fix(deploy): resolve the reserved address by the VM using it - #83
Conversation
The preflight looked the address up by the VM's name, which assumes every address is named after the VM it serves. That does not hold across the fleet, so a full roll aborted on the first VM where the names differ. Every caller wants the address value, so identify it by its user instead, keeping a name lookup for an address reserved before its VM exists. Latent until now: --only narrows the preflight to the selected VM, so earlier single-node rolls never audited the one that breaks the assumption.
There was a problem hiding this comment.
Pull request overview
This PR updates the GCP deployment/audit script (deploy/gcp/seeders.sh) to resolve a VM’s reserved static IP by looking up the address resource via the VM that is using it, instead of assuming the address resource name matches the VM name. This prevents full-fleet roll preflights from aborting when a VM and its reserved address have different names.
Changes:
- Update
addr()to first find the reserved IP viagcloud compute addresses listfiltered byusers:(<vm>)in the target region. - Keep the existing
addresses describe <name>lookup as a fallback for the--createpath (reserved address created before the VM exists).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ip="$(get_cmd gcloud compute addresses list --project="${PROJECT}" \ | ||
| --filter="region:(${region}) AND users:(${name})" --format='value(address)' | head -n1)" |
A server-side --filter over a repeated field is rejected as an invalid list filter expression by some API versions, which is what broke the firewall preflight. List the addresses once and match in the script instead, and treat more than one address per VM as a misconfiguration rather than picking one.
|
Both points addressed in the latest commit, and the ambiguity one led to dropping the server-side filter entirely. On On the filter itself: the first version matched with Verified against the live fleet and with fixtures:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
deploy/gcp/seeders.sh:116
- The PR description says the new lookup uses a server-side
gcloud ... addresses list --filter="region:(...) AND users:(...)", but this implementation explicitly avoids--filterand does client-side matching instead (see the note about API versions rejecting list filters overusers). Please update the PR description (or implement the described server-side filter with a safe fallback) so the documented behavior matches what actually ships.
# A server-side --filter over the repeated `users` field is rejected as an
# invalid list filter expression by some API versions, so match client-side.
while IFS=';' read -r a_name a_ip a_region a_users; do
|
Good catch — the description was stale and contradicted the code. It described the first version of this branch, which did use Description updated to match what ships. |
The first full-fleet roll aborted in preflight — fail-closed, no VM touched:
addr()resolved the reserved address withaddresses describe <VM name>, which assumes every address is named after the VM it serves. That holds for most of the fleet but not all of it, so the roll aborted on the first VM where the names differ. The address itself is correct and in use — only the lookup assumption was wrong.All four callers want the address value, never its name, so the lookup now identifies the address by the VM using it. It lists the addresses once and matches in the script: a server-side
--filteroveruserswould be a filter on a repeated field, the same shape that made the firewall preflight fail withInvalid list filter expressionon the runner, so the script no longer passes--filterto gcloud anywhere. A name match remains as a fallback for an address reserved before its VM exists, which is the--createpath, and a second address for the same VM fails closed rather than being resolved by picking one.Why it stayed hidden:
--onlynarrows the preflight to the selected VM, so every earlier single-node roll audited a VM whose names happen to match.Verified against the live fleet and with fixtures:
ns<N>.zfnd.orgrecord points at--createpath)--auditclears every infra-scope check on all six, and a full--dry-run --rollplans ns2→ns6 with ns1 last