pkg/helm/run.go,pkg/internal/scaffold: helm operator metrics

[joelanford]

Description of the change:
Adds controller-runtime metrics to the helm-operator.

Motivation for the change:
Improves observability of helm-operator.

[lilic]

lgtm just one question

[lilic]

Why replicasets here?

[joelanford]

This is necessary for code that moves up the owner reference chain from the pod to figure out which resource to use for the metrics service owner reference. Since our default operator workload kind is Deployment, the code needs to be able to GET both pods and replicasets. Since we end up using the replicaset's owner reference, we don't actually need GET permission on deployments.

[hasbro17]

We already have pods and replicasets listed with the verb * above in Lines 164-184

- apiGroups:
  - ""
  resources:
  - pods
...
  verbs:
  - "*"
- apiGroups:
  - apps
  resources:
  - deployments
  - daemonsets
  - replicasets
  - statefulsets
  verbs:
  - "*"

Those are the general default rules and these ones are specific to allowing metrics to be exposed. Do you think it's worth adding a comment in the scaffold so it's obvious to the user why we've repeated the rules for pods and replicasets?

[joelanford]

This would be nice. Lili asked this same question when we introduced the initial helm role generation. I looked into it and it turns out it isn't straigtforward since we unmarshal and remarshal the YAML (thus losing the comments) to update the role for CRD APIs.

[estroz]

/lgtm

after one question

[openshift-ci-robot]

New changes are detected. LGTM label has been removed.

[lilic]

Maybe add a mention in the CHANGELOG that helm now comes with metrics by default. I would also amend the docs to mention that, but that is fine in a later PR as well.

still lgtm

[hasbro17]

LGTM