Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to Helm's recommended standard labels #273

Closed
cognifloyd opened this issue Dec 2, 2021 · 4 comments · Fixed by #351
Closed

Migrate to Helm's recommended standard labels #273

cognifloyd opened this issue Dec 2, 2021 · 4 comments · Fixed by #351
Labels
enhancement New feature or request good first issue Good for newcomers Helm help wanted Extra attention is needed K8s
Milestone

Comments

@cognifloyd
Copy link
Member

cognifloyd commented Dec 2, 2021

Currently we have labeled our Deployments, Pods, and Jobs and other k8s resources with:

metadata:
  labels:
    app: st2auth # or st2api, st2actionrunner, st2web, etc
    tier: backend # or frontend
    vendor: stackstorm
    chart: {{ .Chart.Name }}-{{ .Chart.Version }}
    release: {{ .Release.Name }}
    heritage: {{ .Release.Service }}
spec:
  selector:
    matchLabels:
      app: st2auth
      release: {{ .Release.Name }}
  template:
    metadata:
      labels:
        # repeat the labels from above

We should migrate to the standard labels recommend in Helm's "Best Practices" doc.
https://helm.sh/docs/chart_best_practices/labels/#standard-labels

At a glance, I think these could work:

metadata:
  labels:
    app.kubernetes.io/name: st2auth # or st2api, st2actionrunner, st2web, etc
    app.kubernetes.io/component: backend # or frontend
    app.kubernetes.io/part-of: stackstorm
    app.kubernetes.io/version: {{ .Chart.AppVersion }}
    helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version }}
    app.kubernetes.io/instance: {{ .Release.Name }}
    app.kubernetes.io/managed-by: {{ .Release.Service }}
spec:
  selector:
    matchLabels:
      app.kubernetes.io/name: st2auth
      app.kubernetes.io/instance: {{ .Release.Name }}
  template:
    metadata:
      labels:
        # repeat the labels from above

Both labels and label selectors would need to be updated.

A lot of charts define these labels in a helper template. It would be good to do the same in this chart to reduce the duplication.

@cognifloyd cognifloyd added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers Helm K8s labels Dec 2, 2021
@arm4b
Copy link
Member

arm4b commented Jan 12, 2022

I found that many official charts at ArtifactHub are still following the old standards.

But sounds good!
It would make sense to switch from the old-style labels to the new recommended standards before the v1.0.0 release.

@arms11
Copy link
Contributor

arms11 commented Jun 5, 2022

@cognifloyd would there be any negative impact on the existing resources running in the clusters?

@cognifloyd
Copy link
Member Author

I don't know. I will test upgrading an st2 cluster (from old labels to new) to see what happens before I make the next PR.

@cognifloyd cognifloyd modified the milestones: prod/GA, v1.0.0 Jul 30, 2022
@cognifloyd
Copy link
Member Author

@arms11 In #351, I wrote a migration script to run before running helm upgrade. It adds the new labels before the upgrade so that helm can successfully identify the old resources it needs to update.

The plan is to release v0.110.0 (with #352) and then merge #351 to be included in the v1.0.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers Helm help wanted Extra attention is needed K8s
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants