feat: simplifies Feature API (opendatahub-io#831)

- `FeaturesInitializer` (now named `FeaturesHandler`) becomes an entry point to compose features 
    - it has two modes: cluster and component. Based on this mode it will initialize `.spec.source` transparently 
    - there is no `Prepare` func anymore - it is now part of `Apply` and `Delete` - most importantly `FeaturesHandler` keeps track of created features so there is no need to keep adding to the slice explicitly
- Tests have been adjusted to focus on behavior rather than implementation details

apis/features/v1/features_types.go

@@ -23,6 +23,19 @@ type FeatureTracker struct {
 	Status FeatureTrackerStatus `json:"status,omitempty"`
 }
 
+// NewFeatureTracker instantiate FeatureTracker instance.
+func NewFeatureTracker(name, appNamespace string) *FeatureTracker {
+	return &FeatureTracker{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "features.opendatahub.io/v1",
+			Kind:       "FeatureTracker",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: appNamespace + "-" + name,
+		},
+	}
+}
+
 type FeaturePhase string
 type OwnerType string
 

components/kserve/kserve.go

@@ -109,7 +109,7 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client, resC
 			}
 		}
 		// check on dependent operators if all installed in cluster
-		dependOpsErrors := checkDepedentOps(cli).ErrorOrNil()
+		dependOpsErrors := checkDependentOperators(cli).ErrorOrNil()
 		if dependOpsErrors != nil {
 			return dependOpsErrors
 		}
@@ -187,30 +187,22 @@ func (k *Kserve) configureServerless(instance *dsciv1.DSCInitializationSpec) err
 			return fmt.Errorf("ServiceMesh is need to set to 'Managed' in DSCI CR, it is required by KServe serving field")
 		}
 
-		serverlessInitializer := feature.ComponentFeaturesInitializer(k, instance, k.configureServerlessFeatures())
+		serverlessFeatures := feature.ComponentFeaturesHandler(k, instance, k.configureServerlessFeatures())
 
-		if err := serverlessInitializer.Prepare(); err != nil {
-			return err
-		}
-
-		if err := serverlessInitializer.Apply(); err != nil {
+		if err := serverlessFeatures.Apply(); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
 func (k *Kserve) removeServerlessFeatures(instance *dsciv1.DSCInitializationSpec) error {
-	serverlessInitializer := feature.ComponentFeaturesInitializer(k, instance, k.configureServerlessFeatures())
-
-	if err := serverlessInitializer.Prepare(); err != nil {
-		return err
-	}
+	serverlessFeatures := feature.ComponentFeaturesHandler(k, instance, k.configureServerlessFeatures())
 
-	return serverlessInitializer.Delete()
+	return serverlessFeatures.Delete()
 }
 
-func checkDepedentOps(cli client.Client) *multierror.Error {
+func checkDependentOperators(cli client.Client) *multierror.Error {
 	var multiErr *multierror.Error
 
 	if found, err := deploy.OperatorExists(cli, ServiceMeshOperator); err != nil {

components/kserve/serverless_setup.go

@@ -8,17 +8,12 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh"
 )
 
-const (
-	templatesDir = "templates/serverless"
-)
-
-func (k *Kserve) configureServerlessFeatures() feature.DefinedFeatures {
-	return func(initializer *feature.FeaturesInitializer) error {
-		servingDeployment, err := feature.CreateFeature("serverless-serving-deployment").
-			With(initializer.DSCInitializationSpec).
-			From(initializer.Source).
+func (k *Kserve) configureServerlessFeatures() feature.FeaturesProvider {
+	return func(handler *feature.FeaturesHandler) error {
+		servingDeploymentErr := feature.CreateFeature("serverless-serving-deployment").
+			For(handler).
 			Manifests(
-				path.Join(templatesDir, "serving-install"),
+				path.Join(feature.ServerlessDir, "serving-install"),
 			).
 			WithData(PopulateComponentSettings(k)).
 			PreConditions(
@@ -27,47 +22,46 @@ func (k *Kserve) configureServerlessFeatures() feature.DefinedFeatures {
 				servicemesh.EnsureServiceMeshInstalled,
 				feature.CreateNamespaceIfNotExists(serverless.KnativeServingNamespace),
 			).
+			PostConditions(
+				feature.WaitForPodsToBeReady(serverless.KnativeServingNamespace),
+			).
 			Load()
-		if err != nil {
-			return err
+		if servingDeploymentErr != nil {
+			return servingDeploymentErr
 		}
-		initializer.Features = append(initializer.Features, servingDeployment)
 
-		servingNetIstioSecretFiltering, err := feature.CreateFeature("serverless-net-istio-secret-filtering").
-			With(initializer.DSCInitializationSpec).
-			From(initializer.Source).
+		servingNetIstioSecretFilterinErr := feature.CreateFeature("serverless-net-istio-secret-filtering").
+			For(handler).
 			Manifests(
-				path.Join(templatesDir, "serving-net-istio-secret-filtering.patch.tmpl"),
+				path.Join(feature.ServerlessDir, "serving-net-istio-secret-filtering.patch.tmpl"),
 			).
 			WithData(PopulateComponentSettings(k)).
 			PreConditions(serverless.EnsureServerlessServingDeployed).
 			PostConditions(
 				feature.WaitForPodsToBeReady(serverless.KnativeServingNamespace),
 			).
 			Load()
-		if err != nil {
-			return err
+		if servingNetIstioSecretFilterinErr != nil {
+			return servingNetIstioSecretFilterinErr
 		}
-		initializer.Features = append(initializer.Features, servingNetIstioSecretFiltering)
 
-		servingIstioGateways, err := feature.CreateFeature("serverless-serving-gateways").
-			With(initializer.DSCInitializationSpec).
-			From(initializer.Source).
+		serverlessGwErr := feature.CreateFeature("serverless-serving-gateways").
+			For(handler).
 			PreConditions(serverless.EnsureServerlessServingDeployed).
 			WithData(
+				PopulateComponentSettings(k),
 				serverless.ServingDefaultValues,
 				serverless.ServingIngressDomain,
-				PopulateComponentSettings(k),
 			).
 			WithResources(serverless.ServingCertificateResource).
 			Manifests(
-				path.Join(templatesDir, "serving-istio-gateways"),
+				path.Join(feature.ServerlessDir, "serving-istio-gateways"),
 			).
 			Load()
-		if err != nil {
-			return err
+		if serverlessGwErr != nil {
+			return serverlessGwErr
 		}
-		initializer.Features = append(initializer.Features, servingIstioGateways)
+
 		return nil
 	}
 }

controllers/dscinitialization/servicemesh_setup.go

@@ -11,19 +11,12 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh"
 )
 
-const templatesDir = "templates/servicemesh"
-
 func (r *DSCInitializationReconciler) configureServiceMesh(instance *dsciv1.DSCInitialization) error {
 	switch instance.Spec.ServiceMesh.ManagementState {
 	case operatorv1.Managed:
-		serviceMeshInitializer := feature.ClusterFeaturesInitializer(instance, configureServiceMeshFeatures())
-		if err := serviceMeshInitializer.Prepare(); err != nil {
-			r.Log.Error(err, "failed configuring service mesh resources")
-			r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "failed configuring service mesh resources")
-			return err
-		}
+		serviceMeshFeatures := feature.ClusterFeaturesHandler(instance, configureServiceMeshFeatures())
 
-		if err := serviceMeshInitializer.Apply(); err != nil {
+		if err := serviceMeshFeatures.Apply(); err != nil {
 			r.Log.Error(err, "failed applying service mesh resources")
 			r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "failed applying service mesh resources")
 			return err
@@ -43,15 +36,9 @@ func (r *DSCInitializationReconciler) configureServiceMesh(instance *dsciv1.DSCI
 func (r *DSCInitializationReconciler) removeServiceMesh(instance *dsciv1.DSCInitialization) error {
 	// on condition of Managed, do not handle Removed when set to Removed it trigger DSCI reconcile to cleanup
 	if instance.Spec.ServiceMesh.ManagementState == operatorv1.Managed {
-		serviceMeshInitializer := feature.ClusterFeaturesInitializer(instance, configureServiceMeshFeatures())
-		if err := serviceMeshInitializer.Prepare(); err != nil {
-			r.Log.Error(err, "failed configuring service mesh resources")
-			r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "failed configuring service mesh resources")
-
-			return err
-		}
+		serviceMeshFeatures := feature.ClusterFeaturesHandler(instance, configureServiceMeshFeatures())
 
-		if err := serviceMeshInitializer.Delete(); err != nil {
+		if err := serviceMeshFeatures.Delete(); err != nil {
 			r.Log.Error(err, "failed deleting service mesh resources")
 			r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "failed deleting service mesh resources")
 
@@ -62,15 +49,14 @@ func (r *DSCInitializationReconciler) removeServiceMesh(instance *dsciv1.DSCInit
 	return nil
 }
 
-func configureServiceMeshFeatures() feature.DefinedFeatures {
-	return func(initializer *feature.FeaturesInitializer) error {
-		serviceMeshSpec := initializer.DSCInitializationSpec.ServiceMesh
+func configureServiceMeshFeatures() feature.FeaturesProvider {
+	return func(handler *feature.FeaturesHandler) error {
+		serviceMeshSpec := handler.DSCInitializationSpec.ServiceMesh
 
-		smcpCreation, errSmcp := feature.CreateFeature("mesh-control-plane-creation").
-			With(initializer.DSCInitializationSpec).
-			From(initializer.Source).
+		smcpCreationErr := feature.CreateFeature("mesh-control-plane-creation").
+			For(handler).
 			Manifests(
-				path.Join(templatesDir, "base", "create-smcp.tmpl"),
+				path.Join(feature.ServiceMeshDir, "base", "create-smcp.tmpl"),
 			).
 			PreConditions(
 				servicemesh.EnsureServiceMeshOperatorInstalled,
@@ -81,27 +67,25 @@ func configureServiceMeshFeatures() feature.DefinedFeatures {
 			).
 			Load()
 
-		if errSmcp != nil {
-			return errSmcp
+		if smcpCreationErr != nil {
+			return smcpCreationErr
 		}
-		initializer.Features = append(initializer.Features, smcpCreation)
 
 		if serviceMeshSpec.ControlPlane.MetricsCollection == "Istio" {
-			metricsCollection, errMetrics := feature.CreateFeature("mesh-metrics-collection").
-				With(initializer.DSCInitializationSpec).
-				From(initializer.Source).
-				Manifests(
-					path.Join(templatesDir, "metrics-collection"),
-				).
+			metricsCollectionErr := feature.CreateFeature("mesh-metrics-collection").
+				For(handler).
 				PreConditions(
 					servicemesh.EnsureServiceMeshInstalled,
 				).
+				Manifests(
+					path.Join(feature.ServiceMeshDir, "metrics-collection"),
+				).
 				Load()
-			if errMetrics != nil {
-				return errMetrics
+			if metricsCollectionErr != nil {
+				return metricsCollectionErr
 			}
-			initializer.Features = append(initializer.Features, metricsCollection)
 		}
+
 		return nil
 	}
 }

pkg/feature/builder.go

@@ -13,58 +13,48 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
-	v1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
 	infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/infrastructure/v1"
 )
 
 type partialBuilder func(f *Feature) error
 
 type featureBuilder struct {
-	name     string
-	config   *rest.Config
-	builders []partialBuilder
-	fsys     fs.FS
+	name            string
+	config          *rest.Config
+	builders        []partialBuilder
+	featuresHanlder *FeaturesHandler
+	fsys            fs.FS
 }
 
-func CreateFeature(name string) *featureName { //nolint:golint,revive //No need to export featureBuilder.
-	return &featureName{
+func CreateFeature(name string) *usingFeaturesHandler { //nolint:golint,revive //No need to export featureBuilder.
+	return &usingFeaturesHandler{
 		name: name,
 	}
 }
 
-type featureName struct {
+type usingFeaturesHandler struct {
 	name string
 }
 
-func (fn *featureName) With(spec *v1.DSCInitializationSpec) *featureSource {
-	return &featureSource{
-		spec: spec,
-		name: fn.name,
-	}
-}
-
-type featureSource struct {
-	spec *v1.DSCInitializationSpec
-	name string
-}
-
-func (fo *featureSource) From(source featurev1.Source) *featureBuilder {
+func (u *usingFeaturesHandler) For(featuresHandler *FeaturesHandler) *featureBuilder {
 	createSpec := func(f *Feature) error {
 		f.Spec = &Spec{
-			ServiceMeshSpec: &fo.spec.ServiceMesh,
+			ServiceMeshSpec: &featuresHandler.DSCInitializationSpec.ServiceMesh,
 			Serving:         &infrav1.ServingSpec{},
-			Source:          &source,
-			AppNamespace:    fo.spec.ApplicationsNamespace,
+			Source:          &featuresHandler.source,
+			AppNamespace:    featuresHandler.DSCInitializationSpec.ApplicationsNamespace,
 		}
 
 		return nil
 	}
 
 	fb := &featureBuilder{
-		name: fo.name,
-		fsys: embeddedFiles,
+		name:            u.name,
+		featuresHanlder: featuresHandler,
+		fsys:            embeddedFiles,
 	}
+
 	// Ensures creation of .Spec object is always invoked first
 	fb.builders = append([]partialBuilder{createSpec}, fb.builders...)
 
@@ -172,28 +162,30 @@ func (fb *featureBuilder) WithResources(resources ...Action) *featureBuilder {
 	return fb
 }
 
-func (fb *featureBuilder) Load() (*Feature, error) {
+func (fb *featureBuilder) Load() error {
 	feature := newFeature(fb.name)
 
 	// UsingConfig builder wasn't called while constructing this feature.
 	// Get default settings and create needed clients.
 	if fb.config == nil {
 		if err := fb.withDefaultClient(); err != nil {
-			return nil, err
+			return err
 		}
 	}
 
 	if err := createClients(fb.config)(feature); err != nil {
-		return nil, err
+		return err
 	}
 
 	for i := range fb.builders {
 		if err := fb.builders[i](feature); err != nil {
-			return nil, err
+			return err
 		}
 	}
 
-	return feature, nil
+	fb.featuresHanlder.features = append(fb.featuresHanlder.features, feature)
+
+	return nil
 }
 
 func (fb *featureBuilder) withDefaultClient() error {