Skip to content

node_local_store: prevent racey tests while using mock node store. #35945

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

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

tommyp1ckles
Copy link
Contributor

@tommyp1ckles tommyp1ckles commented Nov 13, 2024

When creating the test node store, the node is stored and is immediately
ready via Get.
However, a observable event is also emitted at the same time.
In the hubble node observer tests, the node was updated directly the
update function - however in some cases this would be overwritten
by the initial node event if the observable emit call was scheduled
late.

Instead, we update the store directly by mutating the node which
forces the updates to be serialized in the order they are called.

Fixes: #34092

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 13, 2024
@tommyp1ckles
Copy link
Contributor Author

/test

When creating the test node store, the node is stored and is immediately
ready via Get.
However, a observable event is also emitted at the same time.
In the hubble node observer tests, the node was updated directly the
update function - however in some cases this would be overwritten
by the initial node event if the observable emit call was scheduled
late.

Instead, we update the store directly by mutating the node which
forces the updates to be serialized in the order they are called.

Fixes: #34092

Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-hubble-nodewatcher-test-flake branch from a15d7e8 to 3776f39 Compare November 13, 2024 04:18
@tommyp1ckles tommyp1ckles added kind/bug/CI This is a bug in the testing code. area/CI Continuous Integration testing issue or flake area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. sig/hubble release-note/ci This PR makes changes to the CI. labels Nov 13, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 13, 2024
@tommyp1ckles
Copy link
Contributor Author

/test

@tommyp1ckles tommyp1ckles marked this pull request as ready for review November 13, 2024 04:19
@tommyp1ckles tommyp1ckles requested a review from a team as a code owner November 13, 2024 04:19
@tommyp1ckles tommyp1ckles requested a review from kaworu November 13, 2024 04:19
Copy link
Member

@kaworu kaworu 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 the fix @tommyp1ckles 🙏

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 13, 2024
@kaworu kaworu added the needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch label Nov 13, 2024
@kaworu kaworu added this pull request to the merge queue Nov 13, 2024
Merged via the queue into main with commit c576142 Nov 13, 2024
139 checks passed
@kaworu kaworu deleted the pr/tp/fix-hubble-nodewatcher-test-flake branch November 13, 2024 16:16
@rastislavs rastislavs mentioned this pull request Nov 20, 2024
14 tasks
@rastislavs rastislavs added backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. and removed needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Nov 20, 2024
@github-actions github-actions bot added backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. and removed backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. labels Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. kind/bug/CI This is a bug in the testing code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Runtime Test (privileged) - Test_LocalNodeWatcher/update
3 participants