redpanda-data / helm-charts

Redpanda Helm Chart
http://redpanda.com
Apache License 2.0
77 stars 96 forks source link

commonLabels are not inherited to the console pods #1581

Closed togge closed 2 weeks ago

togge commented 2 weeks ago

What happened?

Setting the commonLabels works for the statefulset pods, but not for the console pod

What did you expect to happen?

As specified in the documentation:

# -- Additional labels to add to all Kubernetes objects.
# For example, `my.k8s.service: redpanda`.
commonLabels: {}

I would think that the console pod would inherit these labels as well?

How can we reproduce it (as minimally and precisely as possible)?. Please include values file.

   commonLabels:
      ingress.home.arpa/lan: allow
      egress.home.arpa/apiserver: allow
      egress.home.arpa/kubedns: allow
    tls:
      enabled: false
    external:
      enabled: true
      type: NodePort
    storage:
      persistentVolume:
        size: 3Gi
        storageClass: "ceph-block"

Anything else we need to know?

No response

Which are the affected charts?

No response

Chart Version(s)

5.9.9

Cloud provider

Talos

JIRA Link: K8S-405

RafalKorepta commented 2 weeks ago

@togge I see the confusion. That commonLabels is not passed to subcharts. If you would like to use labels in Console subchart you should add the following values to your helm chart values.

console:
  commonLabels:
    ingress.home.arpa/lan: allow
    egress.home.arpa/apiserver: allow
    egress.home.arpa/kubedns: allow

If you will find any problem with that please re-open this issue.

togge commented 2 weeks ago

I see that the labels are applied to the ingress and service for the console, but not the actual pods

Looking at the console/deployment.go vs console/ingress.go i see the difference

deployment.go:114

                ObjectMeta: metav1.ObjectMeta{
                    Annotations: helmette.Merge(map[string]string{
                        "checksum/config": helmette.Sha256Sum(helmette.ToYaml(ConfigMap(dot))),
                    }, values.PodAnnotations),
                    Labels: helmette.Merge(SelectorLabels(dot), values.PodLabels),

ingress.go:79

        ObjectMeta: metav1.ObjectMeta{
            Name:        Fullname(dot),
            Labels:      Labels(dot),
            Annotations: values.Ingress.Annotations,
        },

The Labels func does helmette.Merge with the values.CommonLabels. I would create a pull request, but im not sure if this is intended or not.

togge commented 2 weeks ago

I am not able to reopen the issue, so here's a mention @RafalKorepta :)

chrisseto commented 2 weeks ago

I would create a pull request, but im not sure if this is intended or not.

That line is old enough that I'm not sure we have an exact answer either. I do think it's correct to not include common labels on the Pods themselves as, strictly speaking, the chart doesn't manage the Pods.

It looks like you can satisfy your use case with commonLabels and podLabels though.