operator-framework / operator-lifecycle-manager

A management framework for extending Kubernetes with Operators
https://olm.operatorframework.io
Apache License 2.0
1.71k stars 544 forks source link

Set labels of webhook services with the labels of deployment #2161

Open erkanerol opened 3 years ago

erkanerol commented 3 years ago

Feature Request

Is your feature request related to a problem? Please describe. Thanks to #2054, we are able to label our deployments in our CSV but there is no way to label the services created by OLM for webhooks.

Describe the solution you'd like OLM can put the same labels into the services that it creates for webhooks.

erkanerol commented 3 years ago

Actually, as a user, I want to be able to label everything I deploy with OLM such as role, rolebindings, configmaps, deployments etc. There should be a way to do it.

exdx commented 3 years ago

This looks like a good idea and makes sense -- could you provide more context as to how you're using these labels to manage OLM resources?

We would like to add support for this in the new OLM APIs and potentially the existing model.

erkanerol commented 3 years ago

@exdx Thanks for interesting this feature request and asking the details.

Basically, we are trying to label every k8s object with the recommended labels by k8s community. See https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/

Now, we are adapting our operators for the operands of them. However, the operators are deployed by OLM and we want to add those labels into them (the things we deploy with OLM) via OLM. At the moment, we are able to label deployments of operators. However, it is not possible to label other objects such as services, roles, rolebindings etc.

In our case, the values of the labels are not the same for all objects. We are differentiating them according to their components or versions. Therefore, we want to be able to use different labels for different objects as well.

exdx commented 3 years ago

@erkanerol that makes sense, those common labels are helpful.

Currently adding custom labels to objects embedded in the CSV (like Roles) are not supported, but for other objects packaged into the bundle (things like secrets, services, configmaps) that are not directly tied to the CSV -- if those YAML manifests have labels on them in the bundle, they should persist when installed by OLM onto the cluster. If not, that should be a bug. That would hopefully get you 50% of the way there.

OLM also applies its own labels onto objects that it creates on behalf of users (for various reasons) so those labels have to be merged with user-specified labels when installing objects onto the cluster

oshoval commented 3 years ago

Hi Tested with Simone on a Downstream cluster, it seems cluster role of CNAO isn't labeled as well

$ oc get clusterrole networkaddonsconfigs.networkaddonsoperator.network.kubevirt.io-v1-view -o json | jq .metadata.labels
{
  "olm.opgroup.permissions/aggregate-to-cd83d85afcbcf51a-view": "true",
  "rbac.authorization.k8s.io/aggregate-to-view": "true"
}

Not sure if it was raised already (and its deployed by OLM IIUC) should we open an issue for it ?

Thanks

akihikokuroda commented 2 years ago

Hi! I'm looking at how the labels can be added to the role, rolebinding, clusterrole and clusterrolebinding. It seems doable but setting the labels specified in the deployment in the CSV is an issue. The CSV can have multiple deployments and multiple permissions and clusterpermissions. They are independent so there is no good way to associate the deployment entry and the permission entry. So the labels must be specified one more upper level in the CSV. It should be spec.install.spec.labels. Is this reasonable?

erkanerol commented 2 years ago

@akihikokuroda Thanks for looking this issue.

So the labels must be specified one more upper level in the CSV. It should be spec.install.spec.labels. Is this reasonable?

Yes. It sound reasonable but users may need different labels for different object. For example, in our case, we are using different values for app.kubernetes.io/component label

akihikokuroda commented 2 years ago

I see. The permissions(for Role) and clusterPermissions(for ClusterRole) need labels to be set in StrategyDeploymentPermissions like StrategyDeploymentSpec (probably labels instead of label). So the CSV is something like:

apiVersion: operators.coreos.com/v1alpha1
kind: ClusterServiceVersion
spec:
  install:
    spec:
      clusterPermissions:
      - labels:
          application: example-operator-1
          technology: general
        rules:
        - apiGroups:
          - apps
          .....
      permissions:
      - labels:
          application: example-operator-2
          technology: general
        rules:
        - apiGroups:
          - apps
          .....

Does this satisfy the requirement?

erkanerol commented 2 years ago

@akihikokuroda Partially yes. OLM creates some k8s objects for other objects in CSV. For example, it creates Service object for webhooks in CSV. What do you plan for those objects? Will you use labels of original object (deployment in this case)?

akihikokuroda commented 2 years ago

@erkanerol Yes, everything related to a specific deployment gets the labels in the deployment.

erkanerol commented 2 years ago

@akihikokuroda Sounds reasonable!

akihikokuroda commented 2 years ago

@erkanerol I looked into a little details. There are different group of resources created by installing a bundle.

  1. resources in the manifests directory in the bundle These resource can be set the labels by adding them in the yaml files.
  2. resources related to the deployment section in the CSV I can make code changes to put the labels in the deployment.
  3. resources related to the permission/clusterpermission section in the CSV I can make code changes to put the labels in the permission/clusterpermission sections but I have to make the API definition change first.
  4. resources related to the CSV (not specific in any section) I can make code changes to put the labels in the install sections but I have to make the API definition change first.

The Service object for webhooks in CSV is one of the resources in 1. So you can add any labels in the yaml file without any code changes. Do you need labels for the other resources, too?

With my sample bundle with a webhook, I got 3 services after its install.

memcached-operator-webhook-controller-manager-metrics-service   ClusterIP   10.106.20.232   <none>        8443/TCP   3m52s
memcached-operator-webhook-controller-manager-service           ClusterIP   10.98.62.84     <none>        443/TCP    3m50s
memcached-operator-webhook-webhook-service                      ClusterIP   10.102.21.131   <none>        443/TCP    3m51s

The first and third ones are 1. The second one is 2.

erkanerol commented 2 years ago

@akihikokuroda In our case, we deploy everything with a CSV other than CRDs. Could you please point me out the source of the bundle you used in the test above?

akihikokuroda commented 2 years ago

@erkanerol I made this sample for trying this issue. https://github.com/akihikokuroda/memcached-operator-webhook

suridaddy commented 2 years ago

@akihikokuroda i have similar requirement, but in my case, the operators are developed by others. While doing installation or pre/post-installation, i want to set labels on the generated k8s resources (by the operator). Can i modify the CSV manually to make it work?

Or ,do you have any comment for my case?