Skip to content

Commit e12f781

Browse files
committed
Fix setting finalizer on workspaces
Fix the check for whether a finalizer is necessary to use the flattened workspace. Otherwise, we may miss some components that need storage (and so need to be finalized) if they are defined as plugins. Signed-off-by: Angel Misevski <[email protected]>
1 parent a69bd20 commit e12f781

File tree

2 files changed

+12
-8
lines changed

2 files changed

+12
-8
lines changed

controllers/workspace/devworkspace_controller.go

+9-7
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,6 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
132132
return r.stopWorkspace(workspace, reqLogger)
133133
}
134134

135-
// Set finalizer on DevWorkspace if necessary
136-
if ok, err := r.setFinalizer(ctx, workspace); err != nil {
137-
return reconcile.Result{}, err
138-
} else if !ok {
139-
return reconcile.Result{Requeue: true}, nil
140-
}
141-
142135
// Prepare handling workspace status and condition
143136
reconcileStatus := currentStatus{
144137
Conditions: map[devworkspace.WorkspaceConditionType]string{},
@@ -194,6 +187,15 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
194187
return reconcile.Result{}, nil
195188
}
196189
workspace.Spec.Template = *flattenedWorkspace
190+
// Set finalizer on DevWorkspace if necessary
191+
// Note: we need to check the flattened workspace to see if a finalizer is needed, as plugins could require storage
192+
if isFinalizerNecessary(workspace) {
193+
if ok, err := r.setFinalizer(ctx, clusterWorkspace); err != nil {
194+
return reconcile.Result{}, err
195+
} else if !ok {
196+
return reconcile.Result{Requeue: true}, nil
197+
}
198+
}
197199

198200
devfilePodAdditions, err := containerlib.GetKubeContainersFromDevfile(workspace.Spec.Template)
199201
if err != nil {

controllers/workspace/finalize.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,10 @@ var (
5151
PVCCleanupPodMemoryLimit = resource.MustParse("32Mi")
5252
)
5353

54+
// setFinalizer sets a finalizer on the workspace and syncs the changes to the cluster. No-op if the workspace already
55+
// has the finalizer set.
5456
func (r *DevWorkspaceReconciler) setFinalizer(ctx context.Context, workspace *v1alpha2.DevWorkspace) (ok bool, err error) {
55-
if !isFinalizerNecessary(workspace) || hasFinalizer(workspace) {
57+
if hasFinalizer(workspace) {
5658
return true, nil
5759
}
5860
workspace.SetFinalizers(append(workspace.Finalizers, pvcCleanupFinalizer))

0 commit comments

Comments
 (0)