Make controller terminationGracePeriodSeconds configurable and align it with graceful shutdown timeout (#4556)

charts/gha-runner-scale-set-controller-experimental/templates/deployment.yaml

@@ -40,7 +40,7 @@ spec:
         -
       {{- toYaml . | nindent 10 }}
         {{- end }}
-      terminationGracePeriodSeconds: {{ default 10 (index $podSpec "terminationGracePeriodSeconds") }}
+      terminationGracePeriodSeconds: {{ default 35 (index $podSpec "terminationGracePeriodSeconds") }}
       volumes:
         - name: tmp
           emptyDir: {}

charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_pod_extra_fields_test.yaml

@@ -36,6 +36,20 @@ tests:
           path: spec.template.spec.dnsPolicy
           value: "ClusterFirstWithHostNet"
 
+  - it: should allow overriding terminationGracePeriodSeconds via controller.pod
+    set:
+      controller:
+        pod:
+          spec:
+            terminationGracePeriodSeconds: 60
+    release:
+      name: "test-name"
+      namespace: "test-namespace"
+    asserts:
+      - equal:
+          path: spec.template.spec.terminationGracePeriodSeconds
+          value: 60
+
   - it: should not allow overriding serviceAccountName via controller.pod
     set:
       controller:

charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_smoke_test.yaml

@@ -25,3 +25,6 @@ tests:
       - contains:
           path: spec.template.spec.containers[0].command
           content: "/manager"
+      - equal:
+          path: spec.template.spec.terminationGracePeriodSeconds
+          value: 35

charts/gha-runner-scale-set-controller-experimental/values.yaml

@@ -96,7 +96,9 @@ controller:
       affinity: {}
       # Pod topology spread constraints.
       topologySpreadConstraints: []
-      # Pod termination grace period (overrides default 10s).
+      # Pod termination grace period. Overrides the default of 35s, which must
+      # stay above the manager's graceful shutdown timeout (30s, controller-runtime's
+      # default) so in-flight reconciles can finish before the kubelet sends SIGKILL.
       terminationGracePeriodSeconds: null
       # Additional volumes appended to the default ones.
       volumes: []

charts/gha-runner-scale-set-controller/templates/deployment.yaml

@@ -156,7 +156,7 @@ spec:
         {{- range .Values.volumeMounts }}
         - {{ toYaml . | nindent 10 }}
         {{- end }}
-      terminationGracePeriodSeconds: 10
+      terminationGracePeriodSeconds: {{ default 35 .Values.terminationGracePeriodSeconds }}
       volumes:
       - name: tmp
         emptyDir: {}

charts/gha-runner-scale-set-controller/tests/template_test.go

@@ -339,10 +339,10 @@ func TestTemplate_ControllerDeployment_Defaults(t *testing.T) {
 	assert.Equal(t, "test-arc-gha-rs-controller", deployment.Spec.Template.Spec.ServiceAccountName)
 	assert.Nil(t, deployment.Spec.Template.Spec.SecurityContext)
 	assert.Empty(t, deployment.Spec.Template.Spec.PriorityClassName)
-	assert.Equal(t, int64(10), *deployment.Spec.Template.Spec.TerminationGracePeriodSeconds)
+	assert.Equal(t, int64(35), *deployment.Spec.Template.Spec.TerminationGracePeriodSeconds)
 	assert.Len(t, deployment.Spec.Template.Spec.Volumes, 1)
 	assert.Equal(t, "tmp", deployment.Spec.Template.Spec.Volumes[0].Name)
-	assert.NotNil(t, 10, deployment.Spec.Template.Spec.Volumes[0].EmptyDir)
+	assert.NotNil(t, deployment.Spec.Template.Spec.Volumes[0].EmptyDir)
 
 	assert.Len(t, deployment.Spec.Template.Spec.NodeSelector, 0)
 	assert.Nil(t, deployment.Spec.Template.Spec.Affinity)
@@ -429,13 +429,14 @@ func TestTemplate_ControllerDeployment_Customize(t *testing.T) {
 			"topologySpreadConstraints[0].labelSelector.matchLabels.foo":                                                             "bar",
 			"topologySpreadConstraints[0].maxSkew":                                                                                   "1",
 			"topologySpreadConstraints[0].topologyKey":                                                                               "foo",
-			"priorityClassName":         "test-priority-class",
+			"priorityClassName":             "test-priority-class",
-			"flags.logLevel":            "info",
+			"terminationGracePeriodSeconds": "60",
-			"flags.logFormat":           "json",
+			"flags.logLevel":                "info",
-			"volumes[0].name":           "customMount",
+			"flags.logFormat":               "json",
-			"volumes[0].configMap.name": "my-configmap",
+			"volumes[0].name":               "customMount",
-			"volumeMounts[0].name":      "customMount",
+			"volumes[0].configMap.name":     "my-configmap",
-			"volumeMounts[0].mountPath": "/my/mount/path",
+			"volumeMounts[0].name":          "customMount",
+			"volumeMounts[0].mountPath":     "/my/mount/path",
 		},
 		KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName),
 	}
@@ -477,7 +478,7 @@ func TestTemplate_ControllerDeployment_Customize(t *testing.T) {
 	assert.Equal(t, "gha-rs-controller-sa", deployment.Spec.Template.Spec.ServiceAccountName)
 	assert.Equal(t, int64(1000), *deployment.Spec.Template.Spec.SecurityContext.FSGroup)
 	assert.Equal(t, "test-priority-class", deployment.Spec.Template.Spec.PriorityClassName)
-	assert.Equal(t, int64(10), *deployment.Spec.Template.Spec.TerminationGracePeriodSeconds)
+	assert.Equal(t, int64(60), *deployment.Spec.Template.Spec.TerminationGracePeriodSeconds)
 	assert.Len(t, deployment.Spec.Template.Spec.Volumes, 2)
 	assert.Equal(t, "tmp", deployment.Spec.Template.Spec.Volumes[0].Name)
 	assert.NotNil(t, deployment.Spec.Template.Spec.Volumes[0].EmptyDir)
@@ -748,10 +749,10 @@ func TestTemplate_ControllerDeployment_WatchSingleNamespace(t *testing.T) {
 	assert.Equal(t, "test-arc-gha-rs-controller", deployment.Spec.Template.Spec.ServiceAccountName)
 	assert.Nil(t, deployment.Spec.Template.Spec.SecurityContext)
 	assert.Empty(t, deployment.Spec.Template.Spec.PriorityClassName)
-	assert.Equal(t, int64(10), *deployment.Spec.Template.Spec.TerminationGracePeriodSeconds)
+	assert.Equal(t, int64(35), *deployment.Spec.Template.Spec.TerminationGracePeriodSeconds)
 	assert.Len(t, deployment.Spec.Template.Spec.Volumes, 1)
 	assert.Equal(t, "tmp", deployment.Spec.Template.Spec.Volumes[0].Name)
-	assert.NotNil(t, 10, deployment.Spec.Template.Spec.Volumes[0].EmptyDir)
+	assert.NotNil(t, deployment.Spec.Template.Spec.Volumes[0].EmptyDir)
 
 	assert.Len(t, deployment.Spec.Template.Spec.NodeSelector, 0)
 	assert.Nil(t, deployment.Spec.Template.Spec.Affinity)

charts/gha-runner-scale-set-controller/values.yaml

@@ -66,6 +66,10 @@ resources: {}
 #   cpu: 100m
 #   memory: 128Mi
 
+## This must stay above the manager's graceful shutdown timeout (30s, controller-runtime's
+## default) so in-flight reconciles have a chance to finish before the kubelet sends SIGKILL.
+terminationGracePeriodSeconds: 35
+
 nodeSelector: {}
 
 tolerations: []