Skip to content

Commit a7db052

Browse files
sleshchenkoamisevsk
authored andcommitted
Do nothing on storage finalizing when namespace is terminating or PVC does not exist
Fix error when namespace is not terminated in time Requeue storage finalizer each 10 seconds after job is flushed Wait until devworkspace CRs are removed on uninstall
1 parent ea49717 commit a7db052

File tree

4 files changed

+62
-6
lines changed

4 files changed

+62
-6
lines changed

Makefile

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,10 +211,9 @@ restart_webhook:
211211
uninstall: _kustomize
212212
# It's safer to delete all workspaces before deleting the controller; otherwise we could
213213
# leave workspaces in a hanging state if we add finalizers.
214-
$(K8S_CLI) delete devworkspaces.workspace.devfile.io --all-namespaces --all | true
215-
$(K8S_CLI) delete devworkspacetemplates.workspace.devfile.io --all-namespaces --all | true
216-
# Have to wait for routings to be deleted in case there are finalizers
217-
$(K8S_CLI) delete workspaceroutings.controller.devfile.io --all-namespaces --all --wait | true
214+
$(K8S_CLI) delete devworkspaces.workspace.devfile.io --all-namespaces --all --wait || true
215+
$(K8S_CLI) delete devworkspacetemplates.workspace.devfile.io --all-namespaces --all || true
216+
$(K8S_CLI) delete workspaceroutings.controller.devfile.io --all-namespaces --all --wait || true
218217
ifeq ($(PLATFORM),kubernetes)
219218
$(KUSTOMIZE) build config/cert-manager | $(K8S_CLI) delete --ignore-not-found -f -
220219
else

config/components/rbac/role.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ rules:
2222
- namespaces
2323
verbs:
2424
- get
25+
- list
26+
- watch
2527
- apiGroups:
2628
- ""
2729
resources:

controllers/workspace/finalize.go

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"context"
1717
"fmt"
1818
"path"
19+
"time"
1920

2021
"github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
2122
"github.com/devfile/devworkspace-operator/controllers/workspace/provision"
@@ -68,10 +69,33 @@ func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger,
6869
if wait {
6970
return reconcile.Result{Requeue: true}, nil
7071
}
72+
73+
terminating, err := r.namespaceIsTerminating(ctx, workspace.Namespace)
74+
if err != nil {
75+
return reconcile.Result{}, err
76+
} else if terminating {
77+
//Namespace is terminating, it's redundant to clean PVC files since it's going to be removed
78+
log.Info("Namespace is terminating; clearing storage finalizer")
79+
clearFinalizer(workspace)
80+
return reconcile.Result{}, r.Update(ctx, workspace)
81+
}
82+
83+
pvcExists, err := r.pvcExists(ctx, workspace)
84+
if err != nil {
85+
return reconcile.Result{}, err
86+
} else if !pvcExists {
87+
//PVC does not exist. nothing to clean up
88+
log.Info("PVC does not exit; clearing storage finalizer")
89+
clearFinalizer(workspace)
90+
// job will be clean up by k8s garbage collector
91+
return reconcile.Result{}, r.Update(ctx, workspace)
92+
}
93+
7194
specJob, err := r.getSpecCleanupJob(workspace)
7295
if err != nil {
7396
return reconcile.Result{}, err
7497
}
98+
7599
clusterJob, err := r.getClusterCleanupJob(ctx, workspace)
76100
if err != nil {
77101
return reconcile.Result{}, err
@@ -111,7 +135,8 @@ func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger,
111135
return r.updateWorkspaceStatus(workspace, r.Log, failedStatus, reconcile.Result{}, nil)
112136
}
113137
}
114-
return reconcile.Result{}, nil
138+
// Requeue at least each 10 seconds to check if PVC is not removed by someone else
139+
return reconcile.Result{RequeueAfter: 10 * time.Second}, nil
115140
}
116141

117142
func (r *DevWorkspaceReconciler) getSpecCleanupJob(workspace *v1alpha2.DevWorkspace) (*batchv1.Job, error) {
@@ -222,3 +247,33 @@ func clearFinalizer(workspace *v1alpha2.DevWorkspace) {
222247
}
223248
workspace.SetFinalizers(newFinalizers)
224249
}
250+
251+
func (r *DevWorkspaceReconciler) namespaceIsTerminating(ctx context.Context, namespace string) (bool, error) {
252+
namespacedName := types.NamespacedName{
253+
Name: namespace,
254+
}
255+
n := &corev1.Namespace{}
256+
257+
err := r.Get(ctx, namespacedName, n)
258+
if err != nil {
259+
return false, err
260+
}
261+
262+
return n.Status.Phase == corev1.NamespaceTerminating, nil
263+
}
264+
265+
func (r *DevWorkspaceReconciler) pvcExists(ctx context.Context, workspace *v1alpha2.DevWorkspace) (bool, error) {
266+
namespacedName := types.NamespacedName{
267+
Name: config.ControllerCfg.GetWorkspacePVCName(),
268+
Namespace: workspace.Namespace,
269+
}
270+
err := r.Get(ctx, namespacedName, &corev1.PersistentVolumeClaim{})
271+
if err != nil {
272+
if k8sErrors.IsNotFound(err) {
273+
return false, nil
274+
}
275+
276+
return false, err
277+
}
278+
return true, nil
279+
}

test/e2e/pkg/client/namespace.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (w *K8sClient) WaitNamespaceIsTerminated(namespace string) (err error) {
5151
for {
5252
select {
5353
case <-timeoutC:
54-
return errors.New(fmt.Sprintf("The namespace %s is not terminated and removed after %d.", namespace, timeoutC))
54+
return errors.New(fmt.Sprintf("The namespace %s is not terminated and removed after %ds.", namespace, timeout))
5555
case <-tickC:
5656
var ns *corev1.Namespace
5757
ns, err = w.kubeClient.CoreV1().Namespaces().Get(context.TODO(), namespace, metav1.GetOptions{})

0 commit comments

Comments
 (0)