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

ipcache: Fix wrong assertion in ipcache metadata test #23549

Merged

Conversation

christarazi
Copy link
Member

@christarazi christarazi commented Feb 3, 2023

By code inspection, I realized that labels.LabelKubeAPIServer was not
the right key to use, but rather labels.IDNameKubeAPIServer. The test
still passed because it was asserting if the label was NOT in the label
set, which it wasn't, but not for the right reasons.

Fix the assertion in case of regressions in the future.

Fixes: f1b30c1 ("ipcache: Generate new identity if label is removed")

Signed-off-by: Chris Tarazi chris@isovalent.com

@christarazi christarazi added area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. release-note/misc This PR makes changes that have no direct user impact. labels Feb 3, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Feb 3, 2023
@christarazi christarazi marked this pull request as ready for review February 3, 2023 07:04
@christarazi christarazi requested a review from a team as a code owner February 3, 2023 07:04
@christarazi christarazi removed the area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. label Feb 3, 2023
By code inspection, I realized that labels.LabelKubeAPIServer was not
the right key to use, but rather labels.IDNameKubeAPIServer. The test
still passed because it was asserting if the label was NOT in the label
set, which it wasn't, but not for the right reasons.

Fix the assertion in case of regressions in the future.

Fixes: f1b30c1 ("ipcache: Generate new identity if label is removed")

Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi christarazi force-pushed the pr/christarazi/fix-unit-test-ipcache-md branch from 96d151f to a2b1c52 Compare February 3, 2023 07:06
@christarazi
Copy link
Member Author

This PR is fixing unit test code so only Travis is needed for signal. Marking ready to merge as we have approving reviews.

@christarazi christarazi added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 14, 2023
@nbusseneau nbusseneau merged commit 83af240 into cilium:master Feb 15, 2023
@christarazi christarazi deleted the pr/christarazi/fix-unit-test-ipcache-md branch February 16, 2023 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants