Open kaczyns opened 5 years ago
@stephenkinder Can we get a clarification please - would you prefer:
1) Create the Collection
instances in the kabanero namespace, and then create the pipelines/tasks/etc in pipelinesNamespace
2) Create both the Collection
instances and the pipelines/tasks/etc in pipelinesNamespace
I don't know if there are long-term benefits from doing one over the other.
In order to watch multiple namespaces and support this scenario, we have to move up to operator-sdk v0.11.0 or newer to pick up controller-runtime v0.2.0
@dacleyra has been working thru some of the issues here, and I wanted to write some of this stuff down because it's going to affect other components (potentially). First, there were some questions about the service accounts and roles now, which becomes complicated because you have namespaces where the pipelines are allowed to create applications, as well as namespaces where the pipelines are allowed to be in the first place. Things to keep in mind:
1) kabanero-pipeline-role
is currently a ClusterRole
, established in the install script. I think this can stay that way. This role provides the permissions that a PipelineRun
needs, to build and scan an application. We could move the creation of this cluster role to the kabanero controller, but if we do this, we also need additional logic in the finalizer to remove it when the kabanero instance is deleted, since ClusterRole
is a cluster scope object that can't be owned by a namespace-scoped object.
2) kabanero-pipeline
service account is created in the install script, in the kabanero namespace. The namespace should change to the pipelinesNamespace
. This means the service account must be created by the kabanero controller. Since the service account is likely created in another namespace, it will require a finalizer change to clean it up when the kabanero CR instances is deleted.
3) kabanero-pipeline-cluster-role-binding
is a ClusterRoleBinding
established in the install script. Since this is tied to the service account kabanero-pipeline
, and since the namespace of that service account is now variable, the kabanero controller will need to create this, as well as a finalizer hit to clean it up. Now, I don't see why this can't be just a RoleBinding
but we'll need to find out from @kvijai82 if cluster scope is really necessary. In either case a finalizer hit is required, because it can't be owned across namespaces.
4) kabanero-pipeline-deploy-role
is currently a ClusterRole
, established in the install script. I think it can stay that way, but if not, see 1) for an explanation of what would need to be changed.
5) kabanero-pipeline-deploy-role-binding
is currently a RoleBinding
established in each of the targetNamespaces
namespaces, and linked to the kabanero-pipeline
service account in the kabanero
namespace. The namespace would now be variable, since the kabanero-pipeline
service account is now established in the pipelinesNamespace
namespace. This should be a minor change to targetnamespaces.go
to read the pipelinesNamespace
from the Kabanero CR instance, and make the substitution in the template. Since the object might now be created in a different namespace, we have another finalizer hit here.
6) kabanero-trigger-role
is, I think, not relevant here since the stack controller uses it, and the stack-controller SA and namespace are not changing.
After these changes, someone using the tekton dashboard to establish a GitHub webhook would look to the pipelinesNamespace
to select the pipeline. the PipelineRun
would now run in this namespace. The following problems may occur:
1) The run will attempt to locate the Stack CR
instance to determine the stack image name / tags. Today, the Stack CR instance will always be in the kabanero
namespace. We'll need to look at these tasks and see if they are explicitly looking in the kabanero
namespace, or are looking in the current namespace. If the current namespace, we'll need to change to look in the kabanero
namespace. Long-term, when we support moving the Kabanero and Stack CR instances out of the kabanero
namespace, we'll need to come up with a way to tell the PipelineRun
what namespace it should use to look up the Stack CR instance.
2) The run will attempt to locate the Kabanero CR
instance to determine the governance policy. Again, today the Kabanero CR instance will always be in the kabanero
namespace. If it's hard-coded, great, but if not, we'll need to change that. Long-term, we'll need to figure out how to tell the PipelineRun
what namespace to use to look up the Kabanero CR instance.
3) I have no idea what impact this is going to have on kabanero-eventing
. This may be the show-stopper for us... we'd have to teach eventing how to find the pipelines in the pipelinesNamespace
.
I don't like having a default pipelinesNamespace
- I think we should use the control namespace kabanero
for now, as we do today. That way the short-term repercussions of this change are minimal to nil for users who don't choose to code a pipelinesNamespace
in their Kabanero CR instance.
I think that the kabanero controller should be responsible for establishing all of the roles and bindings, since the Kabanero CR instance has the namespaces coded into it. If we push the namespaces down to the Stack CR instances, that leaves the possibility that someone codes different pipelines namespaces into manually created Stack CR instances, and then we potentially end up with a mess (although it might be a convenient mess for someone who wants to do things in a very particular way). I think having all of the permission/control information in the Kabanero CR instances is easier to understand. I am open to other suggestions though.
Dan mentioned that as of right now, the kabanero operator is namespace-scoped, and so if we create pipelines and tasks in another namespace, we won't be able to watch these and re-create any that are deleted. Once we change to be a cluster-scoped operator (see #284 ) this will be possible. I mean, it's always possible that we can do some off-label thing like create a second client in the stack controller pod which is capable of watching another namespace, and then feed reconcile requests back into the reconciler somehow, but I don't think that's necessary. For now, if someone codes pipelinesNamespace
, we just acknowledge that we can't watch the resources, and that some day in the future we'll be able to do it. Again I'm open to other suggestions here.
That's everything I can think of ATM. It's likely that we can move this along to a point, and then have to stop to work thru more design issues (particularly with eventing), and that's fine.
Another approach worth considering is to support a matching pair of namespaces - myapp
for deploying and myapp-build
for pipelines.
There is a desire to have the collection-level resources (ie pipelines, tasks, etc) deployed in a separate namespace from the rest of Kabanero. Consider the following proposed yaml:
Note the new
pipelinesNamespace
attribute (name subject to change, and also cardinality, but lets go with this for now). When activating the collections, the resources would get created in this alternate namespace. IfpipelinesNamespace
were not specified, we would continue to activate in the current namespace (kabanero in this case).It's not clear whether the collection itself should be activated in the current (kabanero) namespace or in the pipelines namespace. For now lets assume activating in the current namespace. If it's activated in the pipelines namespace, then the CLI services will need to change to watch/create
kind: Collection
instances in the pipelines namespace rather than the current namespace. The kabanero-operator already sets up an environment variable to tell the CLI service what namespace to watch, and it may be as simple as just changing that to a new value, but that also may have un-intended side effects on the CLI, and also would probably require a new role / cluster role, to allow the CLI to search and createkind: Collection
objects across namespaces.