operator-framework / kubectl-operator

Manage Kubernetes Operators from the command line
https://operatorframework.io/
Apache License 2.0
128 stars 37 forks source link

Feature req: Honor operatorframework.io/suggested-namespace #50

Open kingdonb opened 3 years ago

kingdonb commented 3 years ago

I have been working on an operator which is listed in the Operator Hub catalog, and it has some resources (clusterrolebinding) which are expected to be created for service accounts in a certain namespace. The operator uses operatorframework.io/suggested-namespace to cajole the OpenShift console to install it into that namespace.

When using OLM without OpenShift, I have found that suggested-namespace is not apparently honored. The result of this is some ServiceAccounts are created in the operators namespace, and the ClusterRoleBindings are bound to service account in the suggested namespace that do not exist. (It was easy enough to update the ClusterRoleBindings by hand, but this is easier done than said, in other words it will be a bad mark on our documentation if we have to explain this...)

While in the long term the plan is to move away from service accounts and impersonate users instead, I'm not sure how to handle this in the short term. I understood that the OpenShift Console can be used outside of OpenShift, and that can be used to install operators and create namespaces, but that Operator Lifecycle Manager will not create namespaces by itself.

Would this be a reasonable feature addition to the kubectl-operator plugin? Is there somewhere else it should go besides OpenShift Console?

I haven't been through every page of the docs, but is there already a nice way to install the operator into a user-specified namespace at install time? (Would it help if I can create the namespace some other way?) I'm looking for the best way to resolve this, and someone suggested this kubectl plugin project might be a good place for this function to land.

Skimming through the options, I found there is a -n flag for namespace, but it does not appear to have the desired effect.

kubectl operator install -n flux-system flux --create-operator-group

(Flux still lands in the operators namespace.)

joelanford commented 3 years ago

Skimming through the options, I found there is a -n flag for namespace, but it does not appear to have the desired effect.

kubectl operator install -n flux-system flux --create-operator-group

(Flux still lands in the operators namespace.)

This seems like a bug to me at first glance. Would you mind filing a separate issue for that?


Regarding the larger issue, I think it makes sense for kubectl-operator to do something here. But given the existing behavior of requiring the namespace to exist and using the current context with an optional --namespace override, there are some UX concerns to talk about.

Suppose operatorframework.io/suggested-namespace is set:

  1. Should kubectl-operator honor that or the kubectl context? What if the --namespace flag is set?
  2. Should kubectl-operator create the namespace if it doesn't exist?

I see two main options:

  1. For installations, set namespace precedence as:

    1. --namespace flag
    2. operatorframework.io/suggested-namespace
    3. kubectl context

    Also, add a --create-namespace flag to the install command. If false, and namespace DNE, fail.

  2. Treat operatorframework.io/suggested-namespace as purely informational and write a warning to the kubectl-operator log output, something like WARN: operator suggests installing in namespace "foo", but you are installing into namespace "bar"

There's probably some middle ground in between these options as well. I personally feel like there's a little too much "magic" behind option 1, since kubectl commands are typically pretty WYSIWYG.

kingdonb commented 3 years ago

There is a third option off the top of my head, although it might be a bit less practical to implement than the others you've suggested:

Provide an interface (such as something like envsubst) so that the operator can tailor resources like clusterrolebindings it would create toward the specific namespace that the namespaced serviceaccounts they create will have landed in.

I do not have a strong opinion about 1 vs 2. Honoring the suggested-namespace one way or another seems strongly beneficial. I don't necessarily see anything wrong with (1). Users might be surprised to find their operators landing in default or whatever namespace they are context bound to, for operators that do not include a suggested namespace annotation, but they will surely get over it quickly.

(2) seems a bit less helpful to me, but some people might prefer it as there is probably less potential for surprise.

I am just starting out learning operator framework. Thanks for the response!

I'll open a separate issue for the -n flag not working as expected, as you suggested. 👍

kingdonb commented 3 years ago

I don't know what went wrong last time, but the -n option seems to have the desired effect on second glance.

I renamed this issue for clarity, and explained what might have gone wrong in #51.

joelanford commented 3 years ago

Users might be surprised to find their operators landing in default or whatever namespace they are context bound to, for operators that do not include a suggested namespace annotation, but they will surely get over it quickly.

I think (though I could be wrong) that the least surprising thing for kubectl-operator to do is to treat the kube context and --namespace flag exactly like kubectl does for built-in commands. When I run kubectl get or kubectl create, those operations follow the same precedence rules as kubectl-operator.

kubectl operator install is really just some sugar around kubectl create-ing a handful of manifests, so at least from my perspective, doing something other than the normal kubectl config way of handling namespaces would be surprising.

I would agree that operatorframework.io/suggested-namespace and OpenShift Console's automation around this may bias this particular set of users in this regard, which is why I'm in agreement to make this experience better.

As an aside, in an ideal world an operator should be able to be installed in any namespace and have its RBAC correctly bound based on the installation namespace. That's currently a limitation of OLM, so not much we can do about it right now.

(2) seems a bit less helpful to me, but some people might prefer it as there is probably less potential for surprise.

A riff on (2) could be:

kingdonb commented 3 years ago

Related to this issue,

$ kubectl operator install -n keycloak keycloak-operator --create-operator-group
failed to install operator: operator "keycloak-operator" does not support install mode "AllNamespaces"

I am not sure how one can install an operator that does not support install mode "AllNamespaces"

I began working with Operator Hub through the website, where the instructions have been fixed, but operator YAMLs are generally not namespaced correctly, at least according to the suggested-namespace annotation. Found that you can use kubectl operator with the --namespace flag that works somewhat unreliably, it fails to relocate the operator you have already tried the instructions from operatorhub.io (at least for our Flux operator) but it works again after kubectl operator uninstall and removing any remaining stray ClusterServiceVersion, the OLM cleans up and next install attempt with -n can succeed.

You can also use the OpenShift Console to install as I was informed, which honors suggested-namespace and works great for us, this is the preferred method I think as there are no special instructions required, as long as OLM is already in place and console is available somewhere. I am running OpenShift console in dev mode with a cluster-admin bearer token, so I should be able to do anything (I think, unless there are some things disabled by default in dev mode.) I try to install Keycloak, which has an operator of its own to deploy which is actually meant to be confined to within a namespace, at least as it appears:

Screen Shot 2021-08-20 at 9 16 05 AM

... but it does not install with either method. Note that there is no entry widget for the namespace, perhaps it is because I do not have the Project namespace controller operator installed, so there are no Project namespaces for the operator to install itself into (?). Not sure where I can get that as a stand-alone component or if it is what I need. I guess I should try instructions from https://operatorhub.io/operator/keycloak-operator to be complete. I am not sure why the Krew Operator plugin does not support install mode "AllNamespaces" or if there is a flag to support it, --namespace did not seem have that effect.

This issue has a similar title and theme, but it is not the same issue:

I am unfortunately not sure what other ways there are for creating an operator group besides kubectl-operator when you are not on a full distribution of OpenShift OKD4. (And maybe the answer is, it's time for me to properly graduate my experiments to that full environment...) but I think it should also be possible to do this with the krew operator plugin.

It seemed close enough to this issue that I had already opened, I did not want to just spam and open up a separate one, (but if it is really a separate issue, maybe should be documented separately...)

joelanford commented 3 years ago

You should be able to use the -w, --watch flag to kubectl operator install to configure the namespaces to watch. That will trigger the plugin to create an operator group that supports the other install modes:

Also, just note that the OpenShift Console is a downstream OpenShift-specific component and is not under the CNCF operator-framework project, so there may be differences in features between the upstream kubectl plugin and downstream components (like the OpenShift Console) that do similar things. Any issues related to downstream projects should be created in downstream issue trackers.

Just to reiterate:

dmesser commented 3 years ago

@joelanford Would it be bad if we default to creating a default OperatorGroup in alignment with the supported install modes, when the user doesn't supply anything and also there is no OperatorGroup in the target install namespace already?

joelanford commented 3 years ago

@dmesser That would be great actually. I think operator-sdk run bundle does what you're suggesting.

We basically need a set intersection of:

After the result of that set intersection, we could use a precedence order of:

  1. AllNamespaces
  2. OwnNamespace
  3. SingleNamespace
  4. MultiNamespaces

Note that in reality, the only time we'll actually have more than one choice is if:

  1. the operator supports at least AllNamespaces and OwnNamespace
  2. the watch flag is unset
  3. there is no existing operator group

In that case, we would choose AllNamespaces.

SingleNamespace is only an option when --watch is explicitly set to single namespace that is different than the install namespace or an existing OperatorGroup specifies a different namespace.

MultiNamespaces is only an option when --watch is explicitly set to multiple namespaces or an existing OperatorGroup specifies multiples namespaces.

dmesser commented 3 years ago

This sounds good. We should prefer AllNamespaces whenever we can.

joelanford commented 3 years ago

Alright, I think #54 improves the situation for auto-detecting supported install modes.