waitForPod should use different exitCondition depending on restart policy

Author: soltysh

staging/src/k8s.io/kubectl/pkg/cmd/run/run.go

@@ -381,17 +381,21 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e
 		}
 
 		var pod *corev1.Pod
-		leaveStdinOpen := o.LeaveStdinOpen
-		waitForExitCode := !leaveStdinOpen && restartPolicy == corev1.RestartPolicyNever
+		waitForExitCode := !o.LeaveStdinOpen && (restartPolicy == corev1.RestartPolicyNever || restartPolicy == corev1.RestartPolicyOnFailure)
 		if waitForExitCode {
-			pod, err = waitForPod(clientset.CoreV1(), attachablePod.Namespace, attachablePod.Name, opts.GetPodTimeout, podCompleted)
+			// we need different exit condition depending on restart policy
+			// for Never it can either fail or succeed, for OnFailure only
+			// success matters
+			exitCondition := podCompleted
+			if restartPolicy == corev1.RestartPolicyOnFailure {
+				exitCondition = podSucceeded
+			}
+			pod, err = waitForPod(clientset.CoreV1(), attachablePod.Namespace, attachablePod.Name, opts.GetPodTimeout, exitCondition)
 			if err != nil {
 				return err
 			}
-		}
-
-		// after removal is done, return successfully if we are not interested in the exit code
-		if !waitForExitCode {
+		} else {
+			// after removal is done, return successfully if we are not interested in the exit code
 			return nil
 		}
 
@@ -691,6 +695,20 @@ func podCompleted(event watch.Event) (bool, error) {
 	return false, nil
 }
 
+// podSucceeded returns true if the pod has run to completion, false if the pod has not yet
+// reached running state, or an error in any other case.
+func podSucceeded(event watch.Event) (bool, error) {
+	switch event.Type {
+	case watch.Deleted:
+		return false, errors.NewNotFound(schema.GroupResource{Resource: "pods"}, "")
+	}
+	switch t := event.Object.(type) {
+	case *corev1.Pod:
+		return t.Status.Phase == corev1.PodSucceeded, nil
+	}
+	return false, nil
+}
+
 // podRunningAndReady returns true if the pod is running and ready, false if the pod has not
 // yet reached those states, returns ErrPodCompleted if the pod has run to completion, or
 // an error in any other case.

test/e2e/kubectl/kubectl.go

@@ -522,13 +522,21 @@ var _ = SIGDescribe("Kubectl client", func() {
 			_, err = framework.NewKubectlCommand(ns, nsFlag, "run", "-i", "--image="+busyboxImage, "--restart=OnFailure", "failure-2", "--", "/bin/sh", "-c", "cat && exit 42").
 				WithStdinData("abcd1234").
 				Exec()
-			framework.ExpectNoError(err)
+			ee, ok = err.(uexec.ExitError)
+			framework.ExpectEqual(ok, true)
+			if !strings.Contains(ee.String(), "timed out") {
+				framework.Failf("Missing expected 'timed out' error, got: %#v", ee)
+			}
 
 			ginkgo.By("running a failing command without --restart=Never, but with --rm")
 			_, err = framework.NewKubectlCommand(ns, nsFlag, "run", "-i", "--image="+busyboxImage, "--restart=OnFailure", "--rm", "failure-3", "--", "/bin/sh", "-c", "cat && exit 42").
 				WithStdinData("abcd1234").
 				Exec()
-			framework.ExpectNoError(err)
+			ee, ok = err.(uexec.ExitError)
+			framework.ExpectEqual(ok, true)
+			if !strings.Contains(ee.String(), "timed out") {
+				framework.Failf("Missing expected 'timed out' error, got: %#v", ee)
+			}
 			e2epod.WaitForPodToDisappear(f.ClientSet, ns, "failure-3", labels.Everything(), 2*time.Second, wait.ForeverTestTimeout)
 
 			ginkgo.By("running a failing command with --leave-stdin-open")