Fix permissions causing vulkan failures #210
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
October 10, 2025 11:04
In order for Vulkan to work the modelrunner user needs to be a member of the video and render groups (needed to access nodes in /dev/dri). So add modelrunner to video when creating the user. Unfortunately render does not have a fixed GID currently, so we'll have to deal with this another way. Signed-off-by: Piotr Stankiewicz <[email protected]>
The modelrunner user needs to be a member of the render group for Vulkan to work. Unfortunately render does not have a fixed GID, so we can't reliably handle this in the Dockerfile. So have the CLI query the hosts render GID and add it when starting the DMR container. Signed-off-by: Piotr Stankiewicz <[email protected]>
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideOn Linux hosts, this PR retrieves the host 'render' group GID at runtime and adds it to container permissions in both the Go container setup and the docker-run script; it simplifies the Vulkan installation by using the libvulkan1 package; and it adjusts the Dockerfile to add the modelrunner user to the video group and remove obsolete Vulkan environment settings. Sequence diagram for adding host 'render' group to container permissions on LinuxsequenceDiagram
participant Controller as "CreateControllerContainer()"
participant OS as "Linux Host OS"
participant Docker as "Docker Container"
Controller->>OS: exec getent group render
OS-->>Controller: return group info (GID)
Controller->>Docker: append GID to GroupAdd
Docker-->>Controller: container runs with host 'render' group permissions
Sequence diagram for docker-run.sh adding host 'render' groupsequenceDiagram
actor User
participant Script as "docker-run.sh"
participant OS as "Linux Host OS"
participant Docker as "Docker Container"
User->>Script: Run docker-run.sh
Script->>OS: getent group render | cut -d: -f3
OS-->>Script: return GID
Script->>Docker: --group-add <GID>
Docker-->>Script: container runs with host 'render' group permissions
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Signed-off-by: Piotr Stankiewicz <[email protected]>
ericcurtin
approved these changes
Oct 10, 2025
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider using Go’s os/user.LookupGroup instead of shelling out to getent for more portable and testable group lookups.
- Add error handling or a fallback in docker-run.sh around the getent render lookup so the script won’t fail if the render group is absent.
- You’ve duplicated the render GID lookup logic in both Go and bash—consider centralizing it into a shared utility or function to reduce maintenance overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using Go’s os/user.LookupGroup instead of shelling out to getent for more portable and testable group lookups.
- Add error handling or a fallback in docker-run.sh around the getent render lookup so the script won’t fail if the render group is absent.
- You’ve duplicated the render GID lookup logic in both Go and bash—consider centralizing it into a shared utility or function to reduce maintenance overhead.
## Individual Comments
### Comment 1
<location> `scripts/docker-run.sh:10-11` </location>
<code_context>
args+=("--device" "$i")
fi
done
+ args+=("--group-add" "$(getent group render | cut -d: -f3)")
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** No error handling if 'render' group is missing could lead to runtime failures.
Add a check to verify the 'render' group exists before using its ID to prevent passing an invalid value to docker.
```suggestion
render_gid="$(getent group render | cut -d: -f3)"
if [ -n "$render_gid" ]; then
args+=("--group-add" "$render_gid")
fi
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+10
to
11
| args+=("--group-add" "$(getent group render | cut -d: -f3)") | ||
| } |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): No error handling if 'render' group is missing could lead to runtime failures.
Add a check to verify the 'render' group exists before using its ID to prevent passing an invalid value to docker.
Suggested change
| args+=("--group-add" "$(getent group render | cut -d: -f3)") | |
| } | |
| render_gid="$(getent group render | cut -d: -f3)" | |
| if [ -n "$render_gid" ]; then | |
| args+=("--group-add" "$render_gid") | |
| fi | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Simplify Vulkan setup and ensure containers run with appropriate group permissions to fix Vulkan failures
Enhancements: