diff --git a/README.md b/README.md index 13164c9..cfa3555 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,9 @@ claim as a `Released` `IPAddress`. `IPAddress` and reports both in `status.addresses`. - **Address**: `Pending` → `Available` (no `claimRef`) → `Bound` → `Released` (claim deleted under `Retain`; not reusable until an admin - clears `spec.claimRef`, at which point it is `Available` again). `Conflict` + clears `spec.claimRef`, or the class's `spec.releaseDelay` elapses, at + which point it is `Available` again; an address annotated + `ipaddress.local.sdn.cozystack.io/held: "true"` waits for the admin). `Conflict` and `Lost` are driver-owned phases; the core controller treats them as sticky. diff --git a/api/v1alpha1/ipaddressclass_types.go b/api/v1alpha1/ipaddressclass_types.go index 555f2ca..062c369 100644 --- a/api/v1alpha1/ipaddressclass_types.go +++ b/api/v1alpha1/ipaddressclass_types.go @@ -53,6 +53,14 @@ type IPAddressClassSpec struct { // +kubebuilder:default=Retain ReclaimPolicy ReclaimPolicy `json:"reclaimPolicy,omitempty"` + // releaseDelay, under reclaimPolicy Retain, returns a Released address + // of this class to Available once it has been Released this long: the + // core controller clears its claimRef. Unset keeps a Released address + // until an admin clears it. An address annotated + // ipaddress.local.sdn.cozystack.io/held: "true" is exempt. + // +optional + ReleaseDelay *metav1.Duration `json:"releaseDelay,omitempty"` + // parameters is an opaque, driver-specific configuration blob (for // example the address ranges to carve from). The core controller never // interprets it. diff --git a/api/v1alpha1/well_known.go b/api/v1alpha1/well_known.go index 56b32f1..05f5eb2 100644 --- a/api/v1alpha1/well_known.go +++ b/api/v1alpha1/well_known.go @@ -20,6 +20,10 @@ package v1alpha1 // Together with the CRD schemas these constants are the contract between the // core controller and the per-class drivers. const ( + // AnnotationTrue is the value that turns on a boolean annotation of + // this group, such as IsDefaultClassAnnotation. + AnnotationTrue = "true" + // IsDefaultClassAnnotation marks an IPAddressClass as the class for // claims that do not name one. Value must be "true". The analogue of // storageclass.kubernetes.io/is-default-class. @@ -55,6 +59,11 @@ const ( // own bookkeeping, such as reservation placeholders. ServiceAutoClaimAnnotation = "local.sdn.cozystack.io/auto-claim" + // HeldAnnotation set to "true" on an IPAddress exempts it from its + // class's releaseDelay: a Released address stays Released until an + // admin clears its claimRef. + HeldAnnotation = "ipaddress.local.sdn.cozystack.io/held" + // ClaimProtectionFinalizer is placed on every IPAddressClaim by the // core controller so that claim deletion runs the reclaim flow for the // bound addresses before the claim disappears. @@ -89,6 +98,12 @@ const ( ReasonWaitingForProvisioning = "WaitingForProvisioning" ReasonAddressLost = "AddressLost" + // Reasons of the Bound condition on an IPAddress, which the core sets + // with its phase; its lastTransitionTime dates the last change. + ReasonAvailable = "Available" + ReasonReleased = "Released" + ReasonPending = "Pending" + // ReasonClassInUse is recorded on a deleted class that still has // addresses. ReasonClassInUse = "ClassInUse" diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 76fd6c2..57da440 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -274,6 +274,11 @@ func (in *IPAddressClassList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IPAddressClassSpec) DeepCopyInto(out *IPAddressClassSpec) { *out = *in + if in.ReleaseDelay != nil { + in, out := &in.ReleaseDelay, &out.ReleaseDelay + *out = new(v1.Duration) + **out = **in + } if in.Parameters != nil { in, out := &in.Parameters, &out.Parameters *out = new(runtime.RawExtension) diff --git a/chart/address-controller/crds/local.sdn.cozystack.io_ipaddressclasses.yaml b/chart/address-controller/crds/local.sdn.cozystack.io_ipaddressclasses.yaml index 4974bc4..792d220 100644 --- a/chart/address-controller/crds/local.sdn.cozystack.io_ipaddressclasses.yaml +++ b/chart/address-controller/crds/local.sdn.cozystack.io_ipaddressclasses.yaml @@ -80,6 +80,14 @@ spec: - Retain - Delete type: string + releaseDelay: + description: |- + releaseDelay, under reclaimPolicy Retain, returns a Released address + of this class to Available once it has been Released this long: the + core controller clears its claimRef. Unset keeps a Released address + until an admin clears it. An address annotated + ipaddress.local.sdn.cozystack.io/held: "true" is exempt. + type: string required: - provisioner type: object diff --git a/docs/design.md b/docs/design.md index 2c38817..16c56b7 100644 --- a/docs/design.md +++ b/docs/design.md @@ -271,10 +271,10 @@ State machine (core-owned transitions solid, driver-owned dashed): ``` (new) ──► Pending ──► Available ◄────────────────┐ - (invalid │ │ ▲ │ admin clears + (invalid │ │ ▲ │ admin clears claimRef, address │ core matches / driver │ claimRef stays │ pre-binds + core accepts │ - Pending) │ ▼ │ │ + Pending) │ ▼ │ │ or releaseDelay elapses │ Bound ───────────────► Released │ ┆ claim deleted │ │ ┆ (Retain) │ (Delete: object @@ -301,7 +301,8 @@ Each pass: 5. **Unbound.** No `claimRef` → `Available`. This single rule is also the recycling path: an admin clears the `claimRef` of a `Released` address and it becomes matchable again — the deliberate, manual step PV - semantics prescribe for `Retain`. + semantics prescribe for `Retain`. A class may automate the step with + `spec.releaseDelay` (step 7). 6. **Bound.** With a `claimRef`: - claim exists, UID matches → `Bound`; - claim exists, `claimRef.uid` empty → wait (the claim reconciler is @@ -313,6 +314,16 @@ Each pass: `Released`). This duplicates the claim-side flow on purpose: reclaim must happen even if the claim vanished without its finalizer running (e.g. the finalizer was force-removed). +7. **Release delay.** A `Released` address of a class with + `spec.releaseDelay` has its `claimRef` cleared once the delay has passed + since it was released, and step 5 makes it `Available`. The release time + is the `lastTransitionTime` of the address's `Bound` condition, which + the core sets with every phase it owns (`True` for `Bound`; `False` with + reason `Available`, `Released` or `Pending` otherwise). An address + annotated `ipaddress.local.sdn.cozystack.io/held: "true"` is exempt and + stays `Released` until an admin acts. An address `Released` before this + condition existed is dated from the first pass that sets it. Changing a + class's delay re-evaluates its `Released` addresses. ## 6. The reclaim flow (claim deletion) diff --git a/internal/controller/ipaddress_controller.go b/internal/controller/ipaddress_controller.go index 05fd45c..1433982 100644 --- a/internal/controller/ipaddress_controller.go +++ b/internal/controller/ipaddress_controller.go @@ -18,8 +18,11 @@ package controller import ( "context" + "time" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" @@ -42,12 +45,15 @@ type IPAddressReconciler struct { client.Client Scheme *runtime.Scheme Recorder record.EventRecorder + // Now returns the current time; nil means time.Now. + Now func() time.Time } // +kubebuilder:rbac:groups=local.sdn.cozystack.io,resources=ipaddresses,verbs=get;list;watch;update;patch;delete // +kubebuilder:rbac:groups=local.sdn.cozystack.io,resources=ipaddresses/status,verbs=get;update;patch // +kubebuilder:rbac:groups=local.sdn.cozystack.io,resources=ipaddresses/finalizers,verbs=update // +kubebuilder:rbac:groups=local.sdn.cozystack.io,resources=ipaddressclaims,verbs=get;list;watch +// +kubebuilder:rbac:groups=local.sdn.cozystack.io,resources=ipaddressclasses,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch // Reconcile drives an IPAddress's phase bookkeeping. @@ -95,9 +101,13 @@ func (r *IPAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( switch { case claim == nil: - // The claim is gone but the claim-side reclaim never ran (stale - // UID, or the claim disappeared without the finalizer flow). - return ctrl.Result{}, r.reclaim(ctx, addr) + // The claim is gone: reclaim it, unless the claim-side flow already + // did (a stale UID, or a claim that vanished without its + // finalizer), and then count down a Released address's delay. + if err := r.reclaim(ctx, addr); err != nil { + return ctrl.Result{}, err + } + return r.releaseAfterDelay(ctx, addr) case !claim.DeletionTimestamp.IsZero(): // The claim-side flow owns the transition; nothing to do here. return ctrl.Result{}, nil @@ -170,13 +180,88 @@ func (r *IPAddressReconciler) reclaim(ctx context.Context, addr *localv1alpha1.I } func (r *IPAddressReconciler) setPhase(ctx context.Context, addr *localv1alpha1.IPAddress, phase localv1alpha1.IPAddressPhase) error { - if addr.Status.Phase == phase { + if !markPhase(addr, phase) { return nil } - addr.Status.Phase = phase return r.Status().Update(ctx, addr) } +// releaseAfterDelay returns a Released address to the pool once its +// class's releaseDelay has passed since it was Released, by clearing its +// claimRef; the next pass makes it Available. An address annotated held, +// or of a class without a delay, is left alone. +func (r *IPAddressReconciler) releaseAfterDelay(ctx context.Context, addr *localv1alpha1.IPAddress) (ctrl.Result, error) { + if addr.Status.Phase != localv1alpha1.IPAddressReleased || addr.Annotations[localv1alpha1.HeldAnnotation] == localv1alpha1.AnnotationTrue { + return ctrl.Result{}, nil + } + class := &localv1alpha1.IPAddressClass{} + if err := r.Get(ctx, types.NamespacedName{Name: addr.Spec.ClassName}, class); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + if class.Spec.ReleaseDelay == nil || class.Spec.ReleaseDelay.Duration <= 0 { + return ctrl.Result{}, nil + } + released := meta.FindStatusCondition(addr.Status.Conditions, localv1alpha1.ConditionBound) + if released == nil || released.Reason != localv1alpha1.ReasonReleased { + return ctrl.Result{}, nil + } + now := time.Now + if r.Now != nil { + now = r.Now + } + if remaining := released.LastTransitionTime.Add(class.Spec.ReleaseDelay.Duration).Sub(now()); remaining > 0 { + return ctrl.Result{RequeueAfter: remaining}, nil + } + ref := addr.Spec.ClaimRef + addr.Spec.ClaimRef = nil + if err := r.Update(ctx, addr); err != nil { + return ctrl.Result{}, err + } + r.Recorder.Eventf(addr, "Normal", "ReleaseDelayElapsed", + "released by claim %s/%s for %s; returned to the pool", ref.Namespace, ref.Name, class.Spec.ReleaseDelay.Duration) + return ctrl.Result{}, nil +} + +// markPhase sets a core-owned phase on the in-memory address together with +// its Bound condition, whose lastTransitionTime then dates the change. It +// reports whether anything changed. +func markPhase(addr *localv1alpha1.IPAddress, phase localv1alpha1.IPAddressPhase) bool { + cond := metav1.Condition{ + Type: localv1alpha1.ConditionBound, + Status: metav1.ConditionFalse, + ObservedGeneration: addr.Generation, + } + switch phase { + case localv1alpha1.IPAddressBound: + cond.Status, cond.Reason, cond.Message = metav1.ConditionTrue, localv1alpha1.ReasonBound, "bound to its claim" + case localv1alpha1.IPAddressReleased: + cond.Reason, cond.Message = localv1alpha1.ReasonReleased, "its claim was deleted under reclaimPolicy Retain" + case localv1alpha1.IPAddressAvailable: + cond.Reason, cond.Message = localv1alpha1.ReasonAvailable, "bound to no claim" + default: + cond.Reason, cond.Message = localv1alpha1.ReasonPending, "spec.address does not parse" + } + changed := addr.Status.Phase != phase + addr.Status.Phase = phase + return meta.SetStatusCondition(&addr.Status.Conditions, cond) || changed +} + +// releasedAddressesForClass maps a class event to its Released addresses, +// so a changed releaseDelay takes effect. +func (r *IPAddressReconciler) releasedAddressesForClass(ctx context.Context, o client.Object) []reconcile.Request { + list := &localv1alpha1.IPAddressList{} + if err := r.List(ctx, list, client.MatchingFields{IPAddressClassNameIndex: o.GetName()}); err != nil { + return nil + } + var reqs []reconcile.Request + for _, addr := range list.Items { + if addr.Status.Phase == localv1alpha1.IPAddressReleased { + reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Name: addr.Name}}) + } + } + return reqs +} + // addressesForClaim maps a claim event to the addresses bound to it, so // claim deletion and rebinding propagate promptly. func (r *IPAddressReconciler) addressesForClaim(ctx context.Context, o client.Object) []reconcile.Request { @@ -198,6 +283,7 @@ func (r *IPAddressReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&localv1alpha1.IPAddress{}). Watches(&localv1alpha1.IPAddressClaim{}, handler.EnqueueRequestsFromMapFunc(r.addressesForClaim)). + Watches(&localv1alpha1.IPAddressClass{}, handler.EnqueueRequestsFromMapFunc(r.releasedAddressesForClass)). Named("ipaddress"). Complete(r) } diff --git a/internal/controller/ipaddressclaim_controller.go b/internal/controller/ipaddressclaim_controller.go index 93fec0b..e08178e 100644 --- a/internal/controller/ipaddressclaim_controller.go +++ b/internal/controller/ipaddressclaim_controller.go @@ -166,8 +166,7 @@ func (r *IPAddressClaimReconciler) finalizeClaim(ctx context.Context, claim *loc case addr.Status.Phase == localv1alpha1.IPAddressConflict || addr.Status.Phase == localv1alpha1.IPAddressLost: // Conflict and Lost belong to the driver and stay as recorded. default: // Retain - if addr.Status.Phase != localv1alpha1.IPAddressReleased { - addr.Status.Phase = localv1alpha1.IPAddressReleased + if markPhase(addr, localv1alpha1.IPAddressReleased) { if err := r.Status().Update(ctx, addr); err != nil { return err } @@ -241,7 +240,7 @@ func (r *IPAddressClaimReconciler) defaultClassName(ctx context.Context, claim * classes.Items = slices.DeleteFunc(classes.Items, classDeleting) var defaults []string for _, c := range classes.Items { - if c.Annotations[localv1alpha1.IsDefaultClassAnnotation] == "true" { + if c.Annotations[localv1alpha1.IsDefaultClassAnnotation] == localv1alpha1.AnnotationTrue { defaults = append(defaults, c.Name) } } diff --git a/internal/controller/ipaddressclaim_controller_test.go b/internal/controller/ipaddressclaim_controller_test.go index 550189b..92771c2 100644 --- a/internal/controller/ipaddressclaim_controller_test.go +++ b/internal/controller/ipaddressclaim_controller_test.go @@ -113,7 +113,7 @@ func publicClass(annotations map[string]string) *localv1alpha1.IPAddressClass { } func defaultClass() *localv1alpha1.IPAddressClass { - return publicClass(map[string]string{localv1alpha1.IsDefaultClassAnnotation: "true"}) + return publicClass(map[string]string{localv1alpha1.IsDefaultClassAnnotation: localv1alpha1.AnnotationTrue}) } func pendingClaim(className string) *localv1alpha1.IPAddressClaim { @@ -342,6 +342,9 @@ func TestClaimDeletionRetainReleasesAddress(t *testing.T) { if got.Spec.ClaimRef == nil { t.Error("claimRef cleared on Retain; it must survive until an admin clears it") } + if cond := meta.FindStatusCondition(got.Status.Conditions, localv1alpha1.ConditionBound); cond == nil || cond.Reason != localv1alpha1.ReasonReleased { + t.Errorf("Bound condition = %+v, want reason Released to date the release", cond) + } } func TestClaimDeletionDeletePolicyDeletesAddress(t *testing.T) { diff --git a/internal/controller/release_delay_test.go b/internal/controller/release_delay_test.go new file mode 100644 index 0000000..9bcd135 --- /dev/null +++ b/internal/controller/release_delay_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" + "time" + + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + + localv1alpha1 "github.com/cozystack/address-controller/api/v1alpha1" +) + +var releasedAt = time.Date(2026, 11, 1, 12, 0, 0, 0, time.UTC) + +// delayedClass is the test class with a one-hour releaseDelay. +func delayedClass() *localv1alpha1.IPAddressClass { + class := publicClass(nil) + class.Spec.ReleaseDelay = &metav1.Duration{Duration: time.Hour} + return class +} + +// releasedAddress is an address whose claim is gone, Released at releasedAt. +func releasedAddress() *localv1alpha1.IPAddress { + addr := boundAddress("ip-released", "203.0.113.40", localv1alpha1.IPAddressReleased) + addr.Spec.ClaimRef.UID = "deleted-claim-uid" + addr.Status.Conditions = []metav1.Condition{{ + Type: localv1alpha1.ConditionBound, + Status: metav1.ConditionFalse, + Reason: localv1alpha1.ReasonReleased, + LastTransitionTime: metav1.NewTime(releasedAt), + }} + return addr +} + +func reconcileAddressAt(t *testing.T, r *IPAddressReconciler, now time.Time) ctrl.Result { + t.Helper() + r.Now = func() time.Time { return now } + res, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: "ip-released"}}) + if err != nil { + t.Fatalf("reconcile: %v", err) + } + return res +} + +func TestReleasedAddressReturnsAfterDelay(t *testing.T) { + c := testClient(t, delayedClass(), releasedAddress()) + r := addressReconciler(c) + reconcileAddressAt(t, r, releasedAt.Add(2*time.Hour)) + reconcileAddressAt(t, r, releasedAt.Add(2*time.Hour)) + + got := getAddress(t, c, "ip-released") + if got.Spec.ClaimRef != nil { + t.Errorf("claimRef = %+v, want cleared after the delay", got.Spec.ClaimRef) + } + if got.Status.Phase != localv1alpha1.IPAddressAvailable { + t.Errorf("phase = %q, want Available", got.Status.Phase) + } +} + +func TestReleasedAddressWaitsOutTheDelay(t *testing.T) { + c := testClient(t, delayedClass(), releasedAddress()) + res := reconcileAddressAt(t, addressReconciler(c), releasedAt.Add(10*time.Minute)) + + if got := getAddress(t, c, "ip-released"); got.Spec.ClaimRef == nil { + t.Error("claimRef cleared before the delay elapsed") + } + if res.RequeueAfter != 50*time.Minute { + t.Errorf("RequeueAfter = %s, want the remaining 50m", res.RequeueAfter) + } +} + +func TestReleasedAddressStaysWithoutDelayOrWhenHeld(t *testing.T) { + held := releasedAddress() + held.Annotations = map[string]string{localv1alpha1.HeldAnnotation: localv1alpha1.AnnotationTrue} + cases := map[string]struct { + class *localv1alpha1.IPAddressClass + addr *localv1alpha1.IPAddress + }{ + "class has no delay": {publicClass(nil), releasedAddress()}, + "address is held": {delayedClass(), held}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + c := testClient(t, tc.class, tc.addr) + reconcileAddressAt(t, addressReconciler(c), releasedAt.Add(48*time.Hour)) + + if got := getAddress(t, c, "ip-released"); got.Spec.ClaimRef == nil || got.Status.Phase != localv1alpha1.IPAddressReleased { + t.Errorf("phase %q, claimRef %+v; want it kept Released", got.Status.Phase, got.Spec.ClaimRef) + } + }) + } +} + +func TestPhaseIsDatedByTheBoundCondition(t *testing.T) { + addr := unboundAddress("ip-41", "203.0.113.41") + c := testClient(t, addr) + reconcileAddress(t, addressReconciler(c), "ip-41") + + got := getAddress(t, c, "ip-41") + cond := meta.FindStatusCondition(got.Status.Conditions, localv1alpha1.ConditionBound) + if cond == nil || cond.Status != metav1.ConditionFalse || cond.Reason != localv1alpha1.ReasonAvailable { + t.Errorf("Bound condition = %+v, want False/Available", cond) + } +} diff --git a/internal/controller/service_controller.go b/internal/controller/service_controller.go index 3c1efde..6f421eb 100644 --- a/internal/controller/service_controller.go +++ b/internal/controller/service_controller.go @@ -164,7 +164,7 @@ func (r *ServiceReconciler) autoClaimClassName(ctx context.Context, svc *corev1. classes.Items = slices.DeleteFunc(classes.Items, classDeleting) var marked []string for _, c := range classes.Items { - if c.Annotations[localv1alpha1.IsAutoClaimClassAnnotation] == "true" { + if c.Annotations[localv1alpha1.IsAutoClaimClassAnnotation] == localv1alpha1.AnnotationTrue { marked = append(marked, c.Name) } } diff --git a/internal/controller/service_controller_test.go b/internal/controller/service_controller_test.go index 979e117..d2bc2d9 100644 --- a/internal/controller/service_controller_test.go +++ b/internal/controller/service_controller_test.go @@ -79,7 +79,7 @@ func getAutoClaim(t *testing.T, c client.Client) *localv1alpha1.IPAddressClaim { } func autoClaimClass(name string) *localv1alpha1.IPAddressClass { - class := publicClass(map[string]string{localv1alpha1.IsAutoClaimClassAnnotation: "true"}) + class := publicClass(map[string]string{localv1alpha1.IsAutoClaimClassAnnotation: localv1alpha1.AnnotationTrue}) class.Name = name return class }