From cd4718678311a80fd8aba127afacdb22dddadae0 Mon Sep 17 00:00:00 2001
From: "Paul \"TBBle\" Hampson" <Paul.Hampson@Pobox.com>
Date: Thu, 16 Jul 2020 16:56:20 +1000
Subject: [PATCH] Split LocalMounter.Mount into Windows and Unix

Due to current deficiencies in the Windows implementation of
containerd's Mount.Mount, we need different behaviour for mounting on
Windows and Unix platforms.

Specifically:
* Windows mounts can only mount in-place, and hence only one mount
  should be in the list.
* BuildKit doesn't own the mount directory, so should not try and remove
  it on unmount.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
---
 snapshot/localmounter.go         | 42 --------------------------------
 snapshot/localmounter_unix.go    | 41 +++++++++++++++++++++++++++++++
 snapshot/localmounter_windows.go | 35 +++++++++++++++++++++++---
 3 files changed, 73 insertions(+), 45 deletions(-)

diff --git a/snapshot/localmounter.go b/snapshot/localmounter.go
index 545c66c5..9ddb7c1a 100644
--- a/snapshot/localmounter.go
+++ b/snapshot/localmounter.go
@@ -1,12 +1,9 @@
 package snapshot
 
 import (
-	"io/ioutil"
-	"os"
 	"sync"
 
 	"github.com/containerd/containerd/mount"
-	"github.com/pkg/errors"
 )
 
 type Mounter interface {
@@ -33,42 +30,3 @@ type localMounter struct {
 	target    string
 	release   func() error
 }
-
-func (lm *localMounter) Mount() (string, error) {
-	lm.mu.Lock()
-	defer lm.mu.Unlock()
-
-	if lm.mounts == nil {
-		mounts, release, err := lm.mountable.Mount()
-		if err != nil {
-			return "", err
-		}
-		lm.mounts = mounts
-		lm.release = release
-	}
-
-	if len(lm.mounts) == 1 && (lm.mounts[0].Type == "bind" || lm.mounts[0].Type == "rbind") {
-		ro := false
-		for _, opt := range lm.mounts[0].Options {
-			if opt == "ro" {
-				ro = true
-				break
-			}
-		}
-		if !ro {
-			return lm.mounts[0].Source, nil
-		}
-	}
-
-	dir, err := ioutil.TempDir("", "buildkit-mount")
-	if err != nil {
-		return "", errors.Wrap(err, "failed to create temp dir")
-	}
-
-	if err := mount.All(lm.mounts, dir); err != nil {
-		os.RemoveAll(dir)
-		return "", errors.Wrapf(err, "failed to mount %s: %+v", dir, lm.mounts)
-	}
-	lm.target = dir
-	return dir, nil
-}
diff --git a/snapshot/localmounter_unix.go b/snapshot/localmounter_unix.go
index 4e285c6c..78d395b1 100644
--- a/snapshot/localmounter_unix.go
+++ b/snapshot/localmounter_unix.go
@@ -3,12 +3,53 @@
 package snapshot
 
 import (
+	"io/ioutil"
 	"os"
 	"syscall"
 
 	"github.com/containerd/containerd/mount"
+	"github.com/pkg/errors"
 )
 
+func (lm *localMounter) Mount() (string, error) {
+	lm.mu.Lock()
+	defer lm.mu.Unlock()
+
+	if lm.mounts == nil {
+		mounts, release, err := lm.mountable.Mount()
+		if err != nil {
+			return "", err
+		}
+		lm.mounts = mounts
+		lm.release = release
+	}
+
+	if len(lm.mounts) == 1 && (lm.mounts[0].Type == "bind" || lm.mounts[0].Type == "rbind") {
+		ro := false
+		for _, opt := range lm.mounts[0].Options {
+			if opt == "ro" {
+				ro = true
+				break
+			}
+		}
+		if !ro {
+			return lm.mounts[0].Source, nil
+		}
+	}
+
+	dir, err := ioutil.TempDir("", "buildkit-mount")
+	if err != nil {
+		return "", errors.Wrap(err, "failed to create temp dir")
+	}
+
+	if err := mount.All(lm.mounts, dir); err != nil {
+		os.RemoveAll(dir)
+		return "", errors.Wrapf(err, "failed to mount %s: %+v", dir, lm.mounts)
+	}
+	lm.target = dir
+	return dir, nil
+}
+
 func (lm *localMounter) Unmount() error {
 	lm.mu.Lock()
 	defer lm.mu.Unlock()
diff --git a/snapshot/localmounter_windows.go b/snapshot/localmounter_windows.go
index 03e8f735..a5cd1a25 100644
--- a/snapshot/localmounter_windows.go
+++ b/snapshot/localmounter_windows.go
@@ -1,11 +1,41 @@
 package snapshot
 
 import (
-	"os"
-
+	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/mount"
+	"github.com/pkg/errors"
 )
 
+func (lm *localMounter) Mount() (string, error) {
+	lm.mu.Lock()
+	defer lm.mu.Unlock()
+
+	if lm.mounts == nil {
+		mounts, release, err := lm.mountable.Mount()
+		if err != nil {
+			return "", err
+		}
+		lm.mounts = mounts
+		lm.release = release
+	}
+
+	// Windows can only mount a single mount at a given location.
+	// Parent layers are carried in Options, opaquely to localMounter.
+	if len(lm.mounts) != 1 {
+		return "", errors.Wrapf(errdefs.ErrNotImplemented, "request to mount %d layers, only 1 is supported", len(lm.mounts))
+	}
+
+	// Windows mounts always activate in-place, so the target of the mount must be the source directory.
+	// See https://github.com/containerd/containerd/pull/2366
+	dir := lm.mounts[0].Source
+
+	if err := lm.mounts[0].Mount(dir); err != nil {
+		return "", errors.Wrapf(err, "failed to mount in-place: %v", lm.mounts[0])
+	}
+	lm.target = dir
+	return lm.target, nil
+}
+
 func (lm *localMounter) Unmount() error {
 	lm.mu.Lock()
 	defer lm.mu.Unlock()
@@ -14,7 +44,6 @@ func (lm *localMounter) Unmount() error {
 		if err := mount.Unmount(lm.target, 0); err != nil {
 			return err
 		}
-		os.RemoveAll(lm.target)
 		lm.target = ""
 	}