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

Support multi-namespace watches #405

Closed
sebgl opened this issue Feb 14, 2019 · 3 comments · Fixed by #1995
Closed

Support multi-namespace watches #405

sebgl opened this issue Feb 14, 2019 · 3 comments · Fixed by #1995
Assignees
Labels
justdoit Continuous improvement not related to a specific feature

Comments

@sebgl
Copy link
Contributor

sebgl commented Feb 14, 2019

Configurable operator meta issue and design proposal.

In order to restrict RBAC permissions required by the operator watching resources in multiple namespaces, we need to support multi-namespaces watch. So far, the controller-runtime only supports watching resources in one namespace, or all of them.

There is already an issue open for it, as a follow-up for the one-namespace restriction: kubernetes-sigs/controller-runtime#218
Looks like it's long-termed planned 👍

operator-sdk folks seem to want that feature as well, and might contribute to the controller-runtime: operator-framework/operator-sdk#767

Meanwhile, the issue above suggests an interesting workaround: implement our own Manager that embeds the controller-runtime Manager, but override the cache to support something like prometheus-operator MultiListWatcher.

My take on it would be to:

  1. Try implementing the multi-namespaces watches in the controller-runtime itself and create a PR upstream.
  2. If 1. turns out not to work that well, use our own cache implementation (the workaround described above).
@sebgl sebgl added loe:large justdoit Continuous improvement not related to a specific feature labels Feb 14, 2019
@sebgl sebgl self-assigned this Feb 14, 2019
@sebgl
Copy link
Contributor Author

sebgl commented Feb 14, 2019

Just noticed there is an in-flight PR for this in the controller-runtime: kubernetes-sigs/controller-runtime#267.
Implementation seems completely ok for us to use 👍
Let's hope it gets merged soon.

@sebgl
Copy link
Contributor Author

sebgl commented Feb 25, 2019

kubernetes-sigs/controller-runtime#267 merged 🎉.
We should be able to test it if we use the controller-runtime latest master (or wait for the next release).

@pebrc
Copy link
Collaborator

pebrc commented Oct 11, 2019

With the upgrade to kubebuilder v2/controller-runtime 0.2 in #1723 completed we still need to enable the multi-namespace cache if a user chooses to restrict ECK to more than one but not all namespaces.

opts := ctrl.Options{
Scheme: clientgoscheme.Scheme,
// restrict the operator to watch resources within a single namespace, unless empty
Namespace: viper.GetString(NamespaceFlagName),
}

needs to get a custom CacheBuilder https://github.com/kubernetes-sigs/controller-runtime/blob/59b131b7cd54d56ec74a66b084a53dd1c3e4843f/pkg/cache/multi_namespace_cache.go#L40

something like

manager.Options{
		NewCache: cache.MultiNamespacedCacheBuilder([]string{"namespace1", "namespace2"}),
	}

Unfortunately we chose to make the the namespace parameter singular, so either we introduce a breaking change between ECK versions and pluralize the parameter or add a new parameter.

NamespaceFlagName,
"",
"namespace in which this operator should manage resources (defaults to all namespaces)",
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
justdoit Continuous improvement not related to a specific feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants