[wsman-mk2] Simplify condition checking (#17961)

Author: Furisto

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

@@ -235,15 +235,15 @@ func (wsc *WorkspaceController) handleWorkspaceStop(ctx context.Context, ws *wor
 		return ctrl.Result{}, fmt.Errorf("workspace content was never ready")
 	}
 
-	if wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionBackupComplete)) {
+	if ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupComplete) {
 		return ctrl.Result{}, nil
 	}
 
-	if wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionBackupFailure)) {
+	if ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupFailure) {
 		return ctrl.Result{}, nil
 	}
 
-	if wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionAborted)) {
+	if ws.IsConditionTrue(workspacev1.WorkspaceConditionAborted) {
 		span.LogKV("event", "workspace was aborted")
 		return ctrl.Result{}, nil
 	}

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

@@ -433,6 +433,10 @@ func (w *Workspace) IsHeadless() bool {
 	return w.Spec.Type != WorkspaceTypeRegular
 }
 
+func (w *Workspace) IsConditionTrue(condition WorkspaceCondition) bool {
+	return wsk8s.ConditionPresentAndTrue(w.Status.Conditions, string(condition))
+}
+
 func init() {
 	SchemeBuilder.Register(&Workspace{}, &WorkspaceList{})
 }

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

@@ -164,17 +164,17 @@ func (m *controllerMetrics) countWorkspaceStop(log *logr.Logger, ws *workspacev1
 	var reason string
 	if c := wsk8s.GetCondition(ws.Status.Conditions, string(workspacev1.WorkspaceConditionFailed)); c != nil {
 		reason = StopReasonFailed
-		if !wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionEverReady)) {
+		if !ws.IsConditionTrue(workspacev1.WorkspaceConditionEverReady) {
 			// Don't record 'failed' if there was a start failure.
 			reason = StopReasonStartFailure
 		} else if strings.Contains(c.Message, "Pod ephemeral local storage usage exceeds the total limit of containers") {
 			reason = StopReasonOutOfSpace
 		}
-	} else if wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionAborted)) {
+	} else if ws.IsConditionTrue(workspacev1.WorkspaceConditionAborted) {
 		reason = StopReasonAborted
-	} else if wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionTimeout)) {
+	} else if ws.IsConditionTrue(workspacev1.WorkspaceConditionTimeout) {
 		reason = StopReasonTimeout
-	} else if wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionClosed)) {
+	} else if ws.IsConditionTrue(workspacev1.WorkspaceConditionClosed) {
 		reason = StopReasonTabClosed
 	} else {
 		reason = StopReasonRegular
@@ -278,10 +278,10 @@ func newMetricState(ws *workspacev1.Workspace) metricState {
 		recordedStartTime:       ws.Status.Phase == workspacev1.WorkspacePhaseRunning,
 		recordedInitFailure:     wsk8s.ConditionWithStatusAndReason(ws.Status.Conditions, string(workspacev1.WorkspaceConditionContentReady), false, workspacev1.ReasonInitializationFailure),
 		recordedStartFailure:    ws.Status.Phase == workspacev1.WorkspacePhaseStopped && isStartFailure(ws),
-		recordedFailure:         wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionFailed)),
-		recordedContentReady:    wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionContentReady)),
-		recordedBackupFailed:    wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionBackupFailure)),
-		recordedBackupCompleted: wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionBackupComplete)),
+		recordedFailure:         ws.IsConditionTrue(workspacev1.WorkspaceConditionFailed),
+		recordedContentReady:    ws.IsConditionTrue(workspacev1.WorkspaceConditionContentReady),
+		recordedBackupFailed:    ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupFailure),
+		recordedBackupCompleted: ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupComplete),
 	}
 }
 

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

@@ -109,14 +109,14 @@ func (r *WorkspaceReconciler) updateWorkspaceStatus(ctx context.Context, workspa
 		workspace.Status.Phase = *phase
 	}
 
-	if failure != "" && !wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionFailed)) {
+	if failure != "" && !workspace.IsConditionTrue(workspacev1.WorkspaceConditionFailed) {
 		// workspaces can fail only once - once there is a failed condition set, stick with it
 		log.Info("workspace failed", "workspace", workspace.Name, "reason", failure)
 		workspace.Status.SetCondition(workspacev1.NewWorkspaceConditionFailed(failure))
 		r.Recorder.Event(workspace, corev1.EventTypeWarning, "Failed", failure)
 	}
 
-	if workspace.IsHeadless() && !wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionsHeadlessTaskFailed)) {
+	if workspace.IsHeadless() && !workspace.IsConditionTrue(workspacev1.WorkspaceConditionsHeadlessTaskFailed) {
 		for _, cs := range pod.Status.ContainerStatuses {
 			if cs.State.Terminated != nil && cs.State.Terminated.Message != "" {
 				workspace.Status.SetCondition(workspacev1.NewWorkspaceConditionHeadlessTaskFailed(cs.State.Terminated.Message))
@@ -156,14 +156,14 @@ func (r *WorkspaceReconciler) updateWorkspaceStatus(ctx context.Context, workspa
 		}
 
 	case pod.Status.Phase == corev1.PodRunning:
-		everReady := wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionEverReady))
+		everReady := workspace.IsConditionTrue(workspacev1.WorkspaceConditionEverReady)
 		if everReady {
 			// If the workspace has been ready before, stay in a Running state, even
 			// if the workspace container is not ready anymore. This is to avoid the workspace
 			// moving back to Initializing and becoming unusable.
 			workspace.Status.Phase = workspacev1.WorkspacePhaseRunning
 		} else {
-			contentReady := wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionContentReady))
+			contentReady := workspace.IsConditionTrue(workspacev1.WorkspaceConditionContentReady)
 			var ideReady bool
 			for _, cs := range pod.Status.ContainerStatuses {
 				if cs.Ready {
@@ -176,7 +176,7 @@ func (r *WorkspaceReconciler) updateWorkspaceStatus(ctx context.Context, workspa
 			if ready {
 				// workspace is ready - hence content init is done
 				workspace.Status.Phase = workspacev1.WorkspacePhaseRunning
-				if !wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionEverReady)) {
+				if !workspace.IsConditionTrue(workspacev1.WorkspaceConditionEverReady) {
 					workspace.Status.SetCondition(workspacev1.NewWorkspaceConditionEverReady())
 				}
 			} else {
@@ -186,7 +186,7 @@ func (r *WorkspaceReconciler) updateWorkspaceStatus(ctx context.Context, workspa
 		}
 
 	case workspace.IsHeadless() && (pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed):
-		if pod.Status.Phase == corev1.PodSucceeded && !wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionEverReady)) {
+		if pod.Status.Phase == corev1.PodSucceeded && !workspace.IsConditionTrue(workspacev1.WorkspaceConditionEverReady) {
 			// Fix for Prebuilds that instantly succeed (e.g. empty task), sometimes we don't observe the
 			// workspace `Running` phase for these, and never had the opportunity to add the EverReady condition.
 			// This would then cause a "start failure" in the metrics. So we retroactively add the EverReady
@@ -229,7 +229,7 @@ func (r *WorkspaceReconciler) checkNodeDisappeared(ctx context.Context, workspac
 	}
 
 	// If NodeDisappeared is already set, return early, we've already made the below checks previously.
-	if wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionNodeDisappeared)) {
+	if workspace.IsConditionTrue(workspacev1.WorkspaceConditionNodeDisappeared) {
 		return nil
 	}
 
@@ -245,17 +245,17 @@ func (r *WorkspaceReconciler) checkNodeDisappeared(ctx context.Context, workspac
 }
 
 func isDisposalFinished(ws *workspacev1.Workspace) bool {
-	return wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionBackupComplete)) ||
-		wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionBackupFailure)) ||
-		wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionAborted)) ||
+	return ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupComplete) ||
+		ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupFailure) ||
+		ws.IsConditionTrue(workspacev1.WorkspaceConditionAborted) ||
 		// Nothing to dispose if content wasn't ready.
-		!wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionContentReady)) ||
+		!ws.IsConditionTrue(workspacev1.WorkspaceConditionContentReady) ||
 		// Can't dispose if node disappeared.
-		wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionNodeDisappeared)) ||
+		ws.IsConditionTrue(workspacev1.WorkspaceConditionNodeDisappeared) ||
 		// Image builds have nothing to dispose.
 		ws.Spec.Type == workspacev1.WorkspaceTypeImageBuild ||
 		// headless workspaces that failed do not need to be backed up
-		wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionsHeadlessTaskFailed))
+		ws.IsConditionTrue(workspacev1.WorkspaceConditionsHeadlessTaskFailed)
 }
 
 // extractFailure returns a pod failure reason and possibly a phase. If phase is nil then

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

@@ -19,7 +19,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
-	wsk8s "github.com/gitpod-io/gitpod/common-go/kubernetes"
 	"github.com/gitpod-io/gitpod/common-go/util"
 	wsactivity "github.com/gitpod-io/gitpod/ws-manager-mk2/pkg/activity"
 	"github.com/gitpod-io/gitpod/ws-manager-mk2/pkg/maintenance"
@@ -81,7 +80,7 @@ func (r *TimeoutReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionTimeout)) {
+	if workspace.IsConditionTrue(workspacev1.WorkspaceConditionTimeout) {
 		// Workspace has already been marked as timed out.
 		// Return and don't requeue another reconciliation.
 		return ctrl.Result{}, nil
@@ -159,7 +158,7 @@ func (r *TimeoutReconciler) isWorkspaceTimedOut(ws *workspacev1.Workspace) (reas
 
 	start := ws.ObjectMeta.CreationTimestamp.Time
 	lastActivity := r.activity.GetLastActivity(ws)
-	isClosed := wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionClosed))
+	isClosed := ws.IsConditionTrue(workspacev1.WorkspaceConditionClosed)
 
 	switch phase {
 	case workspacev1.WorkspacePhasePending:
@@ -213,7 +212,7 @@ func (r *TimeoutReconciler) isWorkspaceTimedOut(ws *workspacev1.Workspace) (reas
 		return decide(*lastActivity, timeout, activity)
 
 	case workspacev1.WorkspacePhaseStopping:
-		if isWorkspaceBeingDeleted(ws) && !wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionBackupComplete)) {
+		if isWorkspaceBeingDeleted(ws) && !ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupComplete) {
 			// Beware: we apply the ContentFinalization timeout only to workspaces which are currently being deleted.
 			//         We basically don't expect a workspace to be in content finalization before it's been deleted.
 			return decide(ws.DeletionTimestamp.Time, timeouts.ContentFinalization, activityBackup)

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

@@ -241,11 +241,11 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
 
 	switch {
 	// if there is a pod, and it's failed, delete it
-	case wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionFailed)) && !isPodBeingDeleted(pod):
+	case workspace.IsConditionTrue(workspacev1.WorkspaceConditionFailed) && !isPodBeingDeleted(pod):
 		return r.deleteWorkspacePod(ctx, pod, "workspace failed")
 
 	// if the pod was stopped by request, delete it
-	case wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionStoppedByRequest)) && !isPodBeingDeleted(pod):
+	case workspace.IsConditionTrue(workspacev1.WorkspaceConditionStoppedByRequest) && !isPodBeingDeleted(pod):
 		var gracePeriodSeconds *int64
 		if c := wsk8s.GetCondition(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionStoppedByRequest)); c != nil {
 			if dt, err := time.ParseDuration(c.Message); err == nil {
@@ -263,11 +263,11 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
 		}
 
 	// if the node disappeared, delete the pod.
-	case wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionNodeDisappeared)) && !isPodBeingDeleted(pod):
+	case workspace.IsConditionTrue(workspacev1.WorkspaceConditionNodeDisappeared) && !isPodBeingDeleted(pod):
 		return r.deleteWorkspacePod(ctx, pod, "node disappeared")
 
 	// if the workspace timed out, delete it
-	case wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionTimeout)) && !isPodBeingDeleted(pod):
+	case workspace.IsConditionTrue(workspacev1.WorkspaceConditionTimeout) && !isPodBeingDeleted(pod):
 		return r.deleteWorkspacePod(ctx, pod, "timed out")
 
 	// if the content initialization failed, delete the pod
@@ -323,23 +323,23 @@ func (r *WorkspaceReconciler) updateMetrics(ctx context.Context, workspace *work
 		lastState.recordedInitFailure = true
 	}
 
-	if !lastState.recordedFailure && wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionFailed)) {
+	if !lastState.recordedFailure && workspace.IsConditionTrue(workspacev1.WorkspaceConditionFailed) {
 		r.metrics.countWorkspaceFailure(&log, workspace)
 		lastState.recordedFailure = true
 	}
 
-	if !lastState.recordedContentReady && wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionContentReady)) {
+	if !lastState.recordedContentReady && workspace.IsConditionTrue(workspacev1.WorkspaceConditionContentReady) {
 		r.metrics.countTotalRestores(&log, workspace)
 		lastState.recordedContentReady = true
 	}
 
-	if !lastState.recordedBackupFailed && wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionBackupFailure)) {
+	if !lastState.recordedBackupFailed && workspace.IsConditionTrue(workspacev1.WorkspaceConditionBackupFailure) {
 		r.metrics.countTotalBackups(&log, workspace)
 		r.metrics.countTotalBackupFailures(&log, workspace)
 		lastState.recordedBackupFailed = true
 	}
 
-	if !lastState.recordedBackupCompleted && wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionBackupComplete)) {
+	if !lastState.recordedBackupCompleted && workspace.IsConditionTrue(workspacev1.WorkspaceConditionBackupComplete) {
 		r.metrics.countTotalBackups(&log, workspace)
 		lastState.recordedBackupCompleted = true
 	}
@@ -368,12 +368,12 @@ func (r *WorkspaceReconciler) updateMetrics(ctx context.Context, workspace *work
 
 func isStartFailure(ws *workspacev1.Workspace) bool {
 	// Consider workspaces that never became ready as start failures.
-	everReady := wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionEverReady))
+	everReady := ws.IsConditionTrue(workspacev1.WorkspaceConditionEverReady)
 	// Except for aborted prebuilds, as they can get aborted before becoming ready, which shouldn't be counted
 	// as a start failure.
-	isAborted := wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionAborted))
+	isAborted := ws.IsConditionTrue(workspacev1.WorkspaceConditionAborted)
 	// Also ignore workspaces that are requested to be stopped before they became ready.
-	isStoppedByRequest := wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionStoppedByRequest))
+	isStoppedByRequest := ws.IsConditionTrue(workspacev1.WorkspaceConditionStoppedByRequest)
 	return !everReady && !isAborted && !isStoppedByRequest
 }
 

components/ws-manager-mk2/pkg/activity/activity.go

@@ -8,7 +8,6 @@ import (
 	"sync"
 	"time"
 
-	wsk8s "github.com/gitpod-io/gitpod/common-go/kubernetes"
 	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
 )
 
@@ -38,7 +37,7 @@ func (w *WorkspaceActivity) GetLastActivity(ws *workspacev1.Workspace) *time.Tim
 
 	// In case we don't have a record of the workspace's last activity, check for the FirstUserActivity condition
 	// to see if the lastActivity got lost on a manager restart.
-	if wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionFirstUserActivity)) {
+	if ws.IsConditionTrue(workspacev1.WorkspaceConditionFirstUserActivity) {
 		// Manager was restarted, consider the workspace's last activity to be the time the manager restarted.
 		return &w.ManagerStartedAt
 	}

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

@@ -495,7 +495,7 @@ func (wsm *WorkspaceManagerServer) MarkActive(ctx context.Context, req *wsmanapi
 
 	// 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))
+	isMarkedClosed := ws.IsConditionTrue(workspacev1.WorkspaceConditionClosed)
 	if req.Closed && !isMarkedClosed {
 		err = wsm.modifyWorkspace(ctx, req.Id, true, func(ws *workspacev1.Workspace) error {
 			ws.Status.SetCondition(workspacev1.NewWorkspaceConditionClosed(metav1.ConditionTrue, "MarkActiveRequest"))