diff --git a/internal/controller/keacluster_controller.go b/internal/controller/keacluster_controller.go index 5bcf614..1e8e421 100644 --- a/internal/controller/keacluster_controller.go +++ b/internal/controller/keacluster_controller.go @@ -3,7 +3,10 @@ package controller import ( "context" "fmt" + "maps" + "slices" "strconv" + "strings" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -240,7 +243,7 @@ func (r *KeaClusterReconciler) reconcileServices(ctx context.Context, c *v1alpha svc := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: serviceName(c.Name), Namespace: c.Namespace}} _, err := ctrl.CreateOrUpdate(ctx, r.Client, svc, func() error { svc.Labels = mergeInto(svc.Labels, commonLabels(c.Name)) - svc.Annotations = mergeInto(svc.Annotations, mergeAnnotations(c.Spec.Service)) + svc.Annotations = syncManagedAnnotations(svc.Annotations, mergeAnnotations(c.Spec.Service)) svc.Spec.Selector = commonLabels(c.Name) svcType := c.Spec.Service.Type if svcType == "" { @@ -274,6 +277,33 @@ func mergeInto(dst, src map[string]string) map[string]string { return dst } +const managedAnnotationsKey = "kea.unkin.net/managed-annotations" + +// syncManagedAnnotations applies want and removes keys this operator set +// previously but no longer wants, leaving foreign keys untouched. +func syncManagedAnnotations(have, want map[string]string) map[string]string { + if have == nil { + have = map[string]string{} + } + for _, k := range strings.Split(have[managedAnnotationsKey], ",") { + if _, ok := want[k]; !ok { + delete(have, k) + } + } + delete(have, managedAnnotationsKey) + keys := slices.Sorted(maps.Keys(want)) + for _, k := range keys { + have[k] = want[k] + } + if len(keys) > 0 { + have[managedAnnotationsKey] = strings.Join(keys, ",") + } + if len(have) == 0 { + return nil + } + return have +} + // setPorts replaces the Service ports, keeping any apiserver-allocated // NodePort so a steady-state reconcile is a no-op. func setPorts(svc *corev1.Service, want ...corev1.ServicePort) { diff --git a/internal/controller/keacluster_test.go b/internal/controller/keacluster_test.go index ad63cd0..11583fb 100644 --- a/internal/controller/keacluster_test.go +++ b/internal/controller/keacluster_test.go @@ -385,6 +385,64 @@ func TestSteadyStateReconcileLeavesServicesAlone(t *testing.T) { } } +// TestRemovedServiceAnnotationIsDropped ensures annotations dropped from the +// spec are removed from the Service while foreign annotations survive. +func TestRemovedServiceAnnotationIsDropped(t *testing.T) { + scheme := testScheme(t) + cluster := newClusterFixture() + cluster.Spec.Service.IPAddressPool = "dhcp" + cluster.Spec.Service.Annotations = map[string]string{"example.com/x": "1"} + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&v1alpha1.KeaCluster{}). + WithObjects(withPeers(cluster)...). + Build() + r := &KeaClusterReconciler{Client: cl, Scheme: scheme} + ctx := context.Background() + key := types.NamespacedName{Namespace: "dhcp-system", Name: "pxe"} + req := ctrl.Request{NamespacedName: key} + if _, err := r.Reconcile(ctx, req); err != nil { + t.Fatalf("first reconcile: %v", err) + } + + const foreign = "field.cattle.io/publicEndpoints" + var svc corev1.Service + if err := cl.Get(ctx, key, &svc); err != nil { + t.Fatal(err) + } + if svc.Annotations["example.com/x"] != "1" || svc.Annotations["purelb.io/service-group"] != "dhcp" { + t.Fatalf("operator annotations missing: %v", svc.Annotations) + } + svc.Annotations[foreign] = "[]" + if err := cl.Update(ctx, &svc); err != nil { + t.Fatal(err) + } + + var kc v1alpha1.KeaCluster + if err := cl.Get(ctx, key, &kc); err != nil { + t.Fatal(err) + } + kc.Spec.Service.Annotations = nil + kc.Spec.Service.IPAddressPool = "" + if err := cl.Update(ctx, &kc); err != nil { + t.Fatal(err) + } + if _, err := r.Reconcile(ctx, req); err != nil { + t.Fatalf("second reconcile: %v", err) + } + if err := cl.Get(ctx, key, &svc); err != nil { + t.Fatal(err) + } + for _, k := range []string{"example.com/x", "purelb.io/service-group", managedAnnotationsKey} { + if _, ok := svc.Annotations[k]; ok { + t.Errorf("stale annotation %s kept: %v", k, svc.Annotations) + } + } + if svc.Annotations[foreign] != "[]" { + t.Errorf("foreign annotation dropped: %v", svc.Annotations) + } +} + func contains(hay, needle string) bool { return len(hay) >= len(needle) && (indexOf(hay, needle) >= 0) }