From 62302d9e5fda8bc98a690e735821b84a30a9f4c2 Mon Sep 17 00:00:00 2001
From: robertdavidsmith <34475852+robertdavidsmith@users.noreply.github.com>
Date: Fri, 19 Jul 2024 14:58:50 +0100
Subject: [PATCH] Helm charts: add scheduler ingester profiling support, must
 specify more profiling details in sched config (#3802)

Co-authored-by: Robert Smith <robert.smith@gresearch.co.uk>
Signed-off-by: Robert Smith <robertdavidsmith@yahoo.com>
---
 .../scheduler-ingester-deployment.yaml        |  2 +-
 .../scheduler-ingester-profiling-ingress.yaml | 35 +++++++++++++++++++
 .../scheduler-ingester-profiling-service.yaml | 20 +++++++++++
 .../scheduler-profiling-ingress.yaml          | 14 ++++----
 docs/developer/pprof.md                       |  4 +++
 5 files changed, 67 insertions(+), 8 deletions(-)
 create mode 100644 deployment/scheduler/templates/scheduler-ingester-profiling-ingress.yaml
 create mode 100644 deployment/scheduler/templates/scheduler-ingester-profiling-service.yaml

diff --git a/deployment/scheduler/templates/scheduler-ingester-deployment.yaml b/deployment/scheduler/templates/scheduler-ingester-deployment.yaml
index b0d243d5fc7..41d2db51ea1 100644
--- a/deployment/scheduler/templates/scheduler-ingester-deployment.yaml
+++ b/deployment/scheduler/templates/scheduler-ingester-deployment.yaml
@@ -48,7 +48,7 @@ spec:
             {{- if and .Values.ingester.applicationConfig.profiling .Values.ingester.applicationConfig.profiling.port }}
             - containerPort: {{ .Values.ingester.applicationConfig.profiling.port }}
               protocol: TCP
-              name: pprof
+              name: profiling
             {{- end }}
             - containerPort: {{ .Values.ingester.applicationConfig.metricsPort }}
               protocol: TCP
diff --git a/deployment/scheduler/templates/scheduler-ingester-profiling-ingress.yaml b/deployment/scheduler/templates/scheduler-ingester-profiling-ingress.yaml
new file mode 100644
index 00000000000..641d74c6e48
--- /dev/null
+++ b/deployment/scheduler/templates/scheduler-ingester-profiling-ingress.yaml
@@ -0,0 +1,35 @@
+{{- if and .Values.ingester.applicationConfig.profiling .Values.ingester.applicationConfig.profiling.hostnames }}
+{{- $root := . -}}
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: {{ include "armada-scheduler.name" . }}-ingester-profiling
+  namespace: {{ .Release.Namespace }}
+  annotations:
+    certmanager.k8s.io/cluster-issuer: {{ required "A value is required for .Values.ingester.applicationConfig.profiling.clusterIssuer" .Values.ingester.applicationConfig.profiling.clusterIssuer }}
+    cert-manager.io/cluster-issuer: {{ required "A value is required for .Values.ingester.applicationConfig.profiling.clusterIssuer" .Values.ingester.applicationConfig.profiling.clusterIssuer }}
+  labels:
+    {{- include "armada-scheduler-ingester.labels.all" . | nindent 4 }}
+spec:
+  rules:
+  {{- range required "A value is required for .Values.ingester.applicationConfig.profiling.hostnames" .Values.ingester.applicationConfig.profiling.hostnames }}
+  - host: {{ .  }}
+    http:
+      paths:
+        - path: /
+          pathType: Prefix
+          backend:
+            service:
+              name: {{ include "armada-scheduler.name" $root }}-ingester-profiling
+              port:
+                number: {{ $root.Values.ingester.applicationConfig.profiling.port }}
+  {{ end -}}
+  tls:
+    - hosts:
+       {{- range required "A value is required for .Values.ingester.applicationConfig.profiling.hostnames" .Values.ingester.applicationConfig.profiling.hostnames }}
+      - {{ . -}}
+       {{ end }}
+      secretName: armada-scheduler-ingester-profiling-service-tls
+---
+{{- end }}
+
diff --git a/deployment/scheduler/templates/scheduler-ingester-profiling-service.yaml b/deployment/scheduler/templates/scheduler-ingester-profiling-service.yaml
new file mode 100644
index 00000000000..6fd27f0d89b
--- /dev/null
+++ b/deployment/scheduler/templates/scheduler-ingester-profiling-service.yaml
@@ -0,0 +1,20 @@
+{{- if and .Values.ingester.applicationConfig.profiling .Values.ingester.applicationConfig.profiling.port }}
+apiVersion: v1
+kind: Service
+metadata:
+  name: {{ include "armada-scheduler.name" . }}-ingester-profiling
+  namespace: {{ .Release.Namespace }}
+  labels:
+    {{- include "armada-scheduler-ingester.labels.all" . | nindent 4 }}
+    name: {{ include "armada-scheduler.name" . }}-ingester-profiling
+spec:
+  selector:
+    app: {{ include "armada-scheduler.name" . }}-ingester
+    {{- include "armada-scheduler-ingester.labels.all" . | nindent 4 }}
+  ports:
+    - name: profiling
+      protocol: TCP
+      port: {{ .Values.ingester.applicationConfig.profiling.port }}
+---
+{{- end }}
+
diff --git a/deployment/scheduler/templates/scheduler-profiling-ingress.yaml b/deployment/scheduler/templates/scheduler-profiling-ingress.yaml
index 64edffb1724..300da8acdc3 100644
--- a/deployment/scheduler/templates/scheduler-profiling-ingress.yaml
+++ b/deployment/scheduler/templates/scheduler-profiling-ingress.yaml
@@ -1,4 +1,4 @@
-{{- if and .Values.scheduler.applicationConfig.profiling .Values.scheduler.applicationConfig.profiling.port }}
+{{- if and .Values.scheduler.applicationConfig.profiling .Values.scheduler.applicationConfig.profiling.hostnames }}
 {{- $root := . -}}
 {{- range $i := until (int .Values.scheduler.replicas) }}
 apiVersion: networking.k8s.io/v1
@@ -7,15 +7,15 @@ metadata:
   name: {{ $root.Values.scheduler.ingress.nameOverride | default (include "armada-scheduler.name" $root) }}-{{ $i }}-profiling
   namespace: {{ $root.Release.Namespace }}
   annotations:
-    certmanager.k8s.io/cluster-issuer: {{ required "A value is required for $root.Values.scheduler.clusterIssuer" $root.Values.scheduler.clusterIssuer }}
-    cert-manager.io/cluster-issuer: {{ required "A value is required for $root.Values.scheduler.clusterIssuer" $root.Values.scheduler.clusterIssuer }}
+    certmanager.k8s.io/cluster-issuer: {{ required "A value is required for $root.Values.scheduler.applicationConfig.profiling.clusterIssuer" $root.Values.scheduler.applicationConfig.profiling.clusterIssuer }}
+    cert-manager.io/cluster-issuer: {{ required "A value is required for $root.Values.scheduler.applicationConfig.profiling.clusterIssuer" $root.Values.scheduler.applicationConfig.profiling.clusterIssuer }}
   labels:
     {{- include "armada-scheduler.labels.all" $root | nindent 4 }}
 spec:
   rules:
-  {{- range required "A value is required for .Values.scheduler.hostnames" $root.Values.scheduler.hostnames }}
+  {{- range required "A value is required for .Values.scheduler.applicationConfig.profiling.hostnames" $root.Values.scheduler.applicationConfig.profiling.hostnames }}
   {{- $splits := splitList "." . -}}
-  {{- $hostname := (list (first $splits) "-" $i "-profiling." (rest $splits | join ".")) | join "" }}
+  {{- $hostname := (list (first $splits) "-" $i "." (rest $splits | join ".")) | join "" }}
   - host: {{ $hostname  }}
     http:
       paths:
@@ -29,9 +29,9 @@ spec:
   {{ end -}}
   tls:
     - hosts:
-       {{- range required "A value is required for .Values.scheduler.hostnames" $root.Values.scheduler.hostnames }}
+       {{- range required "A value is required for .Values.scheduler.applicationConfig.profiling.hostnames" $root.Values.scheduler.applicationConfig.profiling.hostnames }}
        {{- $splits := splitList "." . -}}
-       {{- $hostname := (list (first $splits) "-" $i "-profiling." (rest $splits | join ".")) | join "" }}
+       {{- $hostname := (list (first $splits) "-" $i "." (rest $splits | join ".")) | join "" }}
       - {{ $hostname -}}
        {{ end }}
       secretName: armada-scheduler-{{ $i }}-profiling-service-tls
diff --git a/docs/developer/pprof.md b/docs/developer/pprof.md
index 0d1c22d14a2..6033f73d69e 100644
--- a/docs/developer/pprof.md
+++ b/docs/developer/pprof.md
@@ -5,6 +5,9 @@
   ```
   profiling:
     port: 6060
+    hostnames:
+    - "armada-scheduler-profiling.armada.my-k8s-cluster.com"
+    clusterIssuer: "k8s-cluster-issuer"  # CertManager cluster-issuer
     auth:
       anonymousAuth: true
       permissionGroupMapping:
@@ -12,4 +15,5 @@
   ```
 - It's possible to put pprof behind auth, see [api.md#authentication](./api.md#authentication) and [oidc.md](./oidc.md).
 - For the scheduler, the helm chart will make a service and ingress for every pod. These are named `armada-scheduler-0-profiling` etc.
+- For the scheduler ingester, the helm chart will make a single service and ingress called `armada-scheduler-ingester-profiling`. Note calls to these may not consistently go to the same pod. Use `kubectl port-forward`, or scale the deployment to size 1, if you need to consistently target one pod.
 - For other services, the helm charts do not currently expose the profiling port. You can use `kubectl port-forward` to access these.