Parsing of cimfs mounts & bug fixes

Signed-off-by: Amit Barve <ambarve@microsoft.com>

internal/layers/copy.go

@@ -7,10 +7,13 @@ import (
 
 	"github.com/containerd/containerd/api/types"
 	"github.com/containerd/containerd/errdefs"
-	"github.com/containerd/containerd/mount"
 )
 
-// TODO(ambarve): Below functions are direct copies of functions already defined in containerd-shim-runhcs-v1/rootfs.go, move everything inside this package (update LCOW code to handle that) and then get rid of the copies
+// TODO(ambarve): functions below are direct copies of functions already defined in
+// containerd-shim-runhcs-v1/rootfs.go, or in containerd 2.0 snapshotter/mount packages. Functions that are
+// copied from rootfs.go can be removed from rootfs.go once we are sure everything works as
+// expected. Containerd 2.0 functions are a bit difficult to remove because in order to do that we need to
+// update shim repo to vendor containerd 2.0
 
 // validateRootfsAndLayers checks to ensure we have appropriate information
 // for setting up the container's root filesystem. It ensures the following:
@@ -44,31 +47,45 @@ func validateRootfsAndLayers(rootfs []*types.Mount, layerFolders []string) error
 	return nil
 }
 
-// parseLegacyRootfsMount parses the rootfs mount format that we have traditionally
-// used for both Linux and Windows containers.
-// The mount format consists of:
-//   - The scratch folder path in m.Source, which contains sandbox.vhdx.
-//   - A mount option in the form parentLayerPaths=<JSON>, where JSON is an array of
-//     string paths to read-only layer directories. The exact contents of these layer
-//     directories are intepreteted differently for Linux and Windows containers.
-func parseLegacyRootfsMount(m *types.Mount) (string, []string, error) {
-	// parentLayerPaths are passed in layerN, layerN-1, ..., layer 0
-	//
-	// The OCI spec expects:
-	//   layerN, layerN-1, ..., layer0, scratch
+const (
+	// parentLayerPathsFlag is the options flag used to represent the JSON encoded
+	// list of parent layers required to use the layer
+	parentLayerPathsFlag = "parentLayerPaths="
+
+	// layerCimPathFlag is the option flag used to represent the path at which a layer CIM must be stored. This
+	// flag is only included if an image layer is being extracted onto the snapshot i.e the snapshot key has an
+	// UnpackKeyPrefix.
+	layerCimPathFlag = "cimpath="
+
+	// Similar to ParentLayerPathsFlag this is the optinos flag used to represent the JSON encoded list of
+	// parent layer CIMs
+	parentLayerCimPathsFlag = "parentCimPaths="
+)
+
+// GetParentPaths of the mount
+func getParentPaths(m *types.Mount) ([]string, error) {
 	var parentLayerPaths []string
 	for _, option := range m.Options {
-		if strings.HasPrefix(option, mount.ParentLayerPathsFlag) {
-			err := json.Unmarshal([]byte(option[len(mount.ParentLayerPathsFlag):]), &parentLayerPaths)
+		if strings.HasPrefix(option, parentLayerPathsFlag) {
+			err := json.Unmarshal([]byte(option[len(parentLayerPathsFlag):]), &parentLayerPaths)
+			if err != nil {
+				return nil, fmt.Errorf("failed to unmarshal parent layer paths from mount: %w", err)
+			}
+		}
+	}
+	return parentLayerPaths, nil
+}
+
+// GetParentPaths of the mount
+func getParentCimPaths(m *types.Mount) ([]string, error) {
+	var parentLayerCimPaths []string
+	for _, option := range m.Options {
+		if strings.HasPrefix(option, parentLayerCimPathsFlag) {
+			err := json.Unmarshal([]byte(option[len(parentLayerCimPathsFlag):]), &parentLayerCimPaths)
 			if err != nil {
-				// TODO (go1.20): use multierror via fmt.Errorf("...: %w; ...: %w", ...)
-				//nolint:errorlint // non-wrapping format verb for fmt.Errorf
-				return "", nil, fmt.Errorf("unmarshal parent layer paths from mount: %v: %w", err, errdefs.ErrFailedPrecondition)
+				return nil, fmt.Errorf("failed to unmarshal parent layer cim paths from mount: %w", err)
 			}
-			// Would perhaps be worthwhile to check for unrecognized options and return an error,
-			// but since this is a legacy layer mount we don't do that to avoid breaking anyone.
-			break
 		}
 	}
-	return m.Source, parentLayerPaths, nil
+	return parentLayerCimPaths, nil
 }

internal/layers/layers_wcow.go

@@ -5,15 +5,12 @@ package layers
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 	"time"
 
 	"github.com/containerd/containerd/api/types"
-	"github.com/containerd/containerd/mount"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/windows"
@@ -131,21 +128,13 @@ type wcowIsolatedWCIFSLayerCloser struct {
 }
 
 func parseForkedCimMount(containerID string, m *types.Mount) (*wcowHostForkedCIMLayers, error) {
-	var parentLayerPaths []string
-	var parentCimPaths []string
-	for _, option := range m.Options {
-		if strings.HasPrefix(option, mount.ParentLayerPathsFlag) {
-			err := json.Unmarshal([]byte(option[len(mount.ParentLayerPathsFlag):]), &parentLayerPaths)
-			if err != nil {
-				return nil, err
-			}
-			// TODO(ambarve): use proper constatns for path flag
-		} else if strings.HasPrefix(option, "parentCimPaths=") {
-			err := json.Unmarshal([]byte(option[len("parentCimPaths="):]), &parentCimPaths)
-			if err != nil {
-				return nil, err
-			}
-		}
+	parentLayerPaths, err := getParentPaths(m)
+	if err != nil {
+		return nil, err
+	}
+	parentCimPaths, err := getParentCimPaths(m)
+	if err != nil {
+		return nil, err
 	}
 	if len(parentLayerPaths) != len(parentCimPaths) {
 		return nil, fmt.Errorf("invalid mount, number of parent layer paths & cim paths should be same")
@@ -188,7 +177,8 @@ func ParseWCOWLayers(containerID string, vm *uvm.UtilityVM, rootfs []*types.Moun
 	// snapshotter code.
 	switch m.Type {
 	case "windows-layer":
-		scratchLayer, parentLayers, err := parseLegacyRootfsMount(m)
+		scratchLayer := m.Source
+		parentLayers, err := getParentPaths(m)
 		if err != nil {
 			return nil, err
 		}
@@ -415,7 +405,7 @@ func (l *wcowHostForkedCIMLayers) Mount(ctx context.Context) (_ *MountedWCOWLaye
 			RootFS: mountPath,
 			MountedLayerPaths: []MountedWCOWLayer{{
 				LayerID:     layerID,
-				MountedPath: mountPath,
+				MountedPath: volume,
 			}},
 		}, &wcowHostForkedCIMLayerCloser{
 			containerID:      l.containerID,
@@ -570,10 +560,11 @@ func ParseWCOWUVMBootFilesFromLayers(ctx context.Context, rootfs []*types.Mount,
 		// snapshotter code.
 		switch m.Type {
 		case "windows-layer":
-			scratchLayer, parentLayers, err = parseLegacyRootfsMount(m)
+			parentLayers, err = getParentPaths(m)
 			if err != nil {
 				return nil, err
 			}
+			scratchLayer = m.Source
 		default:
 			return nil, fmt.Errorf("mount type %s is not supported for UVM boot", m.Type)
 		}