Merge pull request #121 from erikgb/upgrade-to-ssa

fix: upgrade managed fields in controller resources from CSA to SSA

controllers/namespace_controller.go

@@ -118,6 +118,15 @@ func (r *NamespaceReconciler) propagateMeta(ctx context.Context, ns, parent *cor
 				}
 			}
 		}
+		// Must ensure we set all fields we care for, also labels added when creating namespace
+		labels[constants.LabelCreatedBy] = constants.CreatedBy
+		labels[constants.LabelParent] = parent.Name
+	}
+
+	// Ensure that managed fields are upgraded to SSA before the following SSA.
+	// TODO(migration): This code could be removed after a couple of releases.
+	if err := upgradeManagedFields(ctx, r.Client, ns); err != nil {
+		return err
 	}
 
 	ac := corev1ac.Namespace(ns.Name).
@@ -247,6 +256,13 @@ func (r *NamespaceReconciler) propagateUpdate(ctx context.Context, res *unstruct
 	}
 
 	c2 := cloneResource(res, ns)
+
+	// Ensure that managed fields are upgraded to SSA before the following SSA.
+	// TODO(migration): This code could be removed after a couple of releases.
+	if err := upgradeManagedFields(ctx, r.Client, c2); err != nil {
+		return err
+	}
+
 	if equality.Semantic.Equalities.DeepDerivative(c2, c) {
 		return nil
 	}

controllers/namespace_controller_test.go

@@ -477,6 +477,28 @@ var _ = Describe("Namespace controller", func() {
 		))
 	})
 
+	It("should remove sub namespace labels/annotations set pre SSA migration", func() {
+		ns := &corev1.Namespace{}
+		ns.Name = "pre-ssa-child"
+		Expect(komega.Get(ns)()).To(Succeed())
+		Expect(ns.Labels).To(HaveKeyWithValue("bar.glob/l", "delete-me"))
+		Expect(ns.Annotations).To(HaveKeyWithValue("bar.glob/a", "delete-me"))
+		sn := &accuratev2alpha1.SubNamespace{}
+		sn.Name = "pre-ssa-child"
+		sn.Namespace = "pre-ssa-root"
+		Expect(komega.Update(sn, func() {
+			delete(sn.Spec.Labels, "bar.glob/l")
+			delete(sn.Spec.Annotations, "bar.glob/a")
+		})()).To(Succeed())
+
+		Eventually(komega.Object(ns)).Should(And(
+			HaveField("Labels", Not(HaveKey("bar.glob/l"))),
+			HaveField("Annotations", Not(HaveKey("bar.glob/a"))),
+		))
+		Expect(ns.Labels).To(HaveKeyWithValue("foo.glob/l", "glob"))
+		Expect(ns.Annotations).To(HaveKeyWithValue("foo.glob/a", "glob"))
+	})
+
 	Context("templated namespace", func() {
 		var ns1 *corev1.Namespace
 

controllers/propagate.go

@@ -269,6 +269,13 @@ func (r *PropagateController) propagateUpdate(ctx context.Context, obj, parent *
 
 	if parent != nil {
 		clone := cloneResource(parent, obj.GetNamespace())
+
+		// Ensure that managed fields are upgraded to SSA before the following SSA.
+		// TODO(migration): This code could be removed after a couple of releases.
+		if err := upgradeManagedFields(ctx, r.Client, clone); err != nil {
+			return err
+		}
+
 		if !equality.Semantic.DeepDerivative(clone, obj) {
 			if err := r.Patch(ctx, clone, applyPatch{clone}, fieldOwner, client.ForceOwnership); err != nil {
 				return fmt.Errorf("failed to apply %s/%s: %w", clone.GetNamespace(), clone.GetName(), err)
@@ -302,6 +309,13 @@ func (r *PropagateController) propagateUpdate(ctx context.Context, obj, parent *
 		}
 
 		clone := cloneResource(obj, child.Name)
+
+		// Ensure that managed fields are upgraded to SSA before the following SSA.
+		// TODO(migration): This code could be removed after a couple of releases.
+		if err := upgradeManagedFields(ctx, r.Client, clone); err != nil {
+			return err
+		}
+
 		if equality.Semantic.DeepDerivative(clone, cres) {
 			continue
 		}

controllers/ssa_client.go

@@ -1,20 +1,40 @@
 package controllers
 
 import (
+	"context"
 	"encoding/json"
 
 	accuratev2alpha1 "github.com/cybozu-go/accurate/api/accurate/v2alpha1"
 	accuratev2alpha1ac "github.com/cybozu-go/accurate/internal/applyconfigurations/accurate/v2alpha1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
+	"k8s.io/client-go/util/csaupgrade"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 const (
 	fieldOwner client.FieldOwner = "accurate-controller"
 )
 
+// TODO(migration): This code could be removed after a couple of releases.
+// upgradeManagedFields is a migration function that migrates the ownership of
+// fields from the Update operation to the Apply operation. This is required
+// to ensure that the apply operations will also remove fields that were
+// set by the Update operation.
+func upgradeManagedFields(ctx context.Context, c client.Client, obj client.Object) error {
+	patch, err := csaupgrade.UpgradeManagedFieldsPatch(obj, sets.New(string(fieldOwner)), string(fieldOwner))
+	if err != nil {
+		return err
+	}
+	if patch != nil {
+		return c.Patch(ctx, obj, client.RawPatch(types.JSONPatchType, patch))
+	}
+	// No work to be done - already upgraded
+	return nil
+}
+
 func newSubNamespacePatch(ac *accuratev2alpha1ac.SubNamespaceApplyConfiguration) (*accuratev2alpha1.SubNamespace, client.Patch, error) {
 	sn := &accuratev2alpha1.SubNamespace{}
 	sn.Name = *ac.Name

controllers/subnamespace_controller.go

@@ -136,6 +136,8 @@ func (r *SubNamespaceReconciler) reconcileNS(ctx context.Context, sn *accuratev2
 		)
 	}
 
+	// TODO: upgrade managed fields to SSA when https://github.com/kubernetes/kubernetes/pull/123484 is released
+
 	sn, p, err := newSubNamespacePatch(ac)
 	if err != nil {
 		return err

controllers/suite_test.go

@@ -2,6 +2,7 @@ package controllers
 
 import (
 	"context"
+	"maps"
 	"path/filepath"
 	"testing"
 	"time"
@@ -113,6 +114,33 @@ var _ = BeforeSuite(func() {
 	ns.Labels = map[string]string{constants.LabelTemplate: "prop-tmpl"}
 	Expect(k8sClient.Create(context.Background(), ns)).To(Succeed())
 
+	// Create resources as they would look like before migration to SSA
+	ns = &corev1.Namespace{}
+	ns.Name = "pre-ssa-root"
+	ns.Labels = map[string]string{constants.LabelType: constants.NSTypeRoot}
+	Expect(k8sClient.Create(context.Background(), ns)).To(Succeed())
+
+	sn := &accuratev2alpha1.SubNamespace{}
+	sn.Name = "pre-ssa-child"
+	sn.Namespace = "pre-ssa-root"
+	sn.Spec.Labels = map[string]string{
+		"foo.glob/l": "glob",
+		"bar.glob/l": "delete-me",
+	}
+	sn.Spec.Annotations = map[string]string{
+		"foo.glob/a": "glob",
+		"bar.glob/a": "delete-me",
+	}
+	Expect(k8sClient.Create(context.Background(), sn)).To(Succeed())
+
+	ns = &corev1.Namespace{}
+	ns.Name = sn.Name
+	ns.Finalizers = []string{constants.Finalizer}
+	ns.Labels = map[string]string{constants.LabelCreatedBy: constants.CreatedBy, constants.LabelParent: sn.Namespace}
+	maps.Copy(ns.Labels, sn.Spec.Labels)
+	ns.Annotations = sn.Spec.Annotations
+	// Setting accurate-controller as field owner to simulate existing resource created by Accurate
+	Expect(k8sClient.Create(context.Background(), ns, fieldOwner)).To(Succeed())
 })
 
 var _ = AfterSuite(func() {