diff --git a/components/kserve/kserve_config_handler.go b/components/kserve/kserve_config_handler.go index 8479e207053..e43e7313b2f 100644 --- a/components/kserve/kserve_config_handler.go +++ b/components/kserve/kserve_config_handler.go @@ -148,9 +148,9 @@ func (k *Kserve) configureServerless(ctx context.Context, cli client.Client, log return dependOpsErrors } - serverlessFeatures := feature.ComponentFeaturesHandler(cli, owner, k.GetComponentName(), instance.ApplicationsNamespace, k.configureServerlessFeatures(instance)) + serverlessFeatures := feature.ComponentFeaturesHandler(owner, k.GetComponentName(), instance.ApplicationsNamespace, k.configureServerlessFeatures(instance)) - if err := serverlessFeatures.Apply(ctx); err != nil { + if err := serverlessFeatures.Apply(ctx, cli); err != nil { return err } } @@ -158,9 +158,9 @@ func (k *Kserve) configureServerless(ctx context.Context, cli client.Client, log } func (k *Kserve) removeServerlessFeatures(ctx context.Context, cli client.Client, owner metav1.Object, instance *dsciv1.DSCInitializationSpec) error { - serverlessFeatures := feature.ComponentFeaturesHandler(cli, owner, k.GetComponentName(), instance.ApplicationsNamespace, k.configureServerlessFeatures(instance)) + serverlessFeatures := feature.ComponentFeaturesHandler(owner, k.GetComponentName(), instance.ApplicationsNamespace, k.configureServerlessFeatures(instance)) - return serverlessFeatures.Delete(ctx) + return serverlessFeatures.Delete(ctx, cli) } func checkDependentOperators(ctx context.Context, cli client.Client) *multierror.Error { diff --git a/components/kserve/servicemesh_setup.go b/components/kserve/servicemesh_setup.go index 23e3c5321bb..126e23d88ea 100644 --- a/components/kserve/servicemesh_setup.go +++ b/components/kserve/servicemesh_setup.go @@ -20,8 +20,8 @@ import ( func (k *Kserve) configureServiceMesh(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec) error { if dscispec.ServiceMesh != nil { if dscispec.ServiceMesh.ManagementState == operatorv1.Managed && k.GetManagementState() == operatorv1.Managed { - serviceMeshInitializer := feature.ComponentFeaturesHandler(cli, owner, k.GetComponentName(), dscispec.ApplicationsNamespace, k.defineServiceMeshFeatures(ctx, cli, dscispec)) - return serviceMeshInitializer.Apply(ctx) + serviceMeshInitializer := feature.ComponentFeaturesHandler(owner, k.GetComponentName(), dscispec.ApplicationsNamespace, k.defineServiceMeshFeatures(ctx, cli, dscispec)) + return serviceMeshInitializer.Apply(ctx, cli) } if dscispec.ServiceMesh.ManagementState == operatorv1.Unmanaged && k.GetManagementState() == operatorv1.Managed { return nil @@ -32,8 +32,8 @@ func (k *Kserve) configureServiceMesh(ctx context.Context, cli client.Client, ow } func (k *Kserve) removeServiceMeshConfigurations(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec) error { - serviceMeshInitializer := feature.ComponentFeaturesHandler(cli, owner, k.GetComponentName(), dscispec.ApplicationsNamespace, k.defineServiceMeshFeatures(ctx, cli, dscispec)) - return serviceMeshInitializer.Delete(ctx) + serviceMeshInitializer := feature.ComponentFeaturesHandler(owner, k.GetComponentName(), dscispec.ApplicationsNamespace, k.defineServiceMeshFeatures(ctx, cli, dscispec)) + return serviceMeshInitializer.Delete(ctx, cli) } func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec) feature.FeaturesProvider { diff --git a/controllers/dscinitialization/servicemesh_setup.go b/controllers/dscinitialization/servicemesh_setup.go index 7b5e6bc0977..4e45b96a0ca 100644 --- a/controllers/dscinitialization/servicemesh_setup.go +++ b/controllers/dscinitialization/servicemesh_setup.go @@ -8,6 +8,7 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" @@ -39,7 +40,7 @@ func (r *DSCInitializationReconciler) configureServiceMesh(ctx context.Context, capabilities = append(capabilities, authzCapability) for _, capability := range capabilities { - capabilityErr := capability.Apply(ctx) + capabilityErr := capability.Apply(ctx, r.Client) if capabilityErr != nil { r.Log.Error(capabilityErr, "failed applying service mesh resources") r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "failed applying service mesh resources") @@ -77,7 +78,7 @@ func (r *DSCInitializationReconciler) removeServiceMesh(ctx context.Context, ins capabilities = append(capabilities, authzCapability) for _, capability := range capabilities { - capabilityErr := capability.Delete(ctx) + capabilityErr := capability.Delete(ctx, r.Client) if capabilityErr != nil { r.Log.Error(capabilityErr, "failed deleting service mesh resources") r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "failed deleting service mesh resources") @@ -91,7 +92,7 @@ func (r *DSCInitializationReconciler) removeServiceMesh(ctx context.Context, ins func (r *DSCInitializationReconciler) serviceMeshCapability(instance *dsciv1.DSCInitialization, initialCondition *conditionsv1.Condition) *feature.HandlerWithReporter[*dsciv1.DSCInitialization] { //nolint:lll // Reason: generics are long return feature.NewHandlerWithReporter( - feature.ClusterFeaturesHandler(r.Client, instance, r.serviceMeshCapabilityFeatures(instance)), + feature.ClusterFeaturesHandler(instance, r.serviceMeshCapabilityFeatures(instance)), createCapabilityReporter(r.Client, instance, initialCondition), ) } @@ -119,7 +120,7 @@ func (r *DSCInitializationReconciler) authorizationCapability(ctx context.Contex } return feature.NewHandlerWithReporter( - feature.ClusterFeaturesHandler(r.Client, instance, r.authorizationFeatures(instance)), + feature.ClusterFeaturesHandler(instance, r.authorizationFeatures(instance)), createCapabilityReporter(r.Client, instance, condition), ), nil } @@ -128,7 +129,7 @@ func (r *DSCInitializationReconciler) serviceMeshCapabilityFeatures(instance *ds return func(registry feature.FeaturesRegistry) error { controlPlaneSpec := instance.Spec.ServiceMesh.ControlPlane - meshMetricsCollection := func(_ context.Context, _ *feature.Feature) (bool, error) { + meshMetricsCollection := func(_ context.Context, _ client.Client, _ *feature.Feature) (bool, error) { return controlPlaneSpec.MetricsCollection == "Istio", nil } @@ -221,13 +222,13 @@ func (r *DSCInitializationReconciler) authorizationFeatures(instance *dsciv1.DSC Include(path.Join(Templates.AuthorinoDir, "deployment.injection.patch.tmpl.yaml")), ). PreConditions( - func(ctx context.Context, f *feature.Feature) error { + func(ctx context.Context, cli client.Client, f *feature.Feature) error { namespace, err := servicemesh.FeatureData.Authorization.Namespace.Extract(f) if err != nil { return fmt.Errorf("failed trying to resolve authorization provider namespace for feature '%s': %w", f.Name, err) } - return feature.WaitForPodsToBeReady(namespace)(ctx, f) + return feature.WaitForPodsToBeReady(namespace)(ctx, cli, f) }, ). WithData(servicemesh.FeatureData.ControlPlane.Define(&instance.Spec).AsAction()). diff --git a/pkg/feature/builder.go b/pkg/feature/builder.go index 9cb53c58a70..54fadcabbec 100644 --- a/pkg/feature/builder.go +++ b/pkg/feature/builder.go @@ -4,14 +4,8 @@ import ( "context" "fmt" - "github.com/hashicorp/go-multierror" - "github.com/pkg/errors" - apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/log" featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1" @@ -27,8 +21,6 @@ type featureBuilder struct { owner metav1.Object targetNs string - client client.Client - builders []partialBuilder } @@ -193,16 +185,10 @@ func (fb *featureBuilder) OnDelete(cleanups ...CleanupFunc) *featureBuilder { return fb } -// UsingClient allows to provide a custom client to the feature. If not called, a default client will be created. -func (fb *featureBuilder) UsingClient(cli client.Client) *featureBuilder { - fb.client = cli - return fb -} - // Create creates a new Feature instance and add it to corresponding FeaturesHandler. // The actual feature creation in the cluster is not performed here. func (fb *featureBuilder) Create() (*Feature, error) { - alwaysEnabled := func(_ context.Context, _ *Feature) (bool, error) { + alwaysEnabled := func(_ context.Context, _ client.Client, _ *Feature) (bool, error) { return true, nil } @@ -215,15 +201,6 @@ func (fb *featureBuilder) Create() (*Feature, error) { owner: fb.owner, } - // UsingClient has not been called, so we need to create a new client - if fb.client == nil { - if err := createDefaultClient()(f); err != nil { - return nil, err - } - } else { - f.Client = fb.client - } - for i := range fb.builders { if err := fb.builders[i](f); err != nil { return nil, err @@ -232,39 +209,3 @@ func (fb *featureBuilder) Create() (*Feature, error) { return f, nil } - -func createDefaultClient() partialBuilder { - return func(f *Feature) error { - var err error - - restCfg, err := config.GetConfig() - if errors.Is(err, rest.ErrNotInCluster) { - // rollback to local kubeconfig - this can be helpful when running the process locally i.e. while debugging - kubeconfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( - &clientcmd.ClientConfigLoadingRules{ExplicitPath: clientcmd.RecommendedHomeFile}, - &clientcmd.ConfigOverrides{}, - ) - - restCfg, err = kubeconfig.ClientConfig() - if err != nil { - return err - } - } else if err != nil { - return err - } - - f.Client, err = client.New(restCfg, client.Options{}) - if err != nil { - return errors.WithStack(err) - } - - var multiErr *multierror.Error - s := f.Client.Scheme() - multiErr = multierror.Append(multiErr, - featurev1.AddToScheme(s), - apiextv1.AddToScheme(s), - ) - - return multiErr.ErrorOrNil() - } -} diff --git a/pkg/feature/conditions.go b/pkg/feature/conditions.go index be86b4c0fb4..9a9373ba130 100644 --- a/pkg/feature/conditions.go +++ b/pkg/feature/conditions.go @@ -40,8 +40,8 @@ func (e *MissingOperatorError) Error() string { } func EnsureOperatorIsInstalled(operatorName string) Action { - return func(ctx context.Context, f *Feature) error { - if found, err := cluster.SubscriptionExists(ctx, f.Client, operatorName); !found || err != nil { + return func(ctx context.Context, cli client.Client, f *Feature) error { + if found, err := cluster.SubscriptionExists(ctx, cli, operatorName); !found || err != nil { return fmt.Errorf( "failed to find the pre-requisite operator subscription %q, please ensure operator is installed. %w", operatorName, @@ -53,13 +53,13 @@ func EnsureOperatorIsInstalled(operatorName string) Action { } func WaitForPodsToBeReady(namespace string) Action { - return func(ctx context.Context, f *Feature) error { + return func(ctx context.Context, cli client.Client, f *Feature) error { f.Log.Info("waiting for pods to become ready", "namespace", namespace, "duration (s)", duration.Seconds()) return wait.PollUntilContextTimeout(ctx, interval, duration, false, func(ctx context.Context) (bool, error) { var podList corev1.PodList - err := f.Client.List(ctx, &podList, client.InNamespace(namespace)) + err := cli.List(ctx, &podList, client.InNamespace(namespace)) if err != nil { return false, err } @@ -103,14 +103,14 @@ func WaitForPodsToBeReady(namespace string) Action { } func WaitForResourceToBeCreated(namespace string, gvk schema.GroupVersionKind) Action { - return func(ctx context.Context, f *Feature) error { + return func(ctx context.Context, cli client.Client, f *Feature) error { f.Log.Info("waiting for resource to be created", "namespace", namespace, "resource", gvk) return wait.PollUntilContextTimeout(ctx, interval, duration, false, func(ctx context.Context) (bool, error) { list := &unstructured.UnstructuredList{} list.SetGroupVersionKind(gvk) - err := f.Client.List(ctx, list, client.InNamespace(namespace), client.Limit(1)) + err := cli.List(ctx, list, client.InNamespace(namespace), client.Limit(1)) if err != nil { f.Log.Error(err, "failed waiting for resource", "namespace", namespace, "resource", gvk) diff --git a/pkg/feature/feature.go b/pkg/feature/feature.go index 3a16a2b9a40..33e569a0fd5 100644 --- a/pkg/feature/feature.go +++ b/pkg/feature/feature.go @@ -41,8 +41,7 @@ type Feature struct { Enabled EnabledFunc Managed bool - Client client.Client - Log logr.Logger + Log logr.Logger tracker *featurev1.FeatureTracker source *featurev1.Source @@ -61,7 +60,7 @@ type Feature struct { // Action is a func type which can be used for different purposes during Feature's lifecycle // while having access to Feature struct. -type Action func(ctx context.Context, f *Feature) error +type Action func(ctx context.Context, cli client.Client, f *Feature) error // CleanupFunc defines how to clean up resources associated with a feature. // By default, all resources created by the feature are deleted when the feature is, @@ -70,70 +69,70 @@ type Action func(ctx context.Context, f *Feature) error type CleanupFunc func(ctx context.Context, cli client.Client) error // EnabledFunc is a func type used to determine if a feature should be enabled. -type EnabledFunc func(ctx context.Context, feature *Feature) (bool, error) +type EnabledFunc func(ctx context.Context, cli client.Client, feature *Feature) (bool, error) // Apply applies the feature to the cluster. // It creates a FeatureTracker resource to establish ownership and reports the result of the operation as a condition. -func (f *Feature) Apply(ctx context.Context) error { +func (f *Feature) Apply(ctx context.Context, cli client.Client) error { // If the feature is disabled, but the FeatureTracker exists in the cluster, ensure clean-up is triggered. // This means that the feature was previously enabled, but now it is not anymore. - if enabled, err := f.Enabled(ctx, f); !enabled || err != nil { + if enabled, err := f.Enabled(ctx, cli, f); !enabled || err != nil { if err != nil { return err } - return f.Cleanup(ctx) + return f.Cleanup(ctx, cli) } - if trackerErr := createFeatureTracker(ctx, f); trackerErr != nil { + if trackerErr := createFeatureTracker(ctx, cli, f); trackerErr != nil { return trackerErr } - if _, updateErr := status.UpdateWithRetry(ctx, f.Client, f.tracker, func(saved *featurev1.FeatureTracker) { + if _, updateErr := status.UpdateWithRetry(ctx, cli, f.tracker, func(saved *featurev1.FeatureTracker) { status.SetProgressingCondition(&saved.Status.Conditions, string(featurev1.ConditionReason.FeatureCreated), fmt.Sprintf("Applying feature [%s]", f.Name)) saved.Status.Phase = status.PhaseProgressing }); updateErr != nil { return updateErr } - applyErr := f.applyFeature(ctx) - _, reportErr := createFeatureTrackerStatusReporter(f).ReportCondition(ctx, applyErr) + applyErr := f.applyFeature(ctx, cli) + _, reportErr := createFeatureTrackerStatusReporter(cli, f).ReportCondition(ctx, applyErr) return multierror.Append(applyErr, reportErr).ErrorOrNil() } -func (f *Feature) applyFeature(ctx context.Context) error { +func (f *Feature) applyFeature(ctx context.Context, cli client.Client) error { var multiErr *multierror.Error for _, dataProvider := range f.dataProviders { - multiErr = multierror.Append(multiErr, dataProvider(ctx, f)) + multiErr = multierror.Append(multiErr, dataProvider(ctx, cli, f)) } if errDataLoad := multiErr.ErrorOrNil(); errDataLoad != nil { return &withConditionReasonError{reason: featurev1.ConditionReason.LoadTemplateData, err: errDataLoad} } for _, precondition := range f.preconditions { - multiErr = multierror.Append(multiErr, precondition(ctx, f)) + multiErr = multierror.Append(multiErr, precondition(ctx, cli, f)) } if preconditionsErr := multiErr.ErrorOrNil(); preconditionsErr != nil { return &withConditionReasonError{reason: featurev1.ConditionReason.PreConditions, err: preconditionsErr} } for _, clusterOperation := range f.clusterOperations { - if errClusterOperation := clusterOperation(ctx, f); errClusterOperation != nil { + if errClusterOperation := clusterOperation(ctx, cli, f); errClusterOperation != nil { return &withConditionReasonError{reason: featurev1.ConditionReason.ResourceCreation, err: errClusterOperation} } } for i := range f.appliers { r := f.appliers[i] - if processErr := r.Apply(ctx, f.Client, f.data, DefaultMetaOptions(f)...); processErr != nil { + if processErr := r.Apply(ctx, cli, f.data, DefaultMetaOptions(f)...); processErr != nil { return &withConditionReasonError{reason: featurev1.ConditionReason.ApplyManifests, err: processErr} } } for _, postcondition := range f.postconditions { - multiErr = multierror.Append(multiErr, postcondition(ctx, f)) + multiErr = multierror.Append(multiErr, postcondition(ctx, cli, f)) } if postConditionErr := multiErr.ErrorOrNil(); postConditionErr != nil { return &withConditionReasonError{reason: featurev1.ConditionReason.PostConditions, err: postConditionErr} @@ -142,14 +141,14 @@ func (f *Feature) applyFeature(ctx context.Context) error { return nil } -func (f *Feature) Cleanup(ctx context.Context) error { +func (f *Feature) Cleanup(ctx context.Context, cli client.Client) error { // Ensure associated FeatureTracker instance has been removed as last one // in the chain of cleanups. f.addCleanup(removeFeatureTracker(f)) var cleanupErrors *multierror.Error for _, cleanupFunc := range f.cleanups { - cleanupErrors = multierror.Append(cleanupErrors, cleanupFunc(ctx, f.Client)) + cleanupErrors = multierror.Append(cleanupErrors, cleanupFunc(ctx, cli)) } return cleanupErrors.ErrorOrNil() diff --git a/pkg/feature/feature_data.go b/pkg/feature/feature_data.go index 0cfe5ee1f46..82c8076fe4d 100644 --- a/pkg/feature/feature_data.go +++ b/pkg/feature/feature_data.go @@ -4,6 +4,8 @@ import ( "context" "fmt" + "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/provider" ) @@ -13,8 +15,8 @@ import ( // // If the value is static, consider using provider.ValueOf(variable).Get as passed provider function. func Entry[T any](key string, providerFunc provider.DataProviderFunc[T]) Action { - return func(ctx context.Context, f *Feature) error { - data, err := providerFunc(ctx, f.Client) + return func(ctx context.Context, cli client.Client, f *Feature) error { + data, err := providerFunc(ctx, cli) if err != nil { return err } diff --git a/pkg/feature/feature_tracker_handler.go b/pkg/feature/feature_tracker_handler.go index 6ca1f682941..6ba728a11d6 100644 --- a/pkg/feature/feature_tracker_handler.go +++ b/pkg/feature/feature_tracker_handler.go @@ -30,8 +30,8 @@ func (e *withConditionReasonError) Error() string { // createFeatureTracker creates a FeatureTracker, persists it in the cluster, // and attaches it to the provided Feature instance. -func createFeatureTracker(ctx context.Context, f *Feature) error { - tracker, errGet := getFeatureTracker(ctx, f.Client, f.Name, f.TargetNamespace) +func createFeatureTracker(ctx context.Context, cli client.Client, f *Feature) error { + tracker, errGet := getFeatureTracker(ctx, cli, f.Name, f.TargetNamespace) if client.IgnoreNotFound(errGet) != nil { return errGet } @@ -43,18 +43,18 @@ func createFeatureTracker(ctx context.Context, f *Feature) error { AppNamespace: f.TargetNamespace, } if f.owner != nil { - ownerRef := cluster.OwnedBy(f.owner, f.Client.Scheme()) + ownerRef := cluster.OwnedBy(f.owner, cli.Scheme()) if errMetaOpts := cluster.ApplyMetaOptions(tracker, ownerRef); errMetaOpts != nil { return fmt.Errorf("failed adding owner to FeatureTracker %s: %w", tracker.Name, errMetaOpts) } } - if errCreate := f.Client.Create(ctx, tracker); errCreate != nil { + if errCreate := cli.Create(ctx, tracker); errCreate != nil { return fmt.Errorf("failed creating FeatureTracker %s: %w", tracker.Name, errCreate) } } - if errGVK := ensureGVKSet(tracker, f.Client.Scheme()); errGVK != nil { + if errGVK := ensureGVKSet(tracker, cli.Scheme()); errGVK != nil { return fmt.Errorf("failed ensuring GVK is set for %s: %w", tracker.Name, errGVK) } @@ -77,7 +77,7 @@ func removeFeatureTracker(f *Feature) CleanupFunc { } if associatedTracker != nil { - return client.IgnoreNotFound(f.Client.Delete(ctx, associatedTracker)) + return client.IgnoreNotFound(cli.Delete(ctx, associatedTracker)) } return nil @@ -109,8 +109,8 @@ func ensureGVKSet(obj runtime.Object, scheme *runtime.Scheme) error { return nil } -func createFeatureTrackerStatusReporter(f *Feature) *status.Reporter[*featurev1.FeatureTracker] { - return status.NewStatusReporter(f.Client, f.tracker, func(err error) status.SaveStatusFunc[*featurev1.FeatureTracker] { +func createFeatureTrackerStatusReporter(cli client.Client, f *Feature) *status.Reporter[*featurev1.FeatureTracker] { + return status.NewStatusReporter(cli, f.tracker, func(err error) status.SaveStatusFunc[*featurev1.FeatureTracker] { updatedCondition := func(saved *featurev1.FeatureTracker) { status.SetCompleteCondition(&saved.Status.Conditions, string(featurev1.ConditionReason.FeatureCreated), fmt.Sprintf("Applied feature [%s] successfully", f.Name)) saved.Status.Phase = status.PhaseReady diff --git a/pkg/feature/handler.go b/pkg/feature/handler.go index 834234adee0..890a51a1fcc 100644 --- a/pkg/feature/handler.go +++ b/pkg/feature/handler.go @@ -15,8 +15,8 @@ import ( ) type featuresHandler interface { - Apply(ctx context.Context) error - Delete(ctx context.Context) error + Apply(ctx context.Context, cli client.Client) error + Delete(ctx context.Context, cli client.Client) error } type FeaturesRegistry interface { @@ -28,7 +28,6 @@ var _ featuresHandler = (*FeaturesHandler)(nil) // FeaturesHandler provides a structured way to manage and coordinate the creation, application, // and deletion of features needed in particular Data Science Cluster configuration. type FeaturesHandler struct { - cli client.Client source featurev1.Source owner metav1.Object targetNamespace string @@ -45,7 +44,7 @@ func (fh *FeaturesHandler) Add(builders ...*featureBuilder) error { for i := range builders { fb := builders[i] - feature, err := fb.UsingClient(fh.cli). + feature, err := fb. TargetNamespace(fh.targetNamespace). OwnedBy(fh.owner). Source(fh.source). @@ -57,7 +56,7 @@ func (fh *FeaturesHandler) Add(builders ...*featureBuilder) error { return multiErr.ErrorOrNil() } -func (fh *FeaturesHandler) Apply(ctx context.Context) error { +func (fh *FeaturesHandler) Apply(ctx context.Context, cli client.Client) error { fh.features = make([]*Feature, 0) for _, featuresProvider := range fh.featuresProviders { @@ -68,7 +67,7 @@ func (fh *FeaturesHandler) Apply(ctx context.Context) error { var multiErr *multierror.Error for _, f := range fh.features { - if applyErr := f.Apply(ctx); applyErr != nil { + if applyErr := f.Apply(ctx, cli); applyErr != nil { multiErr = multierror.Append(multiErr, fmt.Errorf("failed applying FeatureHandler features. cause: %w", applyErr)) } } @@ -78,7 +77,7 @@ func (fh *FeaturesHandler) Apply(ctx context.Context) error { // Delete executes registered clean-up tasks for handled Features in the opposite order they were initiated. // This approach assumes that Features are either instantiated in the correct sequence or are self-contained. -func (fh *FeaturesHandler) Delete(ctx context.Context) error { +func (fh *FeaturesHandler) Delete(ctx context.Context, cli client.Client) error { fh.features = make([]*Feature, 0) for _, featuresProvider := range fh.featuresProviders { @@ -89,7 +88,7 @@ func (fh *FeaturesHandler) Delete(ctx context.Context) error { var multiErr *multierror.Error for i := len(fh.features) - 1; i >= 0; i-- { - if cleanupErr := fh.features[i].Cleanup(ctx); cleanupErr != nil { + if cleanupErr := fh.features[i].Cleanup(ctx, cli); cleanupErr != nil { multiErr = multierror.Append(multiErr, fmt.Errorf("failed executing cleanup in FeatureHandler. cause: %w", cleanupErr)) } } @@ -101,9 +100,8 @@ func (fh *FeaturesHandler) Delete(ctx context.Context) error { // and add them to the handler's registry. type FeaturesProvider func(registry FeaturesRegistry) error -func ClusterFeaturesHandler(cli client.Client, dsci *dsciv1.DSCInitialization, def ...FeaturesProvider) *FeaturesHandler { +func ClusterFeaturesHandler(dsci *dsciv1.DSCInitialization, def ...FeaturesProvider) *FeaturesHandler { return &FeaturesHandler{ - cli: cli, targetNamespace: dsci.Spec.ApplicationsNamespace, owner: dsci, source: featurev1.Source{Type: featurev1.DSCIType, Name: dsci.Name}, @@ -111,9 +109,8 @@ func ClusterFeaturesHandler(cli client.Client, dsci *dsciv1.DSCInitialization, d } } -func ComponentFeaturesHandler(cli client.Client, owner metav1.Object, componentName, targetNamespace string, def ...FeaturesProvider) *FeaturesHandler { +func ComponentFeaturesHandler(owner metav1.Object, componentName, targetNamespace string, def ...FeaturesProvider) *FeaturesHandler { return &FeaturesHandler{ - cli: cli, owner: owner, targetNamespace: targetNamespace, source: featurev1.Source{Type: featurev1.ComponentType, Name: componentName}, @@ -143,16 +140,16 @@ func NewHandlerWithReporter[T client.Object](handler *FeaturesHandler, reporter } } -func (h HandlerWithReporter[T]) Apply(ctx context.Context) error { - applyErr := h.handler.Apply(ctx) +func (h HandlerWithReporter[T]) Apply(ctx context.Context, cli client.Client) error { + applyErr := h.handler.Apply(ctx, cli) _, reportErr := h.reporter.ReportCondition(ctx, applyErr) // We could have failed during Apply phase as well as during reporting. // We should return both errors to the caller. return multierror.Append(applyErr, reportErr).ErrorOrNil() } -func (h HandlerWithReporter[T]) Delete(ctx context.Context) error { - deleteErr := h.handler.Delete(ctx) +func (h HandlerWithReporter[T]) Delete(ctx context.Context, cli client.Client) error { + deleteErr := h.handler.Delete(ctx, cli) _, reportErr := h.reporter.ReportCondition(ctx, deleteErr) // We could have failed during Delete phase as well as during reporting. // We should return both errors to the caller. diff --git a/pkg/feature/resources.go b/pkg/feature/resources.go index 7b89881c9a7..177660c595e 100644 --- a/pkg/feature/resources.go +++ b/pkg/feature/resources.go @@ -3,14 +3,16 @@ package feature import ( "context" + "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" ) // CreateNamespaceIfNotExists will create a namespace with the given name if it does not exist yet. // It does not set ownership nor apply extra metadata to the existing namespace. func CreateNamespaceIfNotExists(namespace string) Action { - return func(ctx context.Context, f *Feature) error { - _, err := cluster.CreateNamespace(ctx, f.Client, namespace) + return func(ctx context.Context, cli client.Client, _ *Feature) error { + _, err := cluster.CreateNamespace(ctx, cli, namespace) return err } diff --git a/pkg/feature/serverless/conditions.go b/pkg/feature/serverless/conditions.go index 12594c9624a..cbdb7f5306a 100644 --- a/pkg/feature/serverless/conditions.go +++ b/pkg/feature/serverless/conditions.go @@ -16,11 +16,11 @@ const ( KnativeServingNamespace = "knative-serving" ) -func EnsureServerlessAbsent(ctx context.Context, f *feature.Feature) error { +func EnsureServerlessAbsent(ctx context.Context, cli client.Client, f *feature.Feature) error { list := &unstructured.UnstructuredList{} list.SetGroupVersionKind(gvk.KnativeServing) - if err := f.Client.List(ctx, list, client.InNamespace("")); err != nil { + if err := cli.List(ctx, list, client.InNamespace("")); err != nil { return fmt.Errorf("failed to list KnativeServings: %w", err) } @@ -46,8 +46,8 @@ func EnsureServerlessAbsent(ctx context.Context, f *feature.Feature) error { return errors.New("existing KNativeServing resource was found; integrating to an existing installation is not supported") } -func EnsureServerlessOperatorInstalled(ctx context.Context, f *feature.Feature) error { - if err := feature.EnsureOperatorIsInstalled("serverless-operator")(ctx, f); err != nil { +func EnsureServerlessOperatorInstalled(ctx context.Context, cli client.Client, f *feature.Feature) error { + if err := feature.EnsureOperatorIsInstalled("serverless-operator")(ctx, cli, f); err != nil { return fmt.Errorf("failed to find the pre-requisite KNative Serving Operator subscription, please ensure Serverless Operator is installed. %w", err) } diff --git a/pkg/feature/serverless/resources.go b/pkg/feature/serverless/resources.go index cc28390895d..cfd7a3ef65b 100644 --- a/pkg/feature/serverless/resources.go +++ b/pkg/feature/serverless/resources.go @@ -3,13 +3,15 @@ package serverless import ( "context" + "sigs.k8s.io/controller-runtime/pkg/client" + infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh" ) -func ServingCertificateResource(ctx context.Context, f *feature.Feature) error { +func ServingCertificateResource(ctx context.Context, cli client.Client, f *feature.Feature) error { secretData, err := getSecretParams(f) if err != nil { return err @@ -17,7 +19,7 @@ func ServingCertificateResource(ctx context.Context, f *feature.Feature) error { switch secretData.Type { case infrav1.SelfSigned: - return cluster.CreateSelfSignedCertificate(ctx, f.Client, + return cluster.CreateSelfSignedCertificate(ctx, cli, secretData.Name, secretData.Domain, secretData.Namespace, @@ -25,7 +27,7 @@ func ServingCertificateResource(ctx context.Context, f *feature.Feature) error { case infrav1.Provided: return nil default: - return cluster.PropagateDefaultIngressCertificate(ctx, f.Client, secretData.Name, secretData.Namespace) + return cluster.PropagateDefaultIngressCertificate(ctx, cli, secretData.Name, secretData.Namespace) } } diff --git a/pkg/feature/servicemesh/conditions.go b/pkg/feature/servicemesh/conditions.go index ceedc718e69..64bd360b49e 100644 --- a/pkg/feature/servicemesh/conditions.go +++ b/pkg/feature/servicemesh/conditions.go @@ -23,30 +23,30 @@ const ( ) // EnsureAuthNamespaceExists creates a namespace for the Authorization provider and set ownership so it will be garbage collected when the operator is uninstalled. -func EnsureAuthNamespaceExists(ctx context.Context, f *feature.Feature) error { +func EnsureAuthNamespaceExists(ctx context.Context, cli client.Client, f *feature.Feature) error { authNs, err := FeatureData.Authorization.Namespace.Extract(f) if err != nil { return fmt.Errorf("could not get auth from feature: %w", err) } - _, err = cluster.CreateNamespace(ctx, f.Client, authNs, feature.OwnedBy(f), cluster.WithLabels(labels.ODH.OwnedNamespace, "true")) + _, err = cluster.CreateNamespace(ctx, cli, authNs, feature.OwnedBy(f), cluster.WithLabels(labels.ODH.OwnedNamespace, "true")) return err } -func EnsureServiceMeshOperatorInstalled(ctx context.Context, f *feature.Feature) error { - if err := feature.EnsureOperatorIsInstalled("servicemeshoperator")(ctx, f); err != nil { +func EnsureServiceMeshOperatorInstalled(ctx context.Context, cli client.Client, f *feature.Feature) error { + if err := feature.EnsureOperatorIsInstalled("servicemeshoperator")(ctx, cli, f); err != nil { return fmt.Errorf("failed to find the pre-requisite Service Mesh Operator subscription, please ensure Service Mesh Operator is installed. %w", err) } return nil } -func EnsureServiceMeshInstalled(ctx context.Context, f *feature.Feature) error { - if err := EnsureServiceMeshOperatorInstalled(ctx, f); err != nil { +func EnsureServiceMeshInstalled(ctx context.Context, cli client.Client, f *feature.Feature) error { + if err := EnsureServiceMeshOperatorInstalled(ctx, cli, f); err != nil { return err } - if err := WaitForControlPlaneToBeReady(ctx, f); err != nil { + if err := WaitForControlPlaneToBeReady(ctx, cli, f); err != nil { controlPlane, errGet := FeatureData.ControlPlane.Extract(f) if errGet != nil { return fmt.Errorf("failed to get control plane struct: %w", err) @@ -60,7 +60,7 @@ func EnsureServiceMeshInstalled(ctx context.Context, f *feature.Feature) error { return nil } -func WaitForControlPlaneToBeReady(ctx context.Context, f *feature.Feature) error { +func WaitForControlPlaneToBeReady(ctx context.Context, cli client.Client, f *feature.Feature) error { controlPlane, err := FeatureData.ControlPlane.Extract(f) if err != nil { return err @@ -72,7 +72,7 @@ func WaitForControlPlaneToBeReady(ctx context.Context, f *feature.Feature) error f.Log.Info("waiting for control plane components to be ready", "control-plane", smcp, "namespace", smcpNs, "duration (s)", duration.Seconds()) return wait.PollUntilContextTimeout(ctx, interval, duration, false, func(ctx context.Context) (bool, error) { - ready, err := CheckControlPlaneComponentReadiness(ctx, f.Client, smcp, smcpNs) + ready, err := CheckControlPlaneComponentReadiness(ctx, cli, smcp, smcpNs) if ready { f.Log.Info("done waiting for control plane components to be ready", "control-plane", smcp, "namespace", smcpNs) diff --git a/pkg/feature/servicemesh/resources.go b/pkg/feature/servicemesh/resources.go index f07fdd600d9..6eaa8e8103f 100644 --- a/pkg/feature/servicemesh/resources.go +++ b/pkg/feature/servicemesh/resources.go @@ -7,6 +7,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature" @@ -14,7 +15,7 @@ import ( // MeshRefs stores service mesh configuration in the config map, so it can // be easily accessed by other components which rely on this information. -func MeshRefs(ctx context.Context, f *feature.Feature) error { +func MeshRefs(ctx context.Context, cli client.Client, f *feature.Feature) error { meshConfig, err := FeatureData.ControlPlane.Extract(f) if err != nil { return fmt.Errorf("failed to get control plane struct: %w", err) @@ -28,7 +29,7 @@ func MeshRefs(ctx context.Context, f *feature.Feature) error { return cluster.CreateOrUpdateConfigMap( ctx, - f.Client, + cli, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: ConfigMapMeshRef, @@ -42,7 +43,7 @@ func MeshRefs(ctx context.Context, f *feature.Feature) error { // AuthRefs stores authorization configuration in the config map, so it can // be easily accessed by other components which rely on this information. -func AuthRefs(ctx context.Context, f *feature.Feature) error { +func AuthRefs(ctx context.Context, cli client.Client, f *feature.Feature) error { targetNamespace := f.TargetNamespace auth, err := FeatureData.Authorization.Spec.Extract(f) if err != nil { @@ -73,7 +74,7 @@ func AuthRefs(ctx context.Context, f *feature.Feature) error { return cluster.CreateOrUpdateConfigMap( ctx, - f.Client, + cli, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: ConfigMapAuthRef, diff --git a/tests/integration/features/cleanup_int_test.go b/tests/integration/features/cleanup_int_test.go index 54f17d1fb3d..8d0634cfc0c 100644 --- a/tests/integration/features/cleanup_int_test.go +++ b/tests/integration/features/cleanup_int_test.go @@ -46,7 +46,6 @@ var _ = Describe("feature cleanup", func() { Type: featurev1.DSCIType, Name: dsci.Name, }). - UsingClient(envTestClient). PreConditions( feature.CreateNamespaceIfNotExists(namespace), ). @@ -63,7 +62,7 @@ var _ = Describe("feature cleanup", func() { It("should successfully create resource and associated feature tracker", func(ctx context.Context) { // when - Expect(testFeature.Apply(ctx)).Should(Succeed()) + Expect(testFeature.Apply(ctx, envTestClient)).Should(Succeed()) // then Eventually(createdSecretHasOwnerReferenceToOwningFeature(namespace, featureName)). @@ -75,7 +74,7 @@ var _ = Describe("feature cleanup", func() { It("should remove feature tracker on clean-up", func(ctx context.Context) { // when - Expect(testFeature.Cleanup(ctx)).To(Succeed()) + Expect(testFeature.Cleanup(ctx, envTestClient)).To(Succeed()) // then Consistently(createdSecretHasOwnerReferenceToOwningFeature(namespace, featureName)). @@ -111,7 +110,7 @@ var _ = Describe("feature cleanup", func() { err := fixtures.CreateOrUpdateNamespace(ctx, envTestClient, fixtures.NewNamespace("conditional-ns")) Expect(err).To(Not(HaveOccurred())) - featuresHandler = feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler = feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { return registry.Add(feature.Define(featureName). EnabledWhen(namespaceExists). PreConditions( @@ -122,7 +121,7 @@ var _ = Describe("feature cleanup", func() { }) // when - Expect(featuresHandler.Apply(ctx)).Should(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).Should(Succeed()) // then Eventually(createdSecretHasOwnerReferenceToOwningFeature(namespace, featureName)). @@ -138,7 +137,7 @@ var _ = Describe("feature cleanup", func() { Expect(err).To(Not(HaveOccurred())) // Mimic reconcile by re-loading the feature handler - featuresHandler = feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler = feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { return registry.Add(feature.Define(featureName). EnabledWhen(namespaceExists). PreConditions( @@ -148,7 +147,7 @@ var _ = Describe("feature cleanup", func() { ) }) - Expect(featuresHandler.Apply(ctx)).Should(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).Should(Succeed()) // then Consistently(createdSecretHasOwnerReferenceToOwningFeature(namespace, featureName)). @@ -212,8 +211,8 @@ func createdSecretHasOwnerReferenceToOwningFeature(namespace, featureName string } } -func namespaceExists(ctx context.Context, f *feature.Feature) (bool, error) { - namespace, err := fixtures.GetNamespace(ctx, f.Client, "conditional-ns") +func namespaceExists(ctx context.Context, cli client.Client, f *feature.Feature) (bool, error) { + namespace, err := fixtures.GetNamespace(ctx, cli, "conditional-ns") if errors.IsNotFound(err) { return false, nil } diff --git a/tests/integration/features/fixtures/cluster_test_fixtures.go b/tests/integration/features/fixtures/cluster_test_fixtures.go index 0dc13dc8255..0837050fd5a 100644 --- a/tests/integration/features/fixtures/cluster_test_fixtures.go +++ b/tests/integration/features/fixtures/cluster_test_fixtures.go @@ -79,8 +79,8 @@ func GetService(ctx context.Context, client client.Client, namespace, name strin return svc, err } -func CreateSecret(name, namespace string) func(ctx context.Context, f *feature.Feature) error { - return func(ctx context.Context, f *feature.Feature) error { +func CreateSecret(name, namespace string) feature.Action { + return func(ctx context.Context, cli client.Client, f *feature.Feature) error { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -94,7 +94,7 @@ func CreateSecret(name, namespace string) func(ctx context.Context, f *feature.F }, } - return f.Client.Create(ctx, secret) + return cli.Create(ctx, secret) } } diff --git a/tests/integration/features/manifests_int_test.go b/tests/integration/features/manifests_int_test.go index 0575bcbfad7..a5b9171cba7 100644 --- a/tests/integration/features/manifests_int_test.go +++ b/tests/integration/features/manifests_int_test.go @@ -46,7 +46,7 @@ var _ = Describe("Applying resources", func() { It("should be able to process an embedded YAML file", func(ctx context.Context) { // given - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errNsCreate := registry.Add(feature.Define("create-namespaces"). Manifests( manifest.Location(fixtures.TestEmbeddedFiles). @@ -60,7 +60,7 @@ var _ = Describe("Applying resources", func() { }) // when - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // then embeddedNs1, errNS1 := fixtures.GetNamespace(ctx, envTestClient, "embedded-test-ns-1") @@ -76,7 +76,7 @@ var _ = Describe("Applying resources", func() { It("should be able to process an embedded template file", func(ctx context.Context) { // given - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errSvcCreate := registry.Add(feature.Define("create-local-gw-svc"). Manifests( manifest.Location(fixtures.TestEmbeddedFiles). @@ -91,7 +91,7 @@ var _ = Describe("Applying resources", func() { }) // when - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // then service, err := fixtures.GetService(ctx, envTestClient, namespace.Name, "knative-local-gateway") @@ -110,7 +110,7 @@ metadata: Expect(fixtures.CreateFile(tempDir, "namespace.yaml", nsYAML)).To(Succeed()) - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errSvcCreate := registry.Add(feature.Define("create-namespace"). Manifests( manifest.Location(os.DirFS(tempDir)). @@ -124,7 +124,7 @@ metadata: }) // when - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // then realNs, err := fixtures.GetNamespace(ctx, envTestClient, "real-file-test-ns") diff --git a/tests/integration/features/preconditions_int_test.go b/tests/integration/features/preconditions_int_test.go index b5d620b18e0..4673a15abf4 100644 --- a/tests/integration/features/preconditions_int_test.go +++ b/tests/integration/features/preconditions_int_test.go @@ -46,7 +46,7 @@ var _ = Describe("feature preconditions", func() { defer objectCleaner.DeleteAll(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}) // when - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("create-new-ns"). PreConditions(feature.CreateNamespaceIfNotExists(namespace)), ) @@ -57,7 +57,7 @@ var _ = Describe("feature preconditions", func() { }) // then - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // and Eventually(func() error { @@ -81,7 +81,7 @@ var _ = Describe("feature preconditions", func() { defer objectCleaner.DeleteAll(ctx, ns) // when - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("create-new-ns"). PreConditions(feature.CreateNamespaceIfNotExists(namespace)), ) @@ -92,7 +92,7 @@ var _ = Describe("feature preconditions", func() { }) // then - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) }) diff --git a/tests/integration/features/resources_int_test.go b/tests/integration/features/resources_int_test.go index c8dc1219b1a..e8043f92ac1 100644 --- a/tests/integration/features/resources_int_test.go +++ b/tests/integration/features/resources_int_test.go @@ -55,7 +55,7 @@ var _ = Describe("Applying and updating resources", func() { It("should reconcile the resource to its managed state", func(ctx context.Context) { // given managed feature - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { return registry.Add( feature.Define("create-local-gw-svc"). Managed(). @@ -66,7 +66,7 @@ var _ = Describe("Applying and updating resources", func() { WithData(feature.Entry("ControlPlane", provider.ValueOf(dsci.Spec.ServiceMesh.ControlPlane).Get)), ) }) - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // expect created svc to have managed annotation service, err := fixtures.GetService(ctx, envTestClient, testNamespace, "knative-local-gateway") @@ -81,7 +81,7 @@ var _ = Describe("Applying and updating resources", func() { // then // expect that modification is reconciled away - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) updatedService, err := fixtures.GetService(ctx, envTestClient, testNamespace, "knative-local-gateway") Expect(err).ToNot(HaveOccurred()) Expect(updatedService.Annotations).To( @@ -91,7 +91,7 @@ var _ = Describe("Applying and updating resources", func() { It("should not reconcile explicitly opt-ed out resource", func(ctx context.Context) { // given managed feature - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { return registry.Add( feature.Define("create-unmanaged-svc"). Managed(). @@ -102,7 +102,7 @@ var _ = Describe("Applying and updating resources", func() { WithData(feature.Entry("ControlPlane", provider.ValueOf(dsci.Spec.ServiceMesh.ControlPlane).Get)), ) }) - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // when service, err := fixtures.GetService(ctx, envTestClient, testNamespace, "unmanaged-svc") @@ -112,7 +112,7 @@ var _ = Describe("Applying and updating resources", func() { // then // expect that modification is reconciled away - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) updatedService, err := fixtures.GetService(ctx, envTestClient, testNamespace, "unmanaged-svc") Expect(err).ToNot(HaveOccurred()) @@ -127,7 +127,7 @@ var _ = Describe("Applying and updating resources", func() { It("should not reconcile the resource", func(ctx context.Context) { // given unmanaged feature - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { return registry.Add( feature.Define("create-local-gw-svc"). Manifests( @@ -137,7 +137,7 @@ var _ = Describe("Applying and updating resources", func() { WithData(feature.Entry("ControlPlane", provider.ValueOf(dsci.Spec.ServiceMesh.ControlPlane).Get)), ) }) - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // when service, err := fixtures.GetService(ctx, envTestClient, testNamespace, "knative-local-gateway") @@ -147,7 +147,7 @@ var _ = Describe("Applying and updating resources", func() { // then // expect that modification is reconciled away - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) updatedService, err := fixtures.GetService(ctx, envTestClient, testNamespace, "knative-local-gateway") Expect(err).ToNot(HaveOccurred()) Expect(updatedService.Annotations).To( @@ -160,7 +160,7 @@ var _ = Describe("Applying and updating resources", func() { When("a feature is unmanaged but the object is marked as managed", func() { It("should reconcile this resource", func(ctx context.Context) { // given unmanaged feature but object marked with managed annotation - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { return registry.Add( feature.Define("create-managed-svc"). Manifests( @@ -170,7 +170,7 @@ var _ = Describe("Applying and updating resources", func() { WithData(feature.Entry("ControlPlane", provider.ValueOf(dsci.Spec.ServiceMesh.ControlPlane).Get)), ) }) - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // when service, err := fixtures.GetService(ctx, envTestClient, testNamespace, "managed-svc") @@ -183,7 +183,7 @@ var _ = Describe("Applying and updating resources", func() { // then // expect that modification is reconciled away - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) updatedService, err := fixtures.GetService(ctx, envTestClient, testNamespace, "managed-svc") Expect(err).ToNot(HaveOccurred()) Expect(updatedService.Annotations).To( diff --git a/tests/integration/features/serverless_feature_test.go b/tests/integration/features/serverless_feature_test.go index 9e6db931dd3..7f116479592 100644 --- a/tests/integration/features/serverless_feature_test.go +++ b/tests/integration/features/serverless_feature_test.go @@ -63,10 +63,10 @@ var _ = Describe("Serverless feature", func() { return nil } - featuresHandler := feature.ComponentFeaturesHandler(envTestClient, dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) + featuresHandler := feature.ComponentFeaturesHandler(dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) // when - applyErr := featuresHandler.Apply(ctx) + applyErr := featuresHandler.Apply(ctx, envTestClient) // then Expect(applyErr).To(MatchError(ContainSubstring("failed to find the pre-requisite operator subscription \"serverless-operator\""))) @@ -111,10 +111,10 @@ var _ = Describe("Serverless feature", func() { return nil } - featuresHandler := feature.ComponentFeaturesHandler(envTestClient, dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) + featuresHandler := feature.ComponentFeaturesHandler(dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) // then - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) }) It("should succeed if serving is not installed for KNative serving precondition", func(ctx context.Context) { @@ -130,10 +130,10 @@ var _ = Describe("Serverless feature", func() { return nil } - featuresHandler := feature.ComponentFeaturesHandler(envTestClient, dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) + featuresHandler := feature.ComponentFeaturesHandler(dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) // then - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) }) It("should fail if serving is already installed for KNative serving precondition", func(ctx context.Context) { @@ -160,10 +160,10 @@ var _ = Describe("Serverless feature", func() { return nil } - featuresHandler := feature.ComponentFeaturesHandler(envTestClient, dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) + featuresHandler := feature.ComponentFeaturesHandler(dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) // then - Expect(featuresHandler.Apply(ctx)).ToNot(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).ToNot(Succeed()) }) }) @@ -171,17 +171,6 @@ var _ = Describe("Serverless feature", func() { Context("default values", func() { - var testFeature *feature.Feature - - BeforeEach(func() { - // Stubbing feature as we want to test particular functions in isolation - testFeature = &feature.Feature{ - Name: "test-feature", - } - - testFeature.Client = envTestClient - }) - Context("ingress gateway TLS secret name", func() { It("should set default value when value is empty in the DSCI", func(ctx context.Context) { @@ -302,10 +291,10 @@ var _ = Describe("Serverless feature", func() { return nil } - featuresHandler := feature.ComponentFeaturesHandler(envTestClient, dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) + featuresHandler := feature.ComponentFeaturesHandler(dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) // when - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // then Eventually(func() error { @@ -344,10 +333,10 @@ var _ = Describe("Serverless feature", func() { return nil } - featuresHandler := feature.ComponentFeaturesHandler(envTestClient, dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) + featuresHandler := feature.ComponentFeaturesHandler(dsci, kserveComponent.GetComponentName(), dsci.Spec.ApplicationsNamespace, featuresProvider) // when - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // then Consistently(func() error { diff --git a/tests/integration/features/servicemesh_feature_test.go b/tests/integration/features/servicemesh_feature_test.go index 43c0a3b8d0b..16a7b419063 100644 --- a/tests/integration/features/servicemesh_feature_test.go +++ b/tests/integration/features/servicemesh_feature_test.go @@ -53,7 +53,7 @@ var _ = Describe("Service Mesh setup", func() { It("should fail using precondition check", func(ctx context.Context) { // given - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("no-service-mesh-operator-check"). PreConditions(servicemesh.EnsureServiceMeshOperatorInstalled), ) @@ -64,7 +64,7 @@ var _ = Describe("Service Mesh setup", func() { }) // when - applyErr := featuresHandler.Apply(ctx) + applyErr := featuresHandler.Apply(ctx, envTestClient) // then Expect(applyErr).To(MatchError(ContainSubstring("failed to find the pre-requisite operator subscription \"servicemeshoperator\""))) @@ -86,7 +86,7 @@ var _ = Describe("Service Mesh setup", func() { It("should succeed using precondition check", func(ctx context.Context) { // when - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add( feature.Define("service-mesh-operator-check"). WithData(feature.Entry("ControlPlane", provider.ValueOf(dsci.Spec.ServiceMesh.ControlPlane).Get)). @@ -99,7 +99,7 @@ var _ = Describe("Service Mesh setup", func() { }) // when - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) }) @@ -118,7 +118,7 @@ var _ = Describe("Service Mesh setup", func() { dsci.Spec.ServiceMesh.ControlPlane.Name = "test-name" // when - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("service-mesh-control-plane-check"). WithData(feature.Entry("ControlPlane", provider.ValueOf(dsci.Spec.ServiceMesh.ControlPlane).Get)). PreConditions(servicemesh.EnsureServiceMeshInstalled), @@ -130,7 +130,7 @@ var _ = Describe("Service Mesh setup", func() { }) // then - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) }) It("should fail to find Service Mesh Control Plane if not present", func(ctx context.Context) { @@ -139,7 +139,7 @@ var _ = Describe("Service Mesh setup", func() { dsci.Spec.ServiceMesh.ControlPlane.Namespace = "test-namespace" // when - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("no-service-mesh-control-plane-check"). WithData(feature.Entry("ControlPlane", provider.ValueOf(dsci.Spec.ServiceMesh.ControlPlane).Get)). PreConditions(servicemesh.EnsureServiceMeshInstalled), @@ -151,7 +151,7 @@ var _ = Describe("Service Mesh setup", func() { }) // then - Expect(featuresHandler.Apply(ctx)).To(MatchError(ContainSubstring("failed to find Service Mesh Control Plane"))) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(MatchError(ContainSubstring("failed to find Service Mesh Control Plane"))) }) }) @@ -195,7 +195,7 @@ var _ = Describe("Service Mesh setup", func() { createServiceMeshControlPlane(ctx, name, namespace) - handler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + handler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { return registry.Add(feature.Define("control-plane-with-external-authz-provider"). Manifests( manifest.Location(fixtures.TestEmbeddedFiles). @@ -217,7 +217,7 @@ var _ = Describe("Service Mesh setup", func() { // when By("verifying extension provider has been added after applying feature", func() { - Expect(handler.Apply(ctx)).To(Succeed()) + Expect(handler.Apply(ctx, envTestClient)).To(Succeed()) serviceMeshControlPlane, err := getServiceMeshControlPlane(ctx, namespace, name) Expect(err).ToNot(HaveOccurred()) @@ -240,7 +240,7 @@ var _ = Describe("Service Mesh setup", func() { // then By("verifying that extension provider has been removed and namespace is gone too", func() { - Expect(handler.Delete(ctx)).To(Succeed()) + Expect(handler.Delete(ctx, envTestClient)).To(Succeed()) Eventually(func(ctx context.Context) []any { serviceMeshControlPlane, err := getServiceMeshControlPlane(ctx, namespace, name) diff --git a/tests/integration/features/tracker_int_test.go b/tests/integration/features/tracker_int_test.go index 370bfe25117..b4f7bca7944 100644 --- a/tests/integration/features/tracker_int_test.go +++ b/tests/integration/features/tracker_int_test.go @@ -6,6 +6,7 @@ import ( conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" corev1 "k8s.io/api/core/v1" + ctrlruntime "sigs.k8s.io/controller-runtime/pkg/client" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1" @@ -35,7 +36,7 @@ var _ = Describe("Feature tracking capability", func() { Context("Reporting progress when applying Feature", func() { It("should indicate successful installation in FeatureTracker through Status conditions", func(ctx context.Context) { - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("always-working-feature")) Expect(errFeatureAdd).ToNot(HaveOccurred()) @@ -44,7 +45,7 @@ var _ = Describe("Feature tracking capability", func() { }) // when - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // then featureTracker, err := fixtures.GetFeatureTracker(ctx, envTestClient, appNamespace, "always-working-feature") @@ -61,9 +62,9 @@ var _ = Describe("Feature tracking capability", func() { It("should indicate when failure occurs in preconditions through Status conditions", func(ctx context.Context) { // given - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("precondition-fail"). - PreConditions(func(_ context.Context, _ *feature.Feature) error { + PreConditions(func(_ context.Context, _ ctrlruntime.Client, _ *feature.Feature) error { return errors.New("during test always fail") }), ) @@ -74,7 +75,7 @@ var _ = Describe("Feature tracking capability", func() { }) // when - Expect(featuresHandler.Apply(ctx)).ToNot(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).ToNot(Succeed()) // then featureTracker, err := fixtures.GetFeatureTracker(ctx, envTestClient, appNamespace, "precondition-fail") @@ -91,9 +92,9 @@ var _ = Describe("Feature tracking capability", func() { It("should indicate when failure occurs in post-conditions through Status conditions", func(ctx context.Context) { // given - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("post-condition-failure"). - PostConditions(func(_ context.Context, _ *feature.Feature) error { + PostConditions(func(_ context.Context, _ ctrlruntime.Client, _ *feature.Feature) error { return errors.New("during test always fail") }), ) @@ -104,7 +105,7 @@ var _ = Describe("Feature tracking capability", func() { }) // when - Expect(featuresHandler.Apply(ctx)).ToNot(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).ToNot(Succeed()) // then featureTracker, err := fixtures.GetFeatureTracker(ctx, envTestClient, appNamespace, "post-condition-failure") @@ -124,7 +125,7 @@ var _ = Describe("Feature tracking capability", func() { It("should correctly indicate source in the feature tracker", func(ctx context.Context) { // given - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("always-working-feature")) Expect(errFeatureAdd).ToNot(HaveOccurred()) @@ -133,7 +134,7 @@ var _ = Describe("Feature tracking capability", func() { }) // when - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // then featureTracker, err := fixtures.GetFeatureTracker(ctx, envTestClient, appNamespace, "always-working-feature") @@ -148,7 +149,7 @@ var _ = Describe("Feature tracking capability", func() { It("should correctly indicate app namespace in the feature tracker", func(ctx context.Context) { // given - featuresHandler := feature.ClusterFeaturesHandler(envTestClient, dsci, func(registry feature.FeaturesRegistry) error { + featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error { errFeatureAdd := registry.Add(feature.Define("empty-feature")) Expect(errFeatureAdd).ToNot(HaveOccurred()) @@ -157,7 +158,7 @@ var _ = Describe("Feature tracking capability", func() { }) // when - Expect(featuresHandler.Apply(ctx)).To(Succeed()) + Expect(featuresHandler.Apply(ctx, envTestClient)).To(Succeed()) // then featureTracker, err := fixtures.GetFeatureTracker(ctx, envTestClient, appNamespace, "empty-feature") @@ -175,7 +176,6 @@ var _ = Describe("Feature tracking capability", func() { Expect(fixtures.CreateOrUpdateDSCI(ctx, envTestClient, dsci)).ToNot(HaveOccurred()) feature, featErr := feature.Define("empty-feat-with-owner"). - UsingClient(envTestClient). Source(featurev1.Source{ Type: featurev1.DSCIType, Name: dsci.Name, @@ -186,7 +186,7 @@ var _ = Describe("Feature tracking capability", func() { // when Expect(featErr).ToNot(HaveOccurred()) - Expect(feature.Apply(ctx)).To(Succeed()) + Expect(feature.Apply(ctx, envTestClient)).To(Succeed()) // then tracker, err := fixtures.GetFeatureTracker(ctx, envTestClient, appNamespace, "empty-feat-with-owner") @@ -197,7 +197,6 @@ var _ = Describe("Feature tracking capability", func() { It("should not indicate owner in the feature tracker when owner not in feature", func(ctx context.Context) { // given feature, featErr := feature.Define("empty-feat-no-owner"). - UsingClient(envTestClient). Source(featurev1.Source{ Type: featurev1.DSCIType, Name: dsci.Name, @@ -207,7 +206,7 @@ var _ = Describe("Feature tracking capability", func() { // when Expect(featErr).ToNot(HaveOccurred()) - Expect(feature.Apply(ctx)).To(Succeed()) + Expect(feature.Apply(ctx, envTestClient)).To(Succeed()) // then tracker, err := fixtures.GetFeatureTracker(ctx, envTestClient, appNamespace, "empty-feat-no-owner")