-
Notifications
You must be signed in to change notification settings - Fork 3k
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 LookupReservedIdentityByLabels function to return consistent results #26795
Conversation
Commit 287605c5d18e35c860405e517075a1fa85a64235 does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
Commit 287605c5d18e35c860405e517075a1fa85a64235 does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
Commits ab86fcf4d6568446162660c38fd6e79d0374acce, c315ca586c1c2233d28aeed9946249a95bf5831a do not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
1 similar comment
Commits ab86fcf4d6568446162660c38fd6e79d0374acce, c315ca586c1c2233d28aeed9946249a95bf5831a do not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
Commit ab86fcf4d6568446162660c38fd6e79d0374acce does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
Commit 3686ef81ed11f2fa8679c6b554a46cfed37ab41d does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
Commit bd89c81eb22f62e9913d43c3802dc0f3cd03505c does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
1 similar comment
Commit bd89c81eb22f62e9913d43c3802dc0f3cd03505c does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
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.
Thanks for your contribution. Could you go over the guidelines to submit a PR - https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#submitting-a-pull-request? More specifically, the change-id in the sign-off can be dropped from the commit message. Is this a user facing commit? We'll need to add a label accordingly.
In the meantime, I've triggered the tests.
The function results inconsistent results if both fixed and reserved identities are present. Also, this simplifies the logic to make it more readable. Signed-off-by: Satish Matti <smatti@google.com>
Thanks for your comment, I just updated the comment. |
/test |
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.
This is much better. Thank you for reworking this logic!
Thanks @skmatti 🙏 ! |
heads up @ldelossa I switched this to |
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXX
line if the commit addresses a particularGitHub issue.
Fixes: <commit-id>
tag, thenplease add the commit author[s] as reviewer[s] to this issue.
The function results inconsistent results if both fixed and reserved identities are present.
For e.g., the following returns
1
whereas the following returns
128
Also, this simplifies the logic to make it more readable.