daemon: Fix image store choice priority for prior graphdriver state

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 <pawel.gronowski@docker.com>

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() {

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
 }

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))
+			})
+		}
+	}
 }