Merge pull request #100527 from matthyx/automated-cherry-pick-of-#985…

…71-upstream-release-1.18 Automated cherry pick of #98571: Stop probing a pod during graceful shutdown
kubernetes · Apr 9, 2021 · c7c1ad4 · c7c1ad4
2 parents 8e67804 + c294831
commit c7c1ad4
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 17 deletions.
diff --git a/pkg/kubelet/prober/worker.go b/pkg/kubelet/prober/worker.go
@@ -222,6 +222,20 @@ func (w *worker) doProbe() (keepGoing bool) {
 			w.pod.Spec.RestartPolicy != v1.RestartPolicyNever
 	}
 
+	// Graceful shutdown of the pod.
+	if w.pod.ObjectMeta.DeletionTimestamp != nil && (w.probeType == liveness || w.probeType == startup) {
+		klog.V(3).Infof("Pod deletion requested, setting %v probe result to success: %v - %v",
+			w.probeType.String(), format.Pod(w.pod), w.container.Name)
+		if w.probeType == startup {
+			klog.Warningf("Pod deletion requested before container has fully started: %v - %v",
+				format.Pod(w.pod), w.container.Name)
+		}
+		// Set a last result to ensure quiet shutdown.
+		w.resultsManager.Set(w.containerID, results.Success, w.pod)
+		// Stop probing at this point.
+		return false
+	}
+
 	// Probe disabled for InitialDelaySeconds.
 	if int32(time.Since(c.State.Running.StartedAt.Time).Seconds()) < w.spec.InitialDelaySeconds {
 		return true
@@ -230,7 +244,7 @@ func (w *worker) doProbe() (keepGoing bool) {
 	if c.Started != nil && *c.Started {
 		// Stop probing for startup once container has started.
 		if w.probeType == startup {
-			return true
+			return false
 		}
 	} else {
 		// Disable other probes until container has started.

diff --git a/pkg/kubelet/prober/worker_test.go b/pkg/kubelet/prober/worker_test.go
@@ -59,25 +59,47 @@ func TestDoProbe(t *testing.T) {
 		failedStatus.Phase = v1.PodFailed
 
 		tests := []struct {
-			probe          v1.Probe
-			podStatus      *v1.PodStatus
-			expectContinue bool
-			expectSet      bool
-			expectedResult results.Result
+			probe                v1.Probe
+			podStatus            *v1.PodStatus
+			expectContinue       map[string]bool
+			expectSet            bool
+			expectedResult       results.Result
+			setDeletionTimestamp bool
 		}{
 			{ // No status.
-				expectContinue: true,
+				expectContinue: map[string]bool{
+					liveness.String():  true,
+					readiness.String(): true,
+					startup.String():   true,
+				},
 			},
 			{ // Pod failed
 				podStatus: &failedStatus,
 			},
+			{ // Pod deletion
+				podStatus:            &runningStatus,
+				setDeletionTimestamp: true,
+				expectSet:            true,
+				expectContinue: map[string]bool{
+					readiness.String(): true,
+				},
+				expectedResult: results.Success,
+			},
 			{ // No container status
-				podStatus:      &otherStatus,
-				expectContinue: true,
+				podStatus: &otherStatus,
+				expectContinue: map[string]bool{
+					liveness.String():  true,
+					readiness.String(): true,
+					startup.String():   true,
+				},
 			},
 			{ // Container waiting
-				podStatus:      &pendingStatus,
-				expectContinue: true,
+				podStatus: &pendingStatus,
+				expectContinue: map[string]bool{
+					liveness.String():  true,
+					readiness.String(): true,
+					startup.String():   true,
+				},
 				expectSet:      true,
 				expectedResult: results.Failure,
 			},
@@ -87,8 +109,12 @@ func TestDoProbe(t *testing.T) {
 				expectedResult: results.Failure,
 			},
 			{ // Probe successful.
-				podStatus:      &runningStatus,
-				expectContinue: true,
+				podStatus: &runningStatus,
+				expectContinue: map[string]bool{
+					liveness.String():  true,
+					readiness.String(): true,
+					startup.String():   true,
+				},
 				expectSet:      true,
 				expectedResult: results.Success,
 			},
@@ -97,7 +123,11 @@ func TestDoProbe(t *testing.T) {
 				probe: v1.Probe{
 					InitialDelaySeconds: -100,
 				},
-				expectContinue: true,
+				expectContinue: map[string]bool{
+					liveness.String():  true,
+					readiness.String(): true,
+					startup.String():   true,
+				},
 				expectSet:      true,
 				expectedResult: results.Success,
 			},
@@ -108,8 +138,12 @@ func TestDoProbe(t *testing.T) {
 			if test.podStatus != nil {
 				m.statusManager.SetPodStatus(w.pod, *test.podStatus)
 			}
-			if c := w.doProbe(); c != test.expectContinue {
-				t.Errorf("[%s-%d] Expected continue to be %v but got %v", probeType, i, test.expectContinue, c)
+			if test.setDeletionTimestamp {
+				now := metav1.Now()
+				w.pod.ObjectMeta.DeletionTimestamp = &now
+			}
+			if c := w.doProbe(); c != test.expectContinue[probeType.String()] {
+				t.Errorf("[%s-%d] Expected continue to be %v but got %v", probeType, i, test.expectContinue[probeType.String()], c)
 			}
 			result, ok := resultsManager(m, probeType).Get(testContainerID)
 			if ok != test.expectSet {
@@ -301,6 +335,12 @@ func expectContinue(t *testing.T, w *worker, c bool, msg string) {
 	}
 }
 
+func expectStop(t *testing.T, w *worker, c bool, msg string) {
+	if c {
+		t.Errorf("[%s - %s] Expected to stop, but did not", w.probeType, msg)
+	}
+}
+
 func resultsManager(m *manager, probeType probeType) results.Manager {
 	switch probeType {
 	case readiness:
@@ -470,6 +510,6 @@ func TestStartupProbeDisabledByStarted(t *testing.T) {
 	// startupProbe fails, but is disabled
 	m.prober.exec = fakeExecProber{probe.Failure, nil}
 	msg = "Started, probe failure, result success"
-	expectContinue(t, w, w.doProbe(), msg)
+	expectStop(t, w, w.doProbe(), msg)
 	expectResult(t, w, results.Success, msg)
 }