[Timofei Larkin (lllamnyp)]

Adds docs/operating.md, linked from the README, covering operator procedures that a review of cozystack/cozystack#3382 found undocumented.

• Adopting an address a Service already holds.

  1. Create the claim with spec.addressName naming the address. The MetalLB driver then does not provision a different one.
  2. Create the IPAddress pre-bound to the claim. It notes the class and family check that Auto-claim addresses for LoadBalancer Services that name no claim #7 applies to pre-bindings.
  3. Name the claim on the Service. The driver's pin equals the existing one.

  It explains why creating the IPAddress first fails: an address whose claim is missing is reclaimed as orphaned. It also says the hand-written pin is protected only where metallb-iad's optional admission policy is enabled.

• Removing the controllers. The teardown order: claim annotations, claims, leftover addresses, classes, then the driver and the core. It also gives the commands to clear finalizers when the controllers are already gone, with the API group spelled out, because ipaddresses alone resolves to networking.k8s.io.

• Deleting a class. A class that has served claims keeps Released and Available addresses, which hold it under the class-deletion guard (Hold a deleted IPAddressClass until its addresses are gone #10); releaseDelay (Return Released addresses to the pool after a class's releaseDelay #11) returns them to the pool rather than deleting them. So its addresses have to be deleted first, and the class reports ClassInUse until then.

• Quarantining an address. The held annotation pauses the return to the pool, not the delay. Lifting it from an address Released for longer than releaseDelay returns the address at once.

• Upgrading the CRDs. The API is v1alpha1 with 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 and the core-set Bound condition on IPAddress.

Docs only.

[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: 837e7c7a-2e25-4ad4-8419-782dd7537393

• 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: 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.

[Timofei Larkin (lllamnyp)]

Thanks for the review. Addressed in 6c809fc; per point:

[MEDIUM] Pin protection. Qualified. The guide now says removing the hand-written pin takes an allowlisted principal only where metallb-iad's optional admission policy is enabled. Without that policy, removing it is ungated, and only the driver's conflict detection reports the address going to another Service.

[MINOR] Adoption race. Fixed, but not by creating the IPAddress first. An IPAddress whose claimRef names a claim that does not exist yet is treated as orphaned: the address controller's safety-net reclaim runs, so it goes Released under Retain and is deleted under Delete. The guide now does this instead:

1. Create the claim with spec.addressName naming the address. metallb-iad does not provision for a claim that names an address, and the core finds no such address yet, so the claim waits Pending.
2. Create the pre-bound IPAddress. The core accepts it through the pre-binding path.
3. Name the claim on the Service.

The guide also notes that setting spec.addressName takes an allowlisted principal where #13's policy is enabled, and why the other order fails.

Note: teardown. Added under Deleting a class. A class that has served claims keeps Released and Available addresses that hold it under #10's finalizer. releaseDelay returns them to the pool rather than deleting them, so they have to be deleted first. The class reports ClassInUse naming them until then.

Also from the #11 review: a new Quarantining an address section says held pauses the return to the pool, not the delay. Lifting it from an address Released for longer than releaseDelay returns the address on the next pass.

[IvanHunters]

Lifting my request-changes; this is now an approve. All three points are addressed, and I checked the new content against the code rather than only re-reading it.

• The MEDIUM pin claim is qualified correctly: operating.md now says removing a hand-written pin takes an allowlisted principal only where metallb-iad's optional admission policy is enabled, and that without it the removal is ungated and only conflict detection reports it.
• The adoption race is closed, and more robustly than I suggested: the claim names the address in spec.addressName so it waits Pending for exactly that address instead of grabbing another, and the doc warns that creating the IPAddress first gets it reclaimed. I confirmed both against the code: bindAvailableAddress with addressName set and the address absent binds nothing, and ipaddress_controller reclaims a pre-bound address whose claim does not exist yet (boundClaim is nil, so case claim == nil reclaims), while a pre-binding whose claim exists waits.
• Teardown and the held-address lifecycle are documented, including that under Retain a class keeps Released/Available addresses that hold its deletion and releaseDelay returns them to Available rather than deleting them. The finalizer-clear commands are group-qualified (ipaddressclaims.local.sdn.cozystack.io, etc.), with the networking.k8s.io ambiguity called out, and the teardown order keeps the controllers running while objects are deleted.

LGTM.