diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 4161d2b82b9..ecfaefcddd0 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -128,6 +128,11 @@ const ( // Enables kubelet to detect CSI volume condition and send the event of the abnormal volume to the corresponding pod that is using it. CSIVolumeHealth featuregate.Feature = "CSIVolumeHealth" + // owner: @HirazawaUi + // + // Enabling this feature gate will cause the pod's status to change due to a kubelet restart. + ChangeContainerStatusOnKubeletRestart = "ChangeContainerStatusOnKubeletRestart" + // owner: @sanposhiho @wojtek-t // kep: https://kep.k8s.io/5278 // @@ -1112,6 +1117,11 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate {Version: version.MustParse("1.21"), Default: false, PreRelease: featuregate.Alpha}, }, + ChangeContainerStatusOnKubeletRestart: { + {Version: version.MustParse("1.0"), Default: true, PreRelease: featuregate.GA}, + {Version: version.MustParse("1.35"), Default: false, PreRelease: featuregate.Deprecated}, + }, + ClearingNominatedNodeNameAfterBinding: { {Version: version.MustParse("1.34"), Default: false, PreRelease: featuregate.Alpha}, }, @@ -2036,6 +2046,8 @@ var defaultKubernetesFeatureGateDependencies = map[featuregate.Feature][]feature CSIVolumeHealth: {}, + ChangeContainerStatusOnKubeletRestart: {}, + ClearingNominatedNodeNameAfterBinding: {}, ClusterTrustBundle: {}, diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index af1d205eebb..88576bfe02f 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -2408,6 +2408,14 @@ func (kl *Kubelet) convertToAPIContainerStatuses(pod *v1.Pod, podStatus *kubecon oldStatusPtr = &oldStatus } status := convertContainerStatus(cStatus, oldStatusPtr) + if !utilfeature.DefaultFeatureGate.Enabled(features.ChangeContainerStatusOnKubeletRestart) { + if cStatus.State == kubecontainer.ContainerStateRunning { + if oldStatus, ok := oldStatuses[status.Name]; ok && oldStatus.Started != nil { + status.Started = oldStatus.Started + } + } + } + if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { allocatedContainer := kubecontainer.GetContainerSpec(pod, cName) if allocatedContainer != nil { diff --git a/pkg/kubelet/prober/prober_manager.go b/pkg/kubelet/prober/prober_manager.go index b390d5f482e..85737b6ad43 100644 --- a/pkg/kubelet/prober/prober_manager.go +++ b/pkg/kubelet/prober/prober_manager.go @@ -18,16 +18,19 @@ package prober import ( "context" + "sync" "time" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/record" "k8s.io/component-base/metrics" "k8s.io/klog/v2" podutil "k8s.io/kubernetes/pkg/api/v1/pod" + "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/prober/results" "k8s.io/kubernetes/pkg/kubelet/status" @@ -278,6 +281,10 @@ func (m *manager) isContainerStarted(pod *v1.Pod, containerStatus *v1.ContainerS return result == results.Success } + if !utilfeature.DefaultFeatureGate.Enabled(features.ChangeContainerStatusOnKubeletRestart) && containerStatus.Started != nil && *containerStatus.Started { + return true + } + // if there is a startup probe which hasn't run yet, the container is not // started. if _, exists := m.getWorker(pod.UID, containerStatus.Name, startup); exists { @@ -288,6 +295,40 @@ func (m *manager) isContainerStarted(pod *v1.Pod, containerStatus *v1.ContainerS return true } +// setReadyStateOnKubeletRestart sets the ready state of a container to false if it was started +// before kubelet restarted and has a readiness probe, but the pod is not ready yet. +// This is to avoid flapping ready status of containers that were ready before kubelet restarted. +func (m *manager) setReadyStateOnKubeletRestart(ready *bool, pod *v1.Pod, containerStatus *v1.ContainerStatus, containerSpec *v1.Container) { + var containerStartTime time.Time + if containerStatus.State.Running != nil { + containerStartTime = containerStatus.State.Running.StartedAt.Time + } + + if !containerStartTime.IsZero() && containerStartTime.Before(kubeletRestartGracePeriod(m.start)) { + // At this point, the Pod may be in one of the following two states: + // - It has not yet been added to the readinessManager. In this case, we directly set the container status to Ready. + // - It has been added to the readinessManager, but the probe has not yet started execution. + // Therefore, in this case, we also need to set the container status to Ready. + if !*ready { + if _, ok := m.readinessManager.Get(kubecontainer.ParseContainerID(containerStatus.ContainerID)); !ok { + *ready = true + } + } + if containerSpec.ReadinessProbe != nil { + podIsReady := false + for _, c := range pod.Status.Conditions { + if c.Type == v1.PodReady && c.Status == v1.ConditionTrue { + podIsReady = true + break + } + } + if !podIsReady { + *ready = false + } + } + } +} + func (m *manager) UpdatePodStatus(ctx context.Context, pod *v1.Pod, podStatus *v1.PodStatus) { logger := klog.FromContext(ctx) for i, c := range podStatus.ContainerStatuses { @@ -315,6 +356,20 @@ func (m *manager) UpdatePodStatus(ctx context.Context, pod *v1.Pod, podStatus *v logger.Info("Failed to trigger a manual run", "probe", w.probeType.String()) } } + + if !utilfeature.DefaultFeatureGate.Enabled(features.ChangeContainerStatusOnKubeletRestart) { + // Find the container spec for the container status. + var containerSpec *v1.Container + for j := range pod.Spec.Containers { + if pod.Spec.Containers[j].Name == c.Name { + containerSpec = &pod.Spec.Containers[j] + break + } + } + if containerSpec != nil { + m.setReadyStateOnKubeletRestart(&ready, pod, &podStatus.ContainerStatuses[i], containerSpec) + } + } } podStatus.ContainerStatuses[i].Ready = ready } @@ -356,6 +411,9 @@ func (m *manager) UpdatePodStatus(ctx context.Context, pod *v1.Pod, podStatus *v logger.Info("Failed to trigger a manual run", "probe", w.probeType.String()) } } + if !utilfeature.DefaultFeatureGate.Enabled(features.ChangeContainerStatusOnKubeletRestart) { + m.setReadyStateOnKubeletRestart(&ready, pod, &podStatus.InitContainerStatuses[i], &initContainer) + } } podStatus.InitContainerStatuses[i].Ready = ready } @@ -381,3 +439,12 @@ func (m *manager) workerCount() int { defer m.workerLock.RUnlock() return len(m.workers) } + +// kubeletRestartGracePeriod returns a time point that is 10 seconds before the kubelet start time. +// This grace period is used to determine if a container was already running before kubelet restarted. +// If a container's start time is before this grace period, it indicates the container was running +// prior to kubelet restart and should not be immediately marked as failed to avoid unnecessary +// status changes for containers that were previously ready. +func kubeletRestartGracePeriod(start time.Time) time.Time { + return start.Add(-time.Second * 10) +} diff --git a/pkg/kubelet/prober/worker.go b/pkg/kubelet/prober/worker.go index f33340a9af9..cd125d763fb 100644 --- a/pkg/kubelet/prober/worker.go +++ b/pkg/kubelet/prober/worker.go @@ -247,8 +247,25 @@ func (w *worker) doProbe(ctx context.Context) (keepGoing bool) { if !w.containerID.IsEmpty() { w.resultsManager.Remove(w.containerID) } + w.containerID = kubecontainer.ParseContainerID(c.ContainerID) - w.resultsManager.Set(w.containerID, w.initialValue, w.pod) + if !utilfeature.DefaultFeatureGate.Enabled(features.ChangeContainerStatusOnKubeletRestart) { + // On kubelet restart, we don't want to immediately set the probe result to Failure, + // as this could cause a container that was Ready to become NotReady. + isRestart := false + if c.State.Running != nil { + containerStartTime := c.State.Running.StartedAt.Time + if !containerStartTime.IsZero() && containerStartTime.Before(kubeletRestartGracePeriod(w.probeManager.start)) { + isRestart = true + } + } + if !isRestart { + w.resultsManager.Set(w.containerID, w.initialValue, w.pod) + } + } else { + w.resultsManager.Set(w.containerID, w.initialValue, w.pod) + } + // We've got a new container; resume probing. w.onHold = false } diff --git a/pkg/kubelet/prober/worker_test.go b/pkg/kubelet/prober/worker_test.go index fb728d47086..11bcb76f70c 100644 --- a/pkg/kubelet/prober/worker_test.go +++ b/pkg/kubelet/prober/worker_test.go @@ -28,6 +28,7 @@ import ( "k8s.io/client-go/kubernetes/fake" featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/features" + kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" kubepod "k8s.io/kubernetes/pkg/kubelet/pod" "k8s.io/kubernetes/pkg/kubelet/prober/results" "k8s.io/kubernetes/pkg/kubelet/status" @@ -779,3 +780,140 @@ func TestStartupProbeDisabledByStarted(t *testing.T) { expectContinue(t, w, w.doProbe(ctx), msg) expectResult(t, w, results.Success, msg) } + +func TestChangeContainerStatusOnKubeletRestart(t *testing.T) { + logger, ctx := ktesting.NewTestContext(t) + + tests := []struct { + name string + featureEnabled bool + isRestart bool + probeType probeType + initialValue results.Result + expectSet bool + }{ + { + name: "feature enabled, is restart, readiness", + featureEnabled: true, + isRestart: true, + probeType: readiness, + initialValue: results.Failure, + expectSet: true, + }, + { + name: "feature enabled, is restart, liveness", + featureEnabled: true, + isRestart: true, + probeType: liveness, + initialValue: results.Success, + expectSet: true, + }, + { + name: "feature enabled, is restart, startup", + featureEnabled: true, + isRestart: true, + probeType: startup, + initialValue: results.Unknown, + expectSet: true, + }, + { + name: "feature enabled, not restart, readiness", + featureEnabled: true, + isRestart: false, + probeType: readiness, + initialValue: results.Failure, + expectSet: true, + }, + { + name: "feature enabled, not restart, liveness", + featureEnabled: true, + isRestart: false, + probeType: liveness, + initialValue: results.Success, + expectSet: true, + }, + { + name: "feature enabled, not restart, startup", + featureEnabled: true, + isRestart: false, + probeType: startup, + initialValue: results.Unknown, + expectSet: true, + }, + { + name: "feature disabled, is restart, readiness", + featureEnabled: false, + isRestart: true, + probeType: readiness, + expectSet: false, + }, + { + name: "feature disabled, is restart, liveness", + featureEnabled: false, + isRestart: true, + probeType: liveness, + expectSet: false, + }, + { + name: "feature disabled, is restart, startup", + featureEnabled: false, + isRestart: true, + probeType: startup, + expectSet: false, + }, + { + name: "feature disabled, not restart, readiness", + featureEnabled: false, + isRestart: false, + probeType: readiness, + initialValue: results.Failure, + expectSet: true, + }, + { + name: "feature disabled, not restart, liveness", + featureEnabled: false, + isRestart: false, + probeType: liveness, + initialValue: results.Success, + expectSet: true, + }, + { + name: "feature disabled, not restart, startup", + featureEnabled: false, + isRestart: false, + probeType: startup, + initialValue: results.Unknown, + expectSet: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ChangeContainerStatusOnKubeletRestart, tc.featureEnabled) + + m := newTestManager() + podStatus := getTestRunningStatus() + podStatus.ContainerStatuses[0].ContainerID = "test://container-id" + if tc.isRestart { + podStatus.ContainerStatuses[0].State.Running.StartedAt = metav1.Time{Time: m.start.Add(-5 * time.Minute)} + } else { + podStatus.ContainerStatuses[0].State.Running.StartedAt = metav1.Time{Time: m.start.Add(5 * time.Minute)} + } + + w := newTestWorker(m, tc.probeType, v1.Probe{InitialDelaySeconds: 1000}) + m.statusManager.SetPodStatus(logger, w.pod, podStatus) + + w.doProbe(ctx) + + containerID := kubecontainer.ParseContainerID(podStatus.ContainerStatuses[0].ContainerID) + result, ok := resultsManager(m, tc.probeType).Get(containerID) + + if ok != tc.expectSet { + t.Errorf("Expected result to be set: %v, but got: %v", tc.expectSet, ok) + } + if tc.expectSet && result != tc.initialValue { + t.Errorf("Expected result %v, but got: %v", tc.initialValue, result) + } + }) + } +} diff --git a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml index 041d6b28197..2e5c9f75085 100644 --- a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml +++ b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml @@ -175,6 +175,16 @@ lockToDefault: false preRelease: Alpha version: "1.32" +- name: ChangeContainerStatusOnKubeletRestart + versionedSpecs: + - default: true + lockToDefault: false + preRelease: GA + version: "1.0" + - default: false + lockToDefault: false + preRelease: Deprecated + version: "1.35" - name: ClearingNominatedNodeNameAfterBinding versionedSpecs: - default: false