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

k8s: Use Resource[*Pod] in pod watcher for the local pod watching #26181

Merged

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Jun 13, 2023

Now that Resource[*Pod] has been added as a shared resource and it is seeing use from multiple modules, refactor the pod watcher to also use it for the normal case of watching just the local pods.

The "handover" case of watching all pods still creates a separate informer, but this should affect very small set of deployments.

@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 Jun 13, 2023
@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label Jun 14, 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 Jun 14, 2023
@joamaki
Copy link
Contributor Author

joamaki commented Jun 14, 2023

/test

pkg/k8s/watchers/pod.go Outdated Show resolved Hide resolved
Now that Resource[*Pod] has been added as a shared resource and it is
seeing use from multiple modules, refactor the pod watcher to also use
it for the normal case of watching just the local pods.

The "handover" case of watching all pods still creates a separate informer,
but this should affect very small set of deployments.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
@bimmlerd bimmlerd force-pushed the pr/joamaki/pod-watcher-use-resource branch from b9679af to 6c6aefc Compare June 15, 2023 09:23
@bimmlerd
Copy link
Member

/test

@bimmlerd bimmlerd marked this pull request as ready for review June 15, 2023 11:07
@bimmlerd bimmlerd requested a review from a team as a code owner June 15, 2023 11:07
@bimmlerd bimmlerd added the kind/bug This is a bug in the Cilium logic. label Jun 15, 2023
@bimmlerd
Copy link
Member

Labelled this a kind/bug as duplicate informers on the pod resource is not great - I don't think the release-blocker label would be approriate though, as it only occurs if IPAM multi-pool is used which is beta.

@bimmlerd bimmlerd added area/daemon Impacts operation of the Cilium daemon. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. kind/performance There is a performance impact of this. sig/agent Cilium agent related. labels Jun 15, 2023
@bimmlerd
Copy link
Member

AFAIK Nate is on PTO - I'll try unassigning and rerequesting sig-k8s.

@bimmlerd bimmlerd requested review from aanm and removed request for nathanjsweet June 15, 2023 12:39
@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 Jun 15, 2023
@joestringer joestringer merged commit 3db8b14 into cilium:main Jun 15, 2023
59 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/daemon Impacts operation of the Cilium daemon. kind/bug This is a bug in the Cilium logic. kind/performance There is a performance impact of this. 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/agent Cilium agent related. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants