Repository navigation
Document adoption, teardown and CRD upgrades for operators - #14
Timofei Larkin (lllamnyp) wants to merge 2 commits into
Conversation
docs/operating.md covers bringing an address a Service already holds under the ledger, the order in which to remove claims, addresses, classes and controllers so finalizers do not hang, and the schema changes each release makes to the v1alpha1 CRDs. Assisted-By: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
Assisted-By: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
Review: document adoption, teardown and CRD upgrades
Most of the document checks out against the code: the API group and kinds, the IPAddress example fields and the fromClass source, the pre-binding acceptance rule and PreBindingRefused event, the finalizer names, the teardown order, and the v1alpha1-without-conversion CRD-upgrade notes are all accurate.
[MEDIUM] The pin-protection claim describes a policy that is not shipped here
docs/operating.md:34-35: "The hand-written pin stays on the Service. Under the driver's admission policy, removing it later takes an allowlisted principal." That policy is real but it is the metallb-iad admission policy from cozystack/metallb-iad#6, which is not merged, is opt-in, and is off by default. As written the sentence reads as a standing guarantee; on a cluster without that optional policy enabled, removing a hand-written pin is ungated and the only backstop is the driver's after-the-fact Conflict detection. Please qualify it: the protection applies only where the optional metallb-iad admission policy is enabled.
[MINOR] The adoption step order invites a race
The guide creates the IPAddressClaim first (step 1) and the pre-bound IPAddress second (step 2). If the claim reconciles before the admin creates the pre-bound address, the driver allocates a different address for the pending claim, and the admin's hand-written IPAddress is then refused with PreBindingRefused while the ledger holds an address other than the one pinned on the Service. Creating the pre-bound IPAddress first (or together) avoids it.
Note
Worth adding a teardown caveat: under reclaim Retain, a class accumulates Released/Available addresses that the class-deletion finalizer (#10) holds on, and releaseDelay (#11) returns addresses to Available rather than deleting them, so deleting a used class requires deleting its addresses first. Documenting this keeps a held class deletion from looking stuck.
Request changes for the pin-protection statement; the rest are notes.
Adds
docs/operating.md, linked from the README, covering operator procedures that a review of cozystack/cozystack#3382 found undocumented. Stacked on #13.IPAddresspre-bound to it, then name the claim on the Service; the driver's pin equals the existing one. It notes the class and family check that Auto-claim addresses for LoadBalancer Services that name no claim #7 applies to pre-bindings, and that removing the hand-written pin later needs an allowlisted principal under the driver's admission policy.ipaddressesalone resolves tonetworking.k8s.io.v1alpha1with no conversion, so a removed or renamed field invalidates stored objects. A table records each release's schema changes. Unreleased so far:IPAddressClass.spec.releaseDelay(Return Released addresses to the pool after a class's releaseDelay #11) and the core-setBoundcondition onIPAddress.Docs only.