Skip to content

fcvm fleet: an aggregated-streaming switch, off (ejc3/fcvm#1232) - #671

Merged
ejc3 merged 1 commit into
mainfrom
fcvm-fleet-aggregated-switch
Oct 25, 2026
Merged

ejc3 merged 1 commit into
mainfrom
fcvm-fleet-aggregated-switch

Conversation

@ejc3

@ejc3 ejc3 commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Adds local.fcvm_fleet_aggregated (false) for ejc3/fcvm#1232: the GPU hosts' aggregators and the EC2 half's aggregator beside the coordinator serve browsers with TLS under per-launch names, so the stage page can run its aggregated mode. Off, every string and resource staging uses is unchanged.

fcvm side: ejc3/fcvm#1266 → #1267 → #1268 (stacked). Turn the switch on only with a host AMI and a coordinator image built from that stack: an older image refuses --aggregate-tls-port / --ec2-aggregate-tls and does not start.

With the switch on

  • Hosts: --aggregate-port 20300 (private: layout and stats; not admitted) and --aggregate-tls-port 20208, the port after the slots' (base + slots), which the fleet security group's IPv4 range then includes. Over IPv6 a host aggregator has its own address, <host prefix>::1, on 443, which the group already admits.
  • Coordinator: --ec2-aggregate-listen 127.0.0.1:20300 --ec2-aggregate-tls --ec2-aggregate-tls-dir /run/fcvm-fleet/tls. The task gets a volume at /run/fcvm-fleet (its root file system is read-only), net.ipv4.ip_unprivileged_port_start=443 (the image runs as 65534 and the aggregator binds 443), 443 in its security group over IPv4 and IPv6 (direct, no tunnel, no Cloudflare in the media path), and the ECS dualStackIPv6 account setting so the task gets an IPv6 address. The coordinator reads its own interface's public IPv4 address with ec2:DescribeNetworkInterfaces, which its existing ec2:Describe* statement covers.

Off

  • Agent arguments: join(" ", concat([...], [] )), the same string.
  • Coordinator container definition: merge({...}, [for ... in []]...) gives the same JSON. Checked with a scratch Terraform for both values of the switch: off, the JSON and the argument string are identical to the literal they replace.
  • Coordinator security group: a dynamic "ingress" with no element; description unchanged (a description change would replace the group).
  • Task volume and account setting: for_each / count empty.

Tests

python3 scripts/test-fcvm-fleet.py: 36 tests OK. Changed: the coordinator group admits nothing but the gated 443 rule; the slot range ends at the aggregator port only with the switch on. New AggregatedTests: the switch is off, every addition is gated by it, the aggregator port follows the slots and the private port is outside the admitted range. scripts/test-fcvm-fleet-account.py: OK. terraform validate was not run: this box's Terraform (1.16.6) is outside the repository's version constraint.

Not verified until applied with the switch on: that a Fargate task volume takes the image's ownership of /run/fcvm-fleet (fcvm#1268 creates it owned by 65534), and that ip_unprivileged_port_start is accepted as a Fargate systemControls entry.

local.fcvm_fleet_aggregated (false) gates everything the aggregated
streams need, so with it off the agent arguments, the coordinator
command, its task definition and both security groups are what they
were:

- Hosts: --aggregate-port 20300 (private: layout, stats; not admitted)
  and --aggregate-tls-port, the port after the slots' (20208), which the
  fleet security group's IPv4 range then includes. Over IPv6 a host
  aggregator has its own address, <prefix>::1, on 443, already admitted.
- Coordinator: --ec2-aggregate-listen on loopback and
  --ec2-aggregate-tls with its certificate in /run/fcvm-fleet/tls; the
  task gets a volume there (the root file system is read-only),
  net.ipv4.ip_unprivileged_port_start=443 (the image runs as 65534),
  443 in its security group over IPv4 and IPv6, and the account's
  dualStackIPv6 ECS setting so the task has an IPv6 address. The
  coordinator already holds ec2:Describe*, which reading its own
  interface's public IPv4 address needs.

scripts/test-fcvm-fleet.py: the coordinator security group admits
nothing but the gated 443 rule, the slot range ends at the aggregator
port only with the switch on, and AggregatedTests pins the switch off
and every addition gated by it.
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You'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 57 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

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: 2e3792a4-7c3f-4d3c-8703-d4eb9c55d25a
Commits

Reviewing files that changed from the base of the PR and between bae6c2e and ad467aa.

Files selected for processing: 2
  • fcvm-fleet.tf
  • scripts/test-fcvm-fleet.py
  • 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

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-25T13:18:32.570068Z ad467aa Manual request
🔒 Security Review ✅ Completed 2026-10-25T13:20:05.895668Z ad467aa Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

ejc3 commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

@codex review
@coderabbitai review
@greptileai review

ejc3 commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

@codex security review

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: ad467aadb2

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: ad467aadb2

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ejc3
ejc3 merged commit 8a5c613 into main Oct 25, 2026
3 checks passed
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.

1 participant