feat(cleanup): remove -auth-provider namespace when remove Featuretra…

…kcer (#952) * feat(cleanup): remove -auth-provicer namespace when remove FTer - add OwnedBy() into namespace creation step as precondition - add check in testcase --------- Signed-off-by: Wen Zhou <[email protected]> Co-authored-by: Bartosz Majsak <[email protected]>
opendatahub-io · Apr 8, 2024 · 562ae3d · 562ae3d
1 parent 93e2126
commit 562ae3d
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 7 deletions.
diff --git a/components/workbenches/workbenches.go b/components/workbenches/workbenches.go
@@ -124,6 +124,8 @@ func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client,
 			}
 		}
 		if platform == deploy.SelfManagedRhods || platform == deploy.ManagedRhods {
+			// Intentionally leaving the ownership unset for this namespace.
+			// Specifying this label triggers its deletion when the operator is uninstalled.
 			_, err := cluster.CreateNamespace(cli, "rhods-notebooks", cluster.WithLabels(labels.ODH.OwnedNamespace, "true"))
 			if err != nil {
 				return err
@@ -135,7 +137,6 @@ func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client,
 			return err
 		}
 	}
-
 	if err = deploy.DeployManifestsFromPath(cli, owner, notebookControllerPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
 		return fmt.Errorf("failed to apply manifetss %s: %w", notebookControllerPath, err)
 	}

diff --git a/controllers/dscinitialization/servicemesh_setup.go b/controllers/dscinitialization/servicemesh_setup.go
@@ -192,7 +192,9 @@ func (r *DSCInitializationReconciler) authorizationFeatures(instance *dsciv1.DSC
 					return f.ApplyManifest(path.Join(feature.AuthDir, "deployment.injection.patch.tmpl"))
 				},
 			).
-			OnDelete(servicemesh.RemoveExtensionProvider).
+			OnDelete(
+				servicemesh.RemoveExtensionProvider,
+			).
 			Load()
 		if extAuthzErr != nil {
 			return extAuthzErr

diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go
@@ -114,7 +114,8 @@ func CreateOrUpdateConfigMap(c client.Client, name string, namespace string, dat
 	return configMap, c.Update(context.TODO(), configMap)
 }
 
-// CreateNamespace creates namespace required by workbenches component in downstream.
+// CreateNamespace creates a namespace and apply metadata.
+// If a namespace already exists, the operation has no effect on it.
 func CreateNamespace(cli client.Client, namespace string, metaOptions ...MetaOptions) (*corev1.Namespace, error) {
 	desiredNamespace := &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{

diff --git a/pkg/feature/resources.go b/pkg/feature/resources.go
@@ -4,7 +4,8 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 )
 
-// CreateNamespaceIfNotExists will create namespace with the given name if it does not exist yet, but will not own it.
+// 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(f *Feature) error {
 		_, err := cluster.CreateNamespace(f.Client, namespace)

diff --git a/pkg/feature/servicemesh/conditions.go b/pkg/feature/servicemesh/conditions.go
@@ -20,12 +20,13 @@ const (
 	duration = 5 * time.Minute
 )
 
+// EnsureAuthNamespaceExists creates a namespace for the Authorization provider and set ownership so it will be garbage collected when the operator is uninstalled.
 func EnsureAuthNamespaceExists(f *feature.Feature) error {
 	if resolveNsErr := ResolveAuthNamespace(f); resolveNsErr != nil {
 		return resolveNsErr
 	}
 
-	_, err := cluster.CreateNamespace(f.Client, f.Spec.Auth.Namespace)
+	_, err := cluster.CreateNamespace(f.Client, f.Spec.Auth.Namespace, feature.OwnedBy(f))
 	return err
 }
 

diff --git a/tests/integration/features/servicemesh_feature_test.go b/tests/integration/features/servicemesh_feature_test.go
@@ -5,6 +5,7 @@ import (
 	"path"
 
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -202,7 +203,9 @@ var _ = Describe("Service Mesh setup", func() {
 						return feature.CreateFeature("control-plane-with-external-authz-provider").
 							For(handler).
 							Manifests(path.Join(feature.AuthDir, "mesh-authz-ext-provider.patch.tmpl")).
-							OnDelete(servicemesh.RemoveExtensionProvider).
+							OnDelete(
+								servicemesh.RemoveExtensionProvider,
+							).
 							UsingConfig(envTest.Config).
 							Load()
 					})
@@ -231,7 +234,7 @@ var _ = Describe("Service Mesh setup", func() {
 					})
 
 					// then
-					By("verifying that extension provider has been removed", func() {
+					By("verifying that extension provider has been removed and namespace is gone too", func() {
 						Expect(handler.Delete()).To(Succeed())
 						Eventually(func() []interface{} {
 
@@ -241,6 +244,10 @@ var _ = Describe("Service Mesh setup", func() {
 							extensionProviders, found, err := unstructured.NestedSlice(serviceMeshControlPlane.Object, "spec", "techPreview", "meshConfig", "extensionProviders")
 							Expect(err).ToNot(HaveOccurred())
 							Expect(found).To(BeTrue())
+
+							_, err = fixtures.GetNamespace(envTestClient, serviceMeshSpec.Auth.Namespace)
+							Expect(errors.IsNotFound(err)).To(BeTrue())
+
 							return extensionProviders
 
 						}).WithTimeout(fixtures.Timeout).WithPolling(fixtures.Interval).Should(BeEmpty())