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.

Sign up for GitHub

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

Jump to bottom

k8s/watchers: Fix calling Done() with proper error #24616

Merged
merged 1 commit into from Mar 30, 2023
Merged

k8s/watchers: Fix calling Done() with proper error #24616

merged 1 commit into from Mar 30, 2023

Conversation

christarazi
Copy link
Member

Previously, the error was not passed down to the Done() which would
never attempt a retry if there was an error in the node handler.

Fixes: b4335ee ("k8s: Introduce shared resources")
Fixes: #21744
Signed-off-by: Chris Tarazi chris@isovalent.com

@christarazi
k8s/watchers: Fix calling Done() with proper error
37aa64b 
Previously, the error was not passed down to the Done() which would
never attempt a retry if there was an error in the node handler.

Fixes: b4335ee ("k8s: Introduce shared resources")
Fixes: cilium#21744
Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi christarazi added kind/bug This is a bug in the Cilium logic. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. release-note/misc This PR makes changes that have no direct user impact. labels Mar 29, 2023
@christarazi
Copy link
Member Author

christarazi commented Mar 29, 2023 •
edited

/test

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathCustomCalls Basic test with byte-counter Loads byte-counter and gets consistent values, with per-endpoint routes

Failure Output

FAIL: Timed out after 240.000s.

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

Edit:

@christarazi christarazi marked this pull request as ready for review March 29, 2023 02:04
@christarazi christarazi requested a review from a team as a code owner March 29, 2023 02:04
@christarazi
Copy link
Member Author

cc @joamaki

@christarazi
Copy link
Member Author

christarazi commented Mar 30, 2023 •
edited by maintainer-s-little-helper bot

/mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next

👍 created #24641

@christarazi
Copy link
Member Author

Marking ready to merge given we have CI passing besides known flakes and approving reviews.

@christarazi christarazi added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 30, 2023
@borkmann borkmann merged commit fc4c2fe into cilium:master Mar 30, 2023
43 checks passed
@christarazi christarazi deleted the pr/christarazi/fix-local-node-watcher-err branch March 30, 2023 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youngnick youngnick youngnick approved these changes

Assignees
No one assigned
Labels
kind/bug This is a bug in the Cilium logic. kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. 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/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@christarazi @youngnick @borkmann