Merge pull request #46539 from TBBle/containerd_image_store_pr46402

c8d: Just enough Windows support to run the test suite

builder/builder-next/controller.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"runtime"
 	"time"
 
 	ctd "github.com/containerd/containerd"
@@ -91,6 +92,14 @@ func newSnapshotterController(ctx context.Context, rt http.RoundTripper, opt Opt
 	nc := netproviders.Opt{
 		Mode: "host",
 	}
+
+	// HACK! Windows doesn't have 'host' mode networking.
+	if runtime.GOOS == "windows" {
+		nc = netproviders.Opt{
+			Mode: "auto",
+		}
+	}
+
 	dns := getDNSConfig(opt.DNSConfig)
 
 	wo, err := containerd.NewWorkerOpt(opt.Root, opt.ContainerdAddress, opt.Snapshotter, opt.ContainerdNamespace,

daemon/containerd/image_builder.go

@@ -350,6 +350,22 @@ func (rw *rwlayer) Commit() (_ builder.ROLayer, outErr error) {
 		}
 	}()
 
+	// Unmount the layer, required by the containerd windows snapshotter.
+	// The windowsfilter graphdriver does this inside its own Diff method.
+	//
+	// The only place that calls this in-tree is (b *Builder) exportImage and
+	// that is called from the end of (b *Builder) performCopy which has a
+	// `defer rwLayer.Release()` pending.
+	//
+	// After the snapshotter.Commit the source snapshot is deleted anyway and
+	// it shouldn't be accessed afterwards.
+	if rw.root != "" {
+		if err := mount.UnmountAll(rw.root, 0); err != nil && !errors.Is(err, os.ErrNotExist) {
+			log.G(ctx).WithError(err).WithField("root", rw.root).Error("failed to unmount RWLayer")
+			return nil, err
+		}
+	}
+
 	err = snapshotter.Commit(ctx, key, rw.key)
 	if err != nil && !cerrdefs.IsAlreadyExists(err) {
 		return nil, err
@@ -389,7 +405,7 @@ func (rw *rwlayer) Release() (outErr error) {
 	}
 
 	if err := mount.UnmountAll(rw.root, 0); err != nil && !errors.Is(err, os.ErrNotExist) {
-		log.G(context.TODO()).WithError(err).WithField("root", rw.root).Error("failed to unmount ROLayer")
+		log.G(context.TODO()).WithError(err).WithField("root", rw.root).Error("failed to unmount RWLayer")
 		return err
 	}
 	if err := os.Remove(rw.root); err != nil && !errors.Is(err, os.ErrNotExist) {

daemon/containerd/image_commit.go

@@ -264,18 +264,15 @@ func (i *ImageService) createDiff(ctx context.Context, name string, sn snapshots
 
 // applyDiffLayer will apply diff layer content created by createDiff into the snapshotter.
 func (i *ImageService) applyDiffLayer(ctx context.Context, name string, containerID string, sn snapshots.Snapshotter, differ diff.Applier, diffDesc ocispec.Descriptor) (retErr error) {
-	var (
-		key    = uniquePart() + "-" + name
-		mounts []mount.Mount
-		err    error
-	)
+	// Let containerd know that this snapshot is only for diff-applying.
+	key := snapshots.UnpackKeyPrefix + "-" + uniquePart() + "-" + name
 
 	info, err := sn.Stat(ctx, containerID)
 	if err != nil {
 		return err
 	}
 
-	mounts, err = sn.Prepare(ctx, key, info.Parent)
+	mounts, err := sn.Prepare(ctx, key, info.Parent)
 	if err != nil {
 		return fmt.Errorf("failed to prepare snapshot: %w", err)
 	}

daemon/containerd/image_snapshot.go

@@ -87,10 +87,12 @@ func (i *ImageService) prepareInitLayer(ctx context.Context, id string, parent s
 		return err
 	}
 
-	if err := mount.WithTempMount(ctx, mounts, func(root string) error {
-		return setupInit(root)
-	}); err != nil {
-		return err
+	if setupInit != nil {
+		if err := mount.WithTempMount(ctx, mounts, func(root string) error {
+			return setupInit(root)
+		}); err != nil {
+			return err
+		}
 	}
 
 	return snapshotter.Commit(ctx, id+"-init", id+"-init-key")

daemon/containerd/service.go

@@ -19,7 +19,6 @@ import (
 	dimages "github.com/docker/docker/daemon/images"
 	"github.com/docker/docker/daemon/snapshotter"
 	"github.com/docker/docker/errdefs"
-	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/registry"
@@ -156,11 +155,6 @@ func (i *ImageService) UpdateConfig(maxDownloads, maxUploads int) {
 	log.G(context.TODO()).Warn("max downloads and uploads is not yet implemented with the containerd store")
 }
 
-// GetLayerFolders returns the layer folders from an image RootFS.
-func (i *ImageService) GetLayerFolders(img *image.Image, rwLayer layer.RWLayer) ([]string, error) {
-	return nil, errdefs.NotImplemented(errors.New("not implemented"))
-}
-
 // GetContainerLayerSize returns the real size & virtual size of the container.
 func (i *ImageService) GetContainerLayerSize(ctx context.Context, containerID string) (int64, int64, error) {
 	ctr := i.containers.Get(containerID)

daemon/containerd/service_unix.go

@@ -0,0 +1,15 @@
+//go:build linux || freebsd
+
+package containerd
+
+import (
+	"github.com/docker/docker/errdefs"
+	"github.com/docker/docker/image"
+	"github.com/docker/docker/layer"
+	"github.com/pkg/errors"
+)
+
+// GetLayerFolders returns the layer folders from an image RootFS.
+func (i *ImageService) GetLayerFolders(img *image.Image, rwLayer layer.RWLayer, containerID string) ([]string, error) {
+	return nil, errdefs.NotImplemented(errors.New("not implemented"))
+}

daemon/containerd/service_windows.go

@@ -0,0 +1,31 @@
+package containerd
+
+import (
+	"context"
+
+	"github.com/docker/docker/image"
+	"github.com/docker/docker/layer"
+	"github.com/pkg/errors"
+)
+
+// GetLayerFolders returns the layer folders from an image RootFS.
+func (i *ImageService) GetLayerFolders(img *image.Image, rwLayer layer.RWLayer, containerID string) ([]string, error) {
+	if rwLayer != nil {
+		return nil, errors.New("RWLayer is unexpectedly not nil")
+	}
+
+	snapshotter := i.client.SnapshotService(i.StorageDriver())
+	mounts, err := snapshotter.Mounts(context.TODO(), containerID)
+	if err != nil {
+		return nil, errors.Wrapf(err, "snapshotter.Mounts failed: container %s", containerID)
+	}
+
+	// This is the same logic used by the hcsshim containerd runtime shim's createInternal
+	// to convert an array of Mounts into windows layers.
+	// See https://github.com/microsoft/hcsshim/blob/release/0.11/cmd/containerd-shim-runhcs-v1/service_internal.go
+	parentPaths, err := mounts[0].GetParentPaths()
+	if err != nil {
+		return nil, errors.Wrapf(err, "GetParentPaths failed: container %s", containerID)
+	}
+	return append(parentPaths, mounts[0].Source), nil
+}

daemon/daemon.go

@@ -1062,7 +1062,11 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 	// be set through an environment variable, a daemon start parameter, or chosen through
 	// initialization of the layerstore through driver priority order for example.
 	driverName := os.Getenv("DOCKER_DRIVER")
-	if isWindows {
+	if isWindows && d.UsesSnapshotter() {
+		// Containerd WCOW snapshotter
+		driverName = "windows"
+	} else if isWindows {
+		// Docker WCOW graphdriver
 		driverName = "windowsfilter"
 	} else if driverName != "" {
 		log.G(ctx).Infof("Setting the storage driver from the $DOCKER_DRIVER environment variable (%s)", driverName)

daemon/image_service.go

@@ -65,7 +65,7 @@ type ImageService interface {
 
 	// Windows specific
 
-	GetLayerFolders(img *image.Image, rwLayer layer.RWLayer) ([]string, error)
+	GetLayerFolders(img *image.Image, rwLayer layer.RWLayer, containerID string) ([]string, error)
 
 	// Build
 

daemon/images/image.go

@@ -47,7 +47,7 @@ type manifest struct {
 }
 
 func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error {
-	// Only makes sense when conatinerd image store is used
+	// Only makes sense when containerd image store is used
 	panic("not implemented")
 }
 

daemon/images/image_unix.go

@@ -11,7 +11,7 @@ import (
 )
 
 // GetLayerFolders returns the layer folders from an image RootFS
-func (i *ImageService) GetLayerFolders(img *image.Image, rwLayer layer.RWLayer) ([]string, error) {
+func (i *ImageService) GetLayerFolders(img *image.Image, rwLayer layer.RWLayer, containerID string) ([]string, error) {
 	// Windows specific
 	panic("not implemented")
 }

daemon/images/image_windows.go

@@ -15,7 +15,7 @@ func (i *ImageService) GetContainerLayerSize(ctx context.Context, containerID st
 }
 
 // GetLayerFolders returns the layer folders from an image RootFS
-func (i *ImageService) GetLayerFolders(img *image.Image, rwLayer layer.RWLayer) ([]string, error) {
+func (i *ImageService) GetLayerFolders(img *image.Image, rwLayer layer.RWLayer, containerID string) ([]string, error) {
 	folders := []string{}
 	rd := len(img.RootFS.DiffIDs)
 	for index := 1; index <= rd; index++ {

daemon/oci_windows.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/Microsoft/hcsshim"
 	coci "github.com/containerd/containerd/oci"
 	"github.com/containerd/log"
 	containertypes "github.com/docker/docker/api/types/container"
@@ -138,9 +139,9 @@ func (daemon *Daemon) createSpec(ctx context.Context, daemonCfg *configStore, c
 		}
 	}
 	s.Process.User.Username = c.Config.User
-	s.Windows.LayerFolders, err = daemon.imageService.GetLayerFolders(img, c.RWLayer)
+	s.Windows.LayerFolders, err = daemon.imageService.GetLayerFolders(img, c.RWLayer, c.ID)
 	if err != nil {
-		return nil, errors.Wrapf(err, "container %s", c.ID)
+		return nil, errors.Wrapf(err, "GetLayerFolders failed: container %s", c.ID)
 	}
 
 	// Get endpoints for the libnetwork allocated networks to the container
@@ -249,7 +250,24 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S
 			return errors.New("createSpecWindowsFields: BaseFS of container " + c.ID + " is unexpectedly empty")
 		}
 
-		s.Root.Path = c.BaseFS // This is not set for Hyper-V containers
+		if daemon.UsesSnapshotter() {
+			// daemon.Mount() for the snapshotters actually mounts the filesystem to the host
+			// using containerd/mount.All and BaseFS is the directory where this is mounted.
+			// This is consistent with Linux-based graphdriver implementations.
+			// For the windowsfilter graphdriver, the underlying Get() call does not actually mount
+			// the filesystem to a path, and BaseFS is the Volume GUID of the prepared/activated
+			// filesystem.
+
+			// The spec for Root.Path for Windows specifies that for Process-isolated containers,
+			// it must be in the Volume GUID (\\?\\Volume{GUID} style), not a host-mounted directory.
+			backingDevicePath, err := getBackingDeviceForContainerdMount(c.BaseFS)
+			if err != nil {
+				return errors.Wrapf(err, "createSpecWindowsFields: Failed to get backing device of BaseFS of container %s", c.ID)
+			}
+			s.Root.Path = backingDevicePath
+		} else {
+			s.Root.Path = c.BaseFS // This is not set for Hyper-V containers
+		}
 		if !strings.HasSuffix(s.Root.Path, `\`) {
 			s.Root.Path = s.Root.Path + `\` // Ensure a correctly formatted volume GUID path \\?\Volume{GUID}\
 		}
@@ -275,6 +293,48 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S
 	return nil
 }
 
+// getBackingDeviceForContainerdMount extracts the backing device or directory mounted at mountPoint
+// by containerd's mount.Mount implementation for Windows.
+func getBackingDeviceForContainerdMount(mountPoint string) (string, error) {
+	// NOTE: This relies on details of the behaviour of containerd's mount implementation for Windows,
+	// and so is somewhat fragile.
+	// TODO: Upstream this into the mount package.
+	// The implementation would be the same, but it'll be better-encapsulated.
+
+	// See containerd/containerd/mount/mount_windows.go
+	// This is mostly just copied from mount.Unmount
+
+	const sourceStreamName = "containerd.io-source"
+
+	mountPoint = filepath.Clean(mountPoint)
+	adsFile := mountPoint + ":" + sourceStreamName
+	var layerPath string
+
+	if _, err := os.Lstat(adsFile); err == nil {
+		layerPathb, err := os.ReadFile(mountPoint + ":" + sourceStreamName)
+		if err != nil {
+			return "", fmt.Errorf("failed to retrieve layer source for mount %s: %w", mountPoint, err)
+		}
+		layerPath = string(layerPathb)
+	}
+
+	if layerPath == "" {
+		return "", fmt.Errorf("no layer source for mount %s", mountPoint)
+	}
+
+	home, layerID := filepath.Split(layerPath)
+	di := hcsshim.DriverInfo{
+		HomeDir: home,
+	}
+
+	backingDevice, err := hcsshim.GetLayerMountPath(di, layerID)
+	if err != nil {
+		return "", fmt.Errorf("failed to retrieve backing device for layer %s: %w", mountPoint, err)
+	}
+
+	return backingDevice, nil
+}
+
 var errInvalidCredentialSpecSecOpt = errdefs.InvalidParameter(fmt.Errorf("invalid credential spec security option - value must be prefixed by 'file://', 'registry://', or 'raw://' followed by a non-empty value"))
 
 // setWindowsCredentialSpec sets the spec's `Windows.CredentialSpec`