Merge pull request #6034 from claudiubelu/windows/fixes-image-volume

Fixes Windows containers with image volumes

mount/mount_windows.go

@@ -18,6 +18,7 @@ package mount
 
 import (
 	"encoding/json"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -50,15 +51,21 @@ func (m *Mount) Mount(target string) error {
 	if err = hcsshim.ActivateLayer(di, layerID); err != nil {
 		return errors.Wrapf(err, "failed to activate layer %s", m.Source)
 	}
-	defer func() {
-		if err != nil {
-			hcsshim.DeactivateLayer(di, layerID)
-		}
-	}()
 
 	if err = hcsshim.PrepareLayer(di, layerID, parentLayerPaths); err != nil {
 		return errors.Wrapf(err, "failed to prepare layer %s", m.Source)
 	}
+
+	// We can link the layer mount path to the given target. It is an UNC path, and it needs
+	// a trailing backslash.
+	mountPath, err := hcsshim.GetLayerMountPath(di, layerID)
+	if err != nil {
+		return errors.Wrapf(err, "failed to get layer mount path for %s", m.Source)
+	}
+	mountPath = mountPath + `\`
+	if err = os.Symlink(mountPath, target); err != nil {
+		return errors.Wrapf(err, "failed to link mount to taget %s", target)
+	}
 	return nil
 }
 

pkg/cri/opts/container.go

@@ -20,6 +20,8 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	goruntime "runtime"
+	"strings"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/containers"
@@ -75,29 +77,51 @@ func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts {
 		// refer to https://github.com/containerd/containerd/pull/1868
 		// https://github.com/containerd/containerd/pull/1785
 		defer os.Remove(root) // nolint: errcheck
-		if err := mount.All(mounts, root); err != nil {
-			return errors.Wrap(err, "failed to mount")
-		}
-		defer func() {
-			if uerr := mount.Unmount(root, 0); uerr != nil {
-				log.G(ctx).WithError(uerr).Errorf("Failed to unmount snapshot %q", c.SnapshotKey)
+
+		unmounter := func(mountPath string) {
+			if uerr := mount.Unmount(mountPath, 0); uerr != nil {
+				log.G(ctx).WithError(uerr).Errorf("Failed to unmount snapshot %q", root)
 				if err == nil {
 					err = uerr
 				}
 			}
-		}()
+		}
+
+		var mountPaths []string
+		if goruntime.GOOS == "windows" {
+			for _, m := range mounts {
+				// appending the layerID to the root.
+				mountPath := filepath.Join(root, filepath.Base(m.Source))
+				mountPaths = append(mountPaths, mountPath)
+				if err := m.Mount(mountPath); err != nil {
+					return err
+				}
+
+				defer unmounter(m.Source)
+			}
+		} else {
+			mountPaths = append(mountPaths, root)
+			if err := mount.All(mounts, root); err != nil {
+				return errors.Wrap(err, "failed to mount")
+			}
+			defer unmounter(root)
+		}
 
 		for host, volume := range volumeMounts {
-			src := filepath.Join(root, volume)
-			if _, err := os.Stat(src); err != nil {
-				if os.IsNotExist(err) {
-					// Skip copying directory if it does not exist.
-					continue
+			// The volume may have been defined with a C: prefix, which we can't use here.
+			volume = strings.TrimPrefix(volume, "C:")
+			for _, mountPath := range mountPaths {
+				src := filepath.Join(mountPath, volume)
+				if _, err := os.Stat(src); err != nil {
+					if os.IsNotExist(err) {
+						// Skip copying directory if it does not exist.
+						continue
+					}
+					return errors.Wrap(err, "stat volume in rootfs")
+				}
+				if err := copyExistingContents(src, host); err != nil {
+					return errors.Wrap(err, "taking runtime copy of volume")
 				}
-				return errors.Wrap(err, "stat volume in rootfs")
-			}
-			if err := copyExistingContents(src, host); err != nil {
-				return errors.Wrap(err, "taking runtime copy of volume")
 			}
 		}
 		return nil

pkg/cri/opts/spec_windows.go

@@ -18,6 +18,7 @@ package opts
 
 import (
 	"context"
+	"os"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -119,19 +120,28 @@ func WithWindowsMounts(osi osinterface.OS, config *runtime.ContainerConfig, extr
 			// paths, so don't use it.
 			if !namedPipePath(src) {
 				if _, err := osi.Stat(src); err != nil {
-					// If the source doesn't exist, return an error instead
-					// of creating the source. This aligns with Docker's
-					// behavior on windows.
-					return errors.Wrapf(err, "failed to stat %q", src)
+					// Create the host path if it doesn't exist. This will align
+					// the behavior with the Linux implementation, but it doesn't
+					// align with Docker's behavior on Windows.
+					if !os.IsNotExist(err) {
+						return errors.Wrapf(err, "failed to stat %q", src)
+					}
+					if err := osi.MkdirAll(src, 0755); err != nil {
+						return errors.Wrapf(err, "failed to mkdir %q", src)
+					}
 				}
 				var err error
 				src, err = osi.ResolveSymbolicLink(src)
 				if err != nil {
 					return errors.Wrapf(err, "failed to resolve symlink %q", src)
 				}
-				// hcsshim requires clean path, especially '/' -> '\'.
+				// hcsshim requires clean path, especially '/' -> '\'. Additionally,
+				// for the destination, absolute paths should have the C: prefix.
 				src = filepath.Clean(src)
 				dst = filepath.Clean(dst)
+				if dst[0] == '\\' {
+					dst = "C:" + dst
+				}
 			}
 
 			var options []string