From d3463d0c8944b6c311d86f3c1593be4871685917 Mon Sep 17 00:00:00 2001
From: Bartosz Majsak <bartosz.majsak@gmail.com>
Date: Fri, 12 Jul 2024 14:15:33 +0200
Subject: [PATCH] fix: ensures SMCP is created before any other features
 (#1118)

With the #1052 refactoring, the order of features added to the Registry was
accidentally changed. It results in failing of metrics collection
feature which expects SMCP to be created first, but the creation runs
afterwards. The setup is eventually consistent, as the reconcile will
retry, so this not a bug per se, but results in unnecassary errors.

This fix ensures features are ordered as before and levarages
`.EnabledWhen` instead of wrapping features in `if`s.

(cherry picked from commit d6f25c71f42561658c63368af327b01df1f5e1f5)
---
 .../dscinitialization/servicemesh_setup.go    | 65 +++++++++----------
 1 file changed, 31 insertions(+), 34 deletions(-)

diff --git a/controllers/dscinitialization/servicemesh_setup.go b/controllers/dscinitialization/servicemesh_setup.go
index 6b6aeb025d6..ff952a12366 100644
--- a/controllers/dscinitialization/servicemesh_setup.go
+++ b/controllers/dscinitialization/servicemesh_setup.go
@@ -125,24 +125,28 @@ func (r *DSCInitializationReconciler) authorizationCapability(ctx context.Contex
 
 func (r *DSCInitializationReconciler) serviceMeshCapabilityFeatures(instance *dsciv1.DSCInitialization) feature.FeaturesProvider {
 	return func(registry feature.FeaturesRegistry) error {
-		serviceMeshSpec := instance.Spec.ServiceMesh
+		controlPlaneSpec := instance.Spec.ServiceMesh.ControlPlane
+
+		meshMetricsCollection := func(_ context.Context, _ *feature.Feature) (bool, error) {
+			return controlPlaneSpec.MetricsCollection == "Istio", nil
+		}
 
-		smcp := feature.Define("mesh-control-plane-creation").
-			ManifestsLocation(Templates.Location).
-			Manifests(
-				path.Join(Templates.ServiceMeshDir),
-			).
-			WithData(servicemesh.FeatureData.ControlPlane.Define(&instance.Spec).AsAction()).
-			PreConditions(
-				servicemesh.EnsureServiceMeshOperatorInstalled,
-				feature.CreateNamespaceIfNotExists(serviceMeshSpec.ControlPlane.Namespace),
-			).
-			PostConditions(
-				feature.WaitForPodsToBeReady(serviceMeshSpec.ControlPlane.Namespace),
-			)
-
-		if serviceMeshSpec.ControlPlane.MetricsCollection == "Istio" {
-			metricsCollectionErr := registry.Add(feature.Define("mesh-metrics-collection").
+		return registry.Add(
+			feature.Define("mesh-control-plane-creation").
+				ManifestsLocation(Templates.Location).
+				Manifests(
+					path.Join(Templates.ServiceMeshDir),
+				).
+				WithData(servicemesh.FeatureData.ControlPlane.Define(&instance.Spec).AsAction()).
+				PreConditions(
+					servicemesh.EnsureServiceMeshOperatorInstalled,
+					feature.CreateNamespaceIfNotExists(controlPlaneSpec.Namespace),
+				).
+				PostConditions(
+					feature.WaitForPodsToBeReady(controlPlaneSpec.Namespace),
+				),
+			feature.Define("mesh-metrics-collection").
+				EnabledWhen(meshMetricsCollection).
 				ManifestsLocation(Templates.Location).
 				Manifests(
 					path.Join(Templates.MetricsDir),
@@ -152,23 +156,16 @@ func (r *DSCInitializationReconciler) serviceMeshCapabilityFeatures(instance *ds
 				).
 				PreConditions(
 					servicemesh.EnsureServiceMeshInstalled,
-				))
-
-			if metricsCollectionErr != nil {
-				return metricsCollectionErr
-			}
-		}
-
-		cfgMap := feature.Define("mesh-shared-configmap").
-			WithResources(servicemesh.MeshRefs, servicemesh.AuthRefs).
-			WithData(
-				servicemesh.FeatureData.ControlPlane.Define(&instance.Spec).AsAction(),
-			).
-			WithData(
-				servicemesh.FeatureData.Authorization.All(&instance.Spec)...,
-			)
-
-		return registry.Add(smcp, cfgMap)
+				),
+			feature.Define("mesh-shared-configmap").
+				WithResources(servicemesh.MeshRefs, servicemesh.AuthRefs).
+				WithData(
+					servicemesh.FeatureData.ControlPlane.Define(&instance.Spec).AsAction(),
+				).
+				WithData(
+					servicemesh.FeatureData.Authorization.All(&instance.Spec)...,
+				),
+		)
 	}
 }