From 21c5da8ed6418ee68a28636012f50f2c48b9e29b Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Thu, 26 Feb 2026 11:10:37 +0100 Subject: [PATCH] OCPBUGS-75200: Configure kubelet GracefulNodeShutdown Enable kubelet's GracefulNodeShutdown by setting shutdownGracePeriod and shutdownGracePeriodCriticalPods in the kubelet configuration templates. Without these settings, kubelet exits immediately on SIGTERM during node reboots without terminating pods, causing kube-apiserver to be SIGKILLed when its graceful shutdown exceeds systemd's 90s timeout. Values: - Master/arbiter: 270s total, 240s for critical pods - Worker: 90s total, 60s for critical pods - SNO: disabled (MCO skips drain and uses short grace periods) The 240s critical pod budget provides sufficient headroom above the longest kube-apiserver terminationGracePeriodSeconds (194s on AWS) without requiring platform-specific logic. Signed-off-by: Sascha Grunert --- .../kubelet_config_shutdown_test.go | 117 ++++++++++++++++++ .../_base/files/kubelet.yaml | 4 + .../_base/files/kubelet.yaml | 4 + .../_base/files/kubelet.yaml | 4 + 4 files changed, 129 insertions(+) create mode 100644 pkg/controller/kubelet-config/kubelet_config_shutdown_test.go diff --git a/pkg/controller/kubelet-config/kubelet_config_shutdown_test.go b/pkg/controller/kubelet-config/kubelet_config_shutdown_test.go new file mode 100644 index 0000000000..3e4990709f --- /dev/null +++ b/pkg/controller/kubelet-config/kubelet_config_shutdown_test.go @@ -0,0 +1,117 @@ +package kubeletconfig + +import ( + "testing" + "time" + + osev1 "github.com/openshift/api/config/v1" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// TestShutdownGracePeriodConfiguration verifies that shutdownGracePeriod and +// shutdownGracePeriodCriticalPods are correctly configured in kubelet templates. +// This test ensures that: +// - Master and arbiter nodes get 270s/240s +// - Worker nodes get 90s/60s +// - SNO (SingleReplica) topology gets no shutdownGracePeriod (zero value) +func TestShutdownGracePeriodConfiguration(t *testing.T) { + testCases := []struct { + name string + platform osev1.PlatformType + topology osev1.TopologyMode + nodeRole string + expectedShutdownGracePeriod time.Duration + expectedShutdownGracePeriodCritical time.Duration + }{ + { + name: "AWS master", + platform: osev1.AWSPlatformType, + nodeRole: "master", + expectedShutdownGracePeriod: 270 * time.Second, + expectedShutdownGracePeriodCritical: 240 * time.Second, + }, + { + name: "AWS arbiter", + platform: osev1.AWSPlatformType, + nodeRole: "arbiter", + expectedShutdownGracePeriod: 270 * time.Second, + expectedShutdownGracePeriodCritical: 240 * time.Second, + }, + { + name: "AWS worker", + platform: osev1.AWSPlatformType, + nodeRole: "worker", + expectedShutdownGracePeriod: 90 * time.Second, + expectedShutdownGracePeriodCritical: 60 * time.Second, + }, + { + name: "GCP master", + platform: osev1.GCPPlatformType, + nodeRole: "master", + expectedShutdownGracePeriod: 270 * time.Second, + expectedShutdownGracePeriodCritical: 240 * time.Second, + }, + { + name: "None master", + platform: osev1.NonePlatformType, + nodeRole: "master", + expectedShutdownGracePeriod: 270 * time.Second, + expectedShutdownGracePeriodCritical: 240 * time.Second, + }, + { + name: "None worker", + platform: osev1.NonePlatformType, + nodeRole: "worker", + expectedShutdownGracePeriod: 90 * time.Second, + expectedShutdownGracePeriodCritical: 60 * time.Second, + }, + { + name: "SNO master", + platform: osev1.NonePlatformType, + topology: osev1.SingleReplicaTopologyMode, + nodeRole: "master", + expectedShutdownGracePeriod: 0, + expectedShutdownGracePeriodCritical: 0, + }, + { + name: "SNO worker", + platform: osev1.NonePlatformType, + topology: osev1.SingleReplicaTopologyMode, + nodeRole: "worker", + expectedShutdownGracePeriod: 0, + expectedShutdownGracePeriodCritical: 0, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + f := newFixture(t) + fgHandler := ctrlcommon.NewFeatureGatesHardcodedHandler([]osev1.FeatureGateName{"Example"}, nil) + + cc := newControllerConfig(ctrlcommon.ControllerConfigName, tc.platform) + if tc.topology != "" { + cc.Spec.Infra.Status.ControlPlaneTopology = tc.topology + } + f.ccLister = append(f.ccLister, cc) + + ctrl := f.newController(fgHandler) + + kubeletConfig, err := generateOriginalKubeletConfigIgn(cc, ctrl.templatesDir, tc.nodeRole, &osev1.APIServer{}) + require.NoError(t, err, "Failed to generate kubelet config for %s", tc.name) + + contents, err := ctrlcommon.DecodeIgnitionFileContents(kubeletConfig.Contents.Source, kubeletConfig.Contents.Compression) + require.NoError(t, err, "Failed to decode ignition file contents for %s", tc.name) + + originalKubeConfig, err := DecodeKubeletConfig(contents) + require.NoError(t, err, "Failed to decode kubelet config for %s", tc.name) + + assert.Equal(t, metav1.Duration{Duration: tc.expectedShutdownGracePeriod}, originalKubeConfig.ShutdownGracePeriod, + "shutdownGracePeriod mismatch for %s", tc.name) + assert.Equal(t, metav1.Duration{Duration: tc.expectedShutdownGracePeriodCritical}, originalKubeConfig.ShutdownGracePeriodCriticalPods, + "shutdownGracePeriodCriticalPods mismatch for %s", tc.name) + }) + } +} diff --git a/templates/arbiter/01-arbiter-kubelet/_base/files/kubelet.yaml b/templates/arbiter/01-arbiter-kubelet/_base/files/kubelet.yaml index e536c6f63f..7627036637 100644 --- a/templates/arbiter/01-arbiter-kubelet/_base/files/kubelet.yaml +++ b/templates/arbiter/01-arbiter-kubelet/_base/files/kubelet.yaml @@ -31,6 +31,10 @@ contents: nodeStatusUpdateFrequency: 10s nodeStatusReportFrequency: 5m serverTLSBootstrap: true + {{- if ne .Infra.Status.ControlPlaneTopology "SingleReplica" }} + shutdownGracePeriod: 270s + shutdownGracePeriodCriticalPods: 240s + {{- end }} tlsMinVersion: {{.TLSMinVersion}} tlsCipherSuites: {{- range .TLSCipherSuites }} diff --git a/templates/master/01-master-kubelet/_base/files/kubelet.yaml b/templates/master/01-master-kubelet/_base/files/kubelet.yaml index e536c6f63f..7627036637 100644 --- a/templates/master/01-master-kubelet/_base/files/kubelet.yaml +++ b/templates/master/01-master-kubelet/_base/files/kubelet.yaml @@ -31,6 +31,10 @@ contents: nodeStatusUpdateFrequency: 10s nodeStatusReportFrequency: 5m serverTLSBootstrap: true + {{- if ne .Infra.Status.ControlPlaneTopology "SingleReplica" }} + shutdownGracePeriod: 270s + shutdownGracePeriodCriticalPods: 240s + {{- end }} tlsMinVersion: {{.TLSMinVersion}} tlsCipherSuites: {{- range .TLSCipherSuites }} diff --git a/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml b/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml index d7d6bd556e..bce3b1dcbc 100644 --- a/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml +++ b/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml @@ -31,6 +31,10 @@ contents: nodeStatusUpdateFrequency: 10s nodeStatusReportFrequency: 5m serverTLSBootstrap: true + {{- if ne .Infra.Status.ControlPlaneTopology "SingleReplica" }} + shutdownGracePeriod: 90s + shutdownGracePeriodCriticalPods: 60s + {{- end }} tlsMinVersion: {{.TLSMinVersion}} tlsCipherSuites: {{- range .TLSCipherSuites }}