Update cimfs snapshotter & differ for new hcsshim interface

hcsshim recently [updated](microsoft/hcsshim@1d406d0)
the interface of APIs that are used for importing OCI layers. Plus, it now expects that the CimFS snapshotter mounts contain the full cim paths for parent layers. This change updates the cimfs differ & snapshotter to use that new interface.

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

core/mount/mount_windows.go

@@ -112,9 +112,20 @@ func (m *Mount) mount(target string) (retErr error) {
 	return nil
 }
 
-// ParentLayerPathsFlag is the options flag used to represent the JSON encoded
-// list of parent layers required to use the layer
-const ParentLayerPathsFlag = "parentLayerPaths="
+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 (m *Mount) GetParentPaths() ([]string, error) {
@@ -130,6 +141,40 @@ func (m *Mount) GetParentPaths() ([]string, error) {
 	return parentLayerPaths, nil
 }
 
+// gets the paths of the parent cims of this mount
+func (m *Mount) GetParentCimPaths() ([]string, error) {
+	if m.Type != "CimFS" {
+		return nil, fmt.Errorf("invalid windows mount type: '%s'", m.Type)
+	}
+	var parentLayerCimPaths []string
+	for _, option := range m.Options {
+		paths, found := strings.CutPrefix(option, ParentLayerCimPathsFlag)
+		if found {
+			err := json.Unmarshal([]byte(paths), &parentLayerCimPaths)
+			if err != nil {
+				return nil, fmt.Errorf("failed to unmarshal parent layer cim paths from mount: %w", err)
+			}
+		}
+	}
+	return parentLayerCimPaths, nil
+}
+
+// Only applies to a snapshot created for image extraction, for such a snapshot provides the
+// path to a cim in which image layer will be extracted.
+func (m *Mount) GetCimPath() (string, error) {
+	if m.Type != "CimFS" {
+		return "", fmt.Errorf("invalid windows mount type: '%s'", m.Type)
+	}
+	cimPath := ""
+	for _, option := range m.Options {
+		cimPath, _ = strings.CutPrefix(option, LayerCimPathFlag)
+	}
+	if cimPath == "" {
+		return "", fmt.Errorf("cim path not found")
+	}
+	return cimPath, nil
+}
+
 // Unmount the mount at the provided path
 func Unmount(mount string, flags int) error {
 	mount = filepath.Clean(mount)

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24
 	github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20230306123547-8075edf89bb0
 	github.com/Microsoft/go-winio v0.6.2
-	github.com/Microsoft/hcsshim v0.12.3
+	github.com/Microsoft/hcsshim v0.12.1-0.20240326215926-1d406d0eac55
 	github.com/checkpoint-restore/checkpointctl v1.1.0
 	github.com/checkpoint-restore/go-criu/v7 v7.1.0
 	github.com/containerd/btrfs/v2 v2.0.0
@@ -137,6 +137,6 @@ require (
 	k8s.io/apiserver v0.29.2 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
-	sigs.k8s.io/yaml v1.3.0 // indirect
+	sigs.k8s.io/yaml v1.4.0 // indirect
 	tags.cncf.io/container-device-interface/specs-go v0.7.0 // indirect
 )

go.sum

@@ -10,8 +10,8 @@ github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0
 github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ=
 github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
 github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
-github.com/Microsoft/hcsshim v0.12.3 h1:LS9NXqXhMoqNCplK1ApmVSfB4UnVLRDWRapB6EIlxE0=
-github.com/Microsoft/hcsshim v0.12.3/go.mod h1:Iyl1WVpZzr+UkzjekHZbV8o5Z9ZkxNGx6CtY2Qg/JVQ=
+github.com/Microsoft/hcsshim v0.12.1-0.20240326215926-1d406d0eac55 h1:OUP7yfGXZtKiCjRAx4q4/yMB9LkAXfK/2fm+HuTOfaA=
+github.com/Microsoft/hcsshim v0.12.1-0.20240326215926-1d406d0eac55/go.mod h1:puKXmBgyNxp1EYsdGOyN0Pe0nIMTBWkJJ6Mzl79kYLQ=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
@@ -510,8 +510,8 @@ sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMm
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08=
-sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo=
-sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8=
+sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
+sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY=
 tags.cncf.io/container-device-interface v0.7.2 h1:MLqGnWfOr1wB7m08ieI4YJ3IoLKKozEnnNYBtacDPQU=
 tags.cncf.io/container-device-interface v0.7.2/go.mod h1:Xb1PvXv2BhfNb3tla4r9JL129ck1Lxv9KuU6eVOfKto=
 tags.cncf.io/container-device-interface/specs-go v0.7.0 h1:w/maMGVeLP6TIQJVYT5pbqTi8SCw/iHZ+n4ignuGHqg=

pkg/archive/tar_opts_windows.go

@@ -79,15 +79,17 @@ func WithParentLayers(p []string) WriteDiffOpt {
 	}
 }
 
-func applyWindowsCimLayer(ctx context.Context, root string, r io.Reader, options ApplyOptions) (size int64, err error) {
-	return ocicimlayer.ImportCimLayerFromTar(ctx, r, root, options.Parents)
+func applyWindowsCimLayer(cimPath string, parentLayerCimPaths []string) func(context.Context, string, io.Reader, ApplyOptions) (int64, error) {
+	return func(ctx context.Context, root string, r io.Reader, options ApplyOptions) (int64, error) {
+		return ocicimlayer.ImportCimLayerFromTar(ctx, r, root, cimPath, options.Parents, parentLayerCimPaths)
+	}
 }
 
 // AsCimContainerLayer indicates that the tar stream to apply is that of a Windows container Layer written in
 // the cim format.
-func AsCimContainerLayer() ApplyOpt {
+func AsCimContainerLayer(cimPath string, parentLayerCimPaths []string) ApplyOpt {
 	return func(options *ApplyOptions) error {
-		options.applyFunc = applyWindowsCimLayer
+		options.applyFunc = applyWindowsCimLayer(cimPath, parentLayerCimPaths)
 		return nil
 	}
 }

plugins/diff/windows/cimfs.go

@@ -77,11 +77,26 @@ func NewCimDiff(store content.Store) (CompareApplier, error) {
 // provided mounts. Archive content will be extracted and decompressed if
 // necessary.
 func (c cimDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts []mount.Mount, opts ...diff.ApplyOpt) (d ocispec.Descriptor, err error) {
-	layer, parentLayerPaths, err := cimMountsToLayerAndParents(mounts)
+	if len(mounts) != 1 {
+		return emptyDesc, fmt.Errorf("number of mounts should always be 1 for CimFS layers: %w", errdefs.ErrInvalidArgument)
+	} else if mounts[0].Type != "CimFS" {
+		return emptyDesc, fmt.Errorf("cimDiff does not support layer type %s: %w", mounts[0].Type, errdefs.ErrNotImplemented)
+	}
+
+	m := mounts[0]
+	parentLayerPaths, err := m.GetParentPaths()
+	if err != nil {
+		return emptyDesc, err
+	}
+	parentLayerCimPaths, err := m.GetParentCimPaths()
+	if err != nil {
+		return emptyDesc, err
+	}
+	cimPath, err := m.GetCimPath()
 	if err != nil {
 		return emptyDesc, err
 	}
-	return applyDiffCommon(ctx, c.store, desc, layer, parentLayerPaths, archive.AsCimContainerLayer(), opts...)
+	return applyDiffCommon(ctx, c.store, desc, m.Source, parentLayerPaths, archive.AsCimContainerLayer(cimPath, parentLayerCimPaths), opts...)
 }
 
 // Compare creates a diff between the given mounts and uploads the result
@@ -90,22 +105,3 @@ func (c cimDiff) Compare(ctx context.Context, lower, upper []mount.Mount, opts .
 	// support for generating layer diff of cimfs layers will be added later.
 	return emptyDesc, errdefs.ErrNotImplemented
 }
-
-func cimMountsToLayerAndParents(mounts []mount.Mount) (string, []string, error) {
-	if len(mounts) != 1 {
-		return "", nil, fmt.Errorf("%w: number of mounts should always be 1 for Windows layers", errdefs.ErrInvalidArgument)
-	}
-	mnt := mounts[0]
-	if mnt.Type != "CimFS" {
-		// This is a special case error. When this is received the diff service
-		// will attempt the next differ in the chain.
-		return "", nil, errdefs.ErrNotImplemented
-	}
-
-	parentLayerPaths, err := mnt.GetParentPaths()
-	if err != nil {
-		return "", nil, err
-	}
-
-	return mnt.Source, parentLayerPaths, nil
-}

plugins/snapshots/windows/cimfs.go

@@ -33,7 +33,6 @@ import (
 	"github.com/Microsoft/hcsshim"
 	"github.com/Microsoft/hcsshim/computestorage"
 	"github.com/Microsoft/hcsshim/pkg/cimfs"
-	cimlayer "github.com/Microsoft/hcsshim/pkg/ociwclayer/cim"
 	"github.com/containerd/containerd/v2/core/mount"
 	"github.com/containerd/containerd/v2/core/snapshots"
 	"github.com/containerd/containerd/v2/core/snapshots/storage"
@@ -95,35 +94,29 @@ func NewCimFSSnapshotter(root string) (snapshots.Snapshotter, error) {
 		return nil, fmt.Errorf("failed to init base scratch VHD: %w", err)
 	}
 
+	if err = os.MkdirAll(filepath.Join(baseSn.info.HomeDir, "cim-layers"), 0755); err != nil {
+		return nil, err
+	}
+
 	return &cimFSSnapshotter{
 		windowsBaseSnapshotter: baseSn,
 		cimDir:                 filepath.Join(baseSn.info.HomeDir, "cim-layers"),
 	}, nil
 }
 
-// getCimLayerPath returns the path of the cim file for the given snapshot. Note that this function doesn't
+// getLayerCimPath returns the path of the cim file for the given snapshot. Note that this function doesn't
 // actually check if the cim layer exists it simply does string manipulation to generate the path isCimLayer
 // can be used to verify if it is actually a cim layer.
-func getCimLayerPath(cimDir, snID string) string {
-	return filepath.Join(cimDir, (snID + ".cim"))
+func (s *cimFSSnapshotter) getLayerCimPath(snID string) string {
+	return filepath.Join(s.cimDir, (snID + ".cim"))
 }
 
-// isCimLayer checks if the snapshot referred by the given key is actually a cim layer.  With CimFS
-// snapshotter all the read-only (i.e image) layers are stored in the cim format while we still use VHDs for
-// scratch layers.
-func (s *cimFSSnapshotter) isCimLayer(ctx context.Context, key string) (bool, error) {
-	id, _, _, err := storage.GetInfo(ctx, key)
-	if err != nil {
-		return false, fmt.Errorf("get snapshot info: %w", err)
-	}
-	snCimPath := getCimLayerPath(s.cimDir, id)
-	if _, err := os.Stat(snCimPath); err != nil {
-		if os.IsNotExist(err) {
-			return false, nil
-		}
-		return false, err
+func (s *cimFSSnapshotter) parentIDsToCimPaths(parentIDs []string) []string {
+	cimPaths := []string{}
+	for _, ID := range parentIDs {
+		cimPaths = append(cimPaths, s.getLayerCimPath(ID))
 	}
-	return true, nil
+	return cimPaths
 }
 
 func (s *cimFSSnapshotter) Usage(ctx context.Context, key string) (snapshots.Usage, error) {
@@ -138,15 +131,14 @@ func (s *cimFSSnapshotter) Usage(ctx context.Context, key string) (snapshots.Usa
 	}
 	defer t.Rollback()
 
-	id, _, _, err := storage.GetInfo(ctx, key)
+	id, info, _, err := storage.GetInfo(ctx, key)
 	if err != nil {
 		return snapshots.Usage{}, fmt.Errorf("failed to get snapshot info: %w", err)
 	}
 
-	if ok, err := s.isCimLayer(ctx, key); err != nil {
-		return snapshots.Usage{}, err
-	} else if ok {
-		cimUsage, err := cimfs.GetCimUsage(ctx, getCimLayerPath(s.cimDir, id))
+	if info.Kind == snapshots.KindCommitted {
+		// Committed MUST be a cimfs layer
+		cimUsage, err := cimfs.GetCimUsage(ctx, s.getLayerCimPath(id))
 		if err != nil {
 			return snapshots.Usage{}, err
 		}
@@ -219,7 +211,7 @@ func (s *cimFSSnapshotter) Remove(ctx context.Context, key string) error {
 		return fmt.Errorf("%w: %s", errdefs.ErrFailedPrecondition, err)
 	}
 
-	if err := cimlayer.DestroyCimLayer(s.getSnapshotDir(ID)); err != nil {
+	if err := cimfs.DestroyCim(ctx, s.getLayerCimPath(ID)); err != nil {
 		// Must be cleaned up, any "rm-*" could be removed if no active transactions
 		log.G(ctx).WithError(err).WithField("ID", ID).Warnf("failed to cleanup cim files")
 	}
@@ -327,27 +319,37 @@ func (s *cimFSSnapshotter) mounts(sn storage.Snapshot, key string) []mount.Mount
 
 	source := s.getSnapshotDir(sn.ID)
 	parentLayerPaths := s.parentIDsToParentPaths(sn.ParentIDs)
+	layerCimPaths := s.parentIDsToCimPaths(sn.ParentIDs)
 
 	mountType := "CimFS"
 
 	// error is not checked here, as a string array will never fail to Marshal
 	parentLayersJSON, _ := json.Marshal(parentLayerPaths)
 	parentLayersOption := mount.ParentLayerPathsFlag + string(parentLayersJSON)
+	parentCimLayersJSON, _ := json.Marshal(layerCimPaths)
+	parentCimLayersOption := mount.ParentLayerCimPathsFlag + string(parentCimLayersJSON)
 
 	options := []string{
 		roFlag,
 	}
 	if len(sn.ParentIDs) != 0 {
 		options = append(options, parentLayersOption)
+		options = append(options, parentCimLayersOption)
+	}
+	// if this is an image layer being extracted include a cim path in which the layer
+	// will be extracted.
+	if strings.Contains(key, snapshots.UnpackKeyPrefix) {
+		cimPathOption := s.getLayerCimPath(sn.ID)
+		options = append(options, mount.LayerCimPathFlag+cimPathOption)
 	}
+
 	mounts := []mount.Mount{
 		{
 			Source:  source,
 			Type:    mountType,
 			Options: options,
 		},
 	}
-
 	return mounts
 }
 

script/setup/runhcs-version

@@ -1 +1 @@
-v0.12.3
+1d406d0eac5573287ba7b46a04a58275410137ac