fix: ClusterAutoscaler addon ownership for move (#830)

This commit changes ownership of the CA addon to be owned by the
management cluster (either the bootstrap cluster or the managemen
cluster) which fixes moving the resources only after the cluster
exists on the target cluster.

Author: jimmidyson

pkg/handlers/generic/lifecycle/clusterautoscaler/strategy_crs.go

@@ -119,7 +119,7 @@ func (s crsStrategy) apply(
 		cm.Name,
 		s.client,
 		targetCluster,
-		utils.EnsureCRSForClusterFromObjectsOptions{SetClusterOwnership: false},
+		utils.DefaultEnsureCRSForClusterFromObjectsOptions().WithOwnerCluster(targetCluster),
 		cm,
 	); err != nil {
 		return fmt.Errorf(

pkg/handlers/generic/lifecycle/clusterautoscaler/strategy_helmaddon.go

@@ -12,6 +12,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	caaphv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/sigs.k8s.io/cluster-api-addon-provider-helm/api/v1alpha1"
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/k8s/client"
@@ -105,7 +106,16 @@ func (s helmAddonStrategy) apply(
 
 	// NOTE Unlike other addons, the cluster-autoscaler HelmChartProxy is created in the management cluster
 	// namespace and thus cannot be owned by the workload cluster which will commonly exist in a different namespace.
-	// Deletion is handled by a BeforeClusterDelete hook instead of relying on Kubernetes GC.
+	// Ownership is set up to be owned by the management cluster so that move will work correctly but deletion is handled
+	// by a BeforeClusterDelete hook instead of relying on Kubernetes GC.
+
+	if err = controllerutil.SetOwnerReference(targetCluster, hcp, s.client.Scheme()); err != nil {
+		return fmt.Errorf(
+			"failed to set owner reference on HelmChartProxy %q: %w",
+			hcp.Name,
+			err,
+		)
+	}
 
 	if err = client.ServerSideApply(ctx, s.client, hcp, client.ForceOwnership); err != nil {
 		return fmt.Errorf("failed to apply cluster-autoscaler installation HelmChartProxy: %w", err)

pkg/handlers/utils/utils.go

@@ -22,14 +22,22 @@ import (
 )
 
 type EnsureCRSForClusterFromObjectsOptions struct {
-	// SetClusterOwnership will set the ownership of the CRS to the cluster when set to true
-	SetClusterOwnership bool
+	// OwnerCluster holds the owning cluster for the ClusterResourceSet.
+	// This allows setting the owner to something other than the workload cluster, which is
+	// needed specifically for the ClusterAutoscaler addon which can be deployed in a different
+	// namespace to the target cluster as it must exist in the management cluster.
+	OwnerCluster *clusterv1.Cluster
 }
 
 func DefaultEnsureCRSForClusterFromObjectsOptions() EnsureCRSForClusterFromObjectsOptions {
-	return EnsureCRSForClusterFromObjectsOptions{
-		SetClusterOwnership: true,
-	}
+	return EnsureCRSForClusterFromObjectsOptions{}
+}
+
+func (o EnsureCRSForClusterFromObjectsOptions) WithOwnerCluster(
+	cluster *clusterv1.Cluster,
+) EnsureCRSForClusterFromObjectsOptions {
+	o.OwnerCluster = cluster
+	return o
 }
 
 func EnsureCRSForClusterFromObjects(
@@ -83,10 +91,12 @@ func EnsureCRSForClusterFromObjects(
 		},
 	}
 
-	if opts.SetClusterOwnership {
-		if err := controllerutil.SetOwnerReference(cluster, crs, c.Scheme()); err != nil {
-			return fmt.Errorf("failed to set owner reference: %w", err)
-		}
+	ownerCluster := cluster
+	if opts.OwnerCluster != nil {
+		ownerCluster = opts.OwnerCluster
+	}
+	if err := controllerutil.SetOwnerReference(ownerCluster, crs, c.Scheme()); err != nil {
+		return fmt.Errorf("failed to set owner reference: %w", err)
 	}
 
 	err := client.ServerSideApply(ctx, c, crs, client.ForceOwnership)