diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dc443246..41b6b707d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pkg/driver/kubernetes/run.go b/pkg/driver/kubernetes/run.go index d7f7ae132..a96b17ef2 100644 --- a/pkg/driver/kubernetes/run.go +++ b/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 diff --git a/pkg/driver/kubernetes/run_test.go b/pkg/driver/kubernetes/run_test.go new file mode 100644 index 000000000..134614fcb --- /dev/null +++ b/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} +}