From 1bd111734607a0dfcc0c96407b330b6d0ff49888 Mon Sep 17 00:00:00 2001
From: Molly Miller <33266253+sysvinit@users.noreply.github.com>
Date: Wed, 7 Sep 2022 10:32:01 +0200
Subject: [PATCH] coturn: refactor resource labels, expose ServiceMonitor for
 metrics endpoint (#2677)

* charts/coturn: refactor labels.

This adds the labels app, chart, heritage, and release to the coturn chart
(same as the wire-server charts), and removes the boilerplate for overriding
resource names.

* changelog: update.

* charts/coturn: add optional ServiceMonitor

* charts/coturn: add metrics port to Service.

The Service is headless, so this port is not exposed to the outside world; this
is required so that the metrics endpoint is visible to the metrics collection
agent which consumes the ServiceMonitor.

* changelog: update.
---
 changelog.d/0-release-notes/coturn-labels     |  6 +++
 changelog.d/2-features/coturn-metrics         |  2 +
 changelog.d/5-internal/coturn-labels          |  2 +
 charts/coturn/templates/_helpers.yaml         | 45 -------------------
 .../configmap-coturn-conf-template.yaml       |  4 +-
 charts/coturn/templates/secret.yaml           |  5 ++-
 charts/coturn/templates/service-account.yaml  | 25 +++++++----
 charts/coturn/templates/service.yaml          | 13 ++++--
 charts/coturn/templates/servicemonitor.yaml   | 19 ++++++++
 charts/coturn/templates/statefulset.yaml      | 18 +++++---
 charts/coturn/values.yaml                     |  4 ++
 11 files changed, 75 insertions(+), 68 deletions(-)
 create mode 100644 changelog.d/0-release-notes/coturn-labels
 create mode 100644 changelog.d/2-features/coturn-metrics
 create mode 100644 changelog.d/5-internal/coturn-labels
 delete mode 100644 charts/coturn/templates/_helpers.yaml
 create mode 100644 charts/coturn/templates/servicemonitor.yaml

diff --git a/changelog.d/0-release-notes/coturn-labels b/changelog.d/0-release-notes/coturn-labels
new file mode 100644
index 00000000000..043dfd539f6
--- /dev/null
+++ b/changelog.d/0-release-notes/coturn-labels
@@ -0,0 +1,6 @@
+For users of the (currently alpha) coturn Helm chart, **manual action is
+required** when upgrading to this version. The labels applied to the Kubernetes
+manifests in this chart have changed, in order to match the conventions used
+in the wire-server charts. However, this may mean that upgrading with Helm can
+fail, due to changes to the `StatefulSet` included in this chart -- in this
+case, the `StatefulSet` must be deleted before the chart is upgraded.
diff --git a/changelog.d/2-features/coturn-metrics b/changelog.d/2-features/coturn-metrics
new file mode 100644
index 00000000000..717a9c795a0
--- /dev/null
+++ b/changelog.d/2-features/coturn-metrics
@@ -0,0 +1,2 @@
+The coturn chart now has support for exposing its metric endpoint with a
+ServiceMonitor, which can be ingested by third-party metrics collection tools.
diff --git a/changelog.d/5-internal/coturn-labels b/changelog.d/5-internal/coturn-labels
new file mode 100644
index 00000000000..33c3a3f5b06
--- /dev/null
+++ b/changelog.d/5-internal/coturn-labels
@@ -0,0 +1,2 @@
+The labels applied to resources in the coturn chart have been changed to
+reflect the conventions in the wire-server charts.
diff --git a/charts/coturn/templates/_helpers.yaml b/charts/coturn/templates/_helpers.yaml
deleted file mode 100644
index 32fea225209..00000000000
--- a/charts/coturn/templates/_helpers.yaml
+++ /dev/null
@@ -1,45 +0,0 @@
-{{- define "coturn.name" -}}
-{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
-{{- end }}
-
-{{/*
-Create chart name and version as used by the chart label.
-*/}}
-{{- define "coturn.chart" -}}
-{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
-{{- end }}
-
-{{/*
-Common labels
-*/}}
-{{- define "coturn.labels" -}}
-helm.sh/chart: {{ include "coturn.chart" . }}
-{{ include "coturn.selectorLabels" . }}
-{{- if .Chart.AppVersion }}
-app.kubernetes.io/version: {{ .Values.image.tag | default .Chart.AppVersion | quote }}
-{{- end }}
-app.kubernetes.io/managed-by: {{ .Release.Service }}
-{{- 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 "coturn.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 }}
-
-{{- define "coturn.selectorLabels" -}}
-app.kubernetes.io/name: {{ include "coturn.name" . }}
-app.kubernetes.io/instance: {{ .Release.Name }}
-{{- end }}
diff --git a/charts/coturn/templates/configmap-coturn-conf-template.yaml b/charts/coturn/templates/configmap-coturn-conf-template.yaml
index 76e0f95605d..4a2a4c4c066 100644
--- a/charts/coturn/templates/configmap-coturn-conf-template.yaml
+++ b/charts/coturn/templates/configmap-coturn-conf-template.yaml
@@ -1,9 +1,7 @@
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: {{ include "coturn.fullname" . }}
-  labels:
-    {{- include "coturn.selectorLabels" . | nindent 4 }}
+  name: coturn
 
 data:
   coturn.conf.template: |
diff --git a/charts/coturn/templates/secret.yaml b/charts/coturn/templates/secret.yaml
index af6a8563cf3..6dd55212066 100644
--- a/charts/coturn/templates/secret.yaml
+++ b/charts/coturn/templates/secret.yaml
@@ -8,9 +8,10 @@ kind: Secret
 metadata:
   name: coturn
   labels:
+    app: coturn
     chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
-    release: "{{ .Release.Name }}"
-    heritage: "{{ .Release.Service }}"
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
 type: Opaque
 stringData:
   zrest_secret.txt: |
diff --git a/charts/coturn/templates/service-account.yaml b/charts/coturn/templates/service-account.yaml
index 1bea5d59085..ce2803840f1 100644
--- a/charts/coturn/templates/service-account.yaml
+++ b/charts/coturn/templates/service-account.yaml
@@ -2,16 +2,22 @@
 apiVersion: v1
 kind: ServiceAccount
 metadata:
-  name: {{ include "coturn.fullname" . }}
+  name: coturn
   labels:
-    {{- include "coturn.labels" . | nindent 4 }}
+    app: coturn
+    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:
-  name: {{ include "coturn.fullname" . }}
+  name: coturn
   labels:
-    {{- include "coturn.labels" . | nindent 4 }}
+    app: coturn
+    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
 rules:
   - apiGroups: [""]
     resources: [nodes]
@@ -20,14 +26,17 @@ rules:
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
 metadata:
-  name: {{ include "coturn.fullname" . }}
+  name: coturn
   labels:
-    {{- include "coturn.labels" . | nindent 4 }}
+    app: coturn
+    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
 roleRef:
   kind: ClusterRole
   apiGroup: rbac.authorization.k8s.io
-  name: {{ include "coturn.fullname" . }}
+  name: coturn
 subjects:
   - kind: ServiceAccount
-    name: {{ include "coturn.fullname" . }}
+    name: coturn
     namespace: {{ .Release.Namespace }}
diff --git a/charts/coturn/templates/service.yaml b/charts/coturn/templates/service.yaml
index a5f8f15bd5c..f1420c44d62 100644
--- a/charts/coturn/templates/service.yaml
+++ b/charts/coturn/templates/service.yaml
@@ -2,9 +2,12 @@
 apiVersion: v1
 kind: Service
 metadata:
-  name: {{ include "coturn.fullname" . }}
+  name: coturn
   labels:
-    {{- include "coturn.labels" . | nindent 4 }}
+    app: coturn
+    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
 spec:
   # Needs to be headless
   # See: https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/
@@ -22,5 +25,9 @@ spec:
       port: {{ .Values.coturnTurnTlsListenPort }}
       targetPort: coturn-tls
     {{- end }}
+    - name: status-http
+      port: {{ .Values.coturnMetricsListenPort }}
+      targetPort: status-http
   selector:
-    {{- include "coturn.selectorLabels" . | nindent 4 }}
+    app: coturn
+    release: {{ .Release.Name }}
diff --git a/charts/coturn/templates/servicemonitor.yaml b/charts/coturn/templates/servicemonitor.yaml
new file mode 100644
index 00000000000..a21f0faea4e
--- /dev/null
+++ b/charts/coturn/templates/servicemonitor.yaml
@@ -0,0 +1,19 @@
+{{- if .Values.metrics.serviceMonitor.enabled }}
+apiVersion: monitoring.coreos.com/v1
+kind: ServiceMonitor
+metadata:
+  name: coturn
+  labels:
+    app: coturn
+    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+spec:
+  endpoints:
+    - port: status-http
+      path: /metrics
+  selector:
+    matchLabels:
+      app: coturn
+      release: {{ .Release.Name }}
+{{- end }}
diff --git a/charts/coturn/templates/statefulset.yaml b/charts/coturn/templates/statefulset.yaml
index daf90ace402..8ab28192b5d 100644
--- a/charts/coturn/templates/statefulset.yaml
+++ b/charts/coturn/templates/statefulset.yaml
@@ -1,9 +1,12 @@
 apiVersion: apps/v1
 kind: StatefulSet
 metadata:
-  name: {{ include "coturn.fullname" . }}
+  name: coturn
   labels:
-    {{- include "coturn.labels" . | nindent 4 }}
+    app: coturn
+    chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
 
 spec:
   replicas: {{ .Values.replicaCount }}
@@ -12,10 +15,10 @@ spec:
   # affect upgrades.
   podManagementPolicy: Parallel
 
-  serviceName: {{ include "coturn.fullname" . }}
+  serviceName: coturn
   selector:
     matchLabels:
-      {{- include "coturn.selectorLabels" . | nindent 6 }}
+      app: coturn
   template:
     metadata:
     {{- with .Values.podAnnotations }}
@@ -24,7 +27,8 @@ spec:
     {{- end }}
 
       labels:
-        {{- include "coturn.selectorLabels" . | nindent 8 }}
+        app: coturn
+        release: {{ .Release.Name }}
     spec:
       securityContext:
         {{- toYaml .Values.podSecurityContext | nindent 8 }}
@@ -33,7 +37,7 @@ spec:
       shareProcessNamespace: true
       {{- end }}
       hostNetwork: true
-      serviceAccountName: {{ include "coturn.fullname" . }}
+      serviceAccountName: coturn
       volumes:
         - name: external-ip
           emptyDir: {}
@@ -41,7 +45,7 @@ spec:
           emptyDir: {}
         - name: coturn-config-template
           configMap:
-            name: {{ include "coturn.fullname" . }}
+            name: coturn
         - name: secrets
           secret:
             secretName: coturn
diff --git a/charts/coturn/values.yaml b/charts/coturn/values.yaml
index 1504bbcdcad..eede1626bec 100644
--- a/charts/coturn/values.yaml
+++ b/charts/coturn/values.yaml
@@ -36,6 +36,10 @@ tls:
     pullPolicy: IfNotPresent
     tag: 1aa6cbbf2ce3a5182ec47e3579bbcb8f47e22fdc
 
+metrics:
+  serviceMonitor:
+    enabled: false
+
 # This chart optionally supports waiting for traffic to drain from coturn
 # before pods are terminated. Warning: coturn does not have any way to steer
 # incoming client traffic away from itself on its own, so this functionality