[Timofei Larkin (lllamnyp)]

Implements the release delay of the ip-address-claims proposal (Failure and edge cases: "A class may set releaseDelay: after it elapses the controller clears claimRef and the address returns to Available. An address marked held by an annotation is exempt"). A review of cozystack/cozystack#3382 noted that Released addresses under the default Retain accumulate, attributed to no one, until an admin clears them. Stacked on #10.

Changes

• API.
  • IPAddressClass.spec.releaseDelay, a metav1.Duration. It is optional and applies under Retain only. The CRD and deepcopy are regenerated.
  • ipaddress.local.sdn.cozystack.io/held: "true" on an IPAddress exempts it from the delay.
  • AnnotationTrue names the value that turns on the group's boolean annotations.
• Bound condition on IPAddress. The core sets it together with each phase it owns: True for Bound, and False with reason Available, Released or Pending otherwise. Its lastTransitionTime dates the release. Both the address controller and the claim finalizer set phases through one helper, markPhase.
• Delay.
  • Once releaseDelay has passed since an address was released, the address controller clears its claimRef and the address becomes Available on the next pass.
  • Before then, it requeues for the remaining time.
  • A class event re-evaluates that class's Released addresses, so changing the delay takes effect. The delay is read from the class, not copied onto the address.
• Docs. docs/design.md §5 gains step 7 and the state diagram's new edge; the README lifecycle is updated.

Behaviour to know

• An address that was Released before this version has no Bound condition. Its delay counts from the first pass that sets the condition, so upgrading does not immediately reissue old releases.
• The delay applies only to addresses whose claim is gone. Claims with a controller owner delete their addresses instead (Auto-claim addresses for LoadBalancer Services that name no claim #7), so it never applies to auto-claims.
• Conflict and Lost remain driver-owned and are not touched.

Not in this PR

The Associated condition from the proposal. It is driver-owned, alongside status.associatedTo.

go test ./internal/controller/ and golangci-lint run pass.

[coderabbitai]

Important

• Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 200091c7-43af-4930-8952-68e75156eee4

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[IvanHunters]

Review: return Released addresses to the pool after a class's releaseDelay

Conforms to the design. releaseAfterDelay (internal/controller/ipaddress_controller.go:193) acts only on a Released address that is not held, only for a class whose releaseDelay is set and positive, measures from the Bound/Released condition's lastTransitionTime, and requeues with a strict remaining > 0 so there is no negative or zero RequeueAfter. The clock is injectable. Tests pass.

[NIT] held pauses the action, not the clock

The held check returns early without touching the condition, so releaseDelay keeps elapsing while an address is quarantined. Removing held from an address that has sat quarantined longer than releaseDelay returns it to the pool on the next reconcile, with no cooling-off after the quarantine is lifted. This may be intended (lifting the hold means you want the address back), but it is not obvious and is not documented; a line in the operator guide would help.

Minor notes

• markPhase sets the condition's lastTransitionTime from meta.SetStatusCondition (real time.Now()), not the injectable r.Now, so the clock injection is partial; harmless in production but worth knowing.
• Nothing enforces that releaseDelay is meaningful only under Retain; under Delete the field is silently inert. A CEL note or validation would match the "Retain only" wording.
• See the cross-PR note on #10: a Retain class's releaseDelay returns addresses to Available rather than deleting them, which blocks the class-deletion finalizer. Worth documenting together.

LGTM.

The base branch was changed.

[IvanHunters]

Re-approving. The PR's own changes are unchanged since my review of 080303b; the head moved only because the sibling PRs merged into the base. I diffed releaseAfterDelay at the new head against the approved one and they are identical: Released and not held only, class releaseDelay positive, a strict remaining > 0 requeue, injectable clock. The earlier notes stand as non-blocking (the held check pauses the action not the clock; partial clock injection; no CEL that releaseDelay is Retain-only). LGTM.