[wsman-mk2] Refactor conditions

Author: Furisto

components/ws-daemon/pkg/controller/workspace_controller.go

@@ -170,20 +170,9 @@ func (wsc *WorkspaceController) handleWorkspaceInit(ctx context.Context, ws *wor
 
 			if failure != "" {
 				log.Error(initErr, "could not initialize workspace", "name", ws.Name)
-				ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-					Type:               string(workspacev1.WorkspaceConditionContentReady),
-					Status:             metav1.ConditionFalse,
-					Message:            failure,
-					Reason:             "InitializationFailure",
-					LastTransitionTime: metav1.Now(),
-				})
+				ws.Status.SetCondition(workspacev1.NewWorkspaceConditionContentReady(metav1.ConditionFalse, "InitializationFailure", failure))
 			} else {
-				ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-					Type:               string(workspacev1.WorkspaceConditionContentReady),
-					Status:             metav1.ConditionTrue,
-					Reason:             "InitializationSuccess",
-					LastTransitionTime: metav1.Now(),
-				})
+				ws.Status.SetCondition(workspacev1.NewWorkspaceConditionContentReady(metav1.ConditionTrue, "InitializationSuccess", ""))
 			}
 
 			return wsc.Status().Update(ctx, ws)
@@ -276,20 +265,9 @@ func (wsc *WorkspaceController) handleWorkspaceStop(ctx context.Context, ws *wor
 
 		if disposeErr != nil {
 			log.Error(disposeErr, "failed to dispose workspace", "name", ws.Name)
-			ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-				Type:               string(workspacev1.WorkspaceConditionBackupFailure),
-				Status:             metav1.ConditionTrue,
-				Reason:             "BackupFailed",
-				Message:            disposeErr.Error(),
-				LastTransitionTime: metav1.Now(),
-			})
+			ws.Status.SetCondition(workspacev1.NewWorkspaceConditionBackupFailure(disposeErr.Error()))
 		} else {
-			ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-				Type:               string(workspacev1.WorkspaceConditionBackupComplete),
-				Status:             metav1.ConditionTrue,
-				Reason:             "BackupComplete",
-				LastTransitionTime: metav1.Now(),
-			})
+			ws.Status.SetCondition(workspacev1.NewWorkspaceConditionBackupComplete())
 		}
 
 		return wsc.Status().Update(ctx, ws)

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

@@ -5,6 +5,7 @@
 package v1
 
 import (
+	wsk8s "github.com/gitpod-io/gitpod/common-go/kubernetes"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -140,6 +141,10 @@ type WorkspaceStatus struct {
 	Runtime *WorkspaceRuntimeStatus `json:"runtime,omitempty"`
 }
 
+func (s *WorkspaceStatus) SetCondition(cond metav1.Condition) {
+	s.Conditions = wsk8s.AddUniqueCondition(s.Conditions, cond)
+}
+
 // +kubebuilder:validation:Enum=Deployed;Failed;Timeout;FirstUserActivity;Closed;HeadlessTaskFailed;StoppedByRequest;Aborted;ContentReady;BackupComplete;BackupFailure
 type WorkspaceCondition string
 
@@ -180,6 +185,99 @@ const (
 	WorkspaceConditionBackupFailure WorkspaceCondition = "BackupFailure"
 )
 
+func NewWorkspaceConditionDeployed() metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionDeployed),
+		LastTransitionTime: metav1.Now(),
+		Status:             metav1.ConditionTrue,
+	}
+}
+
+func NewWorkspaceConditionFailed(message string) metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionFailed),
+		LastTransitionTime: metav1.Now(),
+		Status:             metav1.ConditionTrue,
+		Message:            message,
+	}
+}
+
+func NewWorkspaceConditionTimeout(message string) metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionTimeout),
+		LastTransitionTime: metav1.Now(),
+		Status:             metav1.ConditionTrue,
+		Reason:             "TimedOut",
+		Message:            message,
+	}
+}
+
+func NewWorkspaceConditionFirstUserActivity() metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionFirstUserActivity),
+		LastTransitionTime: metav1.Now(),
+		Status:             metav1.ConditionTrue,
+		Reason:             "MarkActiveRequest",
+	}
+}
+
+func NewWorkspaceConditionClosed(status metav1.ConditionStatus) metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionClosed),
+		LastTransitionTime: metav1.Now(),
+		Status:             status,
+		Reason:             "MarkActiveRequest",
+	}
+}
+
+func NewWorkspaceConditionStoppedByRequest(message string) metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionStoppedByRequest),
+		LastTransitionTime: metav1.Now(),
+		Status:             metav1.ConditionTrue,
+		Reason:             "StopWorkspaceRequest",
+		Message:            message,
+	}
+}
+
+func NewWorkspaceConditionAborted(reason string) metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionAborted),
+		LastTransitionTime: metav1.Now(),
+		Status:             metav1.ConditionTrue,
+		Reason:             reason,
+	}
+}
+
+func NewWorkspaceConditionContentReady(status metav1.ConditionStatus, reason, message string) metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionContentReady),
+		LastTransitionTime: metav1.Now(),
+		Status:             status,
+		Reason:             reason,
+		Message:            message,
+	}
+}
+
+func NewWorkspaceConditionBackupComplete() metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionBackupComplete),
+		LastTransitionTime: metav1.Now(),
+		Status:             metav1.ConditionTrue,
+		Reason:             "BackupComplete",
+	}
+}
+
+func NewWorkspaceConditionBackupFailure(message string) metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionBackupFailure),
+		LastTransitionTime: metav1.Now(),
+		Status:             metav1.ConditionTrue,
+		Reason:             "BackupFailed",
+		Message:            message,
+	}
+}
+
 // +kubebuilder:validation:Enum:=Unknown;Pending;Imagebuild;Creating;Initializing;Running;Stopping;Stopped
 type WorkspacePhase string
 

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

@@ -48,22 +48,13 @@ func updateWorkspaceStatus(ctx context.Context, workspace *workspacev1.Workspace
 		// continue below
 	default:
 		// This is exceptional - not sure what to do here. Probably fail the pod
-		workspace.Status.Conditions = wsk8s.AddUniqueCondition(workspace.Status.Conditions, metav1.Condition{
-			Type:               string(workspacev1.WorkspaceConditionFailed),
-			Status:             metav1.ConditionTrue,
-			LastTransitionTime: metav1.Now(),
-			Message:            "multiple pods exists - this should never happen",
-		})
-
+		workspace.Status.SetUniqueCondition(
+			workspacev1.NewWorkspaceConditionFailed("multiple pods exists - this should never happen"))
 		return nil
 	}
 
 	if c := wsk8s.GetCondition(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionDeployed)); c == nil {
-		workspace.Status.Conditions = wsk8s.AddUniqueCondition(workspace.Status.Conditions, metav1.Condition{
-			Type:               string(workspacev1.WorkspaceConditionDeployed),
-			Status:             metav1.ConditionTrue,
-			LastTransitionTime: metav1.Now(),
-		})
+		workspace.Status.SetUniqueCondition(workspacev1.NewWorkspaceConditionDeployed())
 	}
 
 	pod := &pods.Items[0]

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

@@ -103,14 +103,7 @@ func (r *TimeoutReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
 			return err
 		}
 
-		workspace.Status.Conditions = wsk8s.AddUniqueCondition(workspace.Status.Conditions, metav1.Condition{
-			Type:               string(workspacev1.WorkspaceConditionTimeout),
-			Status:             metav1.ConditionTrue,
-			LastTransitionTime: metav1.Now(),
-			Reason:             "TimedOut",
-			Message:            timedout,
-		})
-
+		workspace.Status.SetUniqueCondition(workspacev1.NewWorkspaceConditionTimeout(metav1.ConditionTrue, timedout))
 		return r.Status().Update(ctx, &workspace)
 	}); err != nil {
 		log.Error(err, "Failed to update workspace status with Timeout condition")

components/ws-manager-mk2/service/manager.go

@@ -356,25 +356,14 @@ func (wsm *WorkspaceManagerServer) StopWorkspace(ctx context.Context, req *wsman
 		span.LogKV("policy", "abort")
 		gracePeriod = stopWorkspaceImmediatelyGracePeriod
 		if err = wsm.modifyWorkspace(ctx, req.Id, true, func(ws *workspacev1.Workspace) error {
-			ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-				Type:               string(workspacev1.WorkspaceConditionAborted),
-				Status:             metav1.ConditionTrue,
-				Reason:             "StopWorkspaceRequest",
-				LastTransitionTime: metav1.Now(),
-			})
+			ws.Status.SetCondition(workspacev1.NewWorkspaceConditionAborted("StopWorkspaceRequest"))
 			return nil
 		}); err != nil {
 			log.Error(err, "failed to add Aborted condition to workspace")
 		}
 	}
 	err = wsm.modifyWorkspace(ctx, req.Id, true, func(ws *workspacev1.Workspace) error {
-		ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-			Type:               string(workspacev1.WorkspaceConditionStoppedByRequest),
-			Status:             metav1.ConditionTrue,
-			LastTransitionTime: metav1.Now(),
-			Message:            gracePeriod.String(),
-			Reason:             "StopWorkspaceRequest",
-		})
+		ws.Status.SetCondition(workspacev1.NewWorkspaceConditionStoppedByRequest(gracePeriod.String()))
 		return nil
 	})
 	if err != nil {
@@ -473,27 +462,17 @@ func (wsm *WorkspaceManagerServer) MarkActive(ctx context.Context, req *wsmanapi
 	now := time.Now().UTC()
 	wsm.activity.Store(req.Id, now)
 
-	// We do however maintain the the "closed" flag as annotation on the workspace. This flag should not change
+	// We do however maintain the the "closed" flag as condition on the workspace. This flag should not change
 	// very often and provides a better UX if it persists across ws-manager restarts.
 	isMarkedClosed := wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionClosed))
 	if req.Closed && !isMarkedClosed {
 		err = wsm.modifyWorkspace(ctx, req.Id, true, func(ws *workspacev1.Workspace) error {
-			ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-				Type:               string(workspacev1.WorkspaceConditionClosed),
-				Status:             metav1.ConditionTrue,
-				LastTransitionTime: metav1.NewTime(now),
-				Reason:             "MarkActiveRequest",
-			})
+			ws.Status.SetUniqueCondition(workspacev1.NewWorkspaceConditionClosed(metav1.ConditionTrue))
 			return nil
 		})
 	} else if !req.Closed && isMarkedClosed {
 		err = wsm.modifyWorkspace(ctx, req.Id, true, func(ws *workspacev1.Workspace) error {
-			ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-				Type:               string(workspacev1.WorkspaceConditionClosed),
-				Status:             metav1.ConditionFalse,
-				LastTransitionTime: metav1.NewTime(now),
-				Reason:             "MarkActiveRequest",
-			})
+			ws.Status.SetUniqueCondition(workspacev1.NewWorkspaceConditionClosed(metav1.ConditionFalse))
 			return nil
 		})
 	}
@@ -508,12 +487,7 @@ func (wsm *WorkspaceManagerServer) MarkActive(ctx context.Context, req *wsmanapi
 	// If it's the first call: Mark the pod with FirstUserActivity condition.
 	if firstUserActivity == nil {
 		err := wsm.modifyWorkspace(ctx, req.Id, true, func(ws *workspacev1.Workspace) error {
-			ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-				Type:               string(workspacev1.WorkspaceConditionFirstUserActivity),
-				Status:             metav1.ConditionTrue,
-				LastTransitionTime: metav1.NewTime(now),
-				Reason:             "MarkActiveRequest",
-			})
+			ws.Status.SetUniqueCondition(workspacev1.NewWorkspaceConditionFirstUserActivity())
 			return nil
 		})
 		if err != nil {