diff --git a/api/v1alpha1/well_known.go b/api/v1alpha1/well_known.go index a901910..56b32f1 100644 --- a/api/v1alpha1/well_known.go +++ b/api/v1alpha1/well_known.go @@ -60,6 +60,12 @@ const ( // bound addresses before the claim disappears. ClaimProtectionFinalizer = "local.sdn.cozystack.io/claim-protection" + // ClassProtectionFinalizer is placed on every IPAddressClass by the core + // controller. It holds a deleted class until no IPAddress of the class + // is left: drivers resolve their backend configuration through the + // class to hold and pin its addresses. + ClassProtectionFinalizer = "local.sdn.cozystack.io/class-protection" + // AddressProtectionFinalizer is placed on every IPAddress by the core // controller. It blocks deletion of an address that is still bound to a // live claim. Drivers add their own finalizer on addresses they @@ -83,6 +89,10 @@ const ( ReasonWaitingForProvisioning = "WaitingForProvisioning" ReasonAddressLost = "AddressLost" + // ReasonClassInUse is recorded on a deleted class that still has + // addresses. + ReasonClassInUse = "ClassInUse" + // Event reasons recorded on Services by auto-claim. ReasonAutoClaimed = "AutoClaimed" ReasonAutoClaimNameTaken = "AutoClaimNameTaken" diff --git a/chart/address-controller/templates/role.yaml b/chart/address-controller/templates/role.yaml index cb55212..3241690 100644 --- a/chart/address-controller/templates/role.yaml +++ b/chart/address-controller/templates/role.yaml @@ -53,6 +53,7 @@ rules: - local.sdn.cozystack.io resources: - ipaddressclaims/finalizers + - ipaddressclasses/finalizers - ipaddresses/finalizers verbs: - update @@ -72,6 +73,8 @@ rules: verbs: - get - list + - patch + - update - watch - apiGroups: - local.sdn.cozystack.io diff --git a/cmd/main.go b/cmd/main.go index 7005c3f..7ab2de7 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -228,6 +228,14 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "IPAddressClaim") os.Exit(1) } + if err = (&controller.IPAddressClassReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("ipaddressclass-controller"), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "IPAddressClass") + os.Exit(1) + } if err = (&controller.ServiceReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), diff --git a/docs/design.md b/docs/design.md index 0c28268..2c38817 100644 --- a/docs/design.md +++ b/docs/design.md @@ -148,7 +148,7 @@ The contract is largely a discipline about who may write which field. | `Address.status.associatedTo` | | | never | ✍ | | `Address` protection finalizer | | | ✍ | | | `Address` driver finalizer(s) | | | | ✍ | -| `IPAddressClass` | | ✍ | reads | reads (incl. parameters) | +| `IPAddressClass` | | ✍ | reads; ✍ protection finalizer | reads (incl. parameters) | Two invariants fall out of this table: @@ -253,16 +253,17 @@ idempotent; the pass re-derives everything from the cluster state. by object name. `status.className` records the resolution when one happened this pass and is left untouched otherwise. -**Class deletion.** The class is load-bearing only at two moments: -resolving/stamping an unbound claim, and matching or provisioning *new* -bindings. Everything downstream deliberately avoids depending on it: -satisfied claims skip class resolution entirely (step 5), reclaim reads -the `reclaimPolicy` **copied onto each address** at provisioning time -(§6), and the address reconciler never reads classes at all. So deleting -an `IPAddressClass` — or restarting the controller after its deletion — -leaves every existing binding, every reclaim, and all status upkeep -intact; the only effect is that still-unbound claims of that class stop -progressing, honestly reported as `Pending` with `ClassNotFound`. +**Class deletion.** The core holds a deleted class with the +`local.sdn.cozystack.io/class-protection` finalizer until no `IPAddress` +of the class is left, in any phase, and records a `ClassInUse` event +naming the addresses it waits for. Drivers resolve their backend +configuration through the class to hold and pin its addresses, so a class +must outlive them. The claim side does not depend on this: satisfied +claims skip class resolution entirely (step 5), and reclaim reads the +`reclaimPolicy` **copied onto each address** at provisioning time (§6). A +class removed by force leaves existing bindings, reclaim and status upkeep +intact; still-unbound claims of that class stay `Pending` with +`ClassNotFound`. ## 5. The address reconciliation algorithm diff --git a/internal/controller/indexes.go b/internal/controller/indexes.go index 2d52f88..931a74d 100644 --- a/internal/controller/indexes.go +++ b/internal/controller/indexes.go @@ -30,6 +30,9 @@ import ( // "/" of their spec.claimRef. const IPAddressClaimRefIndex = "spec.claimRef" +// IPAddressClassNameIndex indexes IPAddress objects by spec.className. +const IPAddressClassNameIndex = "spec.className" + // ClaimRefIndexKey builds the index key for a claim. func ClaimRefIndexKey(namespace, name string) string { return namespace + "/" + name @@ -38,14 +41,27 @@ func ClaimRefIndexKey(namespace, name string) string { // SetupIndexes registers the field indexes shared by the controllers. Call // once, before setting up the controllers. func SetupIndexes(ctx context.Context, mgr ctrl.Manager) error { - return mgr.GetFieldIndexer().IndexField(ctx, &localv1alpha1.IPAddress{}, IPAddressClaimRefIndex, - func(o client.Object) []string { - addr := o.(*localv1alpha1.IPAddress) - if addr.Spec.ClaimRef == nil { - return nil - } - return []string{ClaimRefIndexKey(addr.Spec.ClaimRef.Namespace, addr.Spec.ClaimRef.Name)} - }) + indexer := mgr.GetFieldIndexer() + if err := indexer.IndexField(ctx, &localv1alpha1.IPAddress{}, IPAddressClaimRefIndex, claimRefIndex); err != nil { + return err + } + return indexer.IndexField(ctx, &localv1alpha1.IPAddress{}, IPAddressClassNameIndex, classNameIndex) +} + +func claimRefIndex(o client.Object) []string { + addr := o.(*localv1alpha1.IPAddress) + if addr.Spec.ClaimRef == nil { + return nil + } + return []string{ClaimRefIndexKey(addr.Spec.ClaimRef.Namespace, addr.Spec.ClaimRef.Name)} +} + +func classNameIndex(o client.Object) []string { + addr := o.(*localv1alpha1.IPAddress) + if addr.Spec.ClassName == "" { + return nil + } + return []string{addr.Spec.ClassName} } // familyOf reports the address family of a textual IP, or "" if it does not diff --git a/internal/controller/ipaddressclaim_controller.go b/internal/controller/ipaddressclaim_controller.go index 37950b9..93fec0b 100644 --- a/internal/controller/ipaddressclaim_controller.go +++ b/internal/controller/ipaddressclaim_controller.go @@ -236,6 +236,9 @@ func (r *IPAddressClaimReconciler) defaultClassName(ctx context.Context, claim * if err := r.List(ctx, classes); err != nil { return "", err } + // A deleted class is held only until its addresses are gone; drawing + // new claims into it would keep it from draining. + classes.Items = slices.DeleteFunc(classes.Items, classDeleting) var defaults []string for _, c := range classes.Items { if c.Annotations[localv1alpha1.IsDefaultClassAnnotation] == "true" { diff --git a/internal/controller/ipaddressclaim_controller_test.go b/internal/controller/ipaddressclaim_controller_test.go index b35d54b..550189b 100644 --- a/internal/controller/ipaddressclaim_controller_test.go +++ b/internal/controller/ipaddressclaim_controller_test.go @@ -36,6 +36,7 @@ import ( const ( testClassName = "public" + otherClassName = "private" testProvisioner = "metallb.drivers.local.sdn.cozystack.io" testNamespace = "tenant-a" testClaimName = "web" @@ -59,13 +60,8 @@ func testClient(t *testing.T, objs ...client.Object) client.Client { return fake.NewClientBuilder(). WithScheme(testScheme(t)). WithStatusSubresource(&localv1alpha1.IPAddress{}, &localv1alpha1.IPAddressClaim{}). - WithIndex(&localv1alpha1.IPAddress{}, IPAddressClaimRefIndex, func(o client.Object) []string { - addr := o.(*localv1alpha1.IPAddress) - if addr.Spec.ClaimRef == nil { - return nil - } - return []string{ClaimRefIndexKey(addr.Spec.ClaimRef.Namespace, addr.Spec.ClaimRef.Name)} - }). + WithIndex(&localv1alpha1.IPAddress{}, IPAddressClaimRefIndex, claimRefIndex). + WithIndex(&localv1alpha1.IPAddress{}, IPAddressClassNameIndex, classNameIndex). WithObjects(objs...). Build() } @@ -449,7 +445,7 @@ func TestLostAddressIsNotReplaced(t *testing.T) { func TestClaimRefusesMismatchedPreBinding(t *testing.T) { cases := map[string]*localv1alpha1.IPAddress{ - "other class": preBoundAddress("203.0.113.18", "private"), + "other class": preBoundAddress("203.0.113.18", otherClassName), "unrequested family": preBoundAddress("2001:db8::18", testClassName), } for name, addr := range cases { @@ -499,7 +495,7 @@ func TestOwnedClaimDeletionSparesRefusedPreBinding(t *testing.T) { claim.OwnerReferences = []metav1.OwnerReference{{ APIVersion: "v1", Kind: serviceKind, Name: "web-lb", UID: testServiceUID, Controller: new(true), }} - c := testClient(t, publicClass(nil), claim, preBoundAddress("203.0.113.21", "private")) + c := testClient(t, publicClass(nil), claim, preBoundAddress("203.0.113.21", otherClassName)) if err := c.Delete(context.Background(), claim); err != nil { t.Fatal(err) } diff --git a/internal/controller/ipaddressclass_controller.go b/internal/controller/ipaddressclass_controller.go new file mode 100644 index 0000000..445e74b --- /dev/null +++ b/internal/controller/ipaddressclass_controller.go @@ -0,0 +1,105 @@ +/* +Copyright 2026 The Cozystack Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "sort" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + localv1alpha1 "github.com/cozystack/address-controller/api/v1alpha1" +) + +// IPAddressClassReconciler guards class deletion: a deleted class is held +// by the class-protection finalizer until no IPAddress of the class is left. +type IPAddressClassReconciler struct { + client.Client + Scheme *runtime.Scheme + Recorder record.EventRecorder +} + +// +kubebuilder:rbac:groups=local.sdn.cozystack.io,resources=ipaddressclasses,verbs=get;list;watch;update;patch +// +kubebuilder:rbac:groups=local.sdn.cozystack.io,resources=ipaddressclasses/finalizers,verbs=update +// +kubebuilder:rbac:groups=local.sdn.cozystack.io,resources=ipaddresses,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch + +// Reconcile adds the protection finalizer, or releases a deleted class once +// its addresses are gone. +func (r *IPAddressClassReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + class := &localv1alpha1.IPAddressClass{} + if err := r.Get(ctx, req.NamespacedName, class); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + if class.DeletionTimestamp.IsZero() { + if controllerutil.AddFinalizer(class, localv1alpha1.ClassProtectionFinalizer) { + return ctrl.Result{}, r.Update(ctx, class) + } + return ctrl.Result{}, nil + } + if !controllerutil.ContainsFinalizer(class, localv1alpha1.ClassProtectionFinalizer) { + return ctrl.Result{}, nil + } + + addrs := &localv1alpha1.IPAddressList{} + if err := r.List(ctx, addrs, client.MatchingFields{IPAddressClassNameIndex: class.Name}); err != nil { + return ctrl.Result{}, err + } + if len(addrs.Items) > 0 { + names := make([]string, 0, len(addrs.Items)) + for _, addr := range addrs.Items { + names = append(names, addr.Name) + } + sort.Strings(names) + r.Recorder.Eventf(class, "Warning", localv1alpha1.ReasonClassInUse, + "deletion waits for %d IPAddress(es) of this class: %v", len(names), names) + return ctrl.Result{}, nil + } + controllerutil.RemoveFinalizer(class, localv1alpha1.ClassProtectionFinalizer) + return ctrl.Result{}, r.Update(ctx, class) +} + +// classDeleting reports whether a class is being deleted. +func classDeleting(c localv1alpha1.IPAddressClass) bool { + return !c.DeletionTimestamp.IsZero() +} + +// classForAddress maps an IPAddress event to the address's class. +func (r *IPAddressClassReconciler) classForAddress(_ context.Context, o client.Object) []reconcile.Request { + addr := o.(*localv1alpha1.IPAddress) + if addr.Spec.ClassName == "" { + return nil + } + return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: addr.Spec.ClassName}}} +} + +// SetupWithManager sets up the controller with the Manager. +func (r *IPAddressClassReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&localv1alpha1.IPAddressClass{}). + Watches(&localv1alpha1.IPAddress{}, handler.EnqueueRequestsFromMapFunc(r.classForAddress)). + Named("ipaddressclass"). + Complete(r) +} diff --git a/internal/controller/ipaddressclass_controller_test.go b/internal/controller/ipaddressclass_controller_test.go new file mode 100644 index 0000000..73495b7 --- /dev/null +++ b/internal/controller/ipaddressclass_controller_test.go @@ -0,0 +1,123 @@ +/* +Copyright 2026 The Cozystack Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + localv1alpha1 "github.com/cozystack/address-controller/api/v1alpha1" +) + +func reconcileClass(t *testing.T, c client.Client) { + t.Helper() + r := &IPAddressClassReconciler{Client: c, Scheme: c.Scheme(), Recorder: record.NewFakeRecorder(100)} + if _, err := r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: testClassName}, + }); err != nil { + t.Fatalf("reconcile: %v", err) + } +} + +// deletedClass returns a client holding objs and the test class, deleted +// while it carries the protection finalizer. +func deletedClass(t *testing.T, objs ...client.Object) client.Client { + t.Helper() + class := publicClass(nil) + class.Finalizers = []string{localv1alpha1.ClassProtectionFinalizer} + c := testClient(t, append(objs, class)...) + if err := c.Delete(context.Background(), class); err != nil { + t.Fatal(err) + } + return c +} + +func TestClassGetsProtectionFinalizer(t *testing.T) { + c := testClient(t, publicClass(nil)) + reconcileClass(t, c) + + class := &localv1alpha1.IPAddressClass{} + if err := c.Get(context.Background(), types.NamespacedName{Name: testClassName}, class); err != nil { + t.Fatal(err) + } + if !hasFinalizer(class.Finalizers, localv1alpha1.ClassProtectionFinalizer) { + t.Errorf("finalizers = %v, want the class-protection finalizer", class.Finalizers) + } +} + +func TestDeletedClassWaitsForItsAddresses(t *testing.T) { + // Any address of the class holds it, bound or not. + for _, addr := range []*localv1alpha1.IPAddress{ + boundAddress("ip-bound", "203.0.113.30", localv1alpha1.IPAddressBound), + boundAddress("ip-released", "203.0.113.31", localv1alpha1.IPAddressReleased), + } { + t.Run(addr.Name, func(t *testing.T) { + c := deletedClass(t, addr) + reconcileClass(t, c) + + if err := c.Get(context.Background(), types.NamespacedName{Name: testClassName}, &localv1alpha1.IPAddressClass{}); err != nil { + t.Errorf("class gone while %s exists: %v", addr.Name, err) + } + }) + } +} + +func TestDeletedClassIsReleasedWithoutAddresses(t *testing.T) { + other := boundAddress("ip-other", "203.0.113.32", localv1alpha1.IPAddressBound) + other.Spec.ClassName = otherClassName + c := deletedClass(t, other) + reconcileClass(t, c) + + err := c.Get(context.Background(), types.NamespacedName{Name: testClassName}, &localv1alpha1.IPAddressClass{}) + if !apierrors.IsNotFound(err) { + t.Errorf("class still present with no addresses of its own: %v", err) + } +} + +func TestDeletedClassIsNotChosenByDefault(t *testing.T) { + class := defaultClass() + class.Finalizers = []string{localv1alpha1.ClassProtectionFinalizer} + c := testClient(t, class, pendingClaim("")) + if err := c.Delete(context.Background(), class); err != nil { + t.Fatal(err) + } + reconcileClaim(t, claimReconciler(c)) + + if got := getClaim(t, c); got.Status.ClassName != "" { + t.Errorf("claim resolved to the deleted default class %q", got.Status.ClassName) + } +} + +func TestDeletedClassIsNotChosenForAutoClaim(t *testing.T) { + class := autoClaimClass(testClassName) + class.Finalizers = []string{localv1alpha1.ClassProtectionFinalizer} + c := testClient(t, class, loadBalancerService()) + if err := c.Delete(context.Background(), class); err != nil { + t.Fatal(err) + } + reconcileService(t, serviceReconciler(c)) + + if claim := getAutoClaim(t, c); claim != nil { + t.Errorf("claim auto-provisioned from the deleted class %q", claim.Spec.ClassName) + } +} diff --git a/internal/controller/service_controller.go b/internal/controller/service_controller.go index 9bc2174..3c1efde 100644 --- a/internal/controller/service_controller.go +++ b/internal/controller/service_controller.go @@ -159,6 +159,9 @@ func (r *ServiceReconciler) autoClaimClassName(ctx context.Context, svc *corev1. if err := r.List(ctx, classes); err != nil { return "", err } + // A deleted class is held only until its addresses are gone; drawing + // new claims into it would keep it from draining. + classes.Items = slices.DeleteFunc(classes.Items, classDeleting) var marked []string for _, c := range classes.Items { if c.Annotations[localv1alpha1.IsAutoClaimClassAnnotation] == "true" { diff --git a/internal/controller/service_controller_test.go b/internal/controller/service_controller_test.go index fddac96..979e117 100644 --- a/internal/controller/service_controller_test.go +++ b/internal/controller/service_controller_test.go @@ -177,7 +177,7 @@ func TestAutoClaimWithMultipleMarkedClassesClaimsNothing(t *testing.T) { func TestAutoClaimUsesClassNamedByService(t *testing.T) { svc := loadBalancerService() - svc.Annotations = map[string]string{localv1alpha1.ServiceClassAnnotation: "private"} + svc.Annotations = map[string]string{localv1alpha1.ServiceClassAnnotation: otherClassName} c := testClient(t, autoClaimClass(testClassName), svc) reconcileService(t, serviceReconciler(c)) @@ -185,8 +185,8 @@ func TestAutoClaimUsesClassNamedByService(t *testing.T) { if claim == nil { t.Fatal("no claim auto-provisioned") } - if claim.Spec.ClassName != "private" { - t.Errorf("claim className = %q, want the Service's %q", claim.Spec.ClassName, "private") + if claim.Spec.ClassName != otherClassName { + t.Errorf("claim className = %q, want the Service's %q", claim.Spec.ClassName, otherClassName) } }