Merge pull request #3619 from jichenjc/bug/3603

Add force move support for secret

cmd/clusterctl/client/cluster/mover_test.go

@@ -59,6 +59,26 @@ var moveTests = []struct {
 		},
 		wantErr: false,
 	},
+	{
+		name: "Cluster with external objects marked with move label",
+		fields: moveTestsFields{
+			objs: test.NewFakeCluster("ns1", "foo").WithCloudConfigSecret().Objs(),
+		},
+		wantMoveGroups: [][]string{
+			{ //group 1
+				"cluster.x-k8s.io/v1alpha3, Kind=Cluster, ns1/foo",
+				// objects with force move flag
+				"/v1, Kind=Secret, ns1/foo-cloud-config",
+			},
+			{ //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,
+	},
 	{
 		name: "Cluster with machine",
 		fields: moveTestsFields{

cmd/clusterctl/client/cluster/objectgraph.go

@@ -158,7 +158,7 @@ func (o *objectGraph) ownerToVirtualNode(owner metav1.OwnerReference, namespace
 		tenantClusters: make(map[*node]empty),
 		tenantCRSs:     make(map[*node]empty),
 		virtual:        true,
-		forceMove:      o.getForceMove(owner.Kind, owner.APIVersion),
+		forceMove:      o.getForceMove(owner.Kind, owner.APIVersion, nil),
 		isGlobal:       isGlobal,
 	}
 
@@ -173,6 +173,11 @@ func (o *objectGraph) objToNode(obj *unstructured.Unstructured) *node {
 	existingNode, found := o.uidToNode[obj.GetUID()]
 	if found {
 		existingNode.markObserved()
+
+		// In order to compensate the lack of labels when adding a virtual node,
+		// it is required to re-compute the forceMove flag when the real node is processed
+		// Without this, there is the risk that, forceMove will report false negatives depending on the discovery order
+		existingNode.forceMove = o.getForceMove(obj.GetKind(), obj.GetAPIVersion(), obj.GetLabels())
 		return existingNode
 	}
 
@@ -194,15 +199,19 @@ func (o *objectGraph) objToNode(obj *unstructured.Unstructured) *node {
 		tenantClusters: make(map[*node]empty),
 		tenantCRSs:     make(map[*node]empty),
 		virtual:        false,
-		forceMove:      o.getForceMove(obj.GetKind(), obj.GetAPIVersion()),
+		forceMove:      o.getForceMove(obj.GetKind(), obj.GetAPIVersion(), obj.GetLabels()),
 		isGlobal:       isGlobal,
 	}
 
 	o.uidToNode[newNode.identity.UID] = newNode
 	return newNode
 }
 
-func (o *objectGraph) getForceMove(kind string, apiVersion string) bool {
+func (o *objectGraph) getForceMove(kind, apiVersion string, labels map[string]string) bool {
+	if _, ok := labels[clusterctlv1.ClusterctlMoveLabelName]; ok {
+		return true
+	}
+
 	kindAPIStr := getKindAPIString(metav1.TypeMeta{Kind: kind, APIVersion: apiVersion})
 
 	if discoveryType, ok := o.types[kindAPIStr]; ok {

cmd/clusterctl/client/cluster/objectgraph_test.go

@@ -356,6 +356,34 @@ var objectGraphsTests = []struct {
 			},
 		},
 	},
+	{
+		name: "Cluster with force move label",
+		args: objectGraphTestArgs{
+			objs: test.NewFakeCluster("ns1", "cluster1").
+				WithCloudConfigSecret().Objs(),
+		},
+		want: wantGraph{
+			nodes: map[string]wantGraphItem{
+				"cluster.x-k8s.io/v1alpha3, Kind=Cluster, ns1/cluster1": {},
+				"infrastructure.cluster.x-k8s.io/v1alpha3, Kind=GenericInfrastructureCluster, ns1/cluster1": {
+					owners: []string{
+						"cluster.x-k8s.io/v1alpha3, Kind=Cluster, ns1/cluster1",
+					},
+				},
+				"/v1, Kind=Secret, ns1/cluster1-ca": {
+					softOwners: []string{
+						"cluster.x-k8s.io/v1alpha3, Kind=Cluster, ns1/cluster1", //NB. this secret is not linked to the cluster through owner ref
+					},
+				},
+				"/v1, Kind=Secret, ns1/cluster1-kubeconfig": {
+					owners: []string{
+						"cluster.x-k8s.io/v1alpha3, Kind=Cluster, ns1/cluster1",
+					},
+				},
+				"/v1, Kind=Secret, ns1/cluster1-cloud-config": {},
+			},
+		},
+	},
 	{
 		name: "Two clusters",
 		args: objectGraphTestArgs{

cmd/clusterctl/internal/test/fake_objects.go

@@ -37,13 +37,14 @@ import (
 )
 
 type FakeCluster struct {
-	namespace          string
-	name               string
-	controlPlane       *FakeControlPlane
-	machinePools       []*FakeMachinePool
-	machineDeployments []*FakeMachineDeployment
-	machineSets        []*FakeMachineSet
-	machines           []*FakeMachine
+	namespace             string
+	name                  string
+	controlPlane          *FakeControlPlane
+	machinePools          []*FakeMachinePool
+	machineDeployments    []*FakeMachineDeployment
+	machineSets           []*FakeMachineSet
+	machines              []*FakeMachine
+	withCloudConfigSecret bool
 }
 
 // NewFakeCluster return a FakeCluster that can generate a cluster object, all its own ancillary objects:
@@ -69,6 +70,11 @@ func (f *FakeCluster) WithMachinePools(fakeMachinePool ...*FakeMachinePool) *Fak
 	return f
 }
 
+func (f *FakeCluster) WithCloudConfigSecret() *FakeCluster {
+	f.withCloudConfigSecret = true
+	return f
+}
+
 func (f *FakeCluster) WithMachineDeployments(fakeMachineDeployment ...*FakeMachineDeployment) *FakeCluster {
 	f.machineDeployments = append(f.machineDeployments, fakeMachineDeployment...)
 	return f
@@ -150,6 +156,24 @@ func (f *FakeCluster) Objs() []runtime.Object {
 		caSecret,
 	}
 
+	if f.withCloudConfigSecret {
+		cloudSecret := &corev1.Secret{ // provided by the user -- ** NOT RECONCILED **
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Secret",
+				APIVersion: "v1",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      f.name + "-cloud-config",
+				Namespace: f.namespace,
+			},
+		}
+
+		cloudSecret.SetLabels(map[string]string{
+			clusterctlv1.ClusterctlMoveLabelName: "",
+		})
+		objs = append(objs, cloudSecret)
+	}
+
 	// if the cluster has a control plane object
 	if f.controlPlane != nil {
 		// Adds the objects for the controlPlane