diff --git a/controllers/dscinitialization/servicemesh_setup.go b/controllers/dscinitialization/servicemesh_setup.go index fb8168db122..283a0f33570 100644 --- a/controllers/dscinitialization/servicemesh_setup.go +++ b/controllers/dscinitialization/servicemesh_setup.go @@ -203,7 +203,10 @@ func (r *DSCInitializationReconciler) authorizationFeatures(instance *dsciv1.DSC feature.WaitForPodsToBeReady(serviceMeshSpec.ControlPlane.Namespace), ). OnDelete( - servicemesh.RemoveExtensionProvider, + servicemesh.RemoveExtensionProvider( + instance.Spec.ServiceMesh.ControlPlane, + instance.Spec.ApplicationsNamespace+"-auth-provider", + ), ), // We do not have the control over deployment resource creation. diff --git a/pkg/feature/builder.go b/pkg/feature/builder.go index 7ddfbbeb6bd..e182c716be3 100644 --- a/pkg/feature/builder.go +++ b/pkg/feature/builder.go @@ -174,11 +174,7 @@ func (fb *featureBuilder) PostConditions(postconditions ...Action) *featureBuild } // OnDelete allow to add cleanup hooks that are executed when the feature is going to be deleted. -// By default, all resources created by the feature are deleted when the feature is deleted, so there is no need to -// explicitly add cleanup hooks for them. -// -// This is useful when you need to perform some additional cleanup actions such as removing effects of a patch operation. -func (fb *featureBuilder) OnDelete(cleanups ...Action) *featureBuilder { +func (fb *featureBuilder) OnDelete(cleanups ...CleanupFunc) *featureBuilder { fb.builders = append(fb.builders, func(f *Feature) error { f.addCleanup(cleanups...) diff --git a/pkg/feature/feature.go b/pkg/feature/feature.go index 5ff39b08a48..967e284cacc 100644 --- a/pkg/feature/feature.go +++ b/pkg/feature/feature.go @@ -51,7 +51,7 @@ type Feature struct { appliers []resource.Applier - cleanups []Action + cleanups []CleanupFunc clusterOperations []Action preconditions []Action postconditions []Action @@ -62,6 +62,12 @@ type Feature struct { // while having access to Feature struct. type Action func(ctx context.Context, 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, +// so there is no need to explicitly add cleanup hooks for them. +// This is useful when you need to perform some additional cleanup actions such as removing effects of a patch operation. +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) @@ -138,26 +144,17 @@ func (f *Feature) applyFeature(ctx context.Context) error { func (f *Feature) Cleanup(ctx context.Context) error { // Ensure associated FeatureTracker instance has been removed as last one // in the chain of cleanups. - f.addCleanup(removeFeatureTracker) + f.addCleanup(removeFeatureTracker(f)) var cleanupErrors *multierror.Error - - for _, dataProvider := range f.dataProviders { - cleanupErrors = multierror.Append(cleanupErrors, dataProvider(ctx, f)) - } - - if dataLoadErr := cleanupErrors.ErrorOrNil(); dataLoadErr != nil { - return dataLoadErr - } - for _, cleanupFunc := range f.cleanups { - cleanupErrors = multierror.Append(cleanupErrors, cleanupFunc(ctx, f)) + cleanupErrors = multierror.Append(cleanupErrors, cleanupFunc(ctx, f.Client)) } return cleanupErrors.ErrorOrNil() } -func (f *Feature) addCleanup(cleanupFuncs ...Action) { +func (f *Feature) addCleanup(cleanupFuncs ...CleanupFunc) { f.cleanups = append(f.cleanups, cleanupFuncs...) } diff --git a/pkg/feature/feature_tracker_handler.go b/pkg/feature/feature_tracker_handler.go index f9976df05e2..403d87abbea 100644 --- a/pkg/feature/feature_tracker_handler.go +++ b/pkg/feature/feature_tracker_handler.go @@ -30,7 +30,7 @@ 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) + tracker, errGet := getFeatureTracker(ctx, f.Client, f.Name, f.TargetNamespace) if client.IgnoreNotFound(errGet) != nil { return errGet } @@ -56,28 +56,30 @@ func createFeatureTracker(ctx context.Context, f *Feature) error { } // removeFeatureTracker removes the FeatureTracker associated with the provided Feature instance if one exists in the cluster. -func removeFeatureTracker(ctx context.Context, f *Feature) error { - associatedTracker := f.tracker - if associatedTracker == nil { - // Check if it is persisted in the cluster, but Feature do not have it attached - if tracker, errGet := getFeatureTracker(ctx, f); client.IgnoreNotFound(errGet) != nil { - return errGet - } else { - associatedTracker = tracker +func removeFeatureTracker(f *Feature) CleanupFunc { + return func(ctx context.Context, cli client.Client) error { + associatedTracker := f.tracker + if associatedTracker == nil { + // Check if it is persisted in the cluster, but Feature do not have it attached + if tracker, errGet := getFeatureTracker(ctx, cli, f.Name, f.TargetNamespace); client.IgnoreNotFound(errGet) != nil { + return errGet + } else { + associatedTracker = tracker + } } - } - if associatedTracker != nil { - return client.IgnoreNotFound(f.Client.Delete(ctx, associatedTracker)) - } + if associatedTracker != nil { + return client.IgnoreNotFound(f.Client.Delete(ctx, associatedTracker)) + } - return nil + return nil + } } -func getFeatureTracker(ctx context.Context, f *Feature) (*featurev1.FeatureTracker, error) { - tracker := featurev1.NewFeatureTracker(f.Name, f.TargetNamespace) +func getFeatureTracker(ctx context.Context, cli client.Client, featureName, namespace string) (*featurev1.FeatureTracker, error) { + tracker := featurev1.NewFeatureTracker(featureName, namespace) - if errGet := f.Client.Get(ctx, client.ObjectKeyFromObject(tracker), tracker); errGet != nil { + if errGet := cli.Get(ctx, client.ObjectKeyFromObject(tracker), tracker); errGet != nil { return nil, errGet } diff --git a/pkg/feature/servicemesh/cleanup.go b/pkg/feature/servicemesh/cleanup.go index 87163442591..73ccc211c49 100644 --- a/pkg/feature/servicemesh/cleanup.go +++ b/pkg/feature/servicemesh/cleanup.go @@ -2,65 +2,62 @@ package servicemesh import ( "context" - "fmt" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "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/gvk" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature" ) -func RemoveExtensionProvider(ctx context.Context, f *feature.Feature) error { - extensionName, errExtName := FeatureData.Authorization.ExtensionProviderName.Extract(f) - if errExtName != nil { - return fmt.Errorf("failed to get extension name struct: %w", errExtName) - } +func RemoveExtensionProvider(controlPlane infrav1.ControlPlaneSpec, extensionName string) feature.CleanupFunc { + return func(ctx context.Context, cli client.Client) error { + smcp := &unstructured.Unstructured{} + smcp.SetGroupVersionKind(gvk.ServiceMeshControlPlane) - controlPlane, err := FeatureData.ControlPlane.Extract(f) - if err != nil { - return fmt.Errorf("failed to get control plane struct: %w", err) - } + if err := cli.Get(ctx, client.ObjectKey{ + Namespace: controlPlane.Namespace, + Name: controlPlane.Name, + }, smcp); err != nil { + return client.IgnoreNotFound(err) + } - smcp := &unstructured.Unstructured{} - smcp.SetGroupVersionKind(gvk.ServiceMeshControlPlane) + extensionProviders, found, err := unstructured.NestedSlice(smcp.Object, "spec", "techPreview", "meshConfig", "extensionProviders") + if err != nil { + return err + } + if !found { + return nil + } - if err := f.Client.Get(ctx, client.ObjectKey{ - Namespace: controlPlane.Namespace, - Name: controlPlane.Name, - }, smcp); err != nil { - return client.IgnoreNotFound(err) - } + removed := false - extensionProviders, found, err := unstructured.NestedSlice(smcp.Object, "spec", "techPreview", "meshConfig", "extensionProviders") - if err != nil { - return err - } - if !found { - f.Log.Info("no extension providers found", "feature", f.Name, "control-plane", controlPlane.Name, "namespace", controlPlane.Namespace) - return nil - } + for i, v := range extensionProviders { + extensionProvider, ok := v.(map[string]interface{}) + if !ok { + continue + } - for i, v := range extensionProviders { - extensionProvider, ok := v.(map[string]interface{}) - if !ok { - f.Log.Info("WARN: Unexpected type for extensionProvider, it will not be removed") - continue - } - currentExtensionName, isString := extensionProvider["name"].(string) - if !isString { - f.Log.Info("WARN: Unexpected type for currentExtensionName, it will not be removed") - continue - } - if currentExtensionName == extensionName { - extensionProviders = append(extensionProviders[:i], extensionProviders[i+1:]...) - err = unstructured.SetNestedSlice(smcp.Object, extensionProviders, "spec", "techPreview", "meshConfig", "extensionProviders") - if err != nil { - return err + currentExtensionName, isString := extensionProvider["name"].(string) + if !isString { + continue + } + if currentExtensionName == extensionName { + extensionProviders = append(extensionProviders[:i], extensionProviders[i+1:]...) + err = unstructured.SetNestedSlice(smcp.Object, extensionProviders, "spec", "techPreview", "meshConfig", "extensionProviders") + if err != nil { + return err + } + removed = true + break } - break } - } - return f.Client.Update(ctx, smcp) + if removed { + return cli.Update(ctx, smcp) + } + + return nil + } } diff --git a/tests/integration/features/servicemesh_feature_test.go b/tests/integration/features/servicemesh_feature_test.go index abfd7285a3a..1c77c0eea7c 100644 --- a/tests/integration/features/servicemesh_feature_test.go +++ b/tests/integration/features/servicemesh_feature_test.go @@ -212,7 +212,10 @@ var _ = Describe("Service Mesh setup", func() { servicemesh.FeatureData.ControlPlane.Define(&dsci.Spec).AsAction(), ). OnDelete( - servicemesh.RemoveExtensionProvider, + servicemesh.RemoveExtensionProvider( + dsci.Spec.ServiceMesh.ControlPlane, + dsci.Spec.ApplicationsNamespace+"-auth-provider", + ), )) })