MGMT-17893: Don't destroy cluster on detach

Currently the procedure to detach a cluster that was created using hosts
from a late binding pool is to first delete the `ManagedCluster` object,
then add the `preserveOnDelete: true` field to the `ClusterDeployment`
and then delete that `ClusterDeployment`. But the cluster deployment
controller doesn't look at the `preserveOnDelete` field at all. As a
result the hosts of the cluster are returned to the pool and they are
provisioned again, which effectively destroys the cluster.

This patch changes the deployment manager controller so that in that
case it will check the `preserveOnDelete` field and if it is `true` it
will delete the corresponding `Agent` objects. The result of that is
that the hosts will go back to the pool, but marked the will still be
marked as provisioned and they will not be provisioned again, thus
avoiding the destruction of the cluster.

Note that the `BareMetalHosts` will not be removed, but they will stay
detached.

Related: https://issues.redhat.com/browse/MGMT-17893
Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>

internal/controller/controllers/clusterdeployments_controller.go

@@ -1541,31 +1541,44 @@ func (r *ClusterDeploymentsReconciler) deleteClusterInstall(ctx context.Context,
 	return buildReply(err)
 }
 
-func (r *ClusterDeploymentsReconciler) shouldDeleteAgentOnUnbind(ctx context.Context, agent aiv1beta1.Agent, clusterDeployment types.NamespacedName) bool {
+func (r *ClusterDeploymentsReconciler) shouldDeleteAgentOnUnbind(ctx context.Context, agent aiv1beta1.Agent, clusterDeployment types.NamespacedName) (bool, error) {
 	log := logutil.FromContext(ctx, r.Log).WithFields(logrus.Fields{
 		"cluster_deployment":           clusterDeployment.Name,
 		"cluster_deployment_namespace": clusterDeployment.Namespace,
 	})
 	infraEnvName, ok := agent.Labels[aiv1beta1.InfraEnvNameLabel]
 	if !ok {
 		log.Errorf("Failed to find infraEnv name for agent %s in namespace %s", agent.Name, agent.Namespace)
-		return false
+		return false, nil
 	}
 
 	infraEnv := &aiv1beta1.InfraEnv{}
 	if err := r.Get(ctx, types.NamespacedName{Name: infraEnvName, Namespace: agent.Namespace}, infraEnv); err != nil {
 		log.Errorf("Failed to get infraEnv %s in namespace %s", infraEnvName, agent.Namespace)
-		return false
+		return false, nil
 	}
 
 	if infraEnv.Spec.ClusterRef != nil &&
 		infraEnv.Spec.ClusterRef.Name == clusterDeployment.Name &&
 		infraEnv.Spec.ClusterRef.Namespace == clusterDeployment.Namespace {
 
-		return true
+		return true, nil
 	}
 
-	return false
+	// If the agent came from a late binding pool (the infrastructure environment has no cluster
+	// reference) then we want to delete it only if the cluster deployment is marked to preserve
+	// on delete. This is necessary because otherwise the host will go back to the pool, and it
+	// will be provisioned again, effectively destroying the cluster.
+	if infraEnv.Spec.ClusterRef == nil {
+		clusterDeploymentObject := &hivev1.ClusterDeployment{}
+		err := r.Get(ctx, clusterDeployment, clusterDeploymentObject)
+		if err != nil {
+			return false, err
+		}
+		return clusterDeploymentObject.Spec.PreserveOnDelete, nil
+	}
+
+	return false, nil
 }
 
 func (r *ClusterDeploymentsReconciler) unbindAgents(ctx context.Context, log logrus.FieldLogger, clusterDeployment types.NamespacedName) error {
@@ -1578,7 +1591,11 @@ func (r *ClusterDeploymentsReconciler) unbindAgents(ctx context.Context, log log
 		if clusterAgent.Spec.ClusterDeploymentName != nil &&
 			clusterAgent.Spec.ClusterDeploymentName.Name == clusterDeployment.Name &&
 			clusterAgent.Spec.ClusterDeploymentName.Namespace == clusterDeployment.Namespace {
-			if r.shouldDeleteAgentOnUnbind(ctx, clusterAgent, clusterDeployment) {
+			shouldDeleteAgent, err := r.shouldDeleteAgentOnUnbind(ctx, clusterAgent, clusterDeployment)
+			if err != nil {
+				return err
+			}
+			if shouldDeleteAgent {
 				log.Infof("deleting agent %s in namespace %s", clusterAgent.Name, clusterAgent.Namespace)
 				if err := r.Delete(ctx, &agents.Items[i]); err != nil {
 					log.WithError(err).Errorf("failed to delete agent %s in namespace %s", clusterAgent.Name, clusterAgent.Namespace)

internal/controller/controllers/clusterdeployments_controller_test.go

@@ -4157,6 +4157,15 @@ var _ = Describe("unbindAgents", func() {
 		}
 		Expect(c.Create(ctx, infraEnv)).To(Succeed())
 
+		clusterDeployment := &hivev1.ClusterDeployment{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: clusterReference.Namespace,
+				Name:      clusterReference.Name,
+			},
+			Spec: hivev1.ClusterDeploymentSpec{},
+		}
+		Expect(c.Create(ctx, clusterDeployment)).To(Succeed())
+
 		cdName := types.NamespacedName{
 			Name:      clusterReference.Name,
 			Namespace: clusterReference.Namespace,
@@ -4198,6 +4207,54 @@ var _ = Describe("unbindAgents", func() {
 		err = c.Get(ctx, types.NamespacedName{Name: "test-agent2", Namespace: testNamespace}, agent)
 		Expect(k8serrors.IsNotFound(err)).To(BeTrue())
 	})
+
+	It("deletes late bound agents when cluster deployment is preserved", func() {
+		infraEnv := &aiv1beta1.InfraEnv{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "InfraEnv",
+				APIVersion: fmt.Sprintf("%s/%s", aiv1beta1.GroupVersion.Group, aiv1beta1.GroupVersion.Version),
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      infraEnvName,
+				Namespace: testNamespace,
+			},
+			Spec: aiv1beta1.InfraEnvSpec{ClusterRef: nil},
+		}
+		Expect(c.Create(ctx, infraEnv)).To(Succeed())
+
+		clusterDeployment := &hivev1.ClusterDeployment{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: clusterReference.Namespace,
+				Name:      clusterReference.Name,
+			},
+			Spec: hivev1.ClusterDeploymentSpec{
+				PreserveOnDelete: true,
+			},
+		}
+		Expect(c.Create(ctx, clusterDeployment)).To(Succeed())
+
+		agent := &aiv1beta1.Agent{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-agent3",
+				Namespace: testNamespace,
+				Labels:    map[string]string{aiv1beta1.InfraEnvNameLabel: infraEnvName},
+			},
+			Spec: aiv1beta1.AgentSpec{
+				ClusterDeploymentName: &clusterReference,
+			},
+		}
+		Expect(c.Create(ctx, agent)).To(Succeed())
+
+		cdName := types.NamespacedName{
+			Name:      clusterReference.Name,
+			Namespace: clusterReference.Namespace,
+		}
+		Expect(cr.unbindAgents(ctx, common.GetTestLog(), cdName)).To(Succeed())
+
+		agent = &aiv1beta1.Agent{}
+		err := c.Get(ctx, types.NamespacedName{Name: "test-agent3", Namespace: testNamespace}, agent)
+		Expect(k8serrors.IsNotFound(err)).To(BeTrue())
+	})
 })
 
 var _ = Describe("day2 cluster", func() {