Repository navigation
Hold a deleted IPAddressClass until its addresses are gone - #10
Timofei Larkin (lllamnyp) wants to merge 2 commits into
Conversation
The core now puts a class-protection finalizer on every IPAddressClass and removes it from a deleted class only once no IPAddress of the class remains, recording a ClassInUse event that names the addresses it waits for. Drivers resolve their backend configuration through the class to hold and pin its addresses, so the class must outlive them. Assisted-By: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
Review: hold a deleted IPAddressClass until its addresses are gone
Conforms to the design's class-deletion guard. The class-protection finalizer is added to a live class and removed only when no IPAddress of the class remains, counted through the spec.className index (internal/controller/ipaddressclass_controller.go). The controller never touches the MetalLB pool or advertisement, so the admin's pool survives class deletion, and it does not delete addresses. The release path (a watch requeuing the class on the last address's deletion) works, and I found no deadlock across the class/claim/address finalizers. RBAC is accurate.
[MINOR] A Terminating class is still selected as default and for auto-claim
Neither the default-class selection (ipaddressclaim_controller.go:241) nor the auto-claim-class selection (service_controller.go:164) skips a class with a non-zero DeletionTimestamp. While the finalizer holds a class in Terminating, new claims still resolve to it and auto-claim still mints into it, which keeps minting addresses that extend the hold and can keep the class from draining. Skipping classes with !DeletionTimestamp.IsZero() in those two loops closes it.
Note (cross-PR, not a blocker)
The finalizer holds on an IPAddress of the class in any phase, which the design intends. Combined with reclaim Retain and the releaseDelay of #11, this means a class whose pool was ever used accumulates Released/Available addresses that block its deletion, and releaseDelay returns them to Available rather than deleting them, so it does not help teardown. Deleting such a class then needs the operator to delete its addresses first. Worth documenting in the operator guide (#14) so this reads as expected rather than a stuck deletion.
LGTM, with the Terminating-class selection worth fixing.
The base branch was changed.
The class-protection finalizer keeps a deleted class until its addresses are gone, and while it waited, default-class resolution and auto-claim still chose it, minting addresses that kept it from draining. Both now skip a class that is being deleted. Assisted-By: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
|
Fixed in db6042e: default-class resolution and auto-claim skip a class that is being deleted, so a Terminating class no longer draws new claims while the finalizer holds it. Tests: The cross-PR note on teardown (a used class's This PR now targets |
Implements the class-deletion guard of the
ip-address-claimsproposal (Failure and edge cases: "Class deleted with bound addresses → refused … A finalizer holds the class until its addresses are gone"). A review of the distribution package in cozystack/cozystack#3382 raised it under lifecycle and teardown. Stacked on #7.Changes
IPAddressClassReconcilerputslocal.sdn.cozystack.io/class-protectionon everyIPAddressClass. On a deleted class it removes the finalizer only once noIPAddresswith thatspec.classNameis left, in any phase. While it waits, it records aClassInUseevent naming the addresses. Address events re-trigger the class.IPAddressspec.className.update/patchon classes andupdateonipaddressclasses/finalizers.docs/design.md§4 and the ownership table are updated.Why the class must outlive its addresses
Drivers reach their backend configuration through the class. metallb-iad, after cozystack/metallb-iad#8, finds the pool to hold and pin an address through the class's
parameters.pool. A class deleted while addresses remain would leave them unservable.Behaviour to know
Releasedaddress holds the class too. Deleting a class means deleting itsIPAddressobjects first, or lettingreclaimPolicy: Deletedo it.go test ./internal/controller/andgolangci-lint runpass.