Fix Helm Chart labels and templates. Move version update to labels (#3606)

Fix Helm Chart labels and templates. Move version update to labels.

"app.kubernetes.io/version" and "app.nginx.org/version" should be labels
and not annotations.

This also fixes name generation for the Helm Chart and adds selector
labels.

Author: lucacome

.github/workflows/ci.yml

@@ -242,8 +242,7 @@ jobs:
         working-directory: ${{ github.workspace }}/deployments/helm-chart
       - name: Expose Test Ingresses
         run: |
-          kubectl port-forward service/${{ matrix.type }}-nginx-ingress 8080:80 &
-          kubectl port-forward service/${{ matrix.type }}-nginx-ingress 8443:443 &
+          kubectl port-forward service/${{ matrix.type }}-nginx-ingress-controller 8080:80 8443:443 &
       - name: Test HTTP
         run: |
           counter=0
@@ -393,8 +392,8 @@ jobs:
         run: |
           mv ${{ steps.package.outputs.path }} ${{ github.workspace }}/helm-charts/${{ steps.package-helm.outputs.type }}/
           cd ${{ github.workspace }}/helm-charts
-          helm repo index ${{ needs.package-helm.outputs.type }} --url https://helm.nginx.com/${{ needs.package-helm.outputs.type }}
+          helm repo index ${{ steps.package-helm.outputs.type }} --url https://helm.nginx.com/${{ steps.package-helm.outputs.type }}
           git add -A
           git -c user.name='NGINX Kubernetes Team' -c user.email='[email protected]' \
-          commit -m "NGINX Ingress Controller - Release ${{ needs.package-helm.outputs.type }} ${{ needs.package-helm.outputs.version }}"
+          commit -m "NGINX Ingress Controller - Release ${{ steps.package-helm.outputs.type }} ${{ steps.package-helm.outputs.version }}"
           git push -u origin master

cmd/nginx-ingress/main.go

@@ -43,8 +43,8 @@ import (
 var version string
 
 const (
-	nginxVersionAnnotation = "app.nginx.org/version"
-	versionAnnotation      = "app.kubernetes.io/version"
+	nginxVersionLabel = "app.nginx.org/version"
+	versionLabel      = "app.kubernetes.io/version"
 )
 
 func main() {
@@ -762,21 +762,21 @@ func updateSelfWithVersionInfo(kubeClient *kubernetes.Clientset, version string,
 		return
 	}
 
-	// Copy pod and update the annotations.
+	// Copy pod and update the labels.
 	newPod := pod.DeepCopy()
-	ann := newPod.ObjectMeta.Annotations
-	if ann == nil {
-		ann = make(map[string]string)
+	labels := newPod.ObjectMeta.Labels
+	if labels == nil {
+		labels = make(map[string]string)
 	}
-	ann[nginxVersionAnnotation] = strings.Split(nginxVersion, "/")[1]
-	ann[versionAnnotation] = version
-	newPod.ObjectMeta.Annotations = ann
+	labels[nginxVersionLabel] = strings.TrimSuffix(strings.Split(nginxVersion, "/")[1], "\n")
+	labels[versionLabel] = strings.TrimPrefix(version, "v")
+	newPod.ObjectMeta.Labels = labels
 
 	_, err = kubeClient.CoreV1().Pods(newPod.ObjectMeta.Namespace).Update(context.TODO(), newPod, meta_v1.UpdateOptions{})
 	if err != nil {
-		glog.Errorf("Error updating pod with annotations: %v", err)
+		glog.Errorf("Error updating pod with labels: %v", err)
 		return
 	}
 
-	glog.Infof("Pod annotation updated: %s", pod.ObjectMeta.Name)
+	glog.Infof("Pod label updated: %s", pod.ObjectMeta.Name)
 }

deployments/helm-chart/templates/_helpers.tpl

@@ -4,18 +4,61 @@
 Expand the name of the chart.
 */}}
 {{- define "nginx-ingress.name" -}}
-{{- printf "%s-%s" .Release.Name .Chart.Name | trunc 63 | trimSuffix "-" -}}
+{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
+{{- end }}
+
+{{/*
+Create a default fully qualified app name.
+We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
+If release name contains chart name it will be used as a full name.
+*/}}
+{{- define "nginx-ingress.fullname" -}}
+{{- if .Values.fullnameOverride }}
+{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
+{{- else }}
+{{- $name := default .Chart.Name .Values.nameOverride }}
+{{- if contains $name .Release.Name }}
+{{- .Release.Name | trunc 63 | trimSuffix "-" }}
+{{- else }}
+{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
+{{- end }}
+{{- end }}
+{{- end }}
+
+{{/*
+Create a default fully qualified controller name.
+We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
+*/}}
+{{- define "nginx-ingress.controller.fullname" -}}
+{{- printf "%s-%s" (include "nginx-ingress.fullname" .) .Values.controller.name | trunc 63 | trimSuffix "-" -}}
 {{- end -}}
 
 {{/*
-Create labels
+Create chart name and version as used by the chart label.
+*/}}
+{{- define "nginx-ingress.chart" -}}
+{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
+{{- end }}
+
+{{/*
+Common labels
 */}}
 {{- define "nginx-ingress.labels" -}}
-app.kubernetes.io/name: {{ .Chart.Name }}
-helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version }}
+helm.sh/chart: {{ include "nginx-ingress.chart" . }}
+{{ include "nginx-ingress.selectorLabels" . }}
+{{- if .Chart.AppVersion }}
+app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
+{{- end }}
 app.kubernetes.io/managed-by: {{ .Release.Service }}
+{{- end }}
+
+{{/*
+Selector labels
+*/}}
+{{- define "nginx-ingress.selectorLabels" -}}
+app.kubernetes.io/name: {{ include "nginx-ingress.name" . }}
 app.kubernetes.io/instance: {{ .Release.Name }}
-{{- end -}}
+{{- end }}
 
 {{/*
 Expand the name of the configmap.
@@ -24,7 +67,7 @@ Expand the name of the configmap.
 {{- if .Values.controller.customConfigMap -}}
 {{ .Values.controller.customConfigMap }}
 {{- else -}}
-{{- default (include "nginx-ingress.name" .) .Values.controller.config.name -}}
+{{- default (include "nginx-ingress.fullname" .) .Values.controller.config.name -}}
 {{- end -}}
 {{- end -}}
 
@@ -35,50 +78,29 @@ Expand leader election lock name.
 {{- if .Values.controller.reportIngressStatus.leaderElectionLockName -}}
 {{ .Values.controller.reportIngressStatus.leaderElectionLockName }}
 {{- else -}}
-{{- printf "%s-%s" (include "nginx-ingress.name" .) "leader-election" -}}
+{{- printf "%s-%s" (include "nginx-ingress.fullname" .) "leader-election" -}}
 {{- end -}}
 {{- end -}}
 
 {{/*
 Expand service account name.
 */}}
 {{- define "nginx-ingress.serviceAccountName" -}}
-{{- default (include "nginx-ingress.name" .) .Values.controller.serviceAccount.name -}}
-{{- end -}}
-
-{{/*
-Expand service name.
-*/}}
-{{- define "nginx-ingress.serviceName" -}}
-{{- default (include "nginx-ingress.name" .) .Values.controller.service.name }}
-{{- end -}}
-
-{{/*
-Expand serviceMonitor name.
-*/}}
-{{- define "nginx-ingress.serviceMonitorName" -}}
-{{- default (include "nginx-ingress.name" .) .Values.controller.serviceMonitor.name }}
+{{- default (include "nginx-ingress.fullname" .) .Values.controller.serviceAccount.name -}}
 {{- end -}}
 
 {{/*
 Expand default TLS name.
 */}}
 {{- define "nginx-ingress.defaultTLSName" -}}
-{{- printf "%s-%s" (include "nginx-ingress.name" .) "default-server-tls" -}}
+{{- printf "%s-%s" (include "nginx-ingress.fullname" .) "default-server-tls" -}}
 {{- end -}}
 
 {{/*
 Expand wildcard TLS name.
 */}}
 {{- define "nginx-ingress.wildcardTLSName" -}}
-{{- printf "%s-%s" (include "nginx-ingress.name" .) "wildcard-tls" -}}
-{{- end -}}
-
-{{/*
-Expand app name.
-*/}}
-{{- define "nginx-ingress.appName" -}}
-{{- default (include "nginx-ingress.name" .) .Values.controller.name -}}
+{{- printf "%s-%s" (include "nginx-ingress.fullname" .) "wildcard-tls" -}}
 {{- end -}}
 
 {{- define "nginx-ingress.tag" -}}

deployments/helm-chart/templates/controller-daemonset.yaml

@@ -2,7 +2,7 @@
 apiVersion: apps/v1
 kind: DaemonSet
 metadata:
-  name: {{ default (include "nginx-ingress.name" .) .Values.controller.name }}
+  name: {{ include "nginx-ingress.controller.fullname" . }}
   namespace: {{ .Release.Namespace }}
   labels:
     {{- include "nginx-ingress.labels" . | nindent 4 }}
@@ -12,14 +12,13 @@ metadata:
 spec:
   selector:
     matchLabels:
-      app: {{ include "nginx-ingress.appName" . }}
+      {{- include "nginx-ingress.selectorLabels" . | nindent 6 }}
   template:
     metadata:
       labels:
-        app: {{ include "nginx-ingress.appName" . }}
-        app.kubernetes.io/name: nginx-ingress
+        {{- include "nginx-ingress.selectorLabels" . | nindent 8 }}
 {{- if .Values.nginxServiceMesh.enable }}
-        nsm.nginx.com/daemonset: {{ default (include "nginx-ingress.name" .) .Values.controller.name }}
+        nsm.nginx.com/daemonset: {{ include "nginx-ingress.controller.fullname" . }}
         spiffe.io/spiffeid: "true"
 {{- end }}
 {{- if .Values.controller.pod.extraLabels }}
@@ -198,7 +197,7 @@ spec:
 {{- else if .Values.controller.reportIngressStatus.externalService }}
           - -external-service={{ .Values.controller.reportIngressStatus.externalService }}
 {{- else if and (.Values.controller.service.create) (eq .Values.controller.service.type "LoadBalancer") }}
-          - -external-service={{ include "nginx-ingress.serviceName" . }}
+          - -external-service={{ include "nginx-ingress.controller.fullname" . }}
 {{- end }}
 {{- end }}
           - -enable-leader-election={{ .Values.controller.reportIngressStatus.enableLeaderElection }}

deployments/helm-chart/templates/controller-deployment.yaml

@@ -2,10 +2,9 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
-  name: {{ default (include "nginx-ingress.name" .) .Values.controller.name }}
+  name: {{ include "nginx-ingress.controller.fullname" . }}
   namespace: {{ .Release.Namespace }}
   labels:
-    app.kubernetes.io/name: nginx-ingress
     {{- include "nginx-ingress.labels" . | nindent 4 }}
 {{- if .Values.controller.annotations }}
   annotations: {{ toYaml .Values.controller.annotations | nindent 4 }}
@@ -14,14 +13,13 @@ spec:
   replicas: {{ .Values.controller.replicaCount }}
   selector:
     matchLabels:
-      app: {{ include "nginx-ingress.appName" . }}
+      {{- include "nginx-ingress.selectorLabels" . | nindent 6 }}
   template:
     metadata:
       labels:
-        app: {{ include "nginx-ingress.appName" . }}
-        app.kubernetes.io/name: nginx-ingress
+        {{- include "nginx-ingress.selectorLabels" . | nindent 8 }}
 {{- if .Values.nginxServiceMesh.enable }}
-        nsm.nginx.com/deployment: {{ default (include "nginx-ingress.name" .) .Values.controller.name }}
+        nsm.nginx.com/deployment: {{ include "nginx-ingress.controller.fullname" . }}
         spiffe.io/spiffeid: "true"
 {{- end }}
 {{- if .Values.controller.pod.extraLabels }}
@@ -98,9 +96,9 @@ spec:
           containerPort: 80
         - name: https
           containerPort: 443
-{{ if .Values.controller.customPorts }}
+{{- if .Values.controller.customPorts }}
 {{ toYaml .Values.controller.customPorts | indent 8 }}
-{{ end }}
+{{- end }}
 {{- if .Values.prometheus.create }}
         - name: prometheus
           containerPort: {{ .Values.prometheus.port }}
@@ -202,7 +200,7 @@ spec:
 {{- else if .Values.controller.reportIngressStatus.externalService }}
           - -external-service={{ .Values.controller.reportIngressStatus.externalService }}
 {{- else if and (.Values.controller.service.create) (eq .Values.controller.service.type "LoadBalancer") }}
-          - -external-service={{ include "nginx-ingress.serviceName" . }}
+          - -external-service={{ include "nginx-ingress.controller.fullname" . }}
 {{- end }}
 {{- end }}
           - -enable-leader-election={{ .Values.controller.reportIngressStatus.enableLeaderElection }}

deployments/helm-chart/templates/controller-globalconfiguration.yaml

@@ -2,7 +2,7 @@
 apiVersion: k8s.nginx.org/v1alpha1
 kind: GlobalConfiguration
 metadata:
-  name: {{ include "nginx-ingress.name" . }}
+  name: {{ include "nginx-ingress.fullname" . }}
   namespace: {{ .Release.Namespace }}
   labels:
     {{- include "nginx-ingress.labels" . | nindent 4 }}

deployments/helm-chart/templates/controller-hpa.yaml

@@ -2,7 +2,7 @@
 apiVersion: autoscaling/v2
 kind: HorizontalPodAutoscaler
 metadata:
-  name: {{ include "nginx-ingress.serviceName" . }}
+  name: {{ include "nginx-ingress.controller.fullname" . }}
   namespace: {{ .Release.Namespace }}
   labels:
     {{- include "nginx-ingress.labels" . | nindent 4 }}
@@ -14,7 +14,7 @@ spec:
   scaleTargetRef:
     apiVersion: apps/v1
     kind: Deployment
-    name: {{ default (include "nginx-ingress.name" .) .Values.controller.name }}
+    name: {{ include "nginx-ingress.controller.fullname" . }}
   minReplicas: {{ .Values.controller.autoscaling.minReplicas }}
   maxReplicas: {{ .Values.controller.autoscaling.maxReplicas }}
   metrics:

deployments/helm-chart/templates/controller-ingress-class.yaml

@@ -2,6 +2,8 @@ apiVersion: networking.k8s.io/v1
 kind: IngressClass
 metadata:
   name: {{ .Values.controller.ingressClass }}
+  labels:
+  {{- include "nginx-ingress.labels" . | nindent 4 }}
 {{- if .Values.controller.setAsDefaultIngress }}
   annotations:
     ingressclass.kubernetes.io/is-default-class: "true"

deployments/helm-chart/templates/controller-pdb.yaml

@@ -1,8 +1,8 @@
-{{- if .Values.controller.podDisruptionBudget -}}
+{{- if .Values.controller.podDisruptionBudget.enabled -}}
 apiVersion: policy/v1
 kind: PodDisruptionBudget
 metadata:
-  name: {{ include "nginx-ingress.serviceName" . }}
+  name: {{ include "nginx-ingress.controller.fullname" . }}
   namespace: {{ .Release.Namespace }}
   labels:
     {{- include "nginx-ingress.labels" . | nindent 4 }}
@@ -13,7 +13,7 @@ metadata:
 spec:
   selector:
     matchLabels:
-      app:  {{ include "nginx-ingress.appName" . }}
+      {{- include "nginx-ingress.selectorLabels" . | nindent 6 }}
 {{- if .Values.controller.podDisruptionBudget.minAvailable }}
   minAvailable: {{ .Values.controller.podDisruptionBudget.minAvailable }}
 {{- end }}

deployments/helm-chart/templates/controller-service.yaml

@@ -2,7 +2,7 @@
 apiVersion: v1
 kind: Service
 metadata:
-  name: {{ include "nginx-ingress.serviceName" . }}
+  name: {{ include "nginx-ingress.controller.fullname" . }}
   namespace: {{ .Release.Namespace }}
   labels:
     {{- include "nginx-ingress.labels" . | nindent 4 }}
@@ -61,7 +61,7 @@ spec:
   {{- end }}
 {{- end }}
   selector:
-    app: {{ include "nginx-ingress.appName" . }}
+    {{- include "nginx-ingress.selectorLabels" . | nindent 4 }}
   {{- if .Values.controller.service.externalIPs }}
   externalIPs:
 {{ toYaml .Values.controller.service.externalIPs | indent 4 }}