From e06294f5dea763fd5fe18fd0d76d5e7f3b59c9dd Mon Sep 17 00:00:00 2001 From: Junya Okabe <86868255+Okabe-Junya@users.noreply.github.com> Date: Tue, 7 Jul 2026 07:28:57 +0900 Subject: [PATCH] Make controller terminationGracePeriodSeconds configurable and align it with graceful shutdown timeout (#4556) --- .../templates/deployment.yaml | 2 +- ...ller_deployment_pod_extra_fields_test.yaml | 14 +++++++++++ .../controller_deployment_smoke_test.yaml | 3 +++ .../values.yaml | 4 ++- .../templates/deployment.yaml | 2 +- .../tests/template_test.go | 25 ++++++++++--------- .../values.yaml | 4 +++ 7 files changed, 39 insertions(+), 15 deletions(-) diff --git a/charts/gha-runner-scale-set-controller-experimental/templates/deployment.yaml b/charts/gha-runner-scale-set-controller-experimental/templates/deployment.yaml index 2a5e1c95..f3e76bfb 100644 --- a/charts/gha-runner-scale-set-controller-experimental/templates/deployment.yaml +++ b/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: {} diff --git a/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_pod_extra_fields_test.yaml b/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_pod_extra_fields_test.yaml index bc15523d..c6fb1b0a 100644 --- a/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_pod_extra_fields_test.yaml +++ b/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: diff --git a/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_smoke_test.yaml b/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_smoke_test.yaml index c94c5595..728e5118 100644 --- a/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_smoke_test.yaml +++ b/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 diff --git a/charts/gha-runner-scale-set-controller-experimental/values.yaml b/charts/gha-runner-scale-set-controller-experimental/values.yaml index cdc405b5..1c743c30 100644 --- a/charts/gha-runner-scale-set-controller-experimental/values.yaml +++ b/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: [] diff --git a/charts/gha-runner-scale-set-controller/templates/deployment.yaml b/charts/gha-runner-scale-set-controller/templates/deployment.yaml index 57ad21a3..961f7848 100644 --- a/charts/gha-runner-scale-set-controller/templates/deployment.yaml +++ b/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: {} diff --git a/charts/gha-runner-scale-set-controller/tests/template_test.go b/charts/gha-runner-scale-set-controller/tests/template_test.go index c84e27be..acc7ad8b 100644 --- a/charts/gha-runner-scale-set-controller/tests/template_test.go +++ b/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", - "flags.logLevel": "info", - "flags.logFormat": "json", - "volumes[0].name": "customMount", - "volumes[0].configMap.name": "my-configmap", - "volumeMounts[0].name": "customMount", - "volumeMounts[0].mountPath": "/my/mount/path", + "priorityClassName": "test-priority-class", + "terminationGracePeriodSeconds": "60", + "flags.logLevel": "info", + "flags.logFormat": "json", + "volumes[0].name": "customMount", + "volumes[0].configMap.name": "my-configmap", + "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) diff --git a/charts/gha-runner-scale-set-controller/values.yaml b/charts/gha-runner-scale-set-controller/values.yaml index b83d5053..87fc46c9 100644 --- a/charts/gha-runner-scale-set-controller/values.yaml +++ b/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: []