From c5d0e3e6fa41e4f43cac7d5479f1b3b1b1ca409e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 12 Nov 2025 11:54:17 +0100 Subject: [PATCH 1/2] daemon: Add TestDetermineImageStoreChoice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Gronowski --- daemon/image_store_choice_test.go | 210 ++++++++++++++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100644 daemon/image_store_choice_test.go diff --git a/daemon/image_store_choice_test.go b/daemon/image_store_choice_test.go new file mode 100644 index 0000000000..f54d2a3c73 --- /dev/null +++ b/daemon/image_store_choice_test.go @@ -0,0 +1,210 @@ +package daemon + +import ( + "runtime" + "testing" + + "github.com/moby/moby/v2/daemon/config" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +func TestDetermineImageStoreChoice(t *testing.T) { + str := func(s string) *string { + return &s + } + + tests := []struct { + name string + envDockerDriver *string + envTestUseGraphDriver *string + cfg *config.Config + expectedChoice imageStoreChoice + expectError bool + skipPlatform string + onlyPlatform string + }{ + { + name: "default containerd on non-Windows", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceContainerd, + skipPlatform: "windows", + }, + { + name: "default graphdriver on Windows", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriver, + onlyPlatform: "windows", + }, + { + name: "containerd-snapshotter feature enabled", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{ + "containerd-snapshotter": true, + }, + }, + }, + expectedChoice: imageStoreChoiceContainerdExplicit, + }, + { + name: "containerd-snapshotter feature disabled", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{ + "containerd-snapshotter": false, + }, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + }, + { + name: "TEST_INTEGRATION_USE_GRAPHDRIVER env var set", + envTestUseGraphDriver: str("1"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + }, + { + name: "vfs driver in config", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: "vfs", + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + skipPlatform: "windows", + }, + { + name: "overlay2 driver in config", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: "overlay2", + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + skipPlatform: "windows", + }, + { + name: "btrfs driver in config", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: "btrfs", + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + skipPlatform: "windows", + }, + { + name: "DOCKER_DRIVER env var set to overlay2", + envDockerDriver: str("overlay2"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + skipPlatform: "windows", + }, + { + name: "custom snapshotter", + envDockerDriver: str("my-custom-snapshotter"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceContainerdExplicit, + skipPlatform: "windows", + }, + { + name: "windows driver on Windows", + envDockerDriver: str("windows"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceContainerdExplicit, + onlyPlatform: "windows", + }, + { + name: "windowsfilter driver on Windows", + envDockerDriver: str("windowsfilter"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + onlyPlatform: "windows", + }, + { + name: "TEST_INTEGRATION_USE_GRAPHDRIVER takes precedence over feature flag", + envTestUseGraphDriver: str("1"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{ + "containerd-snapshotter": true, + }, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + }, + { + name: "native driver in config", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: "native", + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceContainerdExplicit, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if tc.skipPlatform != "" && runtime.GOOS == tc.skipPlatform { + t.Skip("Skipping on " + tc.skipPlatform) + } + if tc.onlyPlatform != "" && runtime.GOOS != tc.onlyPlatform { + t.Skip("Skipping on " + tc.onlyPlatform) + } + + if tc.envDockerDriver != nil { + t.Setenv("DOCKER_DRIVER", *tc.envDockerDriver) + } else { + t.Setenv("DOCKER_DRIVER", "") + } + if tc.envTestUseGraphDriver != nil { + t.Setenv("TEST_INTEGRATION_USE_GRAPHDRIVER", *tc.envTestUseGraphDriver) + } else { + t.Setenv("TEST_INTEGRATION_USE_GRAPHDRIVER", "") + } + + choice, err := determineImageStoreChoice(tc.cfg) + if tc.expectError { + assert.Error(t, err, "expected an error but got none") + } else { + assert.NilError(t, err) + } + + assert.Check(t, is.Equal(tc.expectedChoice, choice)) + }) + } +} From 391247ce9619136e3cfd150e0a3719ebdc8e510b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 12 Nov 2025 11:57:19 +0100 Subject: [PATCH 2/2] daemon: Fix image store choice priority for prior graphdriver state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The priority order for determining image store choice was incorrect when a prior graphdriver existed. The issue occurred because the prior graphdriver check happened after processing explicit driver configuration, effectively ignoring user intent when prior state existed. Signed-off-by: Paweł Gronowski --- daemon/daemon.go | 4 +- daemon/image_store_choice.go | 61 +++--- daemon/image_store_choice_test.go | 309 ++++++++++++++++++------------ 3 files changed, 216 insertions(+), 158 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 3e1b421b88..d639b6f0c1 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -873,7 +873,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S } d.configStore.Store(cfgStore) - imgStoreChoice, err := determineImageStoreChoice(config) + imgStoreChoice, err := determineImageStoreChoice(config, determineImageStoreChoiceOptions{}) if err != nil { return nil, err } @@ -1096,7 +1096,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S return nil, err } - driverName := chooseDriver(ctx, cfgStore.GraphDriver, imgStoreChoice) + driverName := getDriverOverride(ctx, cfgStore.GraphDriver, imgStoreChoice) var migrationConfig migration.Config if imgStoreChoice.IsGraphDriver() { diff --git a/daemon/image_store_choice.go b/daemon/image_store_choice.go index 98df037d3c..c7b3ccd556 100644 --- a/daemon/image_store_choice.go +++ b/daemon/image_store_choice.go @@ -48,14 +48,14 @@ func (c imageStoreChoice) IsExplicit() bool { } } -// chooseDriver determines the storage driver name based on environment variables, +// getDriverOverride determines the storage driver name based on environment variables, // configuration, and platform-specific logic. // On Windows we don't support the environment variable, or a user supplied graphdriver, // but it is allowed when using snapshotters. // Unix platforms however run a single graphdriver for all containers, and it can // be set through an environment variable, a daemon start parameter, or chosen through // initialization of the layerstore through driver priority order for example. -func chooseDriver(ctx context.Context, cfgGraphDriver string, imgStoreChoice imageStoreChoice) string { +func getDriverOverride(ctx context.Context, cfgGraphDriver string, imgStoreChoice imageStoreChoice) string { driverName := os.Getenv("DOCKER_DRIVER") if driverName == "" { driverName = cfgGraphDriver @@ -85,9 +85,25 @@ func chooseDriver(ctx context.Context, cfgGraphDriver string, imgStoreChoice ima return driverName } -func determineImageStoreChoice(cfgStore *config.Config) (imageStoreChoice, error) { +type determineImageStoreChoiceOptions struct { + hasPriorDriver func(root string) bool + isRegisteredGraphdriver func(driverName string) bool + runtimeOS string +} + +func determineImageStoreChoice(cfgStore *config.Config, opts determineImageStoreChoiceOptions) (imageStoreChoice, error) { + if opts.hasPriorDriver == nil { + opts.hasPriorDriver = graphdriver.HasPriorDriver + } + if opts.isRegisteredGraphdriver == nil { + opts.isRegisteredGraphdriver = graphdriver.IsRegistered + } + if opts.runtimeOS == "" { + opts.runtimeOS = runtime.GOOS + } + out := imageStoreChoiceContainerd - if runtime.GOOS == "windows" { + if opts.runtimeOS == "windows" { out = imageStoreChoiceGraphdriver } @@ -111,44 +127,27 @@ func determineImageStoreChoice(cfgStore *config.Config) (imageStoreChoice, error out = imageStoreChoiceGraphdriverExplicit } + if out == imageStoreChoiceContainerd { + if opts.hasPriorDriver(cfgStore.Root) { + return imageStoreChoiceGraphdriverPrior, nil + } + } + if driverName != "" { - if !out.IsExplicit() { - switch driverName { - case "vfs", "overlay2": - out = imageStoreChoiceGraphdriverExplicit - case "btrfs": - // The btrfs driver is not heavily used in containerd and has no - // advantage over overlayfs anymore since overlay works fine. - // If btrfs is explicitly chosen, the user most likely means graphdrivers. - out = imageStoreChoiceGraphdriverExplicit - } + if !out.IsExplicit() && opts.isRegisteredGraphdriver(driverName) { + return imageStoreChoiceGraphdriverExplicit, nil } if out.IsGraphDriver() { - if graphdriver.IsRegistered(driverName) { + if opts.isRegisteredGraphdriver(driverName) { return imageStoreChoiceGraphdriverExplicit, nil - } else { + } else if out.IsExplicit() { return imageStoreChoiceGraphdriverExplicit, fmt.Errorf("graphdriver is explicitly enabled but %q is not registered, %v %v", driverName, cfgStore.Features, os.Getenv("TEST_INTEGRATION_USE_GRAPHDRIVER")) } } - if runtime.GOOS == "windows" && !out.IsExplicit() { - switch driverName { - case "windows": - return imageStoreChoiceContainerdExplicit, nil - case "windowsfilter": - return imageStoreChoiceGraphdriverExplicit, nil - } - } - // Assume snapshotter is chosen return imageStoreChoiceContainerdExplicit, nil } - if out == imageStoreChoiceContainerd { - if graphdriver.HasPriorDriver(cfgStore.Root) { - return imageStoreChoiceGraphdriverPrior, nil - } - } - return out, nil } diff --git a/daemon/image_store_choice_test.go b/daemon/image_store_choice_test.go index f54d2a3c73..d44c06c64d 100644 --- a/daemon/image_store_choice_test.go +++ b/daemon/image_store_choice_test.go @@ -1,7 +1,7 @@ package daemon import ( - "runtime" + "slices" "testing" "github.com/moby/moby/v2/daemon/config" @@ -14,36 +14,19 @@ func TestDetermineImageStoreChoice(t *testing.T) { return &s } - tests := []struct { + type testCase struct { name string envDockerDriver *string envTestUseGraphDriver *string + priorGraphDriver bool cfg *config.Config expectedChoice imageStoreChoice expectError bool skipPlatform string onlyPlatform string - }{ - { - name: "default containerd on non-Windows", - cfg: &config.Config{ - CommonConfig: config.CommonConfig{ - Features: map[string]bool{}, - }, - }, - expectedChoice: imageStoreChoiceContainerd, - skipPlatform: "windows", - }, - { - name: "default graphdriver on Windows", - cfg: &config.Config{ - CommonConfig: config.CommonConfig{ - Features: map[string]bool{}, - }, - }, - expectedChoice: imageStoreChoiceGraphdriver, - onlyPlatform: "windows", - }, + } + + tests := []testCase{ { name: "containerd-snapshotter feature enabled", cfg: &config.Config{ @@ -76,83 +59,6 @@ func TestDetermineImageStoreChoice(t *testing.T) { }, expectedChoice: imageStoreChoiceGraphdriverExplicit, }, - { - name: "vfs driver in config", - cfg: &config.Config{ - CommonConfig: config.CommonConfig{ - GraphDriver: "vfs", - Features: map[string]bool{}, - }, - }, - expectedChoice: imageStoreChoiceGraphdriverExplicit, - skipPlatform: "windows", - }, - { - name: "overlay2 driver in config", - cfg: &config.Config{ - CommonConfig: config.CommonConfig{ - GraphDriver: "overlay2", - Features: map[string]bool{}, - }, - }, - expectedChoice: imageStoreChoiceGraphdriverExplicit, - skipPlatform: "windows", - }, - { - name: "btrfs driver in config", - cfg: &config.Config{ - CommonConfig: config.CommonConfig{ - GraphDriver: "btrfs", - Features: map[string]bool{}, - }, - }, - expectedChoice: imageStoreChoiceGraphdriverExplicit, - skipPlatform: "windows", - }, - { - name: "DOCKER_DRIVER env var set to overlay2", - envDockerDriver: str("overlay2"), - cfg: &config.Config{ - CommonConfig: config.CommonConfig{ - Features: map[string]bool{}, - }, - }, - expectedChoice: imageStoreChoiceGraphdriverExplicit, - skipPlatform: "windows", - }, - { - name: "custom snapshotter", - envDockerDriver: str("my-custom-snapshotter"), - cfg: &config.Config{ - CommonConfig: config.CommonConfig{ - Features: map[string]bool{}, - }, - }, - expectedChoice: imageStoreChoiceContainerdExplicit, - skipPlatform: "windows", - }, - { - name: "windows driver on Windows", - envDockerDriver: str("windows"), - cfg: &config.Config{ - CommonConfig: config.CommonConfig{ - Features: map[string]bool{}, - }, - }, - expectedChoice: imageStoreChoiceContainerdExplicit, - onlyPlatform: "windows", - }, - { - name: "windowsfilter driver on Windows", - envDockerDriver: str("windowsfilter"), - cfg: &config.Config{ - CommonConfig: config.CommonConfig{ - Features: map[string]bool{}, - }, - }, - expectedChoice: imageStoreChoiceGraphdriverExplicit, - onlyPlatform: "windows", - }, { name: "TEST_INTEGRATION_USE_GRAPHDRIVER takes precedence over feature flag", envTestUseGraphDriver: str("1"), @@ -175,36 +81,189 @@ func TestDetermineImageStoreChoice(t *testing.T) { }, expectedChoice: imageStoreChoiceContainerdExplicit, }, + { + name: "vfs driver in config", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: "vfs", + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + }, + { + name: "custom snapshotter", + envDockerDriver: str("my-custom-snapshotter"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceContainerdExplicit, + }, } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - if tc.skipPlatform != "" && runtime.GOOS == tc.skipPlatform { - t.Skip("Skipping on " + tc.skipPlatform) - } - if tc.onlyPlatform != "" && runtime.GOOS != tc.onlyPlatform { - t.Skip("Skipping on " + tc.onlyPlatform) - } + nonWindows := []testCase{ + { + name: "default containerd on non-Windows", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceContainerd, + }, + } - if tc.envDockerDriver != nil { - t.Setenv("DOCKER_DRIVER", *tc.envDockerDriver) - } else { - t.Setenv("DOCKER_DRIVER", "") - } - if tc.envTestUseGraphDriver != nil { - t.Setenv("TEST_INTEGRATION_USE_GRAPHDRIVER", *tc.envTestUseGraphDriver) - } else { - t.Setenv("TEST_INTEGRATION_USE_GRAPHDRIVER", "") - } + for _, gd := range []string{"fuse-overlayfs", "overlay2", "btrfs", "zfs"} { + nonWindows = append(nonWindows, testCase{ + name: gd + " driver in config", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: gd, + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + }) - choice, err := determineImageStoreChoice(tc.cfg) - if tc.expectError { - assert.Error(t, err, "expected an error but got none") - } else { - assert.NilError(t, err) - } + nonWindows = append(nonWindows, testCase{ + name: gd + " driver in config with prior data", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: gd, + Features: map[string]bool{}, + }, + }, + priorGraphDriver: true, + expectedChoice: imageStoreChoiceGraphdriverPrior, + }) - assert.Check(t, is.Equal(tc.expectedChoice, choice)) + nonWindows = append(nonWindows, testCase{ + name: gd + " driver in config with containerd snapshotter feature enabled", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: gd, + Features: map[string]bool{ + "containerd-snapshotter": true, + }, + }, + }, + expectedChoice: imageStoreChoiceContainerdExplicit, + }) + + nonWindows = append(nonWindows, testCase{ + name: gd + " driver in config with containerd snapshotter feature disabled", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: gd, + Features: map[string]bool{ + "containerd-snapshotter": false, + }, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + }) + + nonWindows = append(nonWindows, testCase{ + name: gd + " driver in config with TEST_INTEGRATION_USE_GRAPHDRIVER env var set", + envTestUseGraphDriver: str("1"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + GraphDriver: gd, + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, }) } + + windows := []testCase{ + { + name: "default graphdriver on Windows", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriver, + }, + { + name: "windows driver on Windows", + envDockerDriver: str("windows"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceContainerdExplicit, + }, + { + name: "windowsfilter driver on Windows", + envDockerDriver: str("windowsfilter"), + cfg: &config.Config{ + CommonConfig: config.CommonConfig{ + Features: map[string]bool{}, + }, + }, + expectedChoice: imageStoreChoiceGraphdriverExplicit, + }, + } + + for i := range nonWindows { + nonWindows[i].skipPlatform = "windows" + } + tests = append(tests, nonWindows...) + + for i := range windows { + windows[i].onlyPlatform = "windows" + } + tests = append(tests, windows...) + + registeredDrivers := []string{"fuse-overlayfs", "overlay2", "btrfs", "zfs", "vfs"} + windowsRegisteredDrivers := []string{"vfs", "windowsfilter"} + + for _, os := range []string{"linux", "windows"} { + for _, tc := range tests { + if tc.skipPlatform != "" && os == tc.skipPlatform { + continue + } + if tc.onlyPlatform != "" && os != tc.onlyPlatform { + continue + } + + t.Run(os+"/"+tc.name, func(t *testing.T) { + if tc.envDockerDriver != nil { + t.Setenv("DOCKER_DRIVER", *tc.envDockerDriver) + } else { + t.Setenv("DOCKER_DRIVER", "") + } + if tc.envTestUseGraphDriver != nil { + t.Setenv("TEST_INTEGRATION_USE_GRAPHDRIVER", *tc.envTestUseGraphDriver) + } else { + t.Setenv("TEST_INTEGRATION_USE_GRAPHDRIVER", "") + } + + choice, err := determineImageStoreChoice(tc.cfg, determineImageStoreChoiceOptions{ + runtimeOS: os, + hasPriorDriver: func(root string) bool { + return tc.priorGraphDriver + }, + isRegisteredGraphdriver: func(driverName string) bool { + if os == "windows" { + return slices.Contains(windowsRegisteredDrivers, driverName) + } + return slices.Contains(registeredDrivers, driverName) + }, + }) + if tc.expectError { + assert.Error(t, err, "expected an error but got none") + } else { + assert.NilError(t, err) + } + + assert.Check(t, is.Equal(tc.expectedChoice, choice)) + }) + } + } }