Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix syncHandler issue in status_controller #2036

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

Dyanngg
Copy link
Contributor

@Dyanngg Dyanngg commented Apr 7, 2021

Fix the issue when the status_controller tries to update NetworkPolicyStatus to Pending, it always assumes the policy is Antrea NetworkPolicy instead of Antrea ClusterNetworkPolicy. This will result in failures in status updates which are visible at lowest klog verbose level.

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this wasn't caught by any unit test?

@Dyanngg
Copy link
Contributor Author

Dyanngg commented Apr 7, 2021

LGTM, this wasn't caught by any unit test?

It seems that there is no UT covering this specific case

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing it!

@codecov-io
Copy link

codecov-io commented Apr 7, 2021

Codecov Report

Merging #2036 (f596ec6) into main (9a9804a) will increase coverage by 13.95%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2036       +/-   ##
===========================================
+ Coverage   41.69%   55.64%   +13.95%     
===========================================
  Files         127      265      +138     
  Lines       15844    19773     +3929     
===========================================
+ Hits         6606    11003     +4397     
+ Misses       8684     7603     -1081     
- Partials      554     1167      +613     
Flag Coverage Δ
kind-e2e-tests 42.09% <0.00%> (?)
unit-tests 41.71% <0.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/controller/networkpolicy/status_controller.go 49.03% <0.00%> (-0.97%) ⬇️
pkg/apiserver/handlers/webhook/mutation_labels.go 24.71% <0.00%> (ø)
pkg/apis/crd/v1alpha2/register.go 83.33% <0.00%> (ø)
pkg/apiserver/registry/networkpolicy/util.go 100.00% <0.00%> (ø)
pkg/agent/flowexporter/utils.go 100.00% <0.00%> (ø)
pkg/legacyapis/core/v1alpha2/webhook.go 0.00% <0.00%> (ø)
pkg/antctl/transform/common/transform.go 0.00% <0.00%> (ø)
pkg/apis/stats/v1alpha1/register.go 90.90% <0.00%> (ø)
...clusterinformation/v1beta1/antreacontrollerinfo.go 0.00% <0.00%> (ø)
pkg/features/antrea_features.go 16.66% <0.00%> (ø)
... and 213 more

@tnqn
Copy link
Member

tnqn commented Apr 7, 2021

/test-all

@tnqn
Copy link
Member

tnqn commented Apr 7, 2021

/test-e2e

@Dyanngg Dyanngg merged commit d5fd8f3 into antrea-io:main Apr 7, 2021
@Dyanngg Dyanngg deleted the fix-status-acnp branch April 7, 2021 17:21
antoninbas pushed a commit to antoninbas/antrea that referenced this pull request Apr 30, 2021
antoninbas pushed a commit to antoninbas/antrea that referenced this pull request Apr 30, 2021
antoninbas pushed a commit to antoninbas/antrea that referenced this pull request Apr 30, 2021
antoninbas pushed a commit to antoninbas/antrea that referenced this pull request May 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants