-
Notifications
You must be signed in to change notification settings - Fork 32
Add dev container setup #318
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 | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,44 @@ | ||||||||||||||||||
| { | ||||||||||||||||||
| "name": "Iris", | ||||||||||||||||||
| // Use the existing Dockerfile | ||||||||||||||||||
| "build": { | ||||||||||||||||||
| "dockerfile": "../docker/Dockerfile", | ||||||||||||||||||
| "context": ".." | ||||||||||||||||||
| }, | ||||||||||||||||||
| // Runs on the HOST before the container is created/started. | ||||||||||||||||||
| // Creates a stable agent socket at ~/.ssh/ssh-agent.sock and optionally loads ~/.ssh/id_rsa. | ||||||||||||||||||
| "initializeCommand": "bash -lc \"bash '${localWorkspaceFolder}/.devcontainer/ensure-ssh-agent.sh'\"", | ||||||||||||||||||
| "runArgs": [ | ||||||||||||||||||
| "--name=${localEnv:USER}-iris-dev", | ||||||||||||||||||
| "--network=host", | ||||||||||||||||||
| "--device=/dev/kfd", | ||||||||||||||||||
| "--device=/dev/dri", | ||||||||||||||||||
| "--cap-add=SYS_PTRACE", | ||||||||||||||||||
| "--group-add=video", | ||||||||||||||||||
|
||||||||||||||||||
| "--group-add=video", | |
| "--group-add=video", | |
| "--group-add=render", |
Copilot
AI
Jan 19, 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 SSH agent socket mount relies on the ensure-ssh-agent.sh script to create the socket at ~/.ssh/ssh-agent.sock on the host. However, if the initializeCommand fails or the socket is not properly created, the container will attempt to mount a non-existent file, which could cause the container to fail to start. Consider adding error handling or verification that the socket exists before attempting to mount it.
| "source=${localEnv:HOME}/.ssh/ssh-agent.sock,target=/tmp/ssh-agent.sock,type=bind" | |
| ], | |
| "remoteEnv": { | |
| "SSH_AUTH_SOCK": "/tmp/ssh-agent.sock" | |
| "source=${localEnv:HOME}/.ssh,target=/tmp/ssh,type=bind" | |
| ], | |
| "remoteEnv": { | |
| "SSH_AUTH_SOCK": "/tmp/ssh/ssh-agent.sock" |
Copilot
AI
Jan 19, 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 attempts to create video and render groups and add the vscode user to them. However, these groups are already created in the Dockerfile (lines 27-28). This creates redundancy and potential confusion. Consider removing the group creation from either the Dockerfile or the postStartCommand to maintain a single source of truth for group setup.
| "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; sudo usermod -aG video,render vscode || true'", |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,28 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Copyright (c) 2026 Advanced Micro Devices, Inc. All rights reserved. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # This script runs on the HOST (via devcontainer.json "initializeCommand"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # It ensures there is an ssh-agent with a stable socket at: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # ~/.ssh/ssh-agent.sock | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # It also tries to load ~/.ssh/id_rsa if present. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # If your key is passphrase-protected and you're non-interactive, it may fail silently. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| SOCK="${HOME}/.ssh/ssh-agent.sock" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| mkdir -p "${HOME}/.ssh" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ -S "${SOCK}" ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 0 | |
| # Socket exists; verify that an ssh-agent is actually responding on it. | |
| SSH_AUTH_SOCK="${SOCK}" ssh-add -l >/dev/null 2>&1 || status=$? | |
| # ssh-add -l exit codes: | |
| # 0: agent running, identities listed | |
| # 1: agent running, no identities | |
| # 2: agent not running / unreachable | |
| if [[ ${status:-0} -eq 0 || ${status:-0} -eq 1 ]]; then | |
| exit 0 | |
| fi |
Copilot
AI
Jan 19, 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 SSH_AUTH_SOCK environment variable is set inline for individual commands (lines 25, 28) rather than being exported once. While this works, it's less efficient and harder to maintain. Consider exporting SSH_AUTH_SOCK="${SOCK}" once after starting the agent and before the ssh-add commands for better readability and consistency.
| if [[ -f "${HOME}/.ssh/id_rsa" ]]; then | |
| SSH_AUTH_SOCK="${SOCK}" ssh-add "${HOME}/.ssh/id_rsa" >/dev/null 2>&1 || true | |
| fi | |
| SSH_AUTH_SOCK="${SOCK}" ssh-add -l >/dev/null 2>&1 || true | |
| export SSH_AUTH_SOCK="${SOCK}" | |
| if [[ -f "${HOME}/.ssh/id_rsa" ]]; then | |
| ssh-add "${HOME}/.ssh/id_rsa" >/dev/null 2>&1 || true | |
| fi | |
| ssh-add -l >/dev/null 2>&1 || true |
Copilot
AI
Jan 19, 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 script silently ignores errors when adding SSH keys with the || true operator. While this prevents the script from failing when keys are passphrase-protected, it also masks other potential errors (e.g., corrupt key files, permission issues). Consider logging a warning message when ssh-add fails to help users understand why their keys may not be loaded.
| SSH_AUTH_SOCK="${SOCK}" ssh-add "${HOME}/.ssh/id_rsa" >/dev/null 2>&1 || true | |
| fi | |
| SSH_AUTH_SOCK="${SOCK}" ssh-add -l >/dev/null 2>&1 || true | |
| if ! SSH_AUTH_SOCK="${SOCK}" ssh-add "${HOME}/.ssh/id_rsa" >/dev/null 2>&1; then | |
| echo "Warning: Failed to add SSH key ${HOME}/.ssh/id_rsa to ssh-agent at ${SOCK}. The key may be passphrase-protected or there may be a problem with the key file or its permissions." >&2 | |
| fi | |
| fi | |
| if ! SSH_AUTH_SOCK="${SOCK}" ssh-add -l >/dev/null 2>&1; then | |
| echo "Warning: ssh-agent at ${SOCK} is running, but no keys are currently loaded or the agent is not responding as expected." >&2 | |
| fi |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,3 +45,5 @@ __pycache__/ | |
| *.pywz | ||
| *.pyzw | ||
| *.pyzwz | ||
|
|
||
| !.devcontainer/devcontainer.json | ||
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 devcontainer configuration lacks inline comments explaining why certain security options are needed (e.g., --cap-add=SYS_PTRACE, --security-opt=seccomp=unconfined). These security-relaxing options could raise concerns during security reviews. Consider adding comments explaining that these are required for ROCm GPU debugging and profiling.