Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 11 additions & 6 deletions controllers/nstemplateset/nstemplateset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down
25 changes: 24 additions & 1 deletion controllers/nstemplateset/nstemplateset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
Expand Down
9 changes: 9 additions & 0 deletions controllers/nstemplateset/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading