Merge pull request #3337 from rsmitty/move

✨ support ability to move custom objects in clusterctl

cmd/clusterctl/api/v1alpha3/labels.go

@@ -30,6 +30,9 @@ const (
 	// Example: resources shared between instances of the same provider:  CRDs,
 	// ValidatingWebhookConfiguration, MutatingWebhookConfiguration, and so on.
 	ClusterctlResourceLifecyleLabelName = "clusterctl.cluster.x-k8s.io/lifecycle"
+
+	// ClusterctlMoveLabelName can be set on CRDs that providers wish to move that are not part of a cluster
+	ClusterctlMoveLabelName = "clusterctl.cluster.x-k8s.io/move"
 )
 
 // ResourceLifecycle configures the lifecycle of a resource

cmd/clusterctl/client/cluster/mover.go

@@ -60,15 +60,15 @@ func (o *objectMover) Move(namespace string, toCluster Client) error {
 	}
 
 	// Gets all the types defines by the CRDs installed by clusterctl plus the ConfigMap/Secret core types.
-	types, err := objectGraph.getDiscoveryTypes()
+	err := objectGraph.getDiscoveryTypes()
 	if err != nil {
 		return err
 	}
 
 	// Discovery the object graph for the selected types:
 	// - Nodes are defined the Kubernetes objects (Clusters, Machines etc.) identified during the discovery process.
 	// - Edges are derived by the OwnerReferences between nodes.
-	if err := objectGraph.Discovery(namespace, types); err != nil {
+	if err := objectGraph.Discovery(namespace); err != nil {
 		return err
 	}
 
@@ -275,7 +275,8 @@ func getMoveSequence(graph *objectGraph) *moveSequence {
 		// NB. it is necessary to filter out nodes not belonging to a cluster because e.g. discovery reads all the secrets,
 		// but only few of them are related to Clusters/Machines etc.
 		moveGroup := moveGroup{}
-		for _, n := range graph.getNodesWithTenants() {
+
+		for _, n := range graph.getMoveNodes() {
 			// If the node was already included in the moveSequence, skip it.
 			if moveSequence.hasNode(n) {
 				continue
@@ -360,7 +361,13 @@ func patchCluster(proxy Proxy, cluster *node, patch client.Patch) error {
 func (o *objectMover) ensureNamespaces(graph *objectGraph, toProxy Proxy) error {
 	ensureNamespaceBackoff := newWriteBackoff()
 	namespaces := sets.NewString()
-	for _, node := range graph.getNodesWithTenants() {
+	for _, node := range graph.getMoveNodes() {
+
+		// ignore global/cluster-wide objects
+		if node.isGlobal {
+			continue
+		}
+
 		namespace := node.identity.Namespace
 
 		// If the namespace was already processed, skip it.
@@ -565,6 +572,11 @@ func (o *objectMover) deleteGroup(group moveGroup) error {
 	for i := range group {
 		nodeToDelete := group[i]
 
+		// Don't delete cluster-wide nodes
+		if nodeToDelete.isGlobal {
+			continue
+		}
+
 		// Delete the Kubernetes object corresponding to the current node.
 		// Nb. The operation is wrapped in a retry loop to make move more resilient to unexpected conditions.
 		err := retryWithExponentialBackoff(deleteSourceObjectBackoff, func() error {

cmd/clusterctl/client/cluster/mover_test.go

@@ -392,6 +392,32 @@ var moveTests = []struct {
 			},
 		},
 	},
+	{
+		name: "Cluster and global + namespaced external objects with force-move label",
+		fields: moveTestsFields{
+			func() []runtime.Object {
+				objs := []runtime.Object{}
+				objs = append(objs, test.NewFakeCluster("ns1", "foo").Objs()...)
+				objs = append(objs, test.NewFakeExternalObject("ns1", "externalTest1").Objs()...)
+				objs = append(objs, test.NewFakeExternalObject("", "externalTest2").Objs()...)
+				return objs
+			}(),
+		},
+		wantMoveGroups: [][]string{
+			{ // group1
+				"cluster.x-k8s.io/v1alpha3, Kind=Cluster, ns1/foo",
+				"external.cluster.x-k8s.io/v1alpha3, Kind=GenericExternalObject, ns1/externalTest1",
+				"external.cluster.x-k8s.io/v1alpha3, Kind=GenericExternalObject, /externalTest2",
+			},
+			{ //group 2 (objects with ownerReferences in group 1)
+				// owned by Clusters
+				"/v1, Kind=Secret, ns1/foo-ca",
+				"/v1, Kind=Secret, ns1/foo-kubeconfig",
+				"infrastructure.cluster.x-k8s.io/v1alpha3, Kind=GenericInfrastructureCluster, ns1/foo",
+			},
+		},
+		wantErr: false,
+	},
 }
 
 func Test_getMoveSequence(t *testing.T) {
@@ -404,11 +430,11 @@ func Test_getMoveSequence(t *testing.T) {
 			graph := getObjectGraphWithObjs(tt.fields.objs)
 
 			// Get all the types to be considered for discovery
-			discoveryTypes, err := getFakeDiscoveryTypes(graph)
+			err := getFakeDiscoveryTypes(graph)
 			g.Expect(err).NotTo(HaveOccurred())
 
 			// trigger discovery the content of the source cluster
-			g.Expect(graph.Discovery("ns1", discoveryTypes)).To(Succeed())
+			g.Expect(graph.Discovery("")).To(Succeed())
 
 			moveSequence := getMoveSequence(graph)
 			g.Expect(moveSequence.groups).To(HaveLen(len(tt.wantMoveGroups)))
@@ -436,11 +462,11 @@ func Test_objectMover_move(t *testing.T) {
 			graph := getObjectGraphWithObjs(tt.fields.objs)
 
 			// Get all the types to be considered for discovery
-			discoveryTypes, err := getFakeDiscoveryTypes(graph)
+			err := getFakeDiscoveryTypes(graph)
 			g.Expect(err).NotTo(HaveOccurred())
 
 			// trigger discovery the content of the source cluster
-			g.Expect(graph.Discovery("ns1", discoveryTypes)).To(Succeed())
+			g.Expect(graph.Discovery("")).To(Succeed())
 
 			// gets a fakeProxy to an empty cluster with all the required CRDs
 			toProxy := getFakeProxyWithCRDs()
@@ -478,10 +504,11 @@ func Test_objectMover_move(t *testing.T) {
 
 				err := csFrom.Get(ctx, key, oFrom)
 				if err == nil {
-					t.Errorf("%v not deleted in source cluster", key)
-					continue
-				}
-				if !apierrors.IsNotFound(err) {
+					if oFrom.GetNamespace() != "" {
+						t.Errorf("%v not deleted in source cluster", key)
+						continue
+					}
+				} else if !apierrors.IsNotFound(err) {
 					t.Errorf("error = %v when checking for %v deleted in source cluster", err, key)
 					continue
 				}
@@ -676,11 +703,11 @@ func Test_objectMover_checkProvisioningCompleted(t *testing.T) {
 			graph := getObjectGraphWithObjs(tt.fields.objs)
 
 			// Get all the types to be considered for discovery
-			discoveryTypes, err := getFakeDiscoveryTypes(graph)
+			err := getFakeDiscoveryTypes(graph)
 			g.Expect(err).NotTo(HaveOccurred())
 
 			// trigger discovery the content of the source cluster
-			g.Expect(graph.Discovery("ns1", discoveryTypes)).To(Succeed())
+			g.Expect(graph.Discovery("")).To(Succeed())
 
 			o := &objectMover{
 				fromProxy: graph.proxy,
@@ -906,6 +933,7 @@ func Test_objectMoverService_ensureNamespaces(t *testing.T) {
 
 	cluster1 := test.NewFakeCluster("namespace-1", "cluster-1")
 	cluster2 := test.NewFakeCluster("namespace-2", "cluster-2")
+	globalObj := test.NewFakeExternalObject("", "eo-1")
 
 	clustersObjs := append(cluster1.Objs(), cluster2.Objs()...)
 
@@ -946,6 +974,15 @@ func Test_objectMoverService_ensureNamespaces(t *testing.T) {
 			},
 			expectedNamespaces: []string{"namespace-1", "namespace-2"},
 		},
+		{
+			name: "ensureNamespaces doesn't fail if no namespace is specified (cluster-wide)",
+			fields: fields{
+				objs: globalObj.Objs(),
+			},
+			args: args{
+				toProxy: test.NewFakeProxy(),
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -955,11 +992,11 @@ func Test_objectMoverService_ensureNamespaces(t *testing.T) {
 			graph := getObjectGraphWithObjs(tt.fields.objs)
 
 			// Get all the types to be considered for discovery
-			discoveryTypes, err := getFakeDiscoveryTypes(graph)
+			err := getFakeDiscoveryTypes(graph)
 			g.Expect(err).NotTo(HaveOccurred())
 
 			// Trigger discovery the content of the source cluster
-			g.Expect(graph.Discovery("", discoveryTypes)).To(Succeed())
+			g.Expect(graph.Discovery("")).To(Succeed())
 
 			mover := objectMover{
 				fromProxy: graph.proxy,
@@ -978,6 +1015,10 @@ func Test_objectMoverService_ensureNamespaces(t *testing.T) {
 			err = csTo.List(ctx, namespaces, client.Continue(namespaces.Continue))
 			g.Expect(err).ToNot(HaveOccurred())
 
+			// Ensure length of namespaces matches what's expected to ensure we're handling
+			// cluster-wide (namespace of "") objects
+			g.Expect(namespaces.Items).To(HaveLen(len(tt.expectedNamespaces)))
+
 			// Loop through each expected result to ensure that it is found in
 			// the actual results.
 			for _, expected := range tt.expectedNamespaces {