diff --git a/controllers/nstemplateset/nstemplateset_controller.go b/controllers/nstemplateset/nstemplateset_controller.go index 888423df..783afd17 100644 --- a/controllers/nstemplateset/nstemplateset_controller.go +++ b/controllers/nstemplateset/nstemplateset_controller.go @@ -200,6 +200,17 @@ func (r *Reconciler) deleteNSTemplateSet(ctx context.Context, nsTmplSet *toolcha } spacename := nsTmplSet.GetName() + // delete cluster resources first + err := r.clusterResources.delete(ctx, nsTmplSet) + if err != nil { + return reconcile.Result{}, err + } + + // clear the ClusterResources status after successful deletion + if err := r.status.clearStatusClusterResources(ctx, nsTmplSet); err != nil { + return reconcile.Result{}, r.status.wrapErrorWithStatusUpdate(ctx, nsTmplSet, r.status.setStatusTerminatingFailed, err, "failed to clear ClusterResources status") + } + // delete all namespace one by one allDeleted, err := r.namespaces.ensureDeleted(ctx, nsTmplSet) // when err, status Update will not trigger reconcile, sending returning error. @@ -216,12 +227,6 @@ func (r *Reconciler) deleteNSTemplateSet(ctx context.Context, nsTmplSet *toolcha }, nil } - // if no namespace was to be deleted, then we can proceed with the cluster resources associated with the user - err = r.clusterResources.delete(ctx, nsTmplSet) - if err != nil { - return reconcile.Result{}, err - } - // if nothing was to be deleted, then we can remove the finalizer and we're done logger.Info("NSTemplateSet resource is ready to be terminated: all related user namespaces have been marked for deletion") util.RemoveFinalizer(nsTmplSet, toolchainv1alpha1.FinalizerName) diff --git a/controllers/nstemplateset/nstemplateset_controller_test.go b/controllers/nstemplateset/nstemplateset_controller_test.go index 7f9bccba..a5730cbd 100644 --- a/controllers/nstemplateset/nstemplateset_controller_test.go +++ b/controllers/nstemplateset/nstemplateset_controller_test.go @@ -1162,7 +1162,8 @@ func TestDeleteNSTemplateSet(t *testing.T) { AssertThatNamespace(t, firstNSName, r.Client).DoesNotExist() // get the NSTemplateSet resource again and check its status AssertThatNSTemplateSet(t, namespaceName, spacename, r.Client). - HasFinalizer(). // the finalizer should NOT have been removed yet + HasFinalizer(). // the finalizer should NOT have been removed yet + HasStatusClusterResourcesNil(). // the cluster resources status should be cleared HasConditions(Terminating()) t.Run("reconcile after first user namespace deletion triggers deletion of the second namespace", func(t *testing.T) { @@ -1214,6 +1215,28 @@ func TestDeleteNSTemplateSet(t *testing.T) { require.EqualError(t, err, "failed to delete cluster resource 'for-johnsmith': mock error") require.Equal(t, controllerruntime.Result{}, result) }) + t.Run("failed to clear cluster resources status", func(t *testing.T) { + // given an NSTemplateSet resource with cluster resources + nsTmplSet := newNSTmplSet(namespaceName, spacename, "advanced", withDeletionTs(), withClusterResources("abcde11"), withStatusClusterResources("abcde11")) + crq := newClusterResourceQuota(spacename, "advanced") + r, fakeClient := prepareController(t, nsTmplSet, crq) + req := newReconcileRequest(namespaceName, spacename) + + // mock status update failure for clearing cluster resources + fakeClient.MockStatusUpdate = func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + if nsTmplSet, ok := obj.(*toolchainv1alpha1.NSTemplateSet); ok && nsTmplSet.Status.ClusterResources == nil { + return fmt.Errorf("mock status update error") + } + return fakeClient.Client.Status().Update(ctx, obj, opts...) + } + + // when reconcile is triggered + result, err := r.Reconcile(context.TODO(), req) + + // then + require.EqualError(t, err, "failed to clear ClusterResources status: mock status update error") + require.Equal(t, controllerruntime.Result{}, result) + }) t.Run("NSTemplateSet deletion errors when namespace is not deleted in 1 min", func(t *testing.T) { // given an NSTemplateSet resource and 1 active user namespaces ("dev") diff --git a/controllers/nstemplateset/status.go b/controllers/nstemplateset/status.go index c4f5c2fd..76dbdfa1 100644 --- a/controllers/nstemplateset/status.go +++ b/controllers/nstemplateset/status.go @@ -290,3 +290,12 @@ func (r *statusManager) setStatusTerminatingFailed(ctx context.Context, nsTmplSe Message: message, }) } + +// clearStatusClusterResources sets the ClusterResources status to nil +func (r *statusManager) clearStatusClusterResources(ctx context.Context, nsTmplSet *toolchainv1alpha1.NSTemplateSet) error { + if nsTmplSet.Status.ClusterResources != nil { + nsTmplSet.Status.ClusterResources = nil + return r.Client.Status().Update(ctx, nsTmplSet) + } + return nil +}