Set watch option #2822
Set watch option #2822
Conversation
| @@ -275,6 +276,9 @@ func main() { | |||
| }), | |||
| Cache: cache.Options{ | |||
| DefaultNamespaces: namespaces, | |||
| DefaultLabelSelector: labels.SelectorFromSet(map[string]string{ | |||
There was a problem hiding this comment.
I followed the docs from the operator-sdk but they're out of date after this release kubernetes-sigs/controller-runtime#2300. This is the best way going forward!
|
I believe the reason for e2e test failures is that in the course of adding sidecars or instrumentations, we sometimes ask the API Server for resources owning the Pod - ReplicaSets, Deployments, and the like. Those don't have the label, and hence we fail. We probably need a more nuanced policy here. Something like:
Incidentally, are we doing the right thing by setting |
|
@swiatekm-sumo i see... i think this makes sense. I'll make some changes and we can test it out. I think we could even put this behind a featuregate given its just an option on the manager config, that would let us know pretty clearly if this is helpful for performance as well. My worry is that there's some unknown unknown consequence of this |
main.go
Outdated
| &v1.Job{}: {}, | ||
| &appsv1.Deployment{}: {}, | ||
| &appsv1.DaemonSet{}: {}, | ||
| &appsv1.StatefulSet{}: {}, |
There was a problem hiding this comment.
I think you need ReplicaSet and CronJob here as well. Or maybe just Job, ReplicaSet and DaemonSet (the immediate Pod owners) are sufficient? Both are probably worth testing.
|
This may also be easier to test via envtest, as we're running into problems at the webhook level. In general, I think we should move our setup code out of |
Description: This is a performance improvement for us to only watch items we manage.
Link to tracking Issue(s):
Partially resolves #2808
Testing: Tested locally
Documentation: n/a