-
Notifications
You must be signed in to change notification settings - Fork 370
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 incorrect results by antctl get networkpolicy
on Pods
#3499
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3499 +/- ##
==========================================
- Coverage 65.46% 63.37% -2.09%
==========================================
Files 278 278
Lines 27771 27774 +3
==========================================
- Hits 18179 17603 -576
- Misses 7666 8332 +666
+ Partials 1926 1839 -87
Flags with carried forward coverage won't be shown. Click here to find out more.
|
return nil, fmt.Errorf("with a name, none of the other fields can be set") | ||
return nil, "", fmt.Errorf("with a name, none of the other fields can be set") | ||
} | ||
// In case of getting policies applied to a specific Pod, the -n option is used to specify the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel it's confusing that "-n" stands for the namespace of the Pod in one case and stands for the namespace filter of networkpolicies in another case.
Could we ask for "-p ns/pod" for the former case to be clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GraysonWu and I thought about this when we discussed how to fix this issue. The problem by going with "-p ns/pod" is that it doesn't make much sense for -p
and -n
to be used in conjunction anymore. Do you think we should disable this usage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to "-p ns/pod" in the latest commit
This PR came late for Antrea 1.6.0, we can set it as stretched goal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/test-all |
/test-integration |
/test-all |
Signed-off-by: Yang Ding <[email protected]>
Signed-off-by: Yang Ding <[email protected]>
d366f3c
to
e6a3079
Compare
Fixed lint issue /test-all /test-integration |
/test-integration |
When the
-p <PodName> -n <PodNamespace>
options are used inantctl get networkpolicy
command, the-n
option should only be used to identify the Pod being queried, but not filter any internal policies based on the Namespace. Otherwise, all ACNPs applied to the Pod will be filtered out.This PR also fixes an issue where duplicate policy names are displayed.
Signed-off-by: Yang Ding [email protected]