Merge pull request #51270 from robmry/inspect-api-bc

Inspect: API v1.51 compatibility

daemon/containerd/image_inspect.go

@@ -12,6 +12,7 @@ import (
 	"github.com/distribution/reference"
 	dockerspec "github.com/moby/docker-image-spec/specs-go/v1"
 	imagetypes "github.com/moby/moby/api/types/image"
+	"github.com/moby/moby/api/types/storage"
 	"github.com/moby/moby/v2/daemon/server/imagebackend"
 	"github.com/moby/moby/v2/internal/sliceutil"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -98,6 +99,9 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts im
 			},
 		},
 		Parent: parent, // field is deprecated with the legacy builder, but returned by the API if present.
+
+		// GraphDriver is omitted in API v1.52 unless using a graphdriver.
+		GraphDriverLegacy: &storage.DriverData{Name: i.snapshotter},
 	}
 
 	if multi.Best != nil {

daemon/server/imagebackend/image.go

@@ -7,6 +7,7 @@ import (
 	"github.com/moby/moby/api/types/container"
 	imagetypes "github.com/moby/moby/api/types/image"
 	"github.com/moby/moby/api/types/registry"
+	"github.com/moby/moby/api/types/storage"
 	"github.com/moby/moby/v2/daemon/internal/filters"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
@@ -92,4 +93,8 @@ type InspectData struct {
 	//
 	// This field is removed in API v1.45, but used for API <= v1.44 responses.
 	ContainerConfig *container.Config
+
+	// GraphDriverLegacy is used for API versions < v1.52, which included the
+	// name of the snapshotter the GraphDriver field.
+	GraphDriverLegacy *storage.DriverData
 }

daemon/server/router/container/inspect.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 
 	"github.com/moby/moby/api/types/container"
+	"github.com/moby/moby/api/types/storage"
 	"github.com/moby/moby/api/types/versions"
 	"github.com/moby/moby/v2/daemon/internal/compat"
 	"github.com/moby/moby/v2/daemon/internal/stringid"
@@ -69,6 +70,15 @@ func (c *containerRouter) getContainersByName(ctx context.Context, w http.Respon
 				},
 			}))
 		}
+
+		// Restore the GraphDriver field, now omitted when a snapshotter is used.
+		// Remove the Storage field that replaced it.
+		if ctr.GraphDriver == nil && ctr.Storage != nil && ctr.Storage.RootFS != nil && ctr.Storage.RootFS.Snapshot != nil {
+			ctr.GraphDriver = &storage.DriverData{
+				Name: ctr.Storage.RootFS.Snapshot.Name,
+			}
+			ctr.Storage = nil
+		}
 	}
 
 	return httputils.WriteJSON(w, http.StatusOK, compat.Wrap(ctr, wrapOpts...))

daemon/server/router/image/image_routes.go

@@ -440,6 +440,11 @@ func (ir *imageRouter) getImagesByName(ctx context.Context, w http.ResponseWrite
 				"Config": legacyConfigFields["v1.50-v1.51"],
 			}))
 		}
+
+		// Restore the GraphDriver field, now omitted when a snapshotter is used.
+		if imageInspect.GraphDriver == nil && inspectData.GraphDriverLegacy != nil {
+			imageInspect.GraphDriver = inspectData.GraphDriverLegacy
+		}
 	} else {
 		if inspectData.Parent != "" {
 			// field is deprecated, but still included in response when present (built with legacy builder).

integration-cli/docker_api_inspect_test.go

@@ -19,16 +19,10 @@ func (s *DockerAPISuite) TestInspectAPIContainerResponse(c *testing.T) {
 
 	keysBase := []string{
 		"Id", "State", "Created", "Path", "Args", "Config", "Image", "NetworkSettings",
-		"ResolvConfPath", "HostnamePath", "HostsPath", "LogPath", "Name", "Driver", "MountLabel", "ProcessLabel",
+		"ResolvConfPath", "HostnamePath", "HostsPath", "LogPath", "Name", "Driver", "MountLabel", "ProcessLabel", "GraphDriver",
 		"Mounts",
 	}
 
-	if testEnv.UsingSnapshotter() {
-		keysBase = append(keysBase, "Storage")
-	} else {
-		keysBase = append(keysBase, "GraphDriver")
-	}
-
 	cases := []struct {
 		version string
 		keys    []string

integration/daemon/default_storage_test.go

@@ -4,6 +4,8 @@ import (
 	"testing"
 
 	containertypes "github.com/moby/moby/api/types/container"
+	"github.com/moby/moby/api/types/storage"
+	"github.com/moby/moby/client"
 	"github.com/moby/moby/v2/internal/testutil"
 	"github.com/moby/moby/v2/internal/testutil/daemon"
 	"gotest.tools/v3/assert"
@@ -84,3 +86,91 @@ func TestGraphDriverPersistence(t *testing.T) {
 	assert.Check(t, containerInspect.GraphDriver != nil, "GraphDriver should be set for graphdriver backend")
 	assert.Check(t, is.Equal(containerInspect.GraphDriver.Name, prevDriver), "Container graphdriver data should match")
 }
+
+// TestInspectGraphDriverAPIBC checks API backward compatibility of the GraphDriver field in image/container inspect.
+func TestInspectGraphDriverAPIBC(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows", "Windows does not support running sub-daemons")
+	t.Setenv("DOCKER_DRIVER", "")
+	t.Setenv("DOCKER_GRAPHDRIVER", "")
+	t.Setenv("TEST_INTEGRATION_USE_GRAPHDRIVER", "")
+	ctx := testutil.StartSpan(baseContext, t)
+
+	tests := []struct {
+		name          string
+		apiVersion    string
+		storageDriver string
+
+		expContainerdSnapshotter bool
+		expGraphDriver           string
+		expRootFSStorage         bool
+	}{
+		{
+			name:                     "vCurrent/containerd",
+			expContainerdSnapshotter: true,
+			expRootFSStorage:         true,
+		},
+		{
+			name:                     "v1.51/containerd",
+			apiVersion:               "v1.51",
+			expContainerdSnapshotter: true,
+			expGraphDriver:           "overlayfs",
+		},
+		{
+			name:           "vCurrent/graphdriver",
+			storageDriver:  "vfs",
+			expGraphDriver: "vfs",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			d := daemon.New(t)
+			defer d.Stop(t)
+			d.StartWithBusybox(ctx, t, "--iptables=false", "--ip6tables=false", "--storage-driver="+tc.storageDriver)
+			c := d.NewClientT(t, client.WithVersion(tc.apiVersion))
+
+			// Check selection of containerd / storage-driver worked.
+			info := d.Info(t)
+			if tc.expContainerdSnapshotter {
+				assert.Check(t, is.Equal(info.Driver, "overlayfs"))
+				assert.Check(t, is.Equal(info.DriverStatus[0][0], "driver-type"))
+				assert.Check(t, is.Equal(info.DriverStatus[0][1], "io.containerd.snapshotter.v1"))
+			} else {
+				assert.Check(t, is.Equal(info.Driver, "vfs"))
+				assert.Check(t, is.Len(info.DriverStatus, 0))
+			}
+
+			const testImage = "busybox:latest"
+			ctr, err := c.ContainerCreate(ctx, &containertypes.Config{Image: testImage}, nil, nil, nil, "test-container")
+			assert.NilError(t, err)
+			defer func() { _ = c.ContainerRemove(ctx, ctr.ID, client.ContainerRemoveOptions{Force: true}) }()
+
+			if imageInspect, err := c.ImageInspect(ctx, testImage); assert.Check(t, err) {
+				if tc.expGraphDriver != "" {
+					if assert.Check(t, imageInspect.GraphDriver != nil) {
+						assert.Check(t, is.Equal(imageInspect.GraphDriver.Name, tc.expGraphDriver))
+					}
+				} else {
+					assert.Check(t, is.Nil(imageInspect.GraphDriver))
+				}
+			}
+
+			if containerInspect, err := c.ContainerInspect(ctx, ctr.ID); assert.Check(t, err) {
+				if tc.expGraphDriver != "" {
+					if assert.Check(t, containerInspect.GraphDriver != nil) {
+						assert.Check(t, is.Equal(containerInspect.GraphDriver.Name, tc.expGraphDriver))
+					}
+				} else {
+					assert.Check(t, is.Nil(containerInspect.GraphDriver))
+				}
+				if tc.expRootFSStorage {
+					assert.DeepEqual(t, containerInspect.Storage, &storage.Storage{
+						RootFS: &storage.RootFSStorage{Snapshot: &storage.RootFSStorageSnapshot{Name: "overlayfs"}},
+					})
+				} else {
+					assert.Check(t, is.Nil(containerInspect.Storage))
+				}
+			}
+		})
+	}
+}