✨ support ability to move custom objects in clusterctl

This PR will add the ability for anything in clusterctl's object graph
that is part of a CRD with the label `clusterctl.cluster.x-k8s.io/move: ""` to also be eligible for moving.
This allows for providers to explicitly tag their own custom resources
and ensure that they're also processed as part of `clusterctl move`

Signed-off-by: Spencer Smith <[email protected]>

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,9 +361,14 @@ 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() {
 		namespace := node.identity.Namespace
 
+		// ignore cluster-wide objects
+		if namespace == "" {
+			continue
+		}
+
 		// If the namespace was already processed, skip it.
 		if namespaces.Has(namespace) {
 			continue

cmd/clusterctl/client/cluster/mover_test.go

@@ -392,6 +392,30 @@ var moveTests = []struct {
 			},
 		},
 	},
+	{
+		name: "Cluster and external object 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", "externalTest").Objs()...)
+				return objs
+			}(),
+		},
+		wantMoveGroups: [][]string{
+			{ // group1
+				"cluster.x-k8s.io/v1alpha3, Kind=Cluster, ns1/foo",
+				"external.cluster.x-k8s.io/v1alpha3, Kind=GenericExternalObject, ns1/externalTest",
+			},
+			{ //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 +428,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("ns1")).To(Succeed())
 
 			moveSequence := getMoveSequence(graph)
 			g.Expect(moveSequence.groups).To(HaveLen(len(tt.wantMoveGroups)))
@@ -436,11 +460,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("ns1")).To(Succeed())
 
 			// gets a fakeProxy to an empty cluster with all the required CRDs
 			toProxy := getFakeProxyWithCRDs()
@@ -676,11 +700,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("ns1")).To(Succeed())
 
 			o := &objectMover{
 				fromProxy: graph.proxy,
@@ -906,6 +930,7 @@ func Test_objectMoverService_ensureNamespaces(t *testing.T) {
 
 	cluster1 := test.NewFakeCluster("namespace-1", "cluster-1")
 	cluster2 := test.NewFakeCluster("namespace-2", "cluster-2")
+	externalObj := test.NewFakeExternalObject("", "eo-1")
 
 	clustersObjs := append(cluster1.Objs(), cluster2.Objs()...)
 
@@ -946,6 +971,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: externalObj.Objs(),
+			},
+			args: args{
+				toProxy: test.NewFakeProxy(),
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -955,11 +989,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 +1012,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 {

cmd/clusterctl/client/cluster/objectgraph.go

@@ -17,6 +17,9 @@ limitations under the License.
 package cluster
 
 import (
+	"fmt"
+	"strings"
+
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -50,6 +53,10 @@ type node struct {
 	// E.g. secrets are soft-owned by a cluster via a naming convention, but without an explicit OwnerReference.
 	softOwners map[*node]empty
 
+	// moveLabel is set to true if the CRD of this object has the "move" label attached.
+	// This ensures the node is moved, regardless of its owner refs.
+	moveLabel bool
+
 	// virtual records if this node was discovered indirectly, e.g. by processing an OwnerRef, but not yet observed as a concrete object.
 	virtual bool
 
@@ -65,6 +72,11 @@ type node struct {
 	tenantCRSs map[*node]empty
 }
 
+type crdInfo struct {
+	typeMeta  metav1.TypeMeta
+	moveLabel bool
+}
+
 // markObserved marks the fact that a node was observed as a concrete object.
 func (n *node) markObserved() {
 	n.virtual = false
@@ -92,6 +104,7 @@ func (n *node) isSoftOwnedBy(other *node) bool {
 type objectGraph struct {
 	proxy     Proxy
 	uidToNode map[types.UID]*node
+	crds      map[string]*crdInfo
 }
 
 func newObjectGraph(proxy Proxy) *objectGraph {
@@ -168,43 +181,63 @@ func (o *objectGraph) objToNode(obj *unstructured.Unstructured) *node {
 		virtual:        false,
 	}
 
+	crdStr := crdTypeMetaToString(metav1.TypeMeta{Kind: newNode.identity.Kind, APIVersion: newNode.identity.APIVersion})
+	if _, ok := o.crds[crdStr]; ok {
+		if o.crds[crdStr].moveLabel {
+			newNode.moveLabel = true
+		}
+	}
+
 	o.uidToNode[newNode.identity.UID] = newNode
 	return newNode
 }
 
 // getDiscoveryTypes returns the list of TypeMeta to be considered for the the move discovery phase.
 // This list includes all the types defines by the CRDs installed by clusterctl and the ConfigMap/Secret core types.
-func (o *objectGraph) getDiscoveryTypes() ([]metav1.TypeMeta, error) {
-	discoveredTypes := []metav1.TypeMeta{}
-
+func (o *objectGraph) getDiscoveryTypes() error {
 	crdList := &apiextensionsv1.CustomResourceDefinitionList{}
 	getDiscoveryTypesBackoff := newReadBackoff()
 	if err := retryWithExponentialBackoff(getDiscoveryTypesBackoff, func() error {
 		return getCRDList(o.proxy, crdList)
 	}); err != nil {
-		return nil, err
+		return err
 	}
 
+	o.crds = make(map[string]*crdInfo)
+
 	for _, crd := range crdList.Items {
 		for _, version := range crd.Spec.Versions {
 			if !version.Storage {
 				continue
 			}
 
-			discoveredTypes = append(discoveredTypes, metav1.TypeMeta{
+			typeMeta := metav1.TypeMeta{
 				Kind: crd.Spec.Names.Kind,
 				APIVersion: metav1.GroupVersion{
 					Group:   crd.Spec.Group,
 					Version: version.Name,
 				}.String(),
-			})
+			}
+
+			o.crds[crdTypeMetaToString(typeMeta)] = &crdInfo{typeMeta: typeMeta}
+			if _, ok := crd.Labels[clusterctlv1.ClusterctlMoveLabelName]; ok {
+				o.crds[crdTypeMetaToString(typeMeta)].moveLabel = true
+			}
+
 		}
 	}
 
-	discoveredTypes = append(discoveredTypes, metav1.TypeMeta{Kind: "Secret", APIVersion: "v1"})
-	discoveredTypes = append(discoveredTypes, metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"})
+	o.crds["secrets.v1"] = &crdInfo{typeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: "v1"}}
+	o.crds["configmaps.v1"] = &crdInfo{typeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}}
+
+	return nil
+}
 
-	return discoveredTypes, nil
+// TODO: There's gotta be a better way to do this, but I couldn't find an easy way to get the full CRD string
+// with lowercase and plural from typemeta
+func crdTypeMetaToString(typeMeta metav1.TypeMeta) string {
+	api := strings.Split(typeMeta.APIVersion, "/")[0]
+	return fmt.Sprintf("%ss.%s", strings.ToLower(typeMeta.Kind), api)
 }
 
 func getCRDList(proxy Proxy, crdList *apiextensionsv1.CustomResourceDefinitionList) error {
@@ -221,7 +254,7 @@ func getCRDList(proxy Proxy, crdList *apiextensionsv1.CustomResourceDefinitionLi
 
 // Discovery reads all the Kubernetes objects existing in a namespace (or in all namespaces if empty) for the types received in input, and then adds
 // everything to the objects graph.
-func (o *objectGraph) Discovery(namespace string, types []metav1.TypeMeta) error {
+func (o *objectGraph) Discovery(namespace string) error {
 	log := logf.Log
 	log.Info("Discovering Cluster API objects")
 
@@ -231,8 +264,8 @@ func (o *objectGraph) Discovery(namespace string, types []metav1.TypeMeta) error
 	}
 
 	discoveryBackoff := newReadBackoff()
-	for i := range types {
-		typeMeta := types[i]
+	for _, crd := range o.crds {
+		typeMeta := crd.typeMeta
 		objList := new(unstructured.UnstructuredList)
 
 		if err := retryWithExponentialBackoff(discoveryBackoff, func() error {
@@ -327,11 +360,12 @@ func (o *objectGraph) getCRSs() []*node {
 	return clusters
 }
 
-// getNodesWithTenants returns the list of nodes existing in the object graph that belong at least to one Cluster or to a ClusterResourceSet.
-func (o *objectGraph) getNodesWithTenants() []*node {
+// getMoveNodes returns the list of nodes existing in the object graph that belong at least to one Cluster or to a ClusterResourceSet
+// or to a CRD containing the "move" label.
+func (o *objectGraph) getMoveNodes() []*node {
 	nodes := []*node{}
 	for _, node := range o.uidToNode {
-		if len(node.tenantClusters) > 0 || len(node.tenantCRSs) > 0 {
+		if len(node.tenantClusters) > 0 || len(node.tenantCRSs) > 0 || node.moveLabel {
 			nodes = append(nodes, node)
 		}
 	}