Recreate a dead Kubernetes workspace pod on devpod up

CHANGELOG.md

@@ -9,6 +9,10 @@ Add new entries to the top of the "Unreleased" section.
 
 ### Fixed
 
+- `devpod up` of a Kubernetes workspace recreates the workspace's pod when it is dead (its phase is `Failed`,
+  `Succeeded` or `Unknown`, its dev container has terminated, or it is being deleted), keeping the workspace's
+  persistent volume claim. It used to reuse such a pod, for example after the node restarted, and then time out
+  waiting for the agent, an issue inherited from upstream.
 - `devpod provider add --use=false` and `devpod provider update --use=false` no longer silently discard `-o`
   options, an issue inherited from upstream. The options are saved and the provider is initialized, but the
   default provider is not changed. `devpod provider add --from-existing --use=false` likewise keeps the template

pkg/driver/kubernetes/run.go

@@ -232,6 +232,17 @@ func (k *KubernetesDriver) runContainer(
 	}
 
 	if existingPod != nil {
+		// A dead pod never comes back, so replace it even if nothing changed
+		if reason := deadPodReason(existingPod); reason != "" {
+			k.Log.Infof("Pod '%s' %s, recreating it", existingPod.Name, reason)
+			err = k.waitPodDeleted(ctx, id)
+			if err != nil {
+				return errors.Wrapf(err, "delete dead pod: %s", id)
+			}
+
+			return k.runPod(ctx, id, pod)
+		}
+
 		existingOptions := &provider2.ProviderKubernetesDriverConfig{}
 		err := json.Unmarshal([]byte(existingPod.GetAnnotations()[DevPodLastAppliedAnnotation]), existingOptions)
 		if err != nil {
@@ -467,6 +478,29 @@ func getPullSecretsName(workspaceID string) string {
 	return fmt.Sprintf("devpod-pull-secret-%s", workspaceID)
 }
 
+// deadPodReason returns why an existing workspace pod can't be reused, or "" if it can. Workspace pods have
+// restart policy Never, so once the pod's phase is terminal or its dev container has terminated (for example
+// because the node restarted), it never runs again.
+func deadPodReason(pod *corev1.Pod) string {
+	if pod.DeletionTimestamp != nil {
+		return "is terminating"
+	}
+
+	switch pod.Status.Phase {
+	case corev1.PodFailed, corev1.PodSucceeded, corev1.PodUnknown:
+		return fmt.Sprintf("is in phase %s", pod.Status.Phase)
+	case corev1.PodPending, corev1.PodRunning:
+	}
+
+	for _, c := range pod.Status.ContainerStatuses {
+		if c.Name == DevContainerName && c.State.Terminated != nil {
+			return fmt.Sprintf("container '%s' has terminated (reason %s, exit code %d)", c.Name, c.State.Terminated.Reason, c.State.Terminated.ExitCode)
+		}
+	}
+
+	return ""
+}
+
 func optionsEqual(a, b *provider2.ProviderKubernetesDriverConfig) bool {
 	// copy a and b and the compare them without the context, config, namespace and podTimeout
 	aCopy := *a

pkg/driver/kubernetes/run_test.go

@@ -0,0 +1,92 @@
+package kubernetes
+
+import (
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestDeadPodReason(t *testing.T) {
+	running := corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}
+	waiting := corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}}
+	killed := corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 255, Reason: "Unknown"}}
+	now := metav1.Now()
+
+	tests := []struct {
+		name string
+		pod  corev1.Pod
+		dead bool
+	}{
+		{
+			name: "running pod is reused",
+			pod:  pod(corev1.PodRunning, containerStatus(DevContainerName, running)),
+		},
+		{
+			name: "pending pod is reused",
+			pod:  pod(corev1.PodPending, containerStatus(DevContainerName, waiting)),
+		},
+		{
+			name: "pending pod without container statuses is reused",
+			pod:  pod(corev1.PodPending),
+		},
+		{
+			name: "terminated sidecar doesn't make the pod dead",
+			pod:  pod(corev1.PodRunning, containerStatus("sidecar", killed), containerStatus(DevContainerName, running)),
+		},
+		{
+			// what a node restart leaves behind
+			name: "failed pod with killed container is dead",
+			pod:  pod(corev1.PodFailed, containerStatus(DevContainerName, killed)),
+			dead: true,
+		},
+		{
+			name: "failed pod is dead",
+			pod:  pod(corev1.PodFailed),
+			dead: true,
+		},
+		{
+			name: "succeeded pod is dead",
+			pod:  pod(corev1.PodSucceeded),
+			dead: true,
+		},
+		{
+			name: "pod in unknown phase is dead",
+			pod:  pod(corev1.PodUnknown),
+			dead: true,
+		},
+		{
+			name: "running pod with terminated dev container is dead",
+			pod:  pod(corev1.PodRunning, containerStatus(DevContainerName, killed)),
+			dead: true,
+		},
+		{
+			name: "terminating pod is dead",
+			pod: func() corev1.Pod {
+				p := pod(corev1.PodRunning, containerStatus(DevContainerName, running))
+				p.DeletionTimestamp = &now
+				return p
+			}(),
+			dead: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			reason := deadPodReason(&tt.pod)
+			if tt.dead && reason == "" {
+				t.Errorf("deadPodReason() = \"\", want a reason")
+			} else if !tt.dead && reason != "" {
+				t.Errorf("deadPodReason() = %q, want \"\"", reason)
+			}
+		})
+	}
+}
+
+func pod(phase corev1.PodPhase, statuses ...corev1.ContainerStatus) corev1.Pod {
+	return corev1.Pod{Status: corev1.PodStatus{Phase: phase, ContainerStatuses: statuses}}
+}
+
+func containerStatus(name string, state corev1.ContainerState) corev1.ContainerStatus {
+	return corev1.ContainerStatus{Name: name, State: state}
+}