Dynamic serviceaccount/role/rolebinding creation #198
Conversation
This PR dyamically creates the above resources within the namespace of the requested Cinder instance. The above resources are no longer created via OLM. This allows the operator to deploy openstack service into any namespace within the OpenShift cluster.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprince, fmount The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
|
||
| // +kubebuilder:validation:Required | ||
| // ServiceAccount - service account name used internally to provide Cinder services the default SA name | ||
| ServiceAccount string `json:"serviceAccount"` |
There was a problem hiding this comment.
Sorry that I'm late to the party (code merged already), but I wonder if these common fields should be included in the CinderServiceTemplate struct. We've been trying to dedup a lot of redundant data structures and code. @fmount I think you are more familiar with this effort, and am curious to hear your thoughts.
There was a problem hiding this comment.
@ASBishop in theory you're right in terms of data structure it can easily be absorbed in CinderServiceTemplate. However, given it's not a field populated by the Spec (but the result of RbacResourceName() function), we should properly populate this field when the Template is generated.
This was the reason DatabaseHostname and a few other fields are still part of the Spec instead of being passed from the upper layer, but I agree we can follow up providing a unified strategy that allows more dedup.
| @@ -852,6 +887,7 @@ func (r *CinderReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst | |||
| ExtraMounts: instance.Spec.ExtraMounts, | |||
| DatabaseHostname: instance.Status.DatabaseHostname, | |||
| TransportURLSecret: instance.Status.TransportURLSecret, | |||
| ServiceAccount: instance.RbacResourceName(), | |||
There was a problem hiding this comment.
Per my previous comment, if the ServiceAccount were part of the CinderServiceTemplate struct, then this field would be automatically populated by L886. Lines L890, L925, L960 and L994 could be eliminated.
This PR dyamically creates the above resources within the namespace of the requested Cinder instance.
The above resources are no longer created via OLM.
This allows the operator to deploy openstack service into any namespace within the OpenShift cluster.