Skip to content

🌱 Remove unused CleanUpManagedFieldsForSSAAdoption code #12788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 23, 2025
Merged

🌱 Remove unused CleanUpManagedFieldsForSSAAdoption code #12788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions controlplane/kubeadm/internal/controllers/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,13 +797,6 @@ func (r *KubeadmControlPlaneReconciler) syncMachines(ctx context.Context, contro
continue
}

// Cleanup managed fields of all Machines.
// We do this so that Machines that were created/patched before the controller adopted Server-Side-Apply (SSA)
// (< v1.4.0) can also work with SSA. Otherwise, fields would be co-owned by our "old" "manager" and
// "capi-kubeadmcontrolplane" and then we would not be able to e.g. drop labels and annotations.
if err := ssa.CleanUpManagedFieldsForSSAAdoption(ctx, r.Client, m, kcpManagerName); err != nil {
return errors.Wrapf(err, "failed to update Machine: failed to adjust the managedFields of the Machine %s", klog.KObj(m))
}
// Update Machine to propagate in-place mutable fields from KCP.
updatedMachine, err := r.updateMachine(ctx, m, controlPlane.KCP, controlPlane.Cluster)
if err != nil {
Expand Down
17 changes: 8 additions & 9 deletions controlplane/kubeadm/internal/controllers/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1803,16 +1803,15 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) {
InfrastructureRef: *infraMachineRef,
Version: "v1.25.3",
FailureDomain: fd,
ProviderID: "provider-id",
// ProviderID is intentionally not set here, this field is set by the Machine controller.
Deletion: clusterv1.MachineDeletionSpec{
NodeDrainTimeoutSeconds: duration5s,
NodeVolumeDetachTimeoutSeconds: duration5s,
NodeDeletionTimeoutSeconds: duration5s,
},
},
}
// Note: use "manager" as the field owner to mimic the manager used before ClusterAPI v1.4.0.
g.Expect(env.Create(ctx, inPlaceMutatingMachine, client.FieldOwner("manager"))).To(Succeed())
g.Expect(env.PatchAndWait(ctx, inPlaceMutatingMachine, client.FieldOwner(kcpManagerName))).To(Succeed())

// Existing machine that is in deleting state
deletingMachine := &clusterv1.Machine{
Expand Down Expand Up @@ -1845,7 +1844,7 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) {
ReadinessGates: mandatoryMachineReadinessGates,
},
}
g.Expect(env.Create(ctx, deletingMachine, client.FieldOwner(classicManager))).To(Succeed())
g.Expect(env.PatchAndWait(ctx, deletingMachine, client.FieldOwner(kcpManagerName))).To(Succeed())
// Delete the machine to put it in the deleting state
g.Expect(env.Delete(ctx, deletingMachine)).To(Succeed())
// Wait till the machine is marked for deletion
Expand Down Expand Up @@ -2094,13 +2093,13 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) {
g.Expect(env.GetAPIReader().Get(ctx, client.ObjectKeyFromObject(updatedDeletingMachine), updatedDeletingMachine)).To(Succeed())

// Verify ManagedFields
g.Expect(updatedDeletingMachine.ManagedFields).ShouldNot(
g.Expect(updatedDeletingMachine.ManagedFields).Should(
ContainElement(ssa.MatchManagedFieldsEntry(kcpManagerName, metav1.ManagedFieldsOperationApply)),
"deleting machine should not contain an entry for SSA manager",
"deleting machine should contain an entry for SSA manager",
)
g.Expect(updatedDeletingMachine.ManagedFields).Should(
ContainElement(ssa.MatchManagedFieldsEntry("manager", metav1.ManagedFieldsOperationUpdate)),
"in-place mutable machine should still contain an entry for old manager",
g.Expect(updatedDeletingMachine.ManagedFields).ShouldNot(
ContainElement(ssa.MatchManagedFieldsEntry(classicManager, metav1.ManagedFieldsOperationUpdate)),
"deleting machine should not contain an entry for old manager",
)

// Verify the machine labels and annotations are unchanged.
Expand Down
13 changes: 0 additions & 13 deletions internal/controllers/machinedeployment/machinedeployment_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,19 +286,6 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope) error {
}
}

// Loop over all MachineSets and cleanup managed fields.
// We do this so that MachineSets that were created/patched before (< v1.4.0) the controller adopted
// Server-Side-Apply (SSA) can also work with SSA. Otherwise, fields would be co-owned by our "old" "manager" and
// "capi-machinedeployment" and then we would not be able to e.g. drop labels and annotations.
// Note: We are cleaning up managed fields for all MachineSets, so we're able to remove this code in a few
// Cluster API releases. If we do this only for selected MachineSets, we would have to keep this code forever.
for idx := range s.machineSets {
machineSet := s.machineSets[idx]
if err := ssa.CleanUpManagedFieldsForSSAAdoption(ctx, r.Client, machineSet, machineDeploymentManagerName); err != nil {
return errors.Wrapf(err, "failed to clean up managedFields of MachineSet %s", klog.KObj(machineSet))
}
}

templateExists := s.infrastructureTemplateExists && (!md.Spec.Template.Spec.Bootstrap.ConfigRef.IsDefined() || s.bootstrapTemplateExists)

if ptr.Deref(md.Spec.Paused, false) {
Expand Down
191 changes: 0 additions & 191 deletions internal/controllers/machinedeployment/machinedeployment_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (

clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
"sigs.k8s.io/cluster-api/controllers/external"
"sigs.k8s.io/cluster-api/internal/util/ssa"
"sigs.k8s.io/cluster-api/util"
v1beta1conditions "sigs.k8s.io/cluster-api/util/conditions/deprecated/v1beta1"
"sigs.k8s.io/cluster-api/util/patch"
Expand Down Expand Up @@ -500,196 +499,6 @@ func TestMachineDeploymentReconciler(t *testing.T) {
})
}

func TestMachineDeploymentReconciler_CleanUpManagedFieldsForSSAAdoption(t *testing.T) {
setup := func(t *testing.T, g *WithT) (*corev1.Namespace, *clusterv1.Cluster) {
t.Helper()

t.Log("Creating the namespace")
ns, err := env.CreateNamespace(ctx, machineDeploymentNamespace)
g.Expect(err).ToNot(HaveOccurred())

t.Log("Creating the Cluster")
cluster := &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns.Name,
Name: "test-cluster",
},
Spec: clusterv1.ClusterSpec{
ControlPlaneRef: clusterv1.ContractVersionedObjectReference{
APIGroup: builder.ControlPlaneGroupVersion.Group,
Kind: builder.GenericControlPlaneKind,
Name: "cp1",
},
},
}
g.Expect(env.Create(ctx, cluster)).To(Succeed())

t.Log("Creating the Cluster Kubeconfig Secret")
g.Expect(env.CreateKubeconfigSecret(ctx, cluster)).To(Succeed())

return ns, cluster
}

teardown := func(t *testing.T, g *WithT, ns *corev1.Namespace, cluster *clusterv1.Cluster) {
t.Helper()

t.Log("Deleting the Cluster")
g.Expect(env.Delete(ctx, cluster)).To(Succeed())
t.Log("Deleting the namespace")
g.Expect(env.Delete(ctx, ns)).To(Succeed())
}

g := NewWithT(t)
namespace, testCluster := setup(t, g)
defer teardown(t, g, namespace, testCluster)

labels := map[string]string{
"foo": "bar",
clusterv1.ClusterNameLabel: testCluster.Name,
}
version := "v1.10.3"
deployment := &clusterv1.MachineDeployment{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "md-",
Namespace: namespace.Name,
Labels: map[string]string{
clusterv1.ClusterNameLabel: testCluster.Name,
},
},
Spec: clusterv1.MachineDeploymentSpec{
Paused: ptr.To(true), // Set this to true as we do not want to test the other parts of the reconciler in this test.
ClusterName: testCluster.Name,
Replicas: ptr.To[int32](2),
Selector: metav1.LabelSelector{
// We're using the same labels for spec.selector and spec.template.labels.
MatchLabels: labels,
},
Rollout: clusterv1.MachineDeploymentRolloutSpec{
Strategy: clusterv1.MachineDeploymentRolloutStrategy{
Type: clusterv1.RollingUpdateMachineDeploymentStrategyType,
RollingUpdate: clusterv1.MachineDeploymentRolloutStrategyRollingUpdate{
MaxUnavailable: intOrStrPtr(0),
MaxSurge: intOrStrPtr(1),
},
},
},
Deletion: clusterv1.MachineDeploymentDeletionSpec{
Order: clusterv1.OldestMachineSetDeletionOrder,
},
Template: clusterv1.MachineTemplateSpec{
ObjectMeta: clusterv1.ObjectMeta{
Labels: labels,
},
Spec: clusterv1.MachineSpec{
ClusterName: testCluster.Name,
Version: version,
InfrastructureRef: clusterv1.ContractVersionedObjectReference{
APIGroup: clusterv1.GroupVersionInfrastructure.Group,
Kind: "GenericInfrastructureMachineTemplate",
Name: "md-template",
},
Bootstrap: clusterv1.Bootstrap{
DataSecretName: ptr.To("data-secret-name"),
},
},
},
},
}
msListOpts := []client.ListOption{
client.InNamespace(namespace.Name),
client.MatchingLabels(labels),
}

// Create infrastructure template resource.
infraResource := map[string]interface{}{
"kind": "GenericInfrastructureMachine",
"apiVersion": clusterv1.GroupVersionInfrastructure.String(),
"metadata": map[string]interface{}{},
"spec": map[string]interface{}{
"size": "3xlarge",
},
}
infraTmpl := &unstructured.Unstructured{
Object: map[string]interface{}{
"kind": "GenericInfrastructureMachineTemplate",
"apiVersion": clusterv1.GroupVersionInfrastructure.String(),
"metadata": map[string]interface{}{
"name": "md-template",
"namespace": namespace.Name,
},
"spec": map[string]interface{}{
"template": infraResource,
},
},
}
t.Log("Creating the infrastructure template")
g.Expect(env.Create(ctx, infraTmpl)).To(Succeed())

// Create the MachineDeployment object and expect Reconcile to be called.
t.Log("Creating the MachineDeployment")
g.Expect(env.Create(ctx, deployment)).To(Succeed())

// Create a MachineSet for the MachineDeployment.
classicManagerMS := &clusterv1.MachineSet{
TypeMeta: metav1.TypeMeta{
Kind: "MachineSet",
APIVersion: clusterv1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: deployment.Name + "-" + "classic-ms",
Namespace: testCluster.Namespace,
Labels: labels,
},
Spec: clusterv1.MachineSetSpec{
ClusterName: testCluster.Name,
Replicas: ptr.To[int32](0),
Selector: metav1.LabelSelector{
MatchLabels: labels,
},
Template: clusterv1.MachineTemplateSpec{
ObjectMeta: clusterv1.ObjectMeta{
Labels: labels,
},
Spec: clusterv1.MachineSpec{
ClusterName: testCluster.Name,
InfrastructureRef: clusterv1.ContractVersionedObjectReference{
APIGroup: clusterv1.GroupVersionInfrastructure.Group,
Kind: "GenericInfrastructureMachineTemplate",
Name: "md-template",
},
Bootstrap: clusterv1.Bootstrap{
DataSecretName: ptr.To("data-secret-name"),
},
Version: version,
},
},
},
}
ssaManagerMS := classicManagerMS.DeepCopy()
ssaManagerMS.Name = deployment.Name + "-" + "ssa-ms"

// Create one using the "old manager".
g.Expect(env.Create(ctx, classicManagerMS, client.FieldOwner("manager"))).To(Succeed())

// Create one using SSA.
g.Expect(env.Patch(ctx, ssaManagerMS, client.Apply, client.FieldOwner(machineDeploymentManagerName), client.ForceOwnership)).To(Succeed())

// Verify that for both the MachineSets the ManagedFields are updated.
g.Eventually(func(g Gomega) {
machineSets := &clusterv1.MachineSetList{}
g.Expect(env.List(ctx, machineSets, msListOpts...)).To(Succeed())

g.Expect(machineSets.Items).To(HaveLen(2))
for _, ms := range machineSets.Items {
// Verify the ManagedFields are updated.
g.Expect(ms.GetManagedFields()).Should(
ContainElement(ssa.MatchManagedFieldsEntry(machineDeploymentManagerName, metav1.ManagedFieldsOperationApply)))
g.Expect(ms.GetManagedFields()).ShouldNot(
ContainElement(ssa.MatchManagedFieldsEntry("manager", metav1.ManagedFieldsOperationUpdate)))
}
}).Should(Succeed())
}

func TestMachineSetToDeployments(t *testing.T) {
g := NewWithT(t)

Expand Down
8 changes: 0 additions & 8 deletions internal/controllers/machineset/machineset_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,14 +544,6 @@ func (r *Reconciler) syncMachines(ctx context.Context, s *scope) (ctrl.Result, e
}
}

// Cleanup managed fields of all Machines.
// We do this so that Machines that were created/patched before the controller adopted Server-Side-Apply (SSA)
// (< v1.4.0) can also work with SSA. Otherwise, fields would be co-owned by our "old" "manager" and
// "capi-machineset" and then we would not be able to e.g. drop labels and annotations.
if err := ssa.CleanUpManagedFieldsForSSAAdoption(ctx, r.Client, m, machineSetManagerName); err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to update machine: failed to adjust the managedFields of the Machine %q", m.Name)
}

// Update Machine to propagate in-place mutable fields from the MachineSet.
updatedMachine, err := r.computeDesiredMachine(machineSet, m)
if err != nil {
Expand Down
16 changes: 7 additions & 9 deletions internal/controllers/machineset/machineset_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,6 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) {
Kind: "Machine",
},
ObjectMeta: metav1.ObjectMeta{
UID: "abc-123-uid",
Name: "in-place-mutating-machine",
Namespace: namespace.Name,
Labels: map[string]string{
Expand Down Expand Up @@ -1274,15 +1273,14 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) {
},
},
}
g.Expect(env.Create(ctx, inPlaceMutatingMachine, client.FieldOwner(classicManager))).To(Succeed())
g.Expect(env.PatchAndWait(ctx, inPlaceMutatingMachine, client.FieldOwner(machineSetManagerName))).To(Succeed())

deletingMachine := &clusterv1.Machine{
TypeMeta: metav1.TypeMeta{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "Machine",
},
ObjectMeta: metav1.ObjectMeta{
UID: "abc-123-uid",
Name: "deleting-machine",
Namespace: namespace.Name,
Labels: map[string]string{},
Expand All @@ -1301,7 +1299,7 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) {
},
},
}
g.Expect(env.Create(ctx, deletingMachine, client.FieldOwner(classicManager))).To(Succeed())
g.Expect(env.PatchAndWait(ctx, deletingMachine, client.FieldOwner(machineSetManagerName))).To(Succeed())
// Delete the machine to put it in the deleting state
g.Expect(env.Delete(ctx, deletingMachine)).To(Succeed())
// Wait till the machine is marked for deletion
Expand Down Expand Up @@ -1476,13 +1474,13 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) {
g.Expect(env.GetAPIReader().Get(ctx, client.ObjectKeyFromObject(updatedDeletingMachine), updatedDeletingMachine)).To(Succeed())

// Verify ManagedFields
g.Expect(updatedDeletingMachine.ManagedFields).ShouldNot(
g.Expect(updatedDeletingMachine.ManagedFields).Should(
ContainElement(ssa.MatchManagedFieldsEntry(machineSetManagerName, metav1.ManagedFieldsOperationApply)),
"deleting machine should not contain an entry for SSA manager",
"deleting machine should contain an entry for SSA manager",
)
g.Expect(updatedDeletingMachine.ManagedFields).Should(
ContainElement(ssa.MatchManagedFieldsEntry("manager", metav1.ManagedFieldsOperationUpdate)),
"in-place mutable machine should still contain an entry for old manager",
g.Expect(updatedDeletingMachine.ManagedFields).ShouldNot(
ContainElement(ssa.MatchManagedFieldsEntry(classicManager, metav1.ManagedFieldsOperationUpdate)),
"deleting machine should not contain an entry for old manager",
)

// Verify in-place mutable fields are still the same.
Expand Down
Loading