Retry transient fresh LXC starts - #39
Conversation
📝 WalkthroughWalkthroughThe LXC runtime now retries ChangesLXC startup resilience
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/1helm-lxc-runtime`:
- Around line 81-101: The start retry loop in the script overwrites the captured
lxc-start diagnostics on each invocation, allowing a later empty result to erase
useful failure details. Update the variables used by the retry loop around
lxc-start so each attempt’s output is preserved, retaining the last non-empty
message or accumulating all non-empty attempt details before the final
diagnostic print.
- Around line 90-94: The retry path after a non-zero lxc-start exit in the state
polling loop must use the same longer polling window as the success path. Update
the loop around state polling to wait up to 20 seconds before executing
continue, preserving the existing RUNNING check and retry behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a4e07b9-bd7c-4d59-89fa-a869695cc0e4
📒 Files selected for processing (3)
CHANGELOG.mdscripts/1helm-lxc-runtimetest/site.mjs
| local name="$1" start_attempt state_attempt detail="" | ||
| [[ "$(state "$name")" == "RUNNING" ]] && return | ||
| # A freshly unpacked container can hit LXC's daemonized-start socket race: | ||
| # lxc-start exits after wait_on_daemonized_start loses the state message even | ||
| # though the container definition is intact and immediately startable. Check | ||
| # the authoritative state after every invocation and retry that same exact | ||
| # validated machine instead of failing channel creation on the transient. | ||
| for start_attempt in {1..3}; do | ||
| if ! detail="$(lxc-start -P "$LXC_PATH" -n "$name" -d 2>&1)"; then | ||
| for state_attempt in {1..50}; do | ||
| [[ "$(state "$name")" == "RUNNING" ]] && return | ||
| sleep 0.1 | ||
| done | ||
| continue | ||
| fi | ||
| for state_attempt in {1..200}; do | ||
| [[ "$(state "$name")" == "RUNNING" ]] && return | ||
| sleep 0.1 | ||
| done | ||
| done | ||
| die "container did not start" | ||
| [[ -z "$detail" ]] || printf '%s\n' "$detail" >&2 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Preserve startup diagnostics across attempts.
detail is overwritten on every lxc-start invocation. A later empty-output attempt can erase a useful error from an earlier failed attempt, causing the final failure to omit the captured startup details. Store per-attempt output and retain the last non-empty message or append all attempt details.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/1helm-lxc-runtime` around lines 81 - 101, The start retry loop in the
script overwrites the captured lxc-start diagnostics on each invocation,
allowing a later empty result to erase useful failure details. Update the
variables used by the retry loop around lxc-start so each attempt’s output is
preserved, retaining the last non-empty message or accumulating all non-empty
attempt details before the final diagnostic print.
| for state_attempt in {1..50}; do | ||
| [[ "$(state "$name")" == "RUNNING" ]] && return | ||
| sleep 0.1 | ||
| done | ||
| continue |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant script and nearby logic.
git ls-files scripts/1helm-lxc-runtime
wc -l scripts/1helm-lxc-runtime
cat -n scripts/1helm-lxc-runtime | sed -n '1,170p'Repository: gitcommit90/1Helm
Length of output: 8903
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show only the relevant section if the file exists.
sed -n '1,170p' scripts/1helm-lxc-runtime | nl -ba | sed -n '70,120p'Repository: gitcommit90/1Helm
Length of output: 195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,170p' scripts/1helm-lxc-runtime | nl -ba | sed -n '80,115p'Repository: gitcommit90/1Helm
Length of output: 195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if [[ -f scripts/1helm-lxc-runtime ]]; then
echo "FOUND"
nl -ba scripts/1helm-lxc-runtime | sed -n '75,110p'
else
echo "MISSING"
find . -maxdepth 3 -type f | sed -n '1,50p'
fiRepository: gitcommit90/1Helm
Length of output: 201
🌐 Web query:
lxc-start daemonized exit code socket response still starting RUNNING state polling behavior
💡 Result:
When a container is started with lxc-start in daemonized mode, LXC employs a specific synchronization mechanism to ensure the caller can reliably detect when the container has successfully reached the RUNNING state [1][2]. Mechanism Details: 1. Anonymous Socket Pair: To address race conditions where a container's init process might exit before the primary command socket is fully initialized, LXC creates a short-lived anonymous AF_UNIX socket pair (pipe-like behavior) during daemonized initialization [1][2]. 2. Synchronization: The handler uses this socket pair to pass state updates internally. The parent process (the caller) can monitor these sockets to detect the transition to RUNNING [1][2]. 3. State Polling: Tools and callers often interface with LXC's monitor system or command sockets to poll or subscribe to lifecycle state changes (STARTING, RUNNING, STOPPING, etc.) [3][4]. Historically, this relied on lxc-monitord, but modern LXC implementations use native command socket notifications where clients register to receive state change events directly from the container's handler [2][4][5]. 4. Exit Code Handling: If lxc-start fails during the startup phase, it returns specific exit codes to the caller [6]. Generally: - 0-126: Represents the exit code of the application running inside the container [6]. - 128+n: Indicates the application was terminated by signal n [6]. - 255: Indicates an internal LXC error (e.g., failed setup, monitor communication failure) [6]. If a container fails to reach the RUNNING state within a specified timeout (often 5 seconds), the startup process will report a failure to the user, preventing it from incorrectly reporting success for a container that crashed immediately upon launch [2][7]. High-level management tools, such as those used in Proxmox, often implement their own timeout-based polling logic against the monitor socket to ensure that hooks and post-start actions only execute after the container is confirmed to be running [3].
Citations:
- 1: lxc/lxc@b42041e
- 2: start: use separate socket on daemonized start lxc/lxc#1659
- 3: https://lore.proxmox.com/pve-devel/20200908115843.345-2-f.ebner@proxmox.com/
- 4: https://deepwiki.com/lxc/lxc/2.3-command-and-state-system
- 5: start: kill lxc-monitord lxc/lxc#1618
- 6: https://gitea.lierfang.com/proxmox-mirrors/mirror_lxc/commit/e043236e2a35948aeeb3cf85feab7c2f1e2b1fd1
- 7: https://gitea.lierfang.com/proxmox-mirrors/mirror_lxc/commit/7a44c8b447c4cac2b71bf842a61440bae9caf918
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find start() callers and any nearby comments about timing/races.
rg -n "start\(" scripts/1helm-lxc-runtime
rg -n "socket race|RUNNING|50}|200}" scripts/1helm-lxc-runtimeRepository: gitcommit90/1Helm
Length of output: 1048
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path('scripts/1helm-lxc-runtime')
for i, line in enumerate(path.read_text().splitlines(), 1):
if 'start(' in line or 'RUNNING' in line or 'lxc-start' in line:
print(f"{i}:{line}")
PYRepository: gitcommit90/1Helm
Length of output: 1210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path('scripts/1helm-lxc-runtime')
lines = path.read_text().splitlines()
for i, line in enumerate(lines, 1):
if 'start(' in line or 'RUNNING' in line or 'lxc-start' in line:
print(f"{i}:{line}")
PYRepository: gitcommit90/1Helm
Length of output: 1210
scripts/1helm-lxc-runtime:90-94 — Keep the longer polling window before retrying. The 5s wait after a non-zero lxc-start exit is shorter than the 20s success-path poll, so a daemonized start that lost its state message can still be in flight when the next attempt fires and exhaust all 3 tries early. Use the longer window before retrying.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/1helm-lxc-runtime` around lines 90 - 94, The retry path after a
non-zero lxc-start exit in the state polling loop must use the same longer
polling window as the success path. Update the loop around state polling to wait
up to 20 seconds before executing continue, preserving the existing RUNNING
check and retry behavior.
1 participant
Outcome
Fixes fresh Linux channel creation when LXC transiently loses the first daemonized-start state-socket response. The installed helper now checks authoritative state and retries the same exact validated container up to three times.
Evidence
wait_on_daemonized_start/No such file or directorywhile a later automatic retry created the same machine, proving a transient first-start path.bash -n scripts/1helm-lxc-runtimenpm run test:sitenpm run typechecknpm run buildnode --test test/channel-computers.mjsnpm testis running.Summary by CodeRabbit