Skip to content
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.

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

⚠️ Drop ClusterTopologyManagedFieldsAnnotation field from v1beta1 #7845

Merged
merged 1 commit into from
Jan 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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"

knabben marked this conversation as resolved.
Show resolved Hide resolved
"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 {
sbueringer marked this conversation as resolved.
Show resolved Hide resolved
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