Skip to content

Fix starting a workspace once it enters the failed state #424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sleshchenko merged 2 commits into from
May 24, 2021
Merged

Fix starting a workspace once it enters the failed state #424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4006d56
Use devfile/api "Error" devworkspacePhase instead of hard-coded string
amisevsk Apr 30, 2021
a1859b1
Introduce phase to represent failed workspaces that need to be stopped
amisevsk May 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions controllers/workspace/devworkspace_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions controllers/workspace/finalize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -68,7 +65,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)
}
Expand All @@ -85,7 +82,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:
Expand Down
10 changes: 10 additions & 0 deletions controllers/workspace/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down