Helm linkerd-controler-plane Allow Custom labels on Pod Monitor

[jseiser]

Need to be able to set labels on Pod Monitors

add a labels section to podMonitor

Helm Lint/Helm Template

Fixes #11175

Signed-off-by: Justin S [email protected]

[alpeb]

Thanks @jseiser 👍

Can you please run bin/helm-docs to upgrade the chart's README.md file? I don't understand why the helm-docs-diff CI job didn't complain...

[alpeb]

LGTM 👍

[jseiser]

@alpeb

Anything else we need to do on this one?

[mateiidavid]

@jseiser thanks for the change! We generally need 2 reviewers and I took my time with it. I think it'd be great to also include a labels field for pod monitor in code, otherwise things might be lost during deserialization when using the CLI.

I think adding a map here should do the trick

linkerd2/pkg/charts/linkerd2/values.go

Lines 157 to 164 in 33dd17f

	PodMonitor struct {
	Enabled bool `json:"enabled"`
	ScrapeInterval string `json:"scrapeInterval"`
	ScrapeTimeout string `json:"scrapeTimeout"`
	Controller *PodMonitorController `json:"controller"`
	ServiceMirror *PodMonitorComponent `json:"serviceMirror"`
	Proxy *PodMonitorComponent `json:"proxy"`
	}

cc: @alpeb

[alpeb]

It seems that for free-form fields like this one the marshaling is forgiving enough:

$ bin/linkerd install --set podMonitor.enabled=true --set podMonitor.labels.foo=bar | grep -B 6 foo
kind: PodMonitor
metadata:
  name: "linkerd-controller"
  namespace: linkerd
  labels:
    linkerd.io/control-plane-ns: linkerd
    foo: bar
--
kind: PodMonitor
metadata:
  name: "linkerd-service-mirror"
  namespace: linkerd
  labels:
    linkerd.io/control-plane-ns: linkerd
    foo: bar
--
kind: PodMonitor
metadata:
  name: "linkerd-proxy"
  namespace: linkerd
  labels:
    linkerd.io/control-plane-ns: linkerd
    foo: bar
--
          matchNames:
            - {{ .Release.Namespace }}
            - linkerd-viz
            - linkerd-jaeger
      enabled: true
      labels:
        foo: bar

But I think it's still good to add that entry into the PodMonitor struct for completeness sake 👍

[jseiser]

I do not know Go to even attempt to add that.

[mateiidavid]

@alpeb's right, I did a bit of testing on a separate PR and it doesn't seem like it requires us to populate the Go values struct (unless we turn it into an actual CLI flag which I doubt is in the books).

We can forgo consistency here since OP isn't comfortable in Go and since we likely have more similar fields that do not have a corresponding field in the internal model.

Thanks for the PR!