Repository navigation
T26 — Retry the hostname listener and clear its warning without a restart - #1681
norrietaylor merged 7 commits into
Conversation
A host-side proxy whose listen address is held at boot no longer stays down until a daemon restart. start_host_proxies now drives each proxy's bind - and, in a microVM, its host-loopback publish - on a detached task that retries with a doubling, capped backoff until it succeeds (NET-021), so a freed port is picked up in seconds without touching the daemon. bind_listener returns a BindFailure carrying the reason (address and OS error) and the one remedy that clears it, authored once in proxy.rs; the startup retry logs one warning per failed attempt with that report and the next retry delay, records it on the proxy_unavailable note the ListSessions RPC serves, and on recovery logs one info line and clears the note (NET-022). The mTLS proxy rides the same loop. The publish failure report gains its remedy (check the host gvproxy), and the publish attempt cap now bounds a single attempt rather than the whole startup, since the retry supplies the next attempt itself.
…s ran out the coder's uncommitted work as it stood when its 3000s ran out The coder was stopped at the end of its time with these changes not committed. The runtime committed its working tree as it stood so the work is kept: this commit is a snapshot, not a finished step, and may not build or pass its tests.
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
Reviewed against the T26 plan block on #1437. Taking it out of draft: the last commit is the runtime's budget snapshot, but its content is the finished CLI layer (list.rs, the two cli.rs tests, a rustfmt reflow of server.rs), and the head builds and passes every plan test line in CI.
Plan compliance. Exactly the four files the block names.
crates/minimald/src/server.rs:start_host_proxiesspawnsdrive_proxy_until_servingper proxy (egress, and mTLS undernetworking-proxy), retrying bind and the DM1 publish withRetryBackofffrom 1 s doubling to a 30 s cap, recording reason plus remedy per failure and clearing the note on success.crates/minimald/src/net/proxy.rs:bind_listenerreturnsResult<TcpListener, BindFailure>carryingreason,remedyandreported(); the warn moved to the caller, which knows the retry delay.crates/minimal/src/cmd/list.rs:hostname_routing_warningbuilds reason, remedy and the retry hint; used bycmd_lsand by activate.- Observability: one
warn!per failed attempt withreason,reportandnext_retry; oneinfo!withstatus="recovered"on success.
Test lines. All three present and green in the CI tests job (run 35974917428): listener_retries_with_backoff asserts the three warnings, the doubling delays and cap, the note text, and the clear plus recovered log after the port frees; listener_failure_reported_with_remedy and ls_warning_clears_on_recovery drive the compiled min against the in-process test server and assert the ls and session activate stderr. The CLI tests hold an ephemeral port and run the real retry loop through a test-support re-export rather than occupying :7654; same code path, not the production address.
Non-blocking.
- The note is now set asynchronously after
Server::runstarts accepting, so amin lsracing the very first bind attempt can miss the warning for a few milliseconds. - The DM1 publish retry has no test; the plan did not ask for one.
hostname_routing_warningispubin a bin crate;pub(crate)would do.
Approving; merge waits on the runtime review and CodeRabbit now that it is out of draft.
|
Approved: no findings at Nothing to do. Verdict APPROVE, posted as a comment: the App cannot review its own pull request; the check and the label carry it. Clobber: none: |
…hostname-listener-and-clear-its-warnin
The hostname-routing-down warning is shared by `min ls` and `min session activate`, but its recovery sentence hardcoded "run `min ls` again to check". A user mid-activate was told to switch to another command to watch the recovery, which reads as a non-sequitur. `hostname_routing_warning` now takes the command the warning rides on: each path names itself, and the sentence says to re-run the command the user is already in. Review finding on #1681 (low, api-design).
…hostname-listener-and-clear-its-warnin
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
With the proxy port occupied, Notes from the agent: Revise round 1: addressed the single review thread (hardcoded "run |
… merge Completes the merge from origin/main by resolving the proxy.rs conflict. The incoming main version returned Option<TcpListener> and logged internally, which broke server::drive_proxy_until_serving and listener_retries_with_bac 8000 koff. Restore BindFailure with reason + remedy so the retry loop owns the log line and reports the same text through ListSessions, min ls and min session activate.
…hostname-listener-and-clear-its-warnin
|
Round 2 of 10 on a /revise note, pushed as Resolved conflicts in:
Verify Outside the task's
Converted to a draft: something above needs a person before review. With the proxy port occupied, Notes from the agent: Resolved an unresolved merge state on this branch: the latest merge from origin/main took an Option-returning bind_listener that logged internally, which broke server::drive_proxy_until_serving and the listener_retries_with_backoff test. Restored the task's BindFailure reason/remedy pair in bind_listener and updated the pre-existing bind_failure_warns_dns_proxy_unavailable test to assert that shape instead of internal logging. The other files in the merge were already staged and are now part of the completing commit. |
T26 — Retry the hostname listener and clear its warning without a restart
slice: S6
kind: task
covers: NET-020, NET-021, NET-022
size: M
est: 200 code, 260 tests
executor: agent
difficulty: routine
model: standard
diagnostics: the bundle's daemon log tail carries each failed bind with its reason and next retry, and the recovery
observability: one warn log line per failed bind with the reason and the next retry delay; one info line when the listener recovers
layers:
- daemon:
- crates/minimald/src/server.rs: start_host_proxies retries bind and publish with backoff and clears the unavailable flag on success
- crates/minimald/src/net/proxy.rs: bind_listener carries one remedy text for every failure
test: cargo nextest run -p minimald listener_retries_with_backoff
- cli:
- crates/minimal/src/cmd/list.rs: warn_if_hostname_routing_down prints the reason and the remedy
- crates/minimal/tests/cli.rs: hosts the report and clear tests
test: cargo nextest run -p minimal listener_failure_reported_with_remedy
test: cargo nextest run -p minimal ls_warning_clears_on_recovery
produces: none
consumes: none
proof: With the proxy port occupied, activate and ls print the reason and the remedy; free the port and ls clears the warning without a daemon restart.
Out of time
The coder ran out of its 3000s before it finished. If it left work uncommitted, its last commit is the runtime's snapshot of that work as it stood — the commit message says so — and not a finished step: it may not build or pass its tests. So this PR is a draft.
Proof
With the proxy port occupied, activate and ls print the reason and the remedy; free the port and ls clears the warning without a daemon restart.
Commit headers reworded by the runtime to fit commitlint. The header the coder wrote is the first paragraph of each of these commits' bodies:
feat(minimal): the coder's uncommitted work as it stood when its 3000s ran outTests
cargo nextest run -p minimald listener_retries_with_backoffcargo nextest run -p minimal listener_failure_reported_with_remedycargo nextest run -p minimal ls_warning_clears_on_recoveryVerify
min task run agent-verifypassed ([tasks.verify]).Closes #1620