Alvearie / alvearie-helm

repository for the helm chart source and package for Alvearie projects
https://artifacthub.io/packages/helm/linuxforhealth
Apache License 2.0
3 stars 5 forks source link

[FHIR] Use app.kubernetes.io-scoped labels #40

Closed chgl closed 3 years ago

chgl commented 3 years ago

This replaces the previous labels with common/recommended labels: https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels

lmsurpre commented 3 years ago

I find it a little odd that the kubernetes docs say these are the recommended labels but then their own documentation for labels/selects still uses the simpler unprefixed "app" label. I do like the new helper for the the common label annotations.

lmsurpre commented 3 years ago

I didn't realize it at the time, but this is a backwards-breaking change. If you try upgrading an existing install you will get something like this :

Error: UPGRADE FAILED: cannot patch "ibm-fhir-server" with kind Deployment: Deployment.apps "ibm-fhir-server" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/component":"server", "app.kubernetes.io/instance":"ibm-fhir-server", "app.kubernetes.io/name":"ibm-fhir-server"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable

chgl commented 3 years ago

Hm, I wasn't sure at the time either to be honest. I assumed chart testing's upgrade test would catch this: https://github.com/Alvearie/alvearie-helm/runs/3723826708?check_suite_focus=true#step:7:30

And

If upgrade (--upgrade) is true, then this command will validate that 'helm test' passes for the following upgrade paths:

previous chart revision => current chart version (if non-breaking SemVer change) current chart version => current chart version

https://github.com/helm/chart-testing/blob/main/doc/ct_install.md

I'll have to double check why it didn't get caught 🤔