From 4006d565e13127eed8dc58860d63029888cf017d Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Fri, 30 Apr 2021 15:12:26 -0400 Subject: [PATCH 1/2] Use devfile/api "Error" devworkspacePhase instead of hard-coded string Signed-off-by: Angel Misevski --- controllers/workspace/finalize.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/workspace/finalize.go b/controllers/workspace/finalize.go index d0e8c4ce8..8095628c5 100644 --- a/controllers/workspace/finalize.go +++ b/controllers/workspace/finalize.go @@ -68,7 +68,7 @@ func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, storageProvisioner, err := storage.GetProvisioner(workspace) if err != nil { log.Error(err, "Failed to clean up DevWorkspace storage") - failedStatus := currentStatus{phase: "Error"} + failedStatus := currentStatus{phase: dw.DevWorkspaceStatusError} failedStatus.setConditionTrue(dw.DevWorkspaceError, err.Error()) return r.updateWorkspaceStatus(workspace, r.Log, &failedStatus, reconcile.Result{}, nil) } @@ -85,7 +85,7 @@ func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, return reconcile.Result{RequeueAfter: storageErr.RequeueAfter}, nil case *storage.ProvisioningError: log.Error(storageErr, "Failed to clean up DevWorkspace storage") - failedStatus := currentStatus{phase: "Error"} + failedStatus := currentStatus{phase: dw.DevWorkspaceStatusError} failedStatus.setConditionTrue(dw.DevWorkspaceError, err.Error()) return r.updateWorkspaceStatus(workspace, r.Log, &failedStatus, reconcile.Result{}, nil) default: From a1859b1eca5a42c22f38cda9a0c9069810cb056a Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 11 May 2021 15:10:04 -0400 Subject: [PATCH 2/2] Introduce phase to represent failed workspaces that need to be stopped To avoid issues around starting workspaces with `phase: Failed`, introduce an intermediate phase "Failing" to represent a workspace that has hit a failure condition and needs to be stopped. When the controller encounters a DevWorkspace with `phase: Failing` and `.spec.started: true`, it will stop that workspace and update the phase to `Failed`. Signed-off-by: Angel Misevski --- controllers/workspace/devworkspace_controller.go | 13 ++++++++----- controllers/workspace/finalize.go | 3 --- controllers/workspace/status.go | 10 ++++++++++ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index fd116136a..af22707e2 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -124,7 +124,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct } // Stop failed workspaces - if workspace.Status.Phase == dw.DevWorkspaceStatusFailed && workspace.Spec.Started { + if workspace.Status.Phase == devworkspacePhaseFailing && workspace.Spec.Started { patch := []byte(`{"spec":{"started": false}}`) err := r.Client.Patch(context.Background(), workspace, client.RawPatch(types.MergePatchType, patch)) if err != nil { @@ -329,8 +329,8 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct func (r *DevWorkspaceReconciler) stopWorkspace(workspace *dw.DevWorkspace, logger logr.Logger) (reconcile.Result, error) { status := currentStatus{phase: dw.DevWorkspaceStatusStopping} - if workspace.Status.Phase == dw.DevWorkspaceStatusFailed { - status.phase = dw.DevWorkspaceStatusFailed + if workspace.Status.Phase == devworkspacePhaseFailing || workspace.Status.Phase == dw.DevWorkspaceStatusFailed { + status.phase = workspace.Status.Phase failedCondition := getConditionByType(workspace.Status.Conditions, dw.DevWorkspaceFailedStart) if failedCondition != nil { status.setCondition(dw.DevWorkspaceFailedStart, *failedCondition) @@ -343,7 +343,10 @@ func (r *DevWorkspaceReconciler) stopWorkspace(workspace *dw.DevWorkspace, logge } if stopped { - if status.phase != dw.DevWorkspaceStatusFailed { + switch status.phase { + case devworkspacePhaseFailing, dw.DevWorkspaceStatusFailed: + status.phase = dw.DevWorkspaceStatusFailed + default: status.phase = dw.DevWorkspaceStatusStopped } } @@ -385,7 +388,7 @@ func (r *DevWorkspaceReconciler) doStop(workspace *dw.DevWorkspace, logger logr. // in the main reconcile loop. If needed, changes can be flushed to the cluster immediately via `updateWorkspaceStatus()` func (r *DevWorkspaceReconciler) failWorkspace(workspace *dw.DevWorkspace, msg string, logger logr.Logger, status *currentStatus) (reconcile.Result, error) { logger.Info("DevWorkspace failed to start: " + msg) - status.phase = dw.DevWorkspaceStatusFailed + status.phase = devworkspacePhaseFailing status.setConditionTrue(dw.DevWorkspaceFailedStart, msg) if workspace.Spec.Started { return reconcile.Result{Requeue: true}, nil diff --git a/controllers/workspace/finalize.go b/controllers/workspace/finalize.go index 8095628c5..290d2dc0b 100644 --- a/controllers/workspace/finalize.go +++ b/controllers/workspace/finalize.go @@ -30,9 +30,6 @@ import ( const ( storageCleanupFinalizer = "storage.controller.devfile.io" - // devworkspacePhaseTerminating represents a DevWorkspace that has been deleted but is waiting on a finalizer. - // TODO: Should be moved to devfile/api side. - devworkspacePhaseTerminating dw.DevWorkspacePhase = "Terminating" ) func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace) (reconcile.Result, error) { diff --git a/controllers/workspace/status.go b/controllers/workspace/status.go index 6a9dc13e3..a8ee36bf9 100644 --- a/controllers/workspace/status.go +++ b/controllers/workspace/status.go @@ -31,6 +31,16 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +const ( + // devworkspacePhaseTerminating represents a DevWorkspace that has been deleted but is waiting on a finalizer. + // TODO: Should be moved to devfile/api side. + devworkspacePhaseTerminating dw.DevWorkspacePhase = "Terminating" + + // devworkspacePhaseFailing represents a DevWorkspace that has encountered an unrecoverable error and is in + // the process of stopping. + devworkspacePhaseFailing dw.DevWorkspacePhase = "Failing" +) + type currentStatus struct { workspaceConditions // Current workspace phase