[SANDBOX-1399][SANDBOX-1395] apply all cluster resources at once and read the prior state from the nstemplateset status instead of labels

controllers/nstemplateset/features.go

@@ -24,3 +24,9 @@ func shouldCreate(toCreate runtimeclient.Object, nsTmplSet *toolchainv1alpha1.NS
 	}
 	return slices.Contains(utils.SplitCommaSeparatedList(winners), feature)
 }
+
+// featuresChanged returns true if the features on the NSTemplateSet changed since the last time it was applied.
+func featuresChanged(nsTmplSet *toolchainv1alpha1.NSTemplateSet) bool {
+	changed, _ := featureAnnotationNeedsUpdate(nsTmplSet)
+	return changed
+}

controllers/nstemplateset/features_test.go

@@ -98,6 +98,65 @@ func TestShouldCreate(t *testing.T) {
 	}
 }
 
+func TestFeaturesChanged(t *testing.T) {
+	for _, test := range []struct {
+		name           string
+		annoFeatures   string
+		statusFeatures []string
+		changed        bool
+	}{
+		{
+			name:    "should report no change when no features in either annos or status",
+			changed: false,
+		},
+		{
+			name:         "should report change when no features in status",
+			annoFeatures: "feature",
+			changed:      true,
+		},
+		{
+			name:           "should report change when no features in anno",
+			statusFeatures: []string{"feature"},
+			changed:        true,
+		},
+		{
+			name:           "should report no change when features equal",
+			statusFeatures: []string{"feature1", "feature2"},
+			annoFeatures:   "feature1,feature2",
+			changed:        false,
+		},
+		{
+			name:           "should report change when features differ in number",
+			statusFeatures: []string{"feature1", "feature2"},
+			annoFeatures:   "feature1",
+			changed:        true,
+		},
+		{
+			name:           "should report change when features differ",
+			statusFeatures: []string{"feature1", "feature2"},
+			annoFeatures:   "feature1,feature3",
+			changed:        true,
+		},
+		{
+			name:           "should detect duplicates",
+			statusFeatures: []string{"feature1", "feature2", "feature1"},
+			annoFeatures:   "feature2,feature1",
+			changed:        false,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			// given
+			nsts := newNSTmplSet("default", "ns", "base", withNSTemplateSetFeatureAnnotation(test.annoFeatures), withStatusFeatureToggles(test.statusFeatures))
+
+			// when
+			changed := featuresChanged(nsts)
+
+			// then
+			assert.Equal(t, test.changed, changed)
+		})
+	}
+}
+
 func p(s string) *string {
 	return &s
 }

controllers/nstemplateset/nstemplateset_controller.go

@@ -133,12 +133,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 	// we proceed with the cluster-scoped resources template, then all namespaces and finally space roles
 	// as we want to be sure that cluster-scoped resources such as quotas are set
 	// even before the namespaces exist
-	if createdOrUpdated, err := r.clusterResources.ensure(ctx, nsTmplSet); err != nil {
+	// NOTE: the cluster resources are applied ALL AT ONCE, unlike the namespaces.
+	// This is a work-in-progress change to unify how we apply cluster resources and namespaces.
+	// In the end, everything will be applied in one go.
+	if err := r.clusterResources.ensure(ctx, nsTmplSet); err != nil {
 		logger.Error(err, "failed to either provision or update cluster resources")
 		return reconcile.Result{}, err
-	} else if createdOrUpdated {
-		// we need to requeue to make sure we apply all cluster resources before continuing further
-		return reconcile.Result{Requeue: true}, nil // wait for cluster resources to be created
 	}
 	if err := r.status.updateStatusClusterResourcesRevisions(ctx, nsTmplSet); err != nil {
 		return reconcile.Result{}, err
@@ -217,10 +217,9 @@ func (r *Reconciler) deleteNSTemplateSet(ctx context.Context, nsTmplSet *toolcha
 	}
 
 	// if no namespace was to be deleted, then we can proceed with the cluster resources associated with the user
-	deletedAny, err := r.clusterResources.delete(ctx, nsTmplSet)
-	if err != nil || deletedAny {
-		// we need to check if there are some more cluster resources left
-		return reconcile.Result{Requeue: true}, err
+	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