[ws-daemon, ws-manager] Make sure to cleanup all workspace state for rejected pods

Author: geropl

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

@@ -227,6 +227,45 @@ func (wsc *WorkspaceController) handleWorkspaceStop(ctx context.Context, ws *wor
 	span, ctx := opentracing.StartSpanFromContext(ctx, "handleWorkspaceStop")
 	defer tracing.FinishSpan(span, &err)
 
+	if ws.IsConditionTrue(workspacev1.WorkspaceConditionPodRejected) {
+		setStateWipedCondition := func(s bool) {
+			err := retry.RetryOnConflict(retryParams, func() error {
+				if err := wsc.Get(ctx, req.NamespacedName, ws); err != nil {
+					return err
+				}
+
+				if s {
+					ws.Status.SetCondition(workspacev1.NewWorkspaceConditionStateWiped("", metav1.ConditionTrue))
+				} else {
+					ws.Status.SetCondition(workspacev1.NewWorkspaceConditionStateWiped("", metav1.ConditionFalse))
+				}
+				return wsc.Client.Status().Update(ctx, ws)
+			})
+			if err != nil {
+				log.Error(err, "failed to set StateWiped condition")
+			}
+		}
+		// in this case we are not interested in any backups, but instead are concerned with completely wiping all state that might be dangling somewhere
+		log.Info("handling workspace stop - wiping mode")
+		err = wsc.operations.WipeWorkspace(ctx, ws.Name)
+		if err != nil {
+			setStateWipedCondition(false)
+			wsc.emitEvent(ws, "Wiping", fmt.Errorf("failed to wipe workspace: %w", err))
+			return ctrl.Result{}, fmt.Errorf("failed to wipe workspace: %w", err)
+		}
+
+		setStateWipedCondition(true)
+
+		return ctrl.Result{}, nil
+	}
+
+	// regular case
+	return wsc.doWorkspaceContentBackup(ctx, span, ws, req)
+}
+
+func (wsc *WorkspaceController) doWorkspaceContentBackup(ctx context.Context, span opentracing.Span, ws *workspacev1.Workspace, req ctrl.Request) (result ctrl.Result, err error) {
+	log := log.FromContext(ctx)
+
 	if c := wsk8s.GetCondition(ws.Status.Conditions, string(workspacev1.WorkspaceConditionContentReady)); c == nil || c.Status == metav1.ConditionFalse {
 		return ctrl.Result{}, fmt.Errorf("workspace content was never ready")
 	}

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

@@ -66,6 +66,8 @@ type WorkspaceOperations interface {
 	BackupWorkspace(ctx context.Context, opts BackupOptions) (*csapi.GitStatus, error)
 	// DeleteWorkspace deletes the content of the workspace from disk
 	DeleteWorkspace(ctx context.Context, instanceID string) error
+	// WipeWorkspace deletes all references to the workspace. Does not fail if parts are already gone, or state is incosistent.
+	WipeWorkspace(ctx context.Context, instanceID string) error
 	// SnapshotIDs generates the name and url for a snapshot
 	SnapshotIDs(ctx context.Context, instanceID string) (snapshotUrl, snapshotName string, err error)
 	// Snapshot takes a snapshot of the workspace
@@ -285,6 +287,36 @@ func (wso *DefaultWorkspaceOperations) DeleteWorkspace(ctx context.Context, inst
 	return nil
 }
 
+func (wso *DefaultWorkspaceOperations) WipeWorkspace(ctx context.Context, instanceID string) error {
+	ws, err := wso.provider.GetAndConnect(ctx, instanceID)
+	if err != nil {
+		// we have to assume everything is fine, and this workspace has already been completely wiped
+		return nil
+	}
+
+	if err = ws.Dispose(ctx, wso.provider.hooks[session.WorkspaceDisposed]); err != nil {
+		glog.WithError(err).WithFields(ws.OWI()).Error("cannot dispose session")
+		return err
+	}
+
+	// remove workspace daemon directory in the node
+	if err := os.RemoveAll(ws.ServiceLocDaemon); err != nil {
+		glog.WithError(err).WithFields(ws.OWI()).Error("cannot delete workspace daemon directory")
+		return err
+	}
+
+	// remove workspace daemon node directory in the node
+	// TODO(gpl): Is this used at all? Can't find any reference
+	if err := os.RemoveAll(ws.ServiceLocNode); err != nil {
+		glog.WithError(err).WithFields(ws.OWI()).Error("cannot delete workspace daemon node directory")
+		return err
+	}
+
+	wso.provider.Remove(ctx, instanceID)
+
+	return nil
+}
+
 func (wso *DefaultWorkspaceOperations) SnapshotIDs(ctx context.Context, instanceID string) (snapshotUrl, snapshotName string, err error) {
 	sess, err := wso.provider.GetAndConnect(ctx, instanceID)
 	if err != nil {

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

@@ -267,6 +267,9 @@ const (
 
 	// WorkspaceConditionPodRejected is true if we detected that the pod was rejected by the node
 	WorkspaceConditionPodRejected WorkspaceCondition = "PodRejected"
+
+	// WorkspaceConditionStateWiped is true once all state has successfully been wiped by ws-daemon. This is only set if PodRejected=true, and the rejected workspace has been deleted.
+	WorkspaceConditionStateWiped WorkspaceCondition = "StateWiped"
 )
 
 func NewWorkspaceConditionDeployed() metav1.Condition {
@@ -304,6 +307,15 @@ func NewWorkspaceConditionPodRejected(message string, status metav1.ConditionSta
 	}
 }
 
+func NewWorkspaceConditionStateWiped(message string, status metav1.ConditionStatus) metav1.Condition {
+	return metav1.Condition{
+		Type:               string(WorkspaceConditionStateWiped),
+		LastTransitionTime: metav1.Now(),
+		Status:             status,
+		Message:            message,
+	}
+}
+
 func NewWorkspaceConditionTimeout(message string) metav1.Condition {
 	return metav1.Condition{
 		Type:               string(WorkspaceConditionTimeout),
@@ -498,6 +510,14 @@ func (w *Workspace) IsConditionTrue(condition WorkspaceCondition) bool {
 	return wsk8s.ConditionPresentAndTrue(w.Status.Conditions, string(condition))
 }
 
+func (w *Workspace) GetConditionState(condition WorkspaceCondition) (state metav1.ConditionStatus, ok bool) {
+	cond := wsk8s.GetCondition(w.Status.Conditions, string(condition))
+	if cond == nil {
+		return "", false
+	}
+	return cond.Status, true
+}
+
 // UpsertConditionOnStatusChange calls SetCondition if the condition does not exist or it's status or message has changed.
 func (w *Workspace) UpsertConditionOnStatusChange(newCondition metav1.Condition) {
 	oldCondition := wsk8s.GetCondition(w.Status.Conditions, newCondition.Type)

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

@@ -284,6 +284,15 @@ func (r *WorkspaceReconciler) checkNodeDisappeared(ctx context.Context, workspac
 }
 
 func isDisposalFinished(ws *workspacev1.Workspace) bool {
+	if ws.IsConditionTrue(workspacev1.WorkspaceConditionPodRejected) {
+		if c := wsk8s.GetCondition(ws.Status.Conditions, string(workspacev1.WorkspaceConditionStateWiped)); c != nil {
+			// If the condition is set, we are done with the disposal
+			return true
+		}
+		// If the condition has not yet been set, we are not done, yet.
+		return false
+	}
+
 	return ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupComplete) ||
 		ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupFailure) ||
 		ws.IsConditionTrue(workspacev1.WorkspaceConditionAborted) ||
@@ -323,6 +332,17 @@ func (r *WorkspaceReconciler) extractFailure(ctx context.Context, ws *workspacev
 		return msg, nil
 	}
 
+	// Check for state wiping failure.
+	if c := wsk8s.GetCondition(ws.Status.Conditions, string(workspacev1.WorkspaceConditionStateWiped)); c != nil && c.Status == metav1.ConditionFalse {
+		msg := c.Message
+		if msg == "" {
+			msg = "Wiping workspace state failed for an unknown reason"
+		} else {
+			msg = fmt.Sprintf("Wiping workspace state failed: %s", msg)
+		}
+		return msg, nil
+	}
+
 	status := pod.Status
 	if status.Phase == corev1.PodFailed && (status.Reason != "" || status.Message != "") {
 		// Don't force the phase to UNKNONWN here to leave a chance that we may detect the actual phase of

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

@@ -224,6 +224,7 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
 				workspace.Status.SetCondition(workspacev1.NewWorkspaceConditionPodRejected(fmt.Sprintf("Pod reached maximum recreations %d, failing", workspace.Status.PodRecreated), metav1.ConditionFalse))
 				return ctrl.Result{Requeue: true}, nil // requeue so we end up in the "Stopped" case below
 			}
+			log.WithValues("PodStarts", workspace.Status.PodStarts, "PodRecreated", workspace.Status.PodRecreated, "Phase", workspace.Status.Phase).Info("trigger pod recreation")
 
 			// Must persist the modification pod starts, and ensure we retry on conflict.
 			// If we fail to persist this value, it's possible that the Pod gets recreated endlessly

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

@@ -461,14 +461,17 @@ var _ = Describe("WorkspaceController", func() {
 			By("rejecting pod")
 			rejectPod(pod)
 
-			// TODO(gpl): how to check for transient states like:
-			// - pod deletion
-			// - PodRejected condition
-			// By("await pod deleted")
-			// expectWorkspaceCleanup(ws, pod)
-			// Eventually(func(g Gomega) {
-			// 	g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}, pod)).To(MatchError(ContainSubstring("not found")))
-			// }, timeout, interval).Should(Succeed())
+			By("await pod being in stopping")
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: ws.Name, Namespace: ws.Namespace}, ws)).To(Succeed())
+				g.Expect(ws.Status.Phase).To(Equal(workspacev1.WorkspacePhaseStopping))
+			}, timeout, interval).Should(Succeed())
+
+			// when a rejected workspace pod is in stopping, ws-daemon wipes the state before it's moved to "stopped"
+			// mimic this ws-daemon behavior
+			updateObjWithRetries(k8sClient, ws, true, func(ws *workspacev1.Workspace) {
+				ws.Status.SetCondition(workspacev1.NewWorkspaceConditionStateWiped("", metav1.ConditionTrue))
+			})
 
 			By("await pod recreation")
 			Eventually(func(g Gomega) {