-
Notifications
You must be signed in to change notification settings - Fork 32
Fix dev container postStartCommand
#336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -39,6 +39,6 @@ | |||||||||||||||||||||
| "SSH_AUTH_SOCK": "/tmp/ssh-agent.sock" | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| "remoteUser": "vscode", | ||||||||||||||||||||||
| "postStartCommand": "bash -lc 'set -e; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; if ! getent group render >/dev/null; then sudo groupadd -r render || true; fi; sudo usermod -aG video,render vscode || true'", | ||||||||||||||||||||||
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID host-render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; sudo usermod -aG video vscode || true; for rcfile in ~/.bashrc ~/.zshrc; do if [ -f \"$rcfile\" ] && ! grep -q \"exec sg host-render\" \"$rcfile\" 2>/dev/null; then echo \"if getent group host-render >/dev/null 2>&1 && ! groups | grep -q host-render; then exec sg host-render \\\"\\$SHELL\\\"; fi\" >> \"$rcfile\"; fi; done'", | ||||||||||||||||||||||
|
||||||||||||||||||||||
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID host-render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; sudo usermod -aG video vscode || true; for rcfile in ~/.bashrc ~/.zshrc; do if [ -f \"$rcfile\" ] && ! grep -q \"exec sg host-render\" \"$rcfile\" 2>/dev/null; then echo \"if getent group host-render >/dev/null 2>&1 && ! groups | grep -q host-render; then exec sg host-render \\\"\\$SHELL\\\"; fi\" >> \"$rcfile\"; fi; done'", | |
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID host-render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; sudo usermod -aG video vscode || true; for rcfile in ~/.bashrc ~/.zshrc; do if [ -f \"$rcfile\" ] && ! grep -q \"iris-devcontainer-host-render\" \"$rcfile\" 2>/dev/null; then { echo \"\"; echo \"# iris-devcontainer-host-render\"; echo \"if case \\$- in *i*) true;; *) false;; esac && getent group host-render >/dev/null 2>&1 && ! id -nG 2>/dev/null | grep -q '\\\\bhost-render\\\\b'; then exec sg host-render \\\"\\$SHELL\\\"; fi\"; } >> \"$rcfile\"; fi; done'", |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description is empty with no explanation of:
- Why the postStartCommand needed to be changed
- What specific issue the old command had
- What the new command does differently
- How this was tested
This makes it difficult for reviewers to understand the motivation and verify correctness. Please add details explaining what problem this fixes and how the new approach solves it.
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The postStartCommand is extremely complex and difficult to maintain as a single-line JSON string with multiple levels of escaping. This 400+ character bash one-liner contains:
- Multiple conditional branches
- GID detection logic
- Group creation
- User modification
- File manipulation with regex checking
- Complex nested quoting and escaping
This violates basic maintainability principles and makes it nearly impossible to:
- Debug issues when they occur
- Test the logic in isolation
- Review changes (as evidenced by this PR being hard to review)
- Understand what the command does without careful parsing
Consider extracting this logic into a separate shell script file (similar to ensure-ssh-agent.sh) that can be version-controlled, tested, and maintained properly. Then reference it from postStartCommand like: "postStartCommand": "bash -lc '${containerWorkspaceFolder}/.devcontainer/setup-gpu-access.sh'"
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID host-render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; sudo usermod -aG video vscode || true; for rcfile in ~/.bashrc ~/.zshrc; do if [ -f \"$rcfile\" ] && ! grep -q \"exec sg host-render\" \"$rcfile\" 2>/dev/null; then echo \"if getent group host-render >/dev/null 2>&1 && ! groups | grep -q host-render; then exec sg host-render \\\"\\$SHELL\\\"; fi\" >> \"$rcfile\"; fi; done'", | |
| "postStartCommand": "bash -lc '${containerWorkspaceFolder}/.devcontainer/setup-gpu-access.sh'", |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old command added the user to both video and render groups, but the new command only adds the user to video and host-render (when HOST_RENDER_GID is found). The render group creation and assignment was removed.
If the container or other scripts expect the user to be in a render group (not host-render), this could break existing functionality. The new approach uses host-render with the host's GID instead of a standard render group, which is a significant behavioral change.
Verify that no other parts of the devcontainer or application assume the user is in a render group, or document why this change is necessary.
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID host-render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; sudo usermod -aG video vscode || true; for rcfile in ~/.bashrc ~/.zshrc; do if [ -f \"$rcfile\" ] && ! grep -q \"exec sg host-render\" \"$rcfile\" 2>/dev/null; then echo \"if getent group host-render >/dev/null 2>&1 && ! groups | grep -q host-render; then exec sg host-render \\\"\\$SHELL\\\"; fi\" >> \"$rcfile\"; fi; done'", | |
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; sudo usermod -aG video vscode || true; for rcfile in ~/.bashrc ~/.zshrc; do if [ -f \"$rcfile\" ] && ! grep -q \"exec sg render\" \"$rcfile\" 2>/dev/null; then echo \"if getent group render >/dev/null 2>&1 && ! groups | grep -q render; then exec sg render \\\"\\$SHELL\\\"; fi\" >> \"$rcfile\"; fi; done'", |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The runArgs on line 17 already includes --group-add=video, which should add the container user to the video group at container startup. The postStartCommand then also creates a video group (if it doesn't exist) and adds the vscode user to it.
This redundancy could cause confusion. If the --group-add=video already handles video group membership, the video group creation in postStartCommand may be unnecessary. Conversely, if the postStartCommand approach is preferred, the --group-add=video in runArgs might be redundant.
Consider clarifying which approach is intended and removing the redundancy, or document why both are necessary.
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID host-render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; sudo usermod -aG video vscode || true; for rcfile in ~/.bashrc ~/.zshrc; do if [ -f \"$rcfile\" ] && ! grep -q \"exec sg host-render\" \"$rcfile\" 2>/dev/null; then echo \"if getent group host-render >/dev/null 2>&1 && ! groups | grep -q host-render; then exec sg host-render \\\"\\$SHELL\\\"; fi\" >> \"$rcfile\"; fi; done'", | |
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID host-render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; for rcfile in ~/.bashrc ~/.zshrc; do if [ -f \"$rcfile\" ] && ! grep -q \"exec sg host-render\" \"$rcfile\" 2>/dev/null; then echo \"if getent group host-render >/dev/null 2>&1 && ! groups | grep -q host-render; then exec sg host-render \\\"\\$SHELL\\\"; fi\" >> \"$rcfile\"; fi; done'", |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The postStartCommand modifies user shell configuration files (.bashrc and .zshrc) by appending shell code directly to them. This is problematic because:
- These files are typically considered user-controlled and modifying them automatically can be unexpected
- The changes persist across container restarts and rebuilds
- If the user has a custom shell configuration, this could interfere with it
- The check
! grep -q "exec sg host-render"only prevents adding the exact same line, but if the user manually removes or modifies this code, it will be re-added on next container start
Consider using the devcontainer's postAttachCommand or postCreateCommand features to run a temporary setup script that doesn't permanently modify user files, or document prominently that the devcontainer will modify these shell configuration files.
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID host-render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; sudo usermod -aG video vscode || true; for rcfile in ~/.bashrc ~/.zshrc; do if [ -f \"$rcfile\" ] && ! grep -q \"exec sg host-render\" \"$rcfile\" 2>/dev/null; then echo \"if getent group host-render >/dev/null 2>&1 && ! groups | grep -q host-render; then exec sg host-render \\\"\\$SHELL\\\"; fi\" >> \"$rcfile\"; fi; done'", | |
| "postStartCommand": "bash -lc 'set -e; HOST_RENDER_GID=$(stat -c \"%g\" /dev/kfd 2>/dev/null || stat -c \"%g\" /dev/dri/renderD128 2>/dev/null || echo \"\"); if [ -n \"$HOST_RENDER_GID\" ]; then if ! getent group $HOST_RENDER_GID >/dev/null; then sudo groupadd -g $HOST_RENDER_GID host-render || true; fi; sudo usermod -aG $HOST_RENDER_GID vscode || true; fi; if ! getent group video >/dev/null; then sudo groupadd -r video || true; fi; sudo usermod -aG video vscode || true'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
postStartCommandusesset -eat the beginning, which causes the script to exit on any error. However, several commands later in the script use|| trueto prevent failures from stopping execution. This creates an inconsistency: some failures are allowed (those with|| true), while others would cause the entire command to fail.More critically, there's a logical issue in the for loop at the end. The loop iterates over
~/.bashrcand~/.zshrc, but if the first rcfile's shell code addition fails for any reason (without|| true), the script will exit due toset -e, preventing the second rcfile from being processed.Consider either:
set -eand handling errors explicitly where needed|| trueto the echo command in the for loop to prevent failures from stopping the script