Repository: openshift/assisted-service (language: python)

Issue:
MGMT-18448: Allow local cluster import to be disabled.
We need to make sure that the local cluster import may be disabled.
The import should be switched off by default and only overridden with an annotation.

This PR implements this functionality.

If the local cluster import functionality is disabled then all local cluster entities controlled
by assisted will be left intact, we will simply skip the reconcile loop.

Customers who need to enable the feature may do so by placing an annotation on the AgentServiceConfig.

## List all the issues related to this PR

- [ ] New Feature <!-- new functionality -->
- [ ] Enhancement <!-- refactor, code changes, improvement, that won't add new features -->
- [x] Bug fix
- [ ] Tests
- [ ] Documentation
- [ ] CI/CD <!-- Notice that changes for Dockerfiles/Jenkinsfiles aren't tested in CI due to a known bug. -->

## What environments does this code impact?

- [x] Automation (CI, tools, etc)
- [x] Cloud
- [x] Operator Managed Deployments
- [x] None

## How was this code tested?

<!-- Please, select one or more if needed: -->

- [ ] assisted-test-infra environment
- [ ] dev-scripts environment
- [ ] Reviewer's test appreciated
- [x] Waiting for CI to do a full test run
- [ ] Manual (Elaborate on how it was tested)
- [x] No tests needed

## Checklist

- [x] Title and description added to both, commit and PR.
- [x] Relevant issues have been associated (see [CONTRIBUTING] guide)
- [x] This change does not require a documentation update (docstring, `docs`, README, etc)
- [x] Does this change include unit-tests (note that code changes require unit-tests)

## Reviewers Checklist

- Are the title and description (in both PR and commit) meaningful and clear?
- Is there a bug required (and linked) for this change?
- Should this PR be backported?

[Kubernetes community documentation]: https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#commit-message-guidelines
[CONTRIBUTING]: https://github.com/openshift/assisted-service/blob/master/CONTRIBUTING.md



Discussion / hints:

Fix patch:
diff --git a/api/v1beta1/agentserviceconfig_types.go b/api/v1beta1/agentserviceconfig_types.go
index f95128cfe7c..ef3ab07680d 100644
--- a/api/v1beta1/agentserviceconfig_types.go
+++ b/api/v1beta1/agentserviceconfig_types.go
@@ -133,7 +133,8 @@ const (
 	ConditionReconcileCompleted conditionsv1.ConditionType = "ReconcileCompleted"
 	// ConditionDeploymentsHealthy reports whether deployments are healthy.
 	ConditionDeploymentsHealthy conditionsv1.ConditionType = "DeploymentsHealthy"
-
+	// ReasonLocalClusterImportNotEnabled when the import of local cluster is not enabled.
+	ReasonLocalClusterImportNotEnabled string = "Local cluster import is not enabled"
 	// ReasonLocalClusterEntitiesCreated when the local cluster is managed.
 	ReasonLocalClusterManaged string = "Local cluster is managed."
 	// ReasonLocalClusterEntitiesRemoved when the local cluster is not managed.
diff --git a/internal/controller/controllers/local_cluster_import_controller.go b/internal/controller/controllers/local_cluster_import_controller.go
index e752ec4d0a5..c1378f87f01 100644
--- a/internal/controller/controllers/local_cluster_import_controller.go
+++ b/internal/controller/controllers/local_cluster_import_controller.go
@@ -35,6 +35,7 @@ const (
 	hubKubeConfigName                                 = "node-kubeconfigs"
 	hubPullSecretNamespace                            = "openshift-config" // #nosec G101
 	hubPullSecretName                                 = "pull-secret"      // #nosec G101
+	importLocalClusterEnabledAnnotation               = "agent-install.openshift.io/enable-local-cluster-import"
 )
 
 type LocalClusterImportReconciler struct {
@@ -126,6 +127,16 @@ func (r *LocalClusterImportReconciler) Reconcile(ctx context.Context, req ctrl.R
 		return ctrl.Result{}, nil
 	}
 
+	// Enable local cluster import if annotation allows this.
+	_, importLocalClusterEnabled := instance.GetAnnotations()[importLocalClusterEnabledAnnotation]
+	if !importLocalClusterEnabled {
+		err := r.setReconciliationStatus(ctx, false, aiv1beta1.ReasonLocalClusterImportNotEnabled, "Local cluster import is not enabled")
+		if err != nil {
+			return ctrl.Result{}, errors.Wrap(err, "Unable to set reconciliation status of LocalClusterImport on AgentServiceConfig")
+		}
+		return ctrl.Result{}, nil
+	}
+
 	hasManagedCluster, err := r.hasLocalManagedCluster(ctx)
 	if err != nil {
 		err = r.setReconciliationStatus(ctx, false, aiv1beta1.ReasonUnableToDetermineLocalClusterManagedStatus, err.Error())
diff --git a/vendor/github.com/openshift/assisted-service/api/v1beta1/agentserviceconfig_types.go b/vendor/github.com/openshift/assisted-service/api/v1beta1/agentserviceconfig_types.go
index f95128cfe7c..ef3ab07680d 100644
--- a/vendor/github.com/openshift/assisted-service/api/v1beta1/agentserviceconfig_types.go
+++ b/vendor/github.com/openshift/assisted-service/api/v1beta1/agentserviceconfig_types.go
@@ -133,7 +133,8 @@ const (
 	ConditionReconcileCompleted conditionsv1.ConditionType = "ReconcileCompleted"
 	// ConditionDeploymentsHealthy reports whether deployments are healthy.
 	ConditionDeploymentsHealthy conditionsv1.ConditionType = "DeploymentsHealthy"
-
+	// ReasonLocalClusterImportNotEnabled when the import of local cluster is not enabled.
+	ReasonLocalClusterImportNotEnabled string = "Local cluster import is not enabled"
 	// ReasonLocalClusterEntitiesCreated when the local cluster is managed.
 	ReasonLocalClusterManaged string = "Local cluster is managed."
 	// ReasonLocalClusterEntitiesRemoved when the local cluster is not managed.


Test patch:
diff --git a/internal/controller/controllers/local_cluster_import_controller_test.go b/internal/controller/controllers/local_cluster_import_controller_test.go
index 1ae2f8cb4ff..1636f06c903 100644
--- a/internal/controller/controllers/local_cluster_import_controller_test.go
+++ b/internal/controller/controllers/local_cluster_import_controller_test.go
@@ -96,6 +96,9 @@ var _ = Describe("Reconcile", func() {
 		agentServiceConfig = &aiv1beta1.AgentServiceConfig{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "agent",
+				Annotations: map[string]string{
+					importLocalClusterEnabledAnnotation: "true",
+				},
 			},
 			Spec: aiv1beta1.AgentServiceConfigSpec{
 				DatabaseStorage: corev1.PersistentVolumeClaimSpec{
@@ -270,6 +273,29 @@ var _ = Describe("Reconcile", func() {
 		Expect(k8serrors.IsNotFound(err)).To(BeTrue())
 	})
 
+	It("cluster import CR's should not be present after reconcile if Local Cluster Import Annotation is not present in AgentServiceConfig", func() {
+		asc := &aiv1beta1.AgentServiceConfig{}
+		clusterDeployment := &hivev1.ClusterDeployment{}
+		agentClusterInstall := &hiveext.AgentClusterInstall{}
+		infraEnv := &aiv1beta1.InfraEnv{}
+		nameSpacedName := types.NamespacedName{
+			Name: "agent",
+		}
+		err := client.Get(ctx, nameSpacedName, asc)
+		Expect(err).To(BeNil())
+		asc.ObjectMeta.Annotations = nil
+		err = client.Update(ctx, asc)
+		Expect(err).To(BeNil())
+		_, err = reconciler.Reconcile(ctx, request)
+		Expect(err).ToNot(HaveOccurred())
+		err = client.Get(ctx, nameSpacedName, clusterDeployment)
+		Expect(k8serrors.IsNotFound(err)).To(BeTrue())
+		err = client.Get(ctx, nameSpacedName, agentClusterInstall)
+		Expect(k8serrors.IsNotFound(err)).To(BeTrue())
+		err = client.Get(ctx, nameSpacedName, infraEnv)
+		Expect(k8serrors.IsNotFound(err)).To(BeTrue())
+	})
+
 	It("cluster import CR's should not be present after reconcile if ManagedCluster is not present", func() {
 		clusterDeployment := &hivev1.ClusterDeployment{}
 		agentClusterInstall := &hiveext.AgentClusterInstall{}
@@ -342,6 +368,41 @@ var _ = Describe("Reconcile", func() {
 		Expect(k8serrors.IsNotFound(err)).To(BeTrue())
 	})
 
+	It("should NOT remove cluster import CR's when AgentServiceConfig has Local Cluster Import Annotation removed", func() {
+		clusterDeployment := &hivev1.ClusterDeployment{}
+		agentClusterInstall := &hiveext.AgentClusterInstall{}
+		infraEnv := &aiv1beta1.InfraEnv{}
+		namespacedname := types.NamespacedName{
+			Namespace: localClusterName,
+			Name:      localClusterName,
+		}
+		awaitReconcile(aiv1beta1.ConditionLocalClusterManaged, aiv1beta1.ReasonLocalClusterManaged, corev1.ConditionTrue, nil)
+		err := client.Get(ctx, namespacedname, clusterDeployment)
+		Expect(err).ToNot(HaveOccurred())
+		err = client.Get(ctx, namespacedname, agentClusterInstall)
+		Expect(err).ToNot(HaveOccurred())
+		err = client.Get(ctx, namespacedname, infraEnv)
+		Expect(err).ToNot(HaveOccurred())
+
+		asc := &aiv1beta1.AgentServiceConfig{}
+		agentNamespacedName := types.NamespacedName{
+			Name: "agent",
+		}
+		err = client.Get(ctx, agentNamespacedName, asc)
+		Expect(err).To(BeNil())
+		asc.ObjectMeta.Annotations = nil
+		err = client.Update(ctx, asc)
+		Expect(err).To(BeNil())
+
+		awaitReconcile(aiv1beta1.ConditionLocalClusterManaged, aiv1beta1.ReasonLocalClusterImportNotEnabled, corev1.ConditionFalse, nil)
+		err = client.Get(ctx, namespacedname, clusterDeployment)
+		Expect(err).ToNot(HaveOccurred())
+		err = client.Get(ctx, namespacedname, agentClusterInstall)
+		Expect(err).ToNot(HaveOccurred())
+		err = client.Get(ctx, namespacedname, infraEnv)
+		Expect(err).ToNot(HaveOccurred())
+	})
+
 	It("should monitor for proxy change and update AgentClusterInstall correctly", func() {
 		clusterDeployment := &hivev1.ClusterDeployment{}
 		agentClusterInstall := &hiveext.AgentClusterInstall{}