Merge branch 'main' into inconsistent-constants-rename

Tiltfile

@@ -3,7 +3,7 @@
 envsubst_cmd = "./hack/tools/bin/envsubst"
 clusterctl_cmd = "./bin/clusterctl"
 kubectl_cmd = "kubectl"
-kubernetes_version = "v1.25.0"
+kubernetes_version = "v1.25.3"
 
 if str(local("command -v " + kubectl_cmd + " || true", quiet = True)) == "":
     fail("Required command '" + kubectl_cmd + "' not found in PATH")

api/v1beta1/machine_types.go

@@ -42,6 +42,9 @@ const (
 	// MachineDeploymentLabelName is the label set on machines if they're controlled by MachineDeployment.
 	MachineDeploymentLabelName = "cluster.x-k8s.io/deployment-name"
 
+	// MachineControlPlaneNameLabel is the label set on machines if they're controlled by a ControlPlane.
+	MachineControlPlaneNameLabel = "cluster.x-k8s.io/control-plane-name"
+
 	// PreDrainDeleteHookAnnotationPrefix annotation specifies the prefix we
 	// search each annotation for during the pre-drain.delete lifecycle hook
 	// to pause reconciliation of deletion. These hooks will prevent removal of

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go

@@ -1026,7 +1026,7 @@ func (r *KubeadmConfigReconciler) storeBootstrapData(ctx context.Context, scope
 	return nil
 }
 
-// Ensure the bootstrap secret has the configOwner as a controller OwnerReference.
+// Ensure the bootstrap secret has the KubeadmConfig as a controller OwnerReference.
 func (r *KubeadmConfigReconciler) ensureBootstrapSecretOwnersRef(ctx context.Context, scope *Scope) error {
 	secret := &corev1.Secret{}
 	err := r.Client.Get(ctx, client.ObjectKey{Namespace: scope.Config.Namespace, Name: scope.Config.Name}, secret)
@@ -1041,11 +1041,14 @@ func (r *KubeadmConfigReconciler) ensureBootstrapSecretOwnersRef(ctx context.Con
 	if err != nil {
 		return errors.Wrapf(err, "failed to add KubeadmConfig %s as ownerReference to bootstrap Secret %s", scope.ConfigOwner.GetName(), secret.GetName())
 	}
+	if c := metav1.GetControllerOf(secret); c != nil && c.Kind != "KubeadmConfig" {
+		secret.OwnerReferences = util.RemoveOwnerRef(secret.OwnerReferences, *c)
+	}
 	secret.OwnerReferences = util.EnsureOwnerRef(secret.OwnerReferences, metav1.OwnerReference{
-		APIVersion: scope.ConfigOwner.GetAPIVersion(),
-		Kind:       scope.ConfigOwner.GetKind(),
-		UID:        scope.ConfigOwner.GetUID(),
-		Name:       scope.ConfigOwner.GetName(),
+		APIVersion: bootstrapv1.GroupVersion.String(),
+		Kind:       "KubeadmConfig",
+		UID:        scope.Config.UID,
+		Name:       scope.Config.Name,
 		Controller: pointer.Bool(true),
 	})
 	err = patchHelper.Patch(ctx, secret)

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller_test.go

@@ -117,6 +117,105 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnEarlyIfKubeadmConfigIsReady(t *
 	g.Expect(result.RequeueAfter).To(Equal(time.Duration(0)))
 }
 
+// Reconcile returns early if the kubeadm config is ready because it should never re-generate bootstrap data.
+func TestKubeadmConfigReconciler_TestSecretOwnerReferenceReconciliation(t *testing.T) {
+	g := NewWithT(t)
+
+	clusterName := "my-cluster"
+	cluster := builder.Cluster(metav1.NamespaceDefault, clusterName).Build()
+	machine := builder.Machine(metav1.NamespaceDefault, "machine").
+		WithVersion("v1.19.1").
+		WithClusterName(clusterName).
+		WithBootstrapTemplate(bootstrapbuilder.KubeadmConfig(metav1.NamespaceDefault, "cfg").Unstructured()).
+		Build()
+	machine.Spec.Bootstrap.DataSecretName = pointer.String("something")
+
+	config := newKubeadmConfig(metav1.NamespaceDefault, "cfg")
+	config.SetOwnerReferences(util.EnsureOwnerRef(config.GetOwnerReferences(), metav1.OwnerReference{
+		APIVersion: machine.APIVersion,
+		Kind:       machine.Kind,
+		Name:       machine.Name,
+		UID:        machine.UID,
+	}))
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      config.Name,
+			Namespace: config.Namespace,
+		},
+		Type: corev1.SecretTypeBootstrapToken,
+	}
+	config.Status.Ready = true
+
+	objects := []client.Object{
+		config,
+		machine,
+		secret,
+		cluster,
+	}
+	myclient := fake.NewClientBuilder().WithObjects(objects...).Build()
+
+	k := &KubeadmConfigReconciler{
+		Client: myclient,
+	}
+
+	request := ctrl.Request{
+		NamespacedName: client.ObjectKey{
+			Namespace: metav1.NamespaceDefault,
+			Name:      "cfg",
+		},
+	}
+	var err error
+	key := client.ObjectKeyFromObject(config)
+	actual := &corev1.Secret{}
+
+	t.Run("KubeadmConfig ownerReference is added on first reconcile", func(t *testing.T) {
+		_, err = k.Reconcile(ctx, request)
+		g.Expect(err).NotTo(HaveOccurred())
+
+		g.Expect(myclient.Get(ctx, key, actual)).To(Succeed())
+
+		controllerOwner := metav1.GetControllerOf(actual)
+		g.Expect(controllerOwner).To(Not(BeNil()))
+		g.Expect(controllerOwner.Kind).To(Equal(config.Kind))
+		g.Expect(controllerOwner.Name).To(Equal(config.Name))
+	})
+
+	t.Run("KubeadmConfig ownerReference re-reconciled without error", func(t *testing.T) {
+		_, err = k.Reconcile(ctx, request)
+		g.Expect(err).NotTo(HaveOccurred())
+
+		g.Expect(myclient.Get(ctx, key, actual)).To(Succeed())
+
+		controllerOwner := metav1.GetControllerOf(actual)
+		g.Expect(controllerOwner).To(Not(BeNil()))
+		g.Expect(controllerOwner.Kind).To(Equal(config.Kind))
+		g.Expect(controllerOwner.Name).To(Equal(config.Name))
+	})
+	t.Run("non-KubeadmConfig controller OwnerReference is replaced", func(t *testing.T) {
+		g.Expect(myclient.Get(ctx, key, actual)).To(Succeed())
+
+		actual.SetOwnerReferences([]metav1.OwnerReference{
+			{
+				APIVersion: machine.APIVersion,
+				Kind:       machine.Kind,
+				Name:       machine.Name,
+				UID:        machine.UID,
+				Controller: pointer.Bool(true),
+			}})
+		g.Expect(myclient.Update(ctx, actual)).To(Succeed())
+
+		_, err = k.Reconcile(ctx, request)
+		g.Expect(err).NotTo(HaveOccurred())
+
+		g.Expect(myclient.Get(ctx, key, actual)).To(Succeed())
+
+		controllerOwner := metav1.GetControllerOf(actual)
+		g.Expect(controllerOwner).To(Not(BeNil()))
+		g.Expect(controllerOwner.Kind).To(Equal(config.Kind))
+		g.Expect(controllerOwner.Name).To(Equal(config.Name))
+	})
+}
+
 // Reconcile returns nil if the referenced Machine cannot be found.
 func TestKubeadmConfigReconciler_Reconcile_ReturnNilIfReferencedMachineIsNotFound(t *testing.T) {
 	g := NewWithT(t)

controlplane/kubeadm/internal/cluster_labels.go

@@ -34,5 +34,6 @@ func ControlPlaneMachineLabelsForCluster(kcp *controlplanev1.KubeadmControlPlane
 	// Always force these labels over the ones coming from the spec.
 	labels[clusterv1.ClusterNameLabel] = clusterName
 	labels[clusterv1.MachineControlPlaneLabelName] = ""
+	labels[clusterv1.MachineControlPlaneNameLabel] = kcp.Name
 	return labels
 }

controlplane/kubeadm/internal/controllers/controller.go

@@ -329,6 +329,19 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster *
 	// source ref (reason@machine/name) so the problem can be easily tracked down to its source machine.
 	conditions.SetAggregate(controlPlane.KCP, controlplanev1.MachinesReadyCondition, ownedMachines.ConditionGetters(), conditions.AddSourceRef(), conditions.WithStepCounterIf(false))
 
+	// Ensure all required labels exist on the controlled Machines.
+	// This logic is needed to add the `cluster.x-k8s.io/control-plane-name` label to Machines
+	// which were created before the `cluster.x-k8s.io/control-plane-name` label was introduced
+	// or if a user manually removed the label.
+	// NOTE: Changes will be applied to the Machines in reconcileControlPlaneConditions.
+	// NOTE: cluster.x-k8s.io/control-plane is already set at this stage (it is used when reading controlPlane.Machines).
+	for i := range controlPlane.Machines {
+		machine := controlPlane.Machines[i]
+		if value, ok := machine.Labels[clusterv1.MachineControlPlaneNameLabel]; !ok || value != kcp.Name {
+			machine.Labels[clusterv1.MachineControlPlaneNameLabel] = kcp.Name
+		}
+	}
+
 	// Updates conditions reporting the status of static pods and the status of the etcd cluster.
 	// NOTE: Conditions reporting KCP operation progress like e.g. Resized or SpecUpToDate are inlined with the rest of the execution.
 	if result, err := r.reconcileControlPlaneConditions(ctx, controlPlane); err != nil || !result.IsZero() {

controlplane/kubeadm/internal/controllers/helpers.go

@@ -275,6 +275,7 @@ func (r *KubeadmControlPlaneReconciler) generateMachine(ctx context.Context, kcp
 			Namespace:   kcp.Namespace,
 			Labels:      internal.ControlPlaneMachineLabelsForCluster(kcp, cluster.Name),
 			Annotations: map[string]string{},
+			// Note: by setting the ownerRef on creation we signal to the Machine controller that this is not a stand-alone Machine.
 			OwnerReferences: []metav1.OwnerReference{
 				*metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")),
 			},

controlplane/kubeadm/internal/controllers/helpers_test.go

@@ -549,13 +549,18 @@ func TestKubeadmControlPlaneReconciler_generateMachine(t *testing.T) {
 	for k, v := range kcpMachineTemplateObjectMeta.Labels {
 		g.Expect(machine.Labels[k]).To(Equal(v))
 	}
+	g.Expect(machine.Labels[clusterv1.ClusterLabelName]).To(Equal(cluster.Name))
+	g.Expect(machine.Labels[clusterv1.MachineControlPlaneLabelName]).To(Equal(""))
+	g.Expect(machine.Labels[clusterv1.MachineControlPlaneNameLabel]).To(Equal(kcp.Name))
+
 	for k, v := range kcpMachineTemplateObjectMeta.Annotations {
 		g.Expect(machine.Annotations[k]).To(Equal(v))
 	}
 
 	// Verify that machineTemplate.ObjectMeta in KCP has not been modified.
 	g.Expect(kcp.Spec.MachineTemplate.ObjectMeta.Labels).NotTo(HaveKey(clusterv1.ClusterNameLabel))
 	g.Expect(kcp.Spec.MachineTemplate.ObjectMeta.Labels).NotTo(HaveKey(clusterv1.MachineControlPlaneLabelName))
+	g.Expect(kcp.Spec.MachineTemplate.ObjectMeta.Labels).NotTo(HaveKey(clusterv1.MachineControlPlaneNameLabel))
 	g.Expect(kcp.Spec.MachineTemplate.ObjectMeta.Annotations).NotTo(HaveKey(controlplanev1.KubeadmClusterConfigurationAnnotation))
 }
 

docs/book/src/developer/tilt.md

@@ -100,7 +100,7 @@ kustomize_substitutions:
   EXP_RUNTIME_SDK: "true"
 ```
 
-{{#tabs name:"tab-tilt-kustomize-substitution" tabs:"AWS,Azure,DigitalOcean,GCP"}}
+{{#tabs name:"tab-tilt-kustomize-substitution" tabs:"AWS,Azure,DigitalOcean,GCP,vSphere"}}
 {{#tab AWS}}
 
 For example, if the yaml contains `${AWS_B64ENCODED_CREDENTIALS}`, you could do the following:
@@ -169,6 +169,15 @@ kustomize_substitutions:
   GCP_B64ENCODED_CREDENTIALS: "your credentials here"
 ```
 
+{{#/tab }}
+{{#tab vSphere}}
+
+```yaml
+kustomize_substitutions:
+  VSPHERE_USERNAME: "administrator@vsphere.local"
+  VSPHERE_PASSWORD: "Admin123"
+```
+
 {{#/tab }}
 {{#/tabs }}
 
@@ -303,7 +312,7 @@ Custom values for variable substitutions can be set using `kustomize_substitutio
 ```yaml
 kustomize_substitutions:
   NAMESPACE: default
-  KUBERNETES_VERSION: v1.25.0
+  KUBERNETES_VERSION: v1.25.3
   CONTROL_PLANE_MACHINE_COUNT: 1
   WORKER_MACHINE_COUNT: 3
 ```

docs/book/src/user/quick-start.md

@@ -1122,7 +1122,7 @@ The Docker provider is not designed for production use and is intended for devel
 
 ```bash
 clusterctl generate cluster capi-quickstart --flavor development \
-  --kubernetes-version v1.25.0 \
+  --kubernetes-version v1.25.3 \
   --control-plane-machine-count=3 \
   --worker-machine-count=3 \
   > capi-quickstart.yaml
@@ -1165,7 +1165,7 @@ clusterctl generate cluster capi-quickstart \
 
 ```bash
 clusterctl generate cluster capi-quickstart \
-  --kubernetes-version v1.25.0 \
+  --kubernetes-version v1.25.3 \
   --control-plane-machine-count=3 \
   --worker-machine-count=3 \
   > capi-quickstart.yaml
@@ -1225,7 +1225,7 @@ You should see an output is similar to this:
 
 ```bash
 NAME                    CLUSTER           INITIALIZED   API SERVER AVAILABLE   REPLICAS   READY   UPDATED   UNAVAILABLE   AGE    VERSION
-capi-quickstart-g2trk   capi-quickstart   true                                 3                  3         3             4m7s   v1.25.0
+capi-quickstart-g2trk   capi-quickstart   true                                 3                  3         3             4m7s   v1.25.3
 ```
 
 <aside class="note warning">
@@ -1427,12 +1427,12 @@ kubectl --kubeconfig=./capi-quickstart.kubeconfig get nodes
 ```
 ```bash
 NAME                                          STATUS   ROLES           AGE   VERSION
-capi-quickstart-g2trk-9xrjv                   Ready    control-plane   12m   v1.25.0
-capi-quickstart-g2trk-bmm9v                   Ready    control-plane   11m   v1.25.0
-capi-quickstart-g2trk-hvs9q                   Ready    control-plane   13m   v1.25.0
-capi-quickstart-md-0-55x6t-5649968bd7-8tq9v   Ready    <none>          12m   v1.25.0
-capi-quickstart-md-0-55x6t-5649968bd7-glnjd   Ready    <none>          12m   v1.25.0
-capi-quickstart-md-0-55x6t-5649968bd7-sfzp6   Ready    <none>          12m   v1.25.0
+capi-quickstart-g2trk-9xrjv                   Ready    control-plane   12m   v1.25.3
+capi-quickstart-g2trk-bmm9v                   Ready    control-plane   11m   v1.25.3
+capi-quickstart-g2trk-hvs9q                   Ready    control-plane   13m   v1.25.3
+capi-quickstart-md-0-55x6t-5649968bd7-8tq9v   Ready    <none>          12m   v1.25.3
+capi-quickstart-md-0-55x6t-5649968bd7-glnjd   Ready    <none>          12m   v1.25.3
+capi-quickstart-md-0-55x6t-5649968bd7-sfzp6   Ready    <none>          12m   v1.25.3
 ```
 
 {{#/tab }}

internal/controllers/machine/machine_controller.go

@@ -760,10 +760,16 @@ func (r *Reconciler) shouldAdopt(m *clusterv1.Machine) bool {
 		return false
 	}
 
-	// If the Machine is originated by a MachineDeployment, this prevents it from being adopted as a stand-alone Machine.
-	// Note: this is required because after restore from a backup both the Machine controller and the
-	// MachineSet controller are racing to adopt Machines, see https://github.com/kubernetes-sigs/cluster-api/issues/7529
-	if _, ok := m.Labels[clusterv1.MachineDeploymentUniqueLabel]; ok {
+	// Note: following checks are required because after restore from a backup both the Machine controller and the
+	// MachineSet/ControlPlane controller are racing to adopt Machines, see https://github.com/kubernetes-sigs/cluster-api/issues/7529
+
+	// If the Machine is originated by a MachineSet, it should not be adopted directly by the Cluster as a stand-alone Machine.
+	if _, ok := m.Labels[clusterv1.MachineSetLabelName]; ok {
+		return false
+	}
+
+	// If the Machine is originated by a ControlPlane object, it should not be adopted directly by the Cluster as a stand-alone Machine.
+	if _, ok := m.Labels[clusterv1.MachineControlPlaneNameLabel]; ok {
 		return false
 	}
 	return true

internal/controllers/machinedeployment/machinedeployment_controller.go

@@ -200,6 +200,7 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
 
 	d.Labels[clusterv1.ClusterNameLabel] = d.Spec.ClusterName
 
+	// Set the MachineDeployment as directly owned by the Cluster (if not already present).
 	if r.shouldAdopt(d) {
 		d.OwnerReferences = util.EnsureOwnerRef(d.OwnerReferences, metav1.OwnerReference{
 			APIVersion: clusterv1.GroupVersion.String(),
@@ -226,6 +227,27 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
 		return ctrl.Result{}, err
 	}
 
+	// If not already present, add a label specifying the MachineDeployment name to MachineSets.
+	// Ensure all required labels exist on the controlled MachineSets.
+	// This logic is needed to add the `cluster.x-k8s.io/deployment-name` label to MachineSets
+	// which were created before the `cluster.x-k8s.io/deployment-name` label was added
+	// to all MachineSets created by a MachineDeployment or if a user manually removed the label.
+	for idx := range msList {
+		machineSet := msList[idx]
+		if name, ok := machineSet.Labels[clusterv1.MachineDeploymentLabelName]; ok && name == d.Name {
+			continue
+		}
+
+		helper, err := patch.NewHelper(machineSet, r.Client)
+		if err != nil {
+			return ctrl.Result{}, errors.Wrapf(err, "failed to apply %s label to MachineSet %q", clusterv1.MachineDeploymentLabelName, machineSet.Name)
+		}
+		machineSet.Labels[clusterv1.MachineDeploymentLabelName] = d.Name
+		if err := helper.Patch(ctx, machineSet); err != nil {
+			return ctrl.Result{}, errors.Wrapf(err, "failed to apply %s label to MachineSet %q", clusterv1.MachineDeploymentLabelName, machineSet.Name)
+		}
+	}
+
 	if d.Spec.Paused {
 		return ctrl.Result{}, r.sync(ctx, d, msList)
 	}

internal/controllers/machinedeployment/machinedeployment_controller_test.go

@@ -203,8 +203,16 @@ func TestMachineDeploymentReconciler(t *testing.T) {
 			})
 		}, timeout).Should(BeTrue())
 
-		// Verify that expected number of machines are created
-		t.Log("Verify expected number of machines are created")
+		t.Log("Verify MachineSet has expected replicas and version")
+		firstMachineSet := machineSets.Items[0]
+		g.Expect(*firstMachineSet.Spec.Replicas).To(BeEquivalentTo(2))
+		g.Expect(*firstMachineSet.Spec.Template.Spec.Version).To(BeEquivalentTo("v1.10.3"))
+
+		t.Log("Verify MachineSet has expected ClusterLabelName and MachineDeploymentLabelName")
+		g.Expect(firstMachineSet.Labels[clusterv1.ClusterLabelName]).To(Equal(testCluster.Name))
+		g.Expect(firstMachineSet.Labels[clusterv1.MachineDeploymentLabelName]).To(Equal(deployment.Name))
+
+		t.Log("Verify expected number of Machines are created")
 		machines := &clusterv1.MachineList{}
 		g.Eventually(func() int {
 			if err := env.List(ctx, machines, client.InNamespace(namespace.Name)); err != nil {
@@ -213,16 +221,13 @@ func TestMachineDeploymentReconciler(t *testing.T) {
 			return len(machines.Items)
 		}, timeout).Should(BeEquivalentTo(*deployment.Spec.Replicas))
 
-		// Verify that machines has MachineSetLabelName and MachineDeploymentLabelName labels
-		t.Log("Verify machines have expected MachineSetLabelName and MachineDeploymentLabelName")
+		t.Log("Verify Machines have expected ClusterLabelName, MachineDeploymentLabelName and MachineSetLabelName")
 		for _, m := range machines.Items {
 			g.Expect(m.Labels[clusterv1.ClusterNameLabel]).To(Equal(testCluster.Name))
+			g.Expect(m.Labels[clusterv1.MachineDeploymentLabelName]).To(Equal(deployment.Name))
+			g.Expect(m.Labels[clusterv1.MachineSetLabelName]).To(Equal(firstMachineSet.Name))
 		}
 
-		firstMachineSet := machineSets.Items[0]
-		g.Expect(*firstMachineSet.Spec.Replicas).To(BeEquivalentTo(2))
-		g.Expect(*firstMachineSet.Spec.Template.Spec.Version).To(BeEquivalentTo("v1.10.3"))
-
 		//
 		// Delete firstMachineSet and expect Reconcile to be called to replace it.
 		//
@@ -348,7 +353,7 @@ func TestMachineDeploymentReconciler(t *testing.T) {
 			clusterv1.ClusterNameLabel: testCluster.Name,
 		}
 
-		t.Log("Updating MachineDeployment label")
+		t.Log("Updating MachineDeployment labels")
 		modifyFunc = func(d *clusterv1.MachineDeployment) {
 			d.Spec.Selector.MatchLabels = newLabels
 			d.Spec.Template.Labels = newLabels