Repository navigation
Opt placeholders out of the core's auto-claim - #5
Conversation
address-controller moved from github.com/lllamnyp to github.com/cozystack. metallb-iad still required the old module, so it could not pick up any API change made since the move. Assisted-By: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
address-controller can now auto-provision an IPAddressClaim for any LoadBalancer Service that names none. Placeholders are such Services, so without an opt-out the core would claim an address for every reservation the driver makes. Placeholders now carry local.sdn.cozystack.io/auto-claim: "false", using the constant the core exports, and the module is bumped to the address-controller commit that introduces it. 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
There was a problem hiding this comment.
Review: opt placeholders out of auto-claim
Correct and necessary. NewPlaceholder stamps local.sdn.cozystack.io/auto-claim: "false" on every reservation placeholder (internal/driver/driver.go:168-170), and TestNewPlaceholderForAllocation asserts it. The core's wantsAutoClaim honours that annotation, so this keeps the core from minting a claim against the driver's own holders.
One thing to flag for release sequencing rather than for the code here: until the core implements the §6 pool-or-pin exclusion, this annotation is the only protection against the core claiming the driver's placeholders. So this PR is a prerequisite for enabling any auto-claim class, as decision 0002 notes. It should land, and land before auto-claim is switched on anywhere.
LGTM.
cozystack/address-controller#7 makes the core controller auto-provision an
IPAddressClaimfor any LoadBalancer Service that names no claim. This driver's reservation placeholders are exactly such Services, so without an opt-out the core would claim an address for every reservation the driver makes. This PR opts them out.Two commits:
github.com/lllamnyp/address-controllertogithub.com/cozystack/address-controller. metallb-iad still required the old module, so it could not pick up any API change made since. The change is imports plusgo get/go mod tidy; no behaviour changes.NewPlaceholderstampslocal.sdn.cozystack.io/auto-claim: "false"using the core's exportedServiceAutoClaimAnnotation, so the two sides share one constant. A driver test covers it, and design §4 notes it alongside the other placeholder properties.Merge order. The module is pinned to the head commit of cozystack/address-controller#7. That repo merges with merge commits, so the pin stays reachable after #7 lands. Re-pinning to the merge commit afterwards is optional cleanup.
Rollout. Deploy this before marking any
IPAddressClasswithis-auto-claim-class. Existing hold placeholders already have an address, and the core skips Services that have one. Only a placeholder created by an older driver and still waiting for MetalLB at the moment a class gets marked could be claimed by mistake.go build ./...,go test ./internal/...andgolangci-lint runpass.