Repository navigation
Conversation
This would previously fail upon executing `groupdel` if the group in question was the only/primary group of a user. With this change, conflicting users are removed first, then the groups are updated in place without being removed, and finally the container user account gets created.
This repurposes the unused `submodules_initialized` function to check for submodule initialization and report a warning message if (1) any of the submodules are not initialized, or (2) the current submodule HEAD does not match what was committed to the current branch.
This fixes a few spelling errors and corrects the documentation for the option_user_build_image option.
malensek
force-pushed
the
tickets/DM-56606
branch
from
October 8, 2026 22:15
8aa8e2e to
4cbc803
Compare
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The collision handling and submodule diagnostics are consistent with the stated requirements, with no unresolved correctness issues found.
0 open findings
What changed in this PR
Fixes UID/GID collisions when creating personalized Qserv build images and improves build diagnostics.
Changes:
- Removes conflicting users before configuring duplicate GIDs.
- Warns about uninitialized or mismatched Git submodules.
- Corrects CLI help text and spelling.
| File | Description |
|---|---|
deploy/docker/build-user/Dockerfile |
Handles user and group collisions safely. |
python/lsst/qserv/admin/qservCli/launch.py |
Adds recursive submodule checks and typo fixes. |
python/lsst/qserv/admin/qservCli/qserv_cli.py |
Corrects user-build image help text. |
bin/qserv |
Corrects a comment typo. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fritzm
reviewed
Oct 9, 2026
| if getent passwd "$USER" > /dev/null; then userdel "$USER"; fi; \ | ||
| groupadd --force --non-unique --gid "$GID" "$GROUP"; \ | ||
| groupmod --non-unique --gid "$GID" "$GROUP"; \ | ||
| useradd --uid "$UID" --gid "$GROUP" "$USER" |
Contributor
There was a problem hiding this comment.
The old busted buggy version was prettier, just sayin'... 🙂
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
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.
Currently,
build-user-build-imagecan fail if the host UID or GID already exists in the base image. To reproduce, create a user with UID / GID set to 1000 / 1000 and build the image:This can be resolved by removing the conflicting user account before configuring the groups. However, simply rearranging the commands can produce another error because
userdelwill automatically remove the user’s primary group as long as it has the same name as the user and the user is the only member, causing the subsequent groupdel command to fail.Proposed Change
We need to remove conflicting users first, then set up the groups (we can use
--non-uniqueto work around GID conflicts), and finally add the user to the image.Additional Build-Related Changes