Skip to content

Document adoption, teardown and CRD upgrades for operators - #14

Open
Timofei Larkin (lllamnyp) wants to merge 3 commits into
mainfrom
docs/operating
Open

Timofei Larkin (lllamnyp) wants to merge 3 commits into
mainfrom
docs/operating

Conversation

@lllamnyp

@lllamnyp Timofei Larkin (lllamnyp) commented Nov 6, 2026 •

Copy link
Copy Markdown
Member

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

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

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

@IvanHunters IvanHunters left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@lllamnyp
Timofei Larkin (lllamnyp) force-pushed the docs/operating branch 2 times, most recently from 73b8bce to 4a7c96e Compare November 10, 2026 09:28
Base automatically changed from feat/addressname-admission-policy to main November 11, 2026 09:26
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>
Adoption now creates the claim with spec.addressName, so the driver does
not provision a different address before the pre-bound IPAddress exists;
creating the IPAddress first would have it reclaimed as orphaned. The pin
protection is stated as depending on metallb-iad's optional admission
policy. New sections cover why a used class waits on its Released and
Available addresses, and that the held annotation pauses the return to
the pool but not the delay.

Assisted-By: LLM
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
@lllamnyp

Copy link
Copy Markdown
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants