-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
bgp: Replaced localNodeStore based nodespecer with resources #23276
bgp: Replaced localNodeStore based nodespecer with resources #23276
Conversation
/test |
/test-backport-1.13 |
[ modified upstream commit 9edbece ] In PR cilium#22397 the nodespecer implementation was turned into a cell. That implementation used the localNodeStore to get access to a generic node object that would contain annotations, labels and podCIDRs. However, it turns out that the localNodeStore doesn't function as one might expect and didn't register updates properly. This commit changes the implementation to use resources to subscribe to changes in the node objects of the API server directly. This is very much like the old pre-modularization implementation, but now using resources instead of informers. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
11a34a9
to
0dcb39f
Compare
/test-backport-1.13 Job 'Cilium-PR-K8s-1.24-kernel-4.9' hit: #22578 (96.78% similarity) Job 'Cilium-PR-K8s-1.21-kernel-4.9' failed: Click to show.Test Name
Failure Output
If it is a flake and a GitHub issue doesn't already exist to track it, comment |
Alright, tests seem mostly ok. We are hitting flake #21519 in The ConformanceGKE test is failing on an issue for which I have not yet been able to find an existing report. But the other backport PR #23284 is also hitting the same issue, so it might be that its broken in the target branch. So ignoring flakes, we should be good to go, marking ready-to-merge. |
In PR #22397 the nodespecer implementation was turned into a cell. That implementation used the localNodeStore to get access to a generic node object that would contain annotations, labels and podCIDRs.
However, it turns out that the localNodeStore doesn't function as one might expect and didn't register updates properly.
This PR changes the implementation to use resources to subscribe to changes in the node objects of the API server directly. This is very much like the old pre-modularization implementation, but now using resources instead of informers.
Fixes: #23155