Repository navigation
fcvm-metal-arm: switch to the second public address and release the first - #675
Conversation
…irst - firecracker-dev.tf: aws_eip.firecracker_dev_next moves from the ENI's secondary private address to the primary one (allow_reassociation), so inbound and outbound traffic both use it; aws_eip.firecracker_dev and its association are removed, which releases that address. The _next name is kept: renaming it in the same apply would allocate a new address. firecracker_dev_public_ip and the SSH command output name the new address; the _next output is gone. - dev-hop-key.tf, ohio.tf, runner-vpc.tf, and through local.firecracker_dev_public_ips fcvm-dns-test.tf and gpu-box.tf: the box's address is the new one only, so the released address is admitted nowhere. - scripts/test-fcvm-arm-address.py: the old address is neither defined nor referenced, and the new one is on the primary private address; it fails on main, where aws_eip.firecracker_dev exists. The secondary private address and its State Manager association stay for now (nothing public maps to them); a follow-up removes them with the host's netplan entry. Tested: terraform validate (1.10.3), terraform fmt -check; the address, DNS test, runner boundary, Ohio, gpu box, secondary address and credential boundary tests
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 45 minutes. View limit details
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. Comment |
…ssh alias, not a literal
claw-master-login.sh, claw-master-mac-setup.sh, mac-ssh-connector-setup.sh and
windows-pc-setup.sh defaulted the host to a literal address via ${FCVM_HOST:-...}.
Once that address is released, a run without FCVM_HOST would connect to whoever
AWS hands it to next.
- Each of those four gets a small fcvm_host function: FCVM_HOST, else the hostname
`ssh -G fcvm-arm` reports (ignored when it equals the alias, which is what ssh
prints for a name with no entry), else exit 2 with a message naming both. It runs
after argument validation; claw-master-login.sh --server keeps its private
address and does not need it.
- windows-pc-bootstrap.sh runs on the box itself, where no alias names the box, so
`print` requires FCVM_HOST and exits 2 with a message when it is unset.
- test-mac-ssh-client.py: fixture host is now 203.0.113.10 (RFC 5737), and the
connector script tests pass FCVM_HOST.
- AGENTS.md: the ARM entry points at `terraform output firecracker_dev_public_ip`
instead of the address and its allocation id; the manual-swap comment no longer
names the ARM allocation id.
- test-fcvm-arm-address.py: HostComesFromTheOperator runs all five scripts with stub
ssh/aws and no FCVM_HOST: each exits 2 naming FCVM_HOST (and fcvm-arm, except the
bootstrap), reaches neither aws nor ssh; the alias hostname is used, FCVM_HOST
wins over it; no host line holds a public IPv4 literal. Run against the previous
scripts it failed 3 of 6.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Held for the owner: this releases fcvm-metal-arm's Elastic IP (irreversible) and drops every connection through it; the description gates it on the owner moving their SSH configs first, and that confirmation has to come from the owner in a jumpbox session, not from GitHub. Also lambda-tests fails: scripts/test-fcvm-arm-address.py needs |
|
Pushed the CI buffering fix, wired the address regression into the existing credential-free test job, and corrected the operator docs. Focused tests passed on ARM; the full PR patch atop current main also passes the 90-test coordinator/client suite. External SSH/allowlist migration is being verified before the address cutover. |
|
@codex review |
|
@codex security review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bef0b0cd6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@coderabbitai review |
|
@greptileai review |
|
|
@codex review |
|
@codex security review |
|
@coderabbitai review |
|
@greptileai review |
|
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
The switch-over from #666. Draft until the owner has moved their own SSH configs to the new address (the
firecracker_dev_public_ip_nextoutput today); the owner marks it ready, or tells the fcvm session to.Changes
aws_eip.firecracker_dev_nextmoves from the ENI's secondary private address to the primary one (allow_reassociation = true), so inbound and outbound traffic both use it.aws_eip.firecracker_devand its association are removed: that address is released.dev-hop-key.tf,ohio.tf,runner-vpc.tf, and vialocal.firecracker_dev_public_ipsfcvm-dns-test.tfandgpu-box.tf. A released address is admitted nowhere, so whoever AWS hands it to next gets no access.firecracker_dev_public_ipandfirecracker_dev_ssh_commandname the new address;firecracker_dev_public_ip_nextis gone._nextname: renaming it in the apply that releases the old address would allocate a new address. A latermovedblock can rename it.During the apply
Connections through the old address drop: SSH sessions to it, and this box's outbound connections, which leave from the primary private address and switch public address in the swap. Remote Control and agent sessions reconnect on their own. The fleet's sessions on this box do not depend on the old address.
Outside Terraform (check before marking ready)
Tests
scripts/test-fcvm-arm-address.py: the old address is neither defined nor referenced, and the new one is on the primary private address (fails on main). Alsoterraform validate(1.10.3),terraform fmt -check, and the DNS test, runner boundary, Ohio, gpu box, secondary address and credential boundary tests.Scripts no longer default to the old address (ec42b95)
claw-master-login.sh,claw-master-mac-setup.sh,mac-ssh-connector-setup.shandwindows-pc-setup.shdefaulted the host to a literal via${FCVM_HOST:-...}. After the release a run withoutFCVM_HOSTwould connect to a stranger.FCVM_HOST, else the hostnamessh -G fcvm-armreports (ignored when it equals the alias), else exits 2 naming both.claw-master-login.sh --serverkeeps its private address.windows-pc-bootstrap.sh printruns on the box itself, so it requiresFCVM_HOSTand exits 2 when unset.test-mac-ssh-client.pyfixtures use 203.0.113.10 (RFC 5737).AGENTS.mdpoints atterraform output firecracker_dev_public_ipinstead of the address and its allocation id.scripts/test-fcvm-arm-address.py(HostComesFromTheOperator): runs all five scripts with stub ssh/aws and noFCVM_HOST; each must exit 2 with a message namingFCVM_HOST(andfcvm-arm, except the bootstrap) and reach neither aws nor ssh. Also checks the alias hostname is used,FCVM_HOSTwins over it, and no host line holds a public IPv4 literal. It failed 3 of 6 against the previous scripts.Tests run:
test-fcvm-arm-address,test-mac-ssh-client,test-windows-pc-setup,test-agents-mdpass.test-claw-master-serverhas 2 failures in the nextjs-dev/metal-box zshenv routing tests, which fail the same way on the parent commit.