feat: add watch-namespace flag to filter namespaces to watch for resources#175
Conversation
…urces Signed-off-by: Joseph Heyburn <jdheyburn@gmail.com>
| flag.BoolVar(&enableHTTP2, "enable-http2", false, | ||
| "If set, HTTP/2 will be enabled for the metrics and webhook servers") | ||
| seenNamespaces := make(map[string]struct{}) | ||
| flag.Func("watch-namespace", "Namespace to watch (repeatable; omit for cluster-wide)", func(s string) error { |
There was a problem hiding this comment.
how can users set this watch-namespace envvar from helm or operator env? i think we need to document that part
There was a problem hiding this comment.
I've got a draft PR for the helm chart here: valkey-io/valkey-helm#172
Is that what you had in mind?
For env var, can any of these arguments today be set via env var? I hadn't had that in scope.
bjosv
left a comment
There was a problem hiding this comment.
Some operators seem to only, or also, use the env. variable WATCH_NAMESPACE, like dragonfly-opeator, cnpg, OT Redis operator and cockroach-operator.
It might be a convention from the Operator SDK, and some state that its simpler with env. than injecting extra args.
An env var would need to be comma separated.
Looking at the helm chart PR which uses your args; your solution seems cleaner.
|
@bjosv I wonder if there is a way to have our args defined as env vars too. |
…/watch-namespace-flag Signed-off-by: Joseph Heyburn <jdheyburn@gmail.com>
There was a problem hiding this comment.
LGTM
The test failed due to an ValkeyNodeFailed event, and this seem to be a pre-existing race condition during initial cluster creation
Failed to reconcile ValkeyNode: Operation cannot be fulfilled on valkeynodes.valkey.io "cluster-sample-0-1": the object has been modified; please apply your changes to the latest version and try again
Then we try again and it works then. In the future, maybe we should change this event to be skipped when its a conflict.
Signed-off-by: Joseph Heyburn <jdheyburn@gmail.com>
|
@bjosv should be ready for final review now. For that race condition, I think we should try to replace whatever Apply is causing that with a Patch if it makes sense to. |
Ah, yes that might be a better solution. We'll see if it pops up again |
|
Helm chart PR to support this value: |
Closes #178
Summary
Adds a new flag
--watch-namespacethat will limit the resources watched by the managers. This will help to reduce memory usage in larger clusters. The flag can be specified multiple times to limit to multiple namespaces.Features / Behaviour Changes
Lower memory use
Implementation
Limitations
Testing
Tested locally by building the image and deploying to local kind cluster.
Checklist
Before submitting the PR make sure the following are checked:
pre-commit run --all-filesor hooks on commit)