[ws-manager-mk2] Fix race where pod gets recreated in Stopped phase (#16622)

* [ws-manager-mk2] Fix race where pod gets recreated in Stopped phase

* [ws-manager-mk2] Add pod creation logs

* Change to Patch

Author: WVerlaek

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

@@ -708,9 +708,6 @@ func newStartWorkspaceContext(ctx context.Context, cfg *config.Configuration, ws
 	span, _ := tracing.FromContext(ctx, "newStartWorkspaceContext")
 	defer tracing.FinishSpan(span, &err)
 
-	// Can't read ws.Status yet at this point (to e.g. get the headless status),
-	// as it very likely hasn't been set yet by the controller.
-	isHeadless := ws.Spec.Type != workspacev1.WorkspaceTypeRegular
 	return &startWorkspaceContext{
 		Labels: map[string]string{
 			"app":                  "gitpod",
@@ -720,13 +717,13 @@ func newStartWorkspaceContext(ctx context.Context, cfg *config.Configuration, ws
 			wsk8s.OwnerLabel:       ws.Spec.Ownership.Owner,
 			wsk8s.TypeLabel:        strings.ToLower(string(ws.Spec.Type)),
 			instanceIDLabel:        ws.Name,
-			headlessLabel:          strconv.FormatBool(isHeadless),
+			headlessLabel:          strconv.FormatBool(ws.IsHeadless()),
 		},
 		Config:         cfg,
 		Workspace:      ws,
 		IDEPort:        23000,
 		SupervisorPort: 22999,
-		Headless:       isHeadless,
+		Headless:       ws.IsHeadless(),
 	}, nil
 }
 

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

@@ -107,17 +107,17 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	r.updateMetrics(ctx, &workspace)
 
-	result, err := r.actOnStatus(ctx, &workspace, workspacePods)
-	if err != nil {
-		return result, err
-	}
-
 	err = r.Status().Update(ctx, &workspace)
 	if err != nil {
 		// log.WithValues("status", workspace).Error(err, "unable to update workspace status")
 		return ctrl.Result{Requeue: true}, err
 	}
 
+	result, err := r.actOnStatus(ctx, &workspace, workspacePods)
+	if err != nil {
+		return result, err
+	}
+
 	if r.OnReconcile != nil {
 		r.OnReconcile(ctx, &workspace)
 	}
@@ -148,16 +148,28 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
 				return ctrl.Result{}, err
 			}
 
+			log.Info("creating workspace Pod for Workspace")
 			err = r.Create(ctx, pod)
 			if errors.IsAlreadyExists(err) {
 				// pod exists, we're good
+				log.Info("Workspace Pod already exists")
 			} else if err != nil {
 				log.Error(err, "unable to create Pod for Workspace", "pod", pod)
 				return ctrl.Result{Requeue: true}, err
 			} else {
 				// TODO(cw): replicate the startup mechanism where pods can fail to be scheduled,
 				//			 need to be deleted and re-created
+				// Must increment and persist the pod starts, and ensure we retry on conflict.
+				// If we fail to persist this value, it's possible that the Pod gets recreated
+				// 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++
+				if err := r.Status().Patch(ctx, workspace, patch); err != nil {
+					log.Error(err, "Failed to patch PodStarts in workspace status")
+					return ctrl.Result{}, err
+				}
 			}
 			r.metrics.rememberWorkspace(workspace, nil)
 

operations/observability/mixins/workspace/dashboards/components/ws-manager-mk2.json

@@ -3485,7 +3485,7 @@
       }
     }
   ],
-  "refresh": "5s",
+  "refresh": "30s",
   "schemaVersion": 37,
   "style": "dark",
   "tags": [
@@ -3497,10 +3497,10 @@
         "current": {
           "selected": true,
           "text": [
-            "ephemeral-wv"
+            "All"
           ],
           "value": [
-            "ephemeral-wv"
+            "$__all"
           ]
         },
         "datasource": {