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 Endpoint Querier RuleIndex In Response #5783

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

qiyueyao
Copy link
Contributor

@qiyueyao qiyueyao commented Dec 9, 2023

The current endpoint querier rule index shows the index of the rule among all matched rules in the original policy for this endpoint.
This change updates the rule index to show the rule index among all rules in the original policy.

Fixes #5782

// an AddressGroup can only be referenced in a rule once
break
}
}
// ingressIndex/egressIndex indicates the rule index among this policy's ingress/egress rules
// Antrea Native NP rules priorities are set as index, but KNP rules have the same default rule priorities
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the reference to "priorities" here. This seems like a regular index to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. Please also add comments on why the index increment is removed from the original place as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the comments.

pkg/controller/networkpolicy/endpoint_querier_test.go Outdated Show resolved Hide resolved
pkg/controller/networkpolicy/endpoint_querier_test.go Outdated Show resolved Hide resolved
@antoninbas antoninbas added kind/bug Categorizes issue or PR as related to a bug. action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. and removed action/backport Indicates a PR that requires backports. labels Dec 11, 2023
@antoninbas
Copy link
Contributor

Probably no need to backport since this has been the behavior for a very long time, unless someone is asking for it.

@qiyueyao
Copy link
Contributor Author

qiyueyao commented Dec 11, 2023

Probably no need to backport since this has been the behavior for a very long time, unless someone is asking for it.

Sure, it's just needed for #5740 . Thanks!

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

pkg/controller/networkpolicy/endpoint_querier.go Outdated Show resolved Hide resolved
The current endpoint querier rule index shows the index
of the rule among all matched rules in the policy for
this endpoint, which is not super useful for the users.
This change updates the rule index to show the rule index
among all rules in the policy.

Fixes antrea-io#5782

Signed-off-by: Qiyue Yao <yaoq@vmware.com>
@qiyueyao
Copy link
Contributor Author

/test-all

@qiyueyao qiyueyao requested a review from tnqn December 11, 2023 21:30
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.

LGTM

@tnqn
Copy link
Member

tnqn commented Dec 12, 2023

/skip-all

@tnqn tnqn merged commit 12afd3f into antrea-io:main Dec 12, 2023
47 of 53 checks passed
@qiyueyao qiyueyao deleted the fix-querier-index branch December 13, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Endpoint Querier RuleIndex not showing expected index
4 participants