Skip to content

Follow up for strorage.conf parsing changes #704

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.

Sign up for GitHub

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

Merged
mtrmac merged 3 commits into from
Mar 19, 2026
Merged

Follow up for strorage.conf parsing changes #704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5eb2c9c
storage: fix typo in rootless_storage_path warning
Luap99 Mar 19, 2026
d2c67fb
storage: move driver option parsing to separate package
Luap99 Mar 19, 2026
2be1be4
storage: add unit tests for ParseDriverOption
Luap99 Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions storage/drivers/btrfs/btrfs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
"github.com/opencontainers/selinux/go-selinux/label"
"github.com/sirupsen/logrus"
graphdriver "go.podman.io/storage/drivers"
"go.podman.io/storage/internal/opts"
"go.podman.io/storage/internal/driver"
"go.podman.io/storage/internal/tempdir"
"go.podman.io/storage/pkg/directory"
"go.podman.io/storage/pkg/fileutils"
Expand Down Expand Up @@ -97,7 +97,7 @@ func parseOptions(opt []string) (btrfsOptions, bool, error) {
var options btrfsOptions
userDiskQuota := false
for _, option := range opt {
driver, key, val, err := opts.ParseDriverOption(option)
driver, key, val, err := driver.ParseDriverOption(option)
if err != nil {
return options, userDiskQuota, err
}
Expand Down
4 changes: 2 additions & 2 deletions storage/drivers/overlay/overlay.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"go.podman.io/storage/drivers/overlayutils"
"go.podman.io/storage/drivers/quota"
"go.podman.io/storage/internal/dedup"
"go.podman.io/storage/internal/opts"
"go.podman.io/storage/internal/driver"
"go.podman.io/storage/internal/staging_lockfile"
"go.podman.io/storage/internal/tempdir"
"go.podman.io/storage/pkg/archive"
Expand Down Expand Up @@ -467,7 +467,7 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error)
func parseOptions(options []string) (*overlayOptions, error) {
o := &overlayOptions{}
for _, option := range options {
driver, key, val, err := opts.ParseDriverOption(option)
driver, key, val, err := driver.ParseDriverOption(option)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions storage/drivers/vfs/driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/vbatts/tar-split/tar/storage"
graphdriver "go.podman.io/storage/drivers"
"go.podman.io/storage/internal/dedup"
"go.podman.io/storage/internal/opts"
"go.podman.io/storage/internal/driver"
"go.podman.io/storage/internal/tempdir"
"go.podman.io/storage/pkg/archive"
"go.podman.io/storage/pkg/directory"
Expand Down Expand Up @@ -46,7 +46,7 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error)
return nil, err
}
for _, option := range options.DriverOptions {
driver, key, val, err := opts.ParseDriverOption(option)
driver, key, val, err := driver.ParseDriverOption(option)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions storage/drivers/zfs/zfs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/opencontainers/selinux/go-selinux/label"
"github.com/sirupsen/logrus"
graphdriver "go.podman.io/storage/drivers"
"go.podman.io/storage/internal/opts"
"go.podman.io/storage/internal/driver"
"go.podman.io/storage/internal/tempdir"
"go.podman.io/storage/pkg/directory"
"go.podman.io/storage/pkg/idtools"
Expand Down Expand Up @@ -123,7 +123,7 @@ func parseOptions(opt []string) (zfsOptions, error) {
var options zfsOptions
options.fsName = ""
for _, option := range opt {
driver, key, val, err := opts.ParseDriverOption(option)
driver, key, val, err := driver.ParseDriverOption(option)
if err != nil {
return options, err
}
Expand Down
2 changes: 1 addition & 1 deletion storage/internal/opts/driver.go → storage/internal/driver/driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package opts
package driver

import (
"fmt"
Expand Down
86 changes: 86 additions & 0 deletions storage/internal/driver/driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package driver_test

import (
"testing"

"github.com/stretchr/testify/assert"
"go.podman.io/storage/internal/driver"
)

func TestParseDriverOption(t *testing.T) {
tests := []struct {
name string
option string
wantDriver string
wantKey string
wantValue string
wantErr string
}{
{
name: "valid option with overlay driver",
option: "overlay.mountopt=nodev",
wantDriver: "overlay",
wantKey: "mountopt",
wantValue: "nodev",
},
{
name: "empty driver string before the dot",
option: ".mountopt=nodev",
wantDriver: "",
wantKey: "mountopt",
wantValue: "nodev",
},
{
name: "not dot in option",
option: "mountopt=nodev",
wantDriver: "",
wantKey: "mountopt",
wantValue: "nodev",
},
{
name: "valid option with overlay2 driver",
option: "overlay2.size=10G",
wantDriver: "overlay2",
wantKey: "size",
wantValue: "10G",
},
{
name: "uppercase driver and key are converted to lowercase",
option: "ZFS.SIZE=20G",
wantDriver: "zfs",
wantKey: "size",
wantValue: "20G",
},
{
name: "spaces are trimmed",
option: " key= val ",
wantDriver: "",
wantKey: "key",
wantValue: "val",
},
{
name: "invalid driver returns an error",
option: "magicfs.size=5G",
wantErr: `unknown driver "magicfs" in option "magicfs.size=5G"`,
},
{
name: "invalid syntax",
option: "myopt",
wantErr: "unable to parse key/value option: myopt",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
driver, key, val, err := driver.ParseDriverOption(tt.option)
if tt.wantErr != "" {
assert.ErrorContains(t, err, tt.wantErr)
return
}

assert.NoError(t, err)
assert.Equal(t, tt.wantDriver, driver)
assert.Equal(t, tt.wantKey, key)
assert.Equal(t, tt.wantValue, val)
})
}
}
8 changes: 4 additions & 4 deletions storage/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"github.com/sirupsen/logrus"
drivers "go.podman.io/storage/drivers"
"go.podman.io/storage/internal/dedup"
"go.podman.io/storage/internal/opts"
"go.podman.io/storage/internal/driver"
"go.podman.io/storage/internal/tempdir"
"go.podman.io/storage/pkg/archive"
"go.podman.io/storage/pkg/directory"
Expand Down Expand Up @@ -3942,13 +3942,13 @@ func GetDefaultMountOptions() ([]string, error) {
}

// GetMountOptions returns the mountoptions for the specified driver and graphDriverOptions
func GetMountOptions(driver string, graphDriverOptions []string) ([]string, error) {
func GetMountOptions(usedDriver string, graphDriverOptions []string) ([]string, error) {
for _, option := range graphDriverOptions {
optDriver, key, val, err := opts.ParseDriverOption(option)
optDriver, key, val, err := driver.ParseDriverOption(option)
if err != nil {
return nil, err
}
if (optDriver == "" || optDriver == driver) && key == "mountopt" {
if (optDriver == "" || optDriver == usedDriver) && key == "mountopt" {
return strings.Split(val, ","), nil
}
}
Expand Down
2 changes: 1 addition & 1 deletion storage/types/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func LoadStoreOptions(opts LoadOptions) (StoreOptions, error) {

if config.Storage.GraphRoot != "" {
if config.Storage.RootlessStoragePath != "" {
logrus.Warn("Both rootless_storage_path and graphroot are set, using graphroot setting only, rootless_storage_path is deprecated and should be removed form the config")
logrus.Warn("Both rootless_storage_path and graphroot are set, using graphroot setting only, rootless_storage_path is deprecated and should be removed from the config")
}
graphRoot, err := expandEnvPath(config.Storage.GraphRoot, rootlessUID)
if err != nil {
Expand Down
Loading