Merge pull request #7845 from knabben/drop-cluster-topo-field

⚠️ Drop ClusterTopologyManagedFieldsAnnotation field from v1beta1

Author: k8s-ci-robot

.golangci.yml

@@ -222,7 +222,7 @@ issues:
   # Specific exclude rules for deprecated fields that are still part of the codebase. These should be removed as the referenced deprecated item is removed from the project.
   - linters:
       - staticcheck
-    text: "SA1019: (bootstrapv1.ClusterStatus|clusterv1.ClusterTopologyManagedFieldsAnnotation|scope.Config.Spec.UseExperimentalRetryJoin|DockerMachine.Spec.Bootstrapped|machineStatus.Bootstrapped) is deprecated"
+    text: "SA1019: (bootstrapv1.ClusterStatus|scope.Config.Spec.UseExperimentalRetryJoin|DockerMachine.Spec.Bootstrapped|machineStatus.Bootstrapped) is deprecated"
   - linters:
     - revive
     text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"

api/v1beta1/common_types.go

@@ -29,22 +29,6 @@ const (
 	// ClusterTopologyOwnedLabel is the label set on all the object which are managed as part of a ClusterTopology.
 	ClusterTopologyOwnedLabel = "topology.cluster.x-k8s.io/owned"
 
-	// ClusterTopologyManagedFieldsAnnotation is the annotation used to store the list of paths managed
-	// by the topology controller; changes to those paths will be considered authoritative.
-	// NOTE: Managed field depends on the last reconciliation of a managed object; this list can
-	// change during the lifecycle of an object, depending on how the corresponding template + patch/variable
-	// changes over time.
-	// NOTE: The topology controller is only concerned about managed paths in the spec; given that
-	// we are dropping spec. from the result to reduce verbosity of the generated annotation.
-	// NOTE: Managed paths are relevant only for unstructured objects where it is not possible
-	// to easily discover which fields have been set by templates + patches/variables at a given reconcile;
-	// instead, it is not necessary to store managed paths for typed objets (e.g. Cluster, MachineDeployments)
-	// given that the topology controller explicitly sets a well-known, immutable list of fields at every reconcile.
-	//
-	// Deprecated: Topology controller is now using server side apply and this annotation will be removed in a future release.
-	// When removing also remove from staticcheck exclude-rules for SA1019 in golangci.yml.
-	ClusterTopologyManagedFieldsAnnotation = "topology.cluster.x-k8s.io/managed-field-paths"
-
 	// ClusterTopologyMachineDeploymentNameLabel is the label set on the generated  MachineDeployment objects
 	// to track the name of the MachineDeployment topology it represents.
 	ClusterTopologyMachineDeploymentNameLabel = "topology.cluster.x-k8s.io/deployment-name"

docs/book/src/developer/providers/v1.3-to-v1.4.md

@@ -21,11 +21,12 @@ maintainers of providers and consumers of our Go API.
 ### Removals
 
 - `clusterctl backup` has been removed.
-- `MachineHealthCheckSuccededCondition` condition type has been removed.
-- `CloneTemplate` and `CloneTemplateInput` has been removed.
-- The option `--list-images` from `init` subcommand has been removed.
+- `api/v1beta1.MachineHealthCheckSuccededCondition` condition type has been removed.
+- `controller/external/util.CloneTemplate` and `controllers/external/util.CloneTemplateInput` has been removed.
+- The option `--list-images` from `clusterctl init` subcommand has been removed.
 - `exp/runtime/server.NewServer` has been removed.
 - `--disable-no-echo` option from `clusterctl describe cluster` subcommand has been removed
+- `api/v1beta1.ClusterTopologyManagedFieldsAnnotation` field has been removed.
 
 ### API Changes
 

internal/controllers/topology/cluster/structuredmerge/serversidepathhelper.go

@@ -18,15 +18,12 @@ package structuredmerge
 
 import (
 	"context"
-	"encoding/json"
 
 	"github.com/pkg/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
 )
 
@@ -59,13 +56,6 @@ func NewServerSidePatchHelper(ctx context.Context, original, modified client.Obj
 				return nil, errors.Wrap(err, "failed to convert original object to Unstructured")
 			}
 		}
-
-		// If the object has been created with previous custom approach for tracking managed fields, cleanup the object.
-		if _, ok := original.GetAnnotations()[clusterv1.ClusterTopologyManagedFieldsAnnotation]; ok {
-			if err := cleanupLegacyManagedFields(ctx, originalUnstructured, c); err != nil {
-				return nil, errors.Wrap(err, "failed to cleanup legacy managed fields from original object")
-			}
-		}
 	}
 
 	modifiedUnstructured := &unstructured.Unstructured{}
@@ -149,55 +139,3 @@ func (h *serverSidePatchHelper) Patch(ctx context.Context) error {
 	}
 	return h.client.Patch(ctx, h.modified, client.Apply, options...)
 }
-
-// cleanupLegacyManagedFields cleanups managed field management in place before introducing SSA.
-// NOTE: this operation can trigger a machine rollout, but this is considered acceptable given that ClusterClass is still alpha
-// and SSA adoption align the topology controller with K8s recommended solution for many controllers authoring the same object.
-func cleanupLegacyManagedFields(ctx context.Context, obj *unstructured.Unstructured, c client.Client) error {
-	base := obj.DeepCopyObject().(*unstructured.Unstructured)
-
-	// Remove the topology.cluster.x-k8s.io/managed-field-paths annotation
-	annotations := obj.GetAnnotations()
-	delete(annotations, clusterv1.ClusterTopologyManagedFieldsAnnotation)
-	obj.SetAnnotations(annotations)
-
-	// Remove managedFieldEntry for manager=manager and operation=update to prevent having two managers holding values set by the topology controller.
-	originalManagedFields := obj.GetManagedFields()
-	managedFields := make([]metav1.ManagedFieldsEntry, 0, len(originalManagedFields))
-	for i := range originalManagedFields {
-		if originalManagedFields[i].Manager == "manager" &&
-			originalManagedFields[i].Operation == metav1.ManagedFieldsOperationUpdate {
-			continue
-		}
-		managedFields = append(managedFields, originalManagedFields[i])
-	}
-
-	// Add a seeding managedFieldEntry for SSA executed by the management controller, to prevent SSA to create/infer
-	// a default managedFieldEntry when the first SSA is applied.
-	// More specifically, if an existing object doesn't have managedFields when applying the first SSA the API server
-	// creates an entry with operation=Update (kind of guessing where the object comes from), but this entry ends up
-	// acting as a co-ownership and we want to prevent this.
-	// NOTE: fieldV1Map cannot be empty, so we add metadata.name which will be cleaned up at the first SSA patch.
-	fieldV1Map := map[string]interface{}{
-		"f:metadata": map[string]interface{}{
-			"f:name": map[string]interface{}{},
-		},
-	}
-	fieldV1, err := json.Marshal(fieldV1Map)
-	if err != nil {
-		return errors.Wrap(err, "failed to create seeding fieldV1Map for cleaning up legacy managed fields")
-	}
-	now := metav1.Now()
-	managedFields = append(managedFields, metav1.ManagedFieldsEntry{
-		Manager:    TopologyManagerName,
-		Operation:  metav1.ManagedFieldsOperationApply,
-		APIVersion: obj.GetAPIVersion(),
-		Time:       &now,
-		FieldsType: "FieldsV1",
-		FieldsV1:   &metav1.FieldsV1{Raw: fieldV1},
-	})
-
-	obj.SetManagedFields(managedFields)
-
-	return c.Patch(ctx, obj, client.MergeFrom(base))
-}

internal/controllers/topology/cluster/structuredmerge/serversidepathhelper_test.go

@@ -487,57 +487,6 @@ func TestServerSideApply(t *testing.T) {
 	})
 }
 
-func TestServerSideApply_CleanupLegacyManagedFields(t *testing.T) {
-	g := NewWithT(t)
-	// Create a namespace for running the test
-	ns, err := env.CreateNamespace(ctx, "ssa")
-	g.Expect(err).ToNot(HaveOccurred())
-
-	// Build the test object to work with.
-	obj := builder.TestInfrastructureCluster(ns.Name, "obj1").WithSpecFields(map[string]interface{}{
-		"spec.foo": "",
-	}).Build()
-	obj.SetAnnotations(map[string]string{clusterv1.ClusterTopologyManagedFieldsAnnotation: "foo"})
-
-	t.Run("Server side apply cleanups legacy managed fields", func(t *testing.T) {
-		g := NewWithT(t)
-
-		// Create the object simulating reconcile with legacy managed fields
-		g.Expect(env.CreateAndWait(ctx, obj.DeepCopy(), client.FieldOwner("manager")))
-
-		// Gets the object and create SSA patch helper triggering cleanup.
-		original := builder.TestInfrastructureCluster("", "").Build()
-		g.Expect(env.GetAPIReader().Get(ctx, client.ObjectKeyFromObject(obj), original)).To(Succeed())
-
-		modified := obj.DeepCopy()
-		_, err := NewServerSidePatchHelper(ctx, original, modified, env.GetClient())
-		g.Expect(err).ToNot(HaveOccurred())
-
-		// Get created object after cleanup
-		got := builder.TestInfrastructureCluster("", "").Build()
-		g.Expect(env.GetAPIReader().Get(ctx, client.ObjectKeyFromObject(obj), got)).To(Succeed())
-
-		// Check annotation has been removed
-		g.Expect(got.GetAnnotations()).ToNot(HaveKey(clusterv1.ClusterTopologyManagedFieldsAnnotation))
-
-		// Check managed fields has been fixed
-		gotManagedFields := got.GetManagedFields()
-		gotLegacyManager, gotSSAManager := false, false
-		for i := range gotManagedFields {
-			if gotManagedFields[i].Manager == "manager" &&
-				gotManagedFields[i].Operation == metav1.ManagedFieldsOperationUpdate {
-				gotLegacyManager = true
-			}
-			if gotManagedFields[i].Manager == TopologyManagerName &&
-				gotManagedFields[i].Operation == metav1.ManagedFieldsOperationApply {
-				gotSSAManager = true
-			}
-		}
-		g.Expect(gotLegacyManager).To(BeFalse())
-		g.Expect(gotSSAManager).To(BeTrue())
-	})
-}
-
 // getTopologyManagedFields returns metadata.managedFields entry tracking
 // server side apply operations for the topology controller.
 func getTopologyManagedFields(original client.Object) map[string]interface{} {