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

resource: Fix flaky TestResource_RepeatedDelete #28588

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Oct 13, 2023

The fake K8s client checks if the events channel becomes full and panics if it does. Fix this by using a hand-rolled ListerWatcher to have full control over the events channel.

Fixes: #28575
Fixes: a7f1eef ("resource: Fix double upserts on subscribe and retrying of delete events")

@joamaki joamaki requested a review from a team as a code owner October 13, 2023 12:16
@joamaki joamaki requested a review from squeed October 13, 2023 12:16
@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 Oct 13, 2023
@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label Oct 13, 2023
@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 Oct 13, 2023
@joamaki
Copy link
Contributor Author

joamaki commented Oct 13, 2023

/test

The fake K8s client checks if the events channel becomes full and panics
if it does. Fix this by using a hand-rolled ListerWatcher to have full
control over the events channel.

Fixes: cilium#28575
Fixes: a7f1eef ("resource: Fix double upserts on subscribe and retrying of delete events")
Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki joamaki force-pushed the pr/joamaki/fix-resource-test-watch-channel-flake branch from ec64e2c to 2435fa6 Compare October 16, 2023 08:04
@joamaki
Copy link
Contributor Author

joamaki commented Oct 17, 2023

/test

@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 Oct 18, 2023
@joestringer joestringer merged commit 50e9666 into cilium:main Oct 18, 2023
61 of 62 checks passed
@giorio94 giorio94 added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Dec 15, 2023
@YutaroHayakawa YutaroHayakawa mentioned this pull request Dec 20, 2023
8 tasks
@YutaroHayakawa YutaroHayakawa added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Dec 20, 2023
@YutaroHayakawa YutaroHayakawa mentioned this pull request Dec 20, 2023
6 tasks
@YutaroHayakawa YutaroHayakawa added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Dec 20, 2023
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: resource package test triggers panic: channel full
6 participants