Skip to content

Commit

Permalink
Drop ClusterTopologyManagedFieldsAnnotation field from v1beta1
Browse files Browse the repository at this point in the history
  • Loading branch information
knabben committed Jan 4, 2023
1 parent cc7b5ad commit 331a8a1
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 133 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
16 changes: 0 additions & 16 deletions api/v1beta1/common_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 4 additions & 3 deletions docs/book/src/developer/providers/v1.3-to-v1.4.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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))
}
Original file line number Diff line number Diff line change
Expand Up @@ -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{} {
Expand Down

0 comments on commit 331a8a1

Please sign in to comment.