Skip to content

Add root dirs to userns - #981

Merged
dougsland merged 2 commits into
containers:mainfrom
engelmi:add-root-dirs-to-userns
Jul 15, 2026
Merged

dougsland merged 2 commits into
containers:mainfrom
engelmi:add-root-dirs-to-userns

Conversation

@engelmi

@engelmi engelmi commented Jul 15, 2026 •

Copy link
Copy Markdown
Member

This PR does

  • Add missing [Container section to userns generated quadlet
  • userns to support --root-dir and --qm-root-dir and update unit tests

Summary by Sourcery

Add root directory support to userns-generated configuration and update related tests.

New Features:

  • Support separate root and QM root directories in userns-generated configuration files.

Enhancements:

  • Adjust quadlet/userns configuration generation to align with new root directory layout.

Tests:

  • Refactor userns unit tests to use the new root and QM root directory structure and validate updated output paths.

@sourcery-ai

sourcery-ai Bot commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This PR updates the userns helper and its tests to support explicit root directory options for qm, and ensures the generated quadlet has the required [Container] section, aligning function signatures and filesystem layout between the script and tests.

File-Level Changes

Change Details Files
Refactor userns test harness to model separate root and qm root directories and use fixed output paths for sysusers and qm drop-in configs.
  • Adjust mock directory layout to introduce MOCK_QM_ROOT_DIR under the test root and place sysusers.d and drop-in paths under realistic etc locations.
  • Add constants for expected sysusers and qm configuration filenames and use them when verifying outputs.
  • Update mock passwd/group generators to always write into MOCK_QM_ROOT_DIR/etc instead of caller-provided paths.
  • Simplify test calls to generate_sysusers_conf and generate_qm_dropin to rely on ROOT_DIR/QM_ROOT_DIR rather than passing explicit file paths.
tests/userns/test.sh
Extend userns script to support --root-dir and --qm-root-dir options and generate configs into canonical locations with a proper [Container] section in the quadlet.
  • Update generate_sysusers_conf to take only an offset parameter, derive passwd/group paths from QM_ROOT_DIR, and write the sysusers configuration into SYSUSERS_CONF under ROOT_DIR/etc/sysusers.d.
  • Update generate_qm_dropin to take only the user name, write the drop-in into QM_CONF under ROOT_DIR/etc/containers/systemd/qm.container.d, and include the missing [Container] section in the generated quadlet.
  • Add argument parsing and handling for --root-dir and --qm-root-dir so the script can target alternate filesystem roots for qm containers.
userns

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In setup_test_env, MOCK_SYSUSERS_DIR is created twice; you can drop the second mkdir -p call to avoid redundant directory creation.
  • The test_generate_qm_dropin_custom_user test currently sets test_user="qm_root", which contradicts the test name and expectation of a custom username; updating this to a distinct value (e.g., custom_user_123) will better validate custom user handling.
  • The change in verify_sysusers_format to local config_file="${1}" is a no-op compared to the original and could be reverted to keep the function focused on behavior changes only.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `setup_test_env`, `MOCK_SYSUSERS_DIR` is created twice; you can drop the second `mkdir -p` call to avoid redundant directory creation.
- The `test_generate_qm_dropin_custom_user` test currently sets `test_user="qm_root"`, which contradicts the test name and expectation of a custom username; updating this to a distinct value (e.g., `custom_user_123`) will better validate custom user handling.
- The change in `verify_sysusers_format` to `local config_file="${1}"` is a no-op compared to the original and could be reverted to keep the function focused on behavior changes only.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the userns script and its associated unit tests to support custom root directories (--root-dir and --qm-root-dir) for the host and QM filesystems. Key feedback includes addressing potential path concatenation bugs caused by trailing slashes in directory variables, fixing a bug where grep checks host files instead of the specified custom root directory, removing a redundant directory creation in the test setup, and correcting a test logic error where the custom user test incorrectly uses the default root user.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread userns
Comment thread userns Outdated
Comment thread userns Outdated
Comment thread userns
Comment thread tests/userns/test.sh
Comment thread tests/userns/test.sh
engelmi added 2 commits July 15, 2026 13:11
By providing the --root-dir and --qm-root-dir option, the userns script is able to
fetch information from QM and create files on the host machine in the proper places.

Signed-off-by: Michael Engel <mengel@redhat.com>
Signed-off-by: Michael Engel <mengel@redhat.com>
@engelmi
engelmi force-pushed the add-root-dirs-to-userns branch from 71e2aaa to afac65a Compare July 15, 2026 11:11
@dougsland

Copy link
Copy Markdown
Member

There is a know issue regarding tests failing.
#978

@dougsland
dougsland merged commit 0b4fca2 into containers:main Jul 15, 2026
25 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants