Skip to content

storage/pkg/config: cover driver options being dropped mid-chain - #1058

Open
vtushar06 wants to merge 1 commit into
podman-container-tools:mainfrom
vtushar06:storage-config-combined-options
Open

storage/pkg/config: cover driver options being dropped mid-chain#1058
vtushar06 wants to merge 1 commit into
podman-container-tools:mainfrom
vtushar06:storage-config-combined-options

Conversation

@vtushar06

Copy link
Copy Markdown

GetGraphDriverOptions appends into one list across all four driver sections, but every test in config_test.go sets a single field on a fresh OptionsConfig and then resets, so nothing covers a branch that ends the chain early.

That is not hypothetical. 5d9d814 fixed btrfs.min_space doing return append(...) instead of doptions = append(...), which silently dropped every option after it, and the package passed both before and after the fix.

I put that line back locally to check. The existing tests still report ok; the one here fails with

--- FAIL: TestCombinedDriverOptions (0.00s)
    config_test.go:210: Expected to find "overlay.mountopt=nodev", got [btrfs.min_space=100]

It sets one field in each of btrfs, overlay, vfs and zfs and asserts all four come back, so it catches an early return anywhere in the chain rather than just that one line.

@mtrmac you asked for something along these lines on #680 ("Some testing of the interactions between per-driver options and the top options in OptionsConfig would be valuable"). This only covers the per-driver half. The OptionsConfig top-level interaction is the other half, and I left it out because the ordering between the global options in storage/types/options.go and the per-driver ones looks undocumented, so I did not want to pin behaviour nobody has ruled on.

GetGraphDriverOptions builds one list across every driver section, but each
test here sets a single field on a fresh OptionsConfig, so a branch that
ends the chain early is invisible. 5d9d814 fixed btrfs.min_space
returning instead of appending and the package passed before and after.

Putting that line back today still gives ok for the existing tests; this
one fails with

    Expected to find "overlay.mountopt=nodev", got [btrfs.min_space=100]

Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>
Copilot AI lite review requested due to automatic review settings August 7, 2026 11:08

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added the storage Related to "storage" package label Aug 7, 2026

@giuseppe giuseppe 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.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

storage Related to "storage" package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants