Merge pull request #12786 from alexandrevilain/feat/clusterctl-block-move-if-cluster-paused

⚠️ feat(clusterctl): block move when Cluster or ClusterClass is paused

Author: k8s-ci-robot

cmd/clusterctl/client/cluster/mover.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -41,6 +42,7 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	logf "sigs.k8s.io/cluster-api/cmd/clusterctl/log"
+	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/yaml"
@@ -232,8 +234,7 @@ func (o *objectMover) checkProvisioningCompleted(ctx context.Context, graph *obj
 	// Checking all the clusters have infrastructure is ready
 	readClusterBackoff := newReadBackoff()
 	clusters := graph.getClusters()
-	for i := range clusters {
-		cluster := clusters[i]
+	for _, cluster := range clusters {
 		clusterObj := &clusterv1.Cluster{}
 		if err := retryWithExponentialBackoff(ctx, readClusterBackoff, func(ctx context.Context) error {
 			return getClusterObj(ctx, o.fromProxy, cluster, clusterObj)
@@ -297,6 +298,25 @@ func getClusterObj(ctx context.Context, proxy Proxy, cluster *node, clusterObj *
 	return nil
 }
 
+// getClusterClassObj retrieves the clusterClassObj corresponding to a node with type ClusterClass.
+func getClusterClassObj(ctx context.Context, proxy Proxy, clusterClass *node, clusterClassObj *clusterv1.ClusterClass) error {
+	c, err := proxy.NewClient(ctx)
+	if err != nil {
+		return err
+	}
+
+	clusterClassObjKey := client.ObjectKey{
+		Namespace: clusterClass.identity.Namespace,
+		Name:      clusterClass.identity.Name,
+	}
+
+	if err := c.Get(ctx, clusterClassObjKey, clusterClassObj); err != nil {
+		return errors.Wrapf(err, "error reading ClusterClass %s/%s",
+			clusterClass.identity.Namespace, clusterClass.identity.Name)
+	}
+	return nil
+}
+
 // getMachineObj retrieves the machineObj corresponding to a node with type Machine.
 func getMachineObj(ctx context.Context, proxy Proxy, machine *node, machineObj *clusterv1.Machine) error {
 	c, err := proxy.NewClient(ctx)
@@ -320,9 +340,17 @@ func (o *objectMover) move(ctx context.Context, graph *objectGraph, toProxy Prox
 	log := logf.Log
 
 	clusters := graph.getClusters()
+	if err := checkClustersNotPaused(ctx, o.fromProxy, clusters); err != nil {
+		return err
+	}
+
 	log.Info("Moving Cluster API objects", "Clusters", len(clusters))
 
 	clusterClasses := graph.getClusterClasses()
+	if err := checkClusterClassesNotPaused(ctx, o.fromProxy, clusterClasses); err != nil {
+		return err
+	}
+
 	log.Info("Moving Cluster API objects", "ClusterClasses", len(clusterClasses))
 
 	// Sets the pause field on the Cluster object in the source management cluster, so the controllers stop reconciling it.
@@ -395,9 +423,17 @@ func (o *objectMover) toDirectory(ctx context.Context, graph *objectGraph, direc
 	log := logf.Log
 
 	clusters := graph.getClusters()
+	if err := checkClustersNotPaused(ctx, o.fromProxy, clusters); err != nil {
+		return err
+	}
+
 	log.Info("Starting move of Cluster API objects", "Clusters", len(clusters))
 
 	clusterClasses := graph.getClusterClasses()
+	if err := checkClusterClassesNotPaused(ctx, o.fromProxy, clusterClasses); err != nil {
+		return err
+	}
+
 	log.Info("Moving Cluster API objects", "ClusterClasses", len(clusterClasses))
 
 	// Sets the pause field on the Cluster object in the source management cluster, so the controllers stop reconciling it.
@@ -570,8 +606,7 @@ func setClusterPause(ctx context.Context, proxy Proxy, clusters []*node, value b
 	patch := client.RawPatch(types.MergePatchType, []byte(fmt.Sprintf("{\"spec\":{\"paused\":%s}}", patchValue)))
 
 	setClusterPauseBackoff := newWriteBackoff()
-	for i := range clusters {
-		cluster := clusters[i]
+	for _, cluster := range clusters {
 		log.V(5).Info("Set Cluster.Spec.Paused", "paused", value, "Cluster", klog.KRef(cluster.identity.Namespace, cluster.identity.Name))
 
 		// Nb. The operation is wrapped in a retry loop to make setClusterPause more resilient to unexpected conditions.
@@ -593,8 +628,7 @@ func setClusterClassPause(ctx context.Context, proxy Proxy, clusterclasses []*no
 	log := logf.Log
 
 	setClusterClassPauseBackoff := newWriteBackoff()
-	for i := range clusterclasses {
-		clusterclass := clusterclasses[i]
+	for _, clusterclass := range clusterclasses {
 		if pause {
 			log.V(5).Info("Set Paused annotation", "ClusterClass", clusterclass.identity.Name, "Namespace", clusterclass.identity.Namespace)
 		} else {
@@ -611,6 +645,44 @@ func setClusterClassPause(ctx context.Context, proxy Proxy, clusterclasses []*no
 	return nil
 }
 
+// checkClustersNotPaused checks that no cluster in the graph is paused before proceeding.
+func checkClustersNotPaused(ctx context.Context, proxy Proxy, clusters []*node) error {
+	paused := []string{}
+	for _, cluster := range clusters {
+		clusterObj := &clusterv1.Cluster{}
+		if err := getClusterObj(ctx, proxy, cluster, clusterObj); err != nil {
+			return err
+		}
+
+		if ptr.Deref(clusterObj.Spec.Paused, false) || annotations.HasPaused(clusterObj) {
+			paused = append(paused, fmt.Sprintf("%s/%s", clusterObj.Namespace, clusterObj.Name))
+		}
+	}
+	if len(paused) > 0 {
+		return errors.Errorf("cannot start operation while the following Clusters are paused: %s", strings.Join(paused, ", "))
+	}
+	return nil
+}
+
+// checkClusterClassesNotPaused checks that no clusterClass in the graph is paused before proceeding.
+func checkClusterClassesNotPaused(ctx context.Context, proxy Proxy, clusterClasses []*node) error {
+	paused := []string{}
+	for _, clusterClass := range clusterClasses {
+		clusterClassObj := &clusterv1.ClusterClass{}
+		if err := getClusterClassObj(ctx, proxy, clusterClass, clusterClassObj); err != nil {
+			return err
+		}
+
+		if annotations.HasPaused(clusterClassObj) {
+			paused = append(paused, fmt.Sprintf("%s/%s", clusterClassObj.Namespace, clusterClassObj.Name))
+		}
+	}
+	if len(paused) > 0 {
+		return errors.Errorf("cannot start operation while the following ClusterClasses are paused: %s", strings.Join(paused, ", "))
+	}
+	return nil
+}
+
 func waitReadyForMove(ctx context.Context, proxy Proxy, nodes []*node, dryRun bool, backoff wait.Backoff) error {
 	if dryRun {
 		return nil
@@ -723,7 +795,8 @@ func pauseClusterClass(ctx context.Context, proxy Proxy, n *node, pause bool, mu
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      n.identity.Name,
 			Namespace: n.identity.Namespace,
-		}}, mutators...)
+		},
+	}, mutators...)
 	if err != nil {
 		return err
 	}
@@ -1072,7 +1145,7 @@ func (o *objectMover) backupTargetObject(ctx context.Context, nodeToCreate *node
 		}
 	}
 
-	err = os.WriteFile(objectFile, byObj, 0600)
+	err = os.WriteFile(objectFile, byObj, 0o600)
 	if err != nil {
 		return err
 	}
@@ -1173,7 +1246,6 @@ func (o *objectMover) deleteGroup(ctx context.Context, group moveGroup) error {
 		err := retryWithExponentialBackoff(ctx, deleteSourceObjectBackoff, func(ctx context.Context) error {
 			return o.deleteSourceObject(ctx, nodeToDelete)
 		})
-
 		if err != nil {
 			errList = append(errList, err)
 		}

cmd/clusterctl/client/cluster/mover_test.go

@@ -98,6 +98,40 @@ var moveTests = []struct {
 		},
 		wantErr: false,
 	},
+	{
+		name: "Paused Cluster",
+		fields: moveTestsFields{
+			objs: test.NewFakeCluster("ns1", "foo").WithPaused().Objs(),
+		},
+		wantMoveGroups: [][]string{
+			{ // group 1
+				clusterv1.GroupVersion.String() + ", Kind=Cluster, ns1/foo",
+			},
+			{ // group 2 (objects with ownerReferences in group 1)
+				// owned by Clusters
+				"/v1, Kind=Secret, ns1/foo-ca",
+				"/v1, Kind=Secret, ns1/foo-kubeconfig",
+				clusterv1.GroupVersionInfrastructure.String() + ", Kind=GenericInfrastructureCluster, ns1/foo",
+			},
+		},
+		wantErr: true,
+	},
+	{
+		name: "Paused ClusterClass",
+		fields: moveTestsFields{
+			objs: test.NewFakeClusterClass("ns1", "class1").WithPaused().Objs(),
+		},
+		wantMoveGroups: [][]string{
+			{ // group 1
+				clusterv1.GroupVersion.String() + ", Kind=ClusterClass, ns1/class1",
+			},
+			{ // group 2
+				clusterv1.GroupVersionInfrastructure.String() + ", Kind=GenericInfrastructureClusterTemplate, ns1/class1",
+				clusterv1.GroupVersionControlPlane.String() + ", Kind=GenericControlPlaneTemplate, ns1/class1",
+			},
+		},
+		wantErr: true,
+	},
 	{
 		name: "Cluster with cloud config secret with the force move label",
 		fields: moveTestsFields{
@@ -923,8 +957,29 @@ func Test_objectMover_restoreTargetObject(t *testing.T) {
 }
 
 func Test_objectMover_toDirectory(t *testing.T) {
-	// NB. we are testing the move and move sequence using the same set of moveTests, but checking the results at different stages of the move process
-	for _, tt := range backupRestoreTests {
+	tests := []struct {
+		name    string
+		fields  moveTestsFields
+		files   map[string]string
+		wantErr bool
+	}{
+		{
+			name: "Cluster is paused",
+			fields: moveTestsFields{
+				objs: test.NewFakeCluster("ns1", "foo").WithPaused().Objs(),
+			},
+			wantErr: true,
+		},
+		{
+			name: "ClusterClass is paused",
+			fields: moveTestsFields{
+				objs: test.NewFakeClusterClass("ns1", "foo").WithPaused().Objs(),
+			},
+			wantErr: true,
+		},
+	}
+	tests = append(tests, backupRestoreTests...)
+	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 

cmd/clusterctl/internal/test/fake_objects.go

@@ -48,6 +48,7 @@ import (
 type FakeCluster struct {
 	namespace              string
 	name                   string
+	paused                 bool
 	controlPlane           *FakeControlPlane
 	machinePools           []*FakeMachinePool
 	machineDeployments     []*FakeMachineDeployment
@@ -117,6 +118,11 @@ func (f *FakeCluster) WithTopologyClassNamespace(namespace string) *FakeCluster
 	return f
 }
 
+func (f *FakeCluster) WithPaused() *FakeCluster {
+	f.paused = true
+	return f
+}
+
 func (f *FakeCluster) Objs() []client.Object {
 	clusterInfrastructure := &fakeinfrastructure.GenericInfrastructureCluster{
 		TypeMeta: metav1.TypeMeta{
@@ -161,6 +167,10 @@ func (f *FakeCluster) Objs() []client.Object {
 		}
 	}
 
+	if f.paused {
+		cluster.Spec.Paused = ptr.To(true)
+	}
+
 	// Ensure the cluster gets a UID to be used by dependant objects for creating OwnerReferences.
 	setUID(cluster)
 
@@ -1486,6 +1496,7 @@ func FakeCRDList() []*apiextensionsv1.CustomResourceDefinition {
 type FakeClusterClass struct {
 	namespace                                 string
 	name                                      string
+	paused                                    bool
 	infrastructureClusterTemplate             *unstructured.Unstructured
 	controlPlaneTemplate                      *unstructured.Unstructured
 	controlPlaneInfrastructureMachineTemplate *unstructured.Unstructured
@@ -1519,6 +1530,11 @@ func (f *FakeClusterClass) WithWorkerMachineDeploymentClasses(classes []*FakeMac
 	return f
 }
 
+func (f *FakeClusterClass) WithPaused() *FakeClusterClass {
+	f.paused = true
+	return f
+}
+
 func (f *FakeClusterClass) Objs() []client.Object {
 	// objMap map where the key is the object to which the owner reference to the cluster class should be added
 	// and the value dictates if the onwner ref needs to be added.
@@ -1546,6 +1562,10 @@ func (f *FakeClusterClass) Objs() []client.Object {
 		objMap[f.controlPlaneInfrastructureMachineTemplate] = true
 	}
 
+	if f.paused {
+		clusterClassBuilder.WithAnnotations(map[string]string{clusterv1.PausedAnnotation: "true"})
+	}
+
 	if len(f.workerMachineDeploymentClasses) > 0 {
 		mdClasses := []clusterv1.MachineDeploymentClass{}
 		for _, fakeMDClass := range f.workerMachineDeploymentClasses {

util/test/builder/builders.go

@@ -342,6 +342,7 @@ func (m *MachinePoolTopologyBuilder) Build() clusterv1.MachinePoolTopology {
 type ClusterClassBuilder struct {
 	namespace                                 string
 	name                                      string
+	annotations                               map[string]string
 	infrastructureClusterTemplate             *unstructured.Unstructured
 	controlPlaneMetadata                      *clusterv1.ObjectMeta
 	controlPlaneReadinessGates                []clusterv1.MachineReadinessGate
@@ -370,6 +371,12 @@ func ClusterClass(namespace, name string) *ClusterClassBuilder {
 	}
 }
 
+// WithAnnotations adds the passed annotations to the ClusterClassBuilder.
+func (c *ClusterClassBuilder) WithAnnotations(annotations map[string]string) *ClusterClassBuilder {
+	c.annotations = annotations
+	return c
+}
+
 // WithInfrastructureClusterTemplate adds the passed InfrastructureClusterTemplate to the ClusterClassBuilder.
 func (c *ClusterClassBuilder) WithInfrastructureClusterTemplate(t *unstructured.Unstructured) *ClusterClassBuilder {
 	c.infrastructureClusterTemplate = t
@@ -502,6 +509,9 @@ func (c *ClusterClassBuilder) Build() *clusterv1.ClusterClass {
 			Variables: c.statusVariables,
 		},
 	}
+	if c.annotations != nil {
+		obj.Annotations = c.annotations
+	}
 	if c.conditions != nil {
 		obj.Status.Conditions = c.conditions
 	}