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

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

Open
kingdonb opened this issue Jul 28, 2021 · 10 comments
Open

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

kingdonb opened this issue Jul 28, 2021 · 10 comments

Comments

@kingdonb
Copy link

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
Copy link
Member

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
Copy link
Author

kingdonb commented Aug 2, 2021

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 kingdonb changed the title Feature req: Install operator into namespace Feature req: Honor operatorframework.io/suggested-namespace Aug 2, 2021
@kingdonb
Copy link
Author

kingdonb commented Aug 2, 2021

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
Copy link
Member

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:

  • Add a new --permit-non-suggested-namespace flag (there's gotta be a better name I'm not thinking of 🤔)
  • If operatorframework.io/suggested-namespace is set, and --permit-non-suggested-namespace is false, enforce that the installation namespace matches. Fail the install otherwise.

@kingdonb
Copy link
Author

kingdonb commented Aug 20, 2021

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
Copy link
Member

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:

  • OwnNamespace == -n <ns1> -w <ns1>
  • SingleNamespace == -n <ns1> -w <ns2>
  • MultiNamespace == -n <ns1> -w <ns2>,<ns3>

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:

  • -n, --namespace is the namespace in which to install the operator itself
  • -w, --watch is the set of namespaces that the operator should be watching and reconciling. This currently defaults to all namespaces.

@dmesser
Copy link

dmesser commented Aug 23, 2021

@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
Copy link
Member

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

We basically need a set intersection of:

  • The install modes supported by operator (note we only know the supported install modes of the channel heads)
  • The install modes compatible with the namespaces from the --watch flag.
  • The install modes compatible with the namespaces watched by the existing OperatorGroup, if it exists.

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
Copy link

dmesser commented Aug 26, 2021

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

@joelanford
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants