WIP: test

Author: geropl

components/ws-manager-api/go/crd/v1/workspace_types.go

@@ -171,7 +171,7 @@ func (ps PortSpec) Equal(other PortSpec) bool {
 // WorkspaceStatus defines the observed state of Workspace
 type WorkspaceStatus struct {
 	PodStarts    int    `json:"podStarts"`
-	PodRecreated int    `json:"podRecreated,omitempty"`
+	PodRecreated int    `json:"podRecreated"`
 	URL          string `json:"url,omitempty" scrub:"redact"`
 	OwnerToken   string `json:"ownerToken,omitempty" scrub:"redact"`
 

components/ws-manager-mk2/controllers/workspace_controller.go

@@ -230,14 +230,22 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
 			// when the workspace stops, due to PodStarts still being 0 when the original Pod
 			// disappears.
 			// Use a Patch instead of an Update, to prevent conflicts.
-			patch := client.MergeFrom(workspace.DeepCopy())
-			workspace.Status.PodStarts = 0
-			workspace.Status.PodRecreated++
+			dc := workspace.DeepCopy()
+			patch := client.MergeFrom(dc)
+
+			// Reset status
+			sc := workspace.Status.DeepCopy()
+			workspace.Status = workspacev1.WorkspaceStatus{}
+			workspace.Status.OwnerToken = sc.OwnerToken
+			workspace.Status.PodRecreated = sc.PodRecreated + 1
+			log.WithValues("ws", workspace.Status.PodRecreated, "oldWs", sc.PodRecreated, "patch", dc.Status.PodRecreated).Info("PATCH")
+
 			workspace.Status.SetCondition(workspacev1.NewWorkspaceConditionPodRejected(fmt.Sprintf("Recreating pod... (%d retry)", workspace.Status.PodRecreated), metav1.ConditionFalse))
 			if err := r.Status().Patch(ctx, workspace, patch); err != nil {
-				log.Error(err, "Failed to patch PodStarts=0,PodRecreated++ in workspace status")
+				log.Error(err, "Failed to patch workspace status-reset")
 				return ctrl.Result{}, err
 			}
+			log.WithValues("ws", workspace.Status.PodRecreated, "oldWs", sc.PodRecreated, "patch", dc.Status.PodRecreated).Info("AFTER PATCH")
 
 			requeueAfter := 5 * time.Second
 			if r.Config.PodRecreationBackoff != 0 {

components/ws-manager-mk2/controllers/workspace_controller_test.go

@@ -389,6 +389,127 @@ var _ = Describe("WorkspaceController", func() {
 			})
 		})
 
+		It("pod rejection should result in a retry", func() {
+			ws := newWorkspace(uuid.NewString(), "default")
+
+			// ### prepare block start
+			By("creating workspace")
+			// Simulate pod getting scheduled to a node.
+			var node corev1.Node
+			node.Name = uuid.NewString()
+			Expect(k8sClient.Create(ctx, &node)).To(Succeed())
+			// Manually create the workspace pod with the node name.
+			// We can't update the pod with the node name, as this operation
+			// is only allowed for the scheduler. So as a hack, we manually
+			// create the workspace's pod.
+			pod := &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       fmt.Sprintf("ws-%s", ws.Name),
+					Namespace:  ws.Namespace,
+					Finalizers: []string{workspacev1.GitpodFinalizerName},
+					Labels: map[string]string{
+						wsk8s.WorkspaceManagedByLabel: constants.ManagedBy,
+					},
+				},
+				Spec: corev1.PodSpec{
+					NodeName: node.Name,
+					Containers: []corev1.Container{{
+						Name:  "workspace",
+						Image: "someimage",
+					}},
+				},
+			}
+
+			Expect(k8sClient.Create(ctx, pod)).To(Succeed())
+			pod = createWorkspaceExpectPod(ws)
+			updateObjWithRetries(k8sClient, pod, false, func(pod *corev1.Pod) {
+				Expect(ctrl.SetControllerReference(ws, pod, k8sClient.Scheme())).To(Succeed())
+			})
+			// mimic the regular "start" phase
+			updateObjWithRetries(k8sClient, ws, true, func(ws *workspacev1.Workspace) {
+				ws.Status.PodStarts = 1
+				ws.Status.PodRecreated = 0
+			})
+
+			// Wait until controller has reconciled at least once (by waiting for the runtime status to get updated).
+			// This is necessary for the metrics to get recorded correctly. If we don't wait, the first reconciliation
+			// might be once the Pod is already in a running state, and hence the metric state might not record e.g. content
+			// restore.
+			// This is only necessary because we manually created the pod, normally the Pod creation is the controller's
+			// first reconciliation which ensures the metrics are recorded from the workspace's initial state.
+
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: ws.Name, Namespace: ws.Namespace}, ws)).To(Succeed())
+				g.Expect(ws.Status.Runtime).ToNot(BeNil())
+				g.Expect(ws.Status.Runtime.PodName).To(Equal(pod.Name))
+			}, timeout, interval).Should(Succeed())
+
+			// Await "deployed" condition, and check we are good
+			expectConditionEventually(ws, string(workspacev1.WorkspaceConditionDeployed), metav1.ConditionTrue, "")
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: ws.Name, Namespace: ws.Namespace}, ws)).To(Succeed())
+				g.Expect(ws.Status.PodStarts).To(Equal(1))
+				g.Expect(ws.Status.PodRecreated).To(Equal(0))
+			}, timeout, interval).Should(Succeed())
+
+			// ### prepare block end
+
+			// ### trigger block start
+			// Make pod be rejected 🪄
+			By("rejecting pod")
+			rejectPod(pod)
+
+			// TODO(gpl): how to check for transient states like:
+			// - pod deletion
+			// - PodRejected condition
+			// By("await pod deleted")
+			// expectWorkspaceCleanup(ws, pod)
+			// Eventually(func(g Gomega) {
+			// 	g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}, pod)).To(MatchError(ContainSubstring("not found")))
+			// }, timeout, interval).Should(Succeed())
+
+			By("await pod recreation")
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: ws.Name, Namespace: ws.Namespace}, ws)).To(Succeed())
+				g.Expect(ws.Status.PodRecreated).To(Equal(1))
+				g.Expect(ws.Status.Phase).To(Equal(workspacev1.WorkspacePhasePending))
+			}, timeout, interval).Should(Succeed())
+			// ### trigger block end
+
+			// ### retry block start
+			// Transition Pod to pending, and expect workspace to reach Creating  phase.
+			// This should also cause create time metrics to be recorded.
+			updateObjWithRetries(k8sClient, pod, true, func(pod *corev1.Pod) {
+				pod.Status.Phase = corev1.PodPending
+				pod.Status.ContainerStatuses = []corev1.ContainerStatus{{
+					State: corev1.ContainerState{
+						Waiting: &corev1.ContainerStateWaiting{
+							Reason: "ContainerCreating",
+						},
+					},
+					Name: "workspace",
+				}}
+			})
+
+			expectPhaseEventually(ws, workspacev1.WorkspacePhaseCreating)
+
+			// Transition Pod to running, and expect workspace to reach Running phase.
+			// This should also cause e.g. startup time metrics to be recorded.
+			updateObjWithRetries(k8sClient, pod, true, func(pod *corev1.Pod) {
+				pod.Status.Phase = corev1.PodRunning
+				pod.Status.ContainerStatuses = []corev1.ContainerStatus{{
+					Name:  "workspace",
+					Ready: true,
+				}}
+			})
+
+			updateObjWithRetries(k8sClient, ws, true, func(ws *workspacev1.Workspace) {
+				ws.Status.SetCondition(workspacev1.NewWorkspaceConditionContentReady(metav1.ConditionTrue, workspacev1.ReasonInitializationSuccess, ""))
+			})
+
+			expectPhaseEventually(ws, workspacev1.WorkspacePhaseRunning)
+			// ### retry block end
+		})
 	})
 
 	Context("with headless workspaces", func() {
@@ -634,6 +755,16 @@ func requestStop(ws *workspacev1.Workspace) {
 	})
 }
 
+func rejectPod(pod *corev1.Pod) {
+	GinkgoHelper()
+	By("adding pod rejected condition")
+	updateObjWithRetries(k8sClient, pod, true, func(pod *corev1.Pod) {
+		pod.Status.Phase = corev1.PodFailed
+		pod.Status.Reason = "NodeAffinity"
+		pod.Status.Message = "Pod was rejected"
+	})
+}
+
 func markReady(ws *workspacev1.Workspace) {
 	GinkgoHelper()
 	By("adding content ready condition")