Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update cimfs snapshotter & differ for new hcsshim interface #10033

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update cimfs snapshotter & differ for new hcsshim interface #10033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions core/mount/mount_windows.go
View file
Open in desktop
Expand Up @@ -112,9 +112,11 @@ 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="
)

// GetParentPaths of the mount
func (m *Mount) GetParentPaths() ([]string, error) {
Expand Down
6 changes: 3 additions & 3 deletions go.mod
View file
Open in desktop
Expand Up @@ -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.20240509152327-46ef279de8db
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
Expand Down Expand Up @@ -113,7 +113,7 @@ require (
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.5.0 // indirect
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/common v0.48.0 // indirect
github.com/prometheus/procfs v0.12.0 // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
Expand All @@ -138,6 +138,6 @@ require (
k8s.io/apiserver v0.30.0 // 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
)
12 changes: 6 additions & 6 deletions go.sum
View file
Open in desktop
Expand Up @@ -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.20240509152327-46ef279de8db h1:xb38qDzEEP0AuYqBC9+MkxeAPgYXU1ff692+4vmwecM=
github.com/Microsoft/hcsshim v0.12.1-0.20240509152327-46ef279de8db/go.mod h1:rGcObLjFs6zKLYt35t+/gTZxPnRQC3Ck/xLghu/d5A0=
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=
Expand Down Expand Up @@ -262,8 +262,8 @@ github.com/prometheus/client_golang v1.19.1/go.mod h1:mP78NwGzrVks5S2H6ab8+ZZGJL
github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/prometheus/client_model v0.5.0 h1:VQw1hfvPvk3Uv6Qf29VrPF32JB6rtbgI6cYPYQjL0Qw=
github.com/prometheus/client_model v0.5.0/go.mod h1:dTiFglRmd66nLR9Pv9f0mZi7B7fk5Pm3gvsjB5tr+kI=
github.com/prometheus/client_model v0.6.1 h1:ZKSh/rekM+n3CeS952MLRAdFwIKqeY8b62p8ais2e9E=
github.com/prometheus/client_model v0.6.1/go.mod h1:OrxVMOVHjw3lKMa8+x6HeMGkHMQyHDk9E3jmP2AmGiY=
github.com/prometheus/common v0.4.1/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4=
github.com/prometheus/common v0.6.0/go.mod h1:eBmuwkDJBwy6iBfxCBob6t6dR6ENT/y+J+Zk0j9GMYc=
github.com/prometheus/common v0.48.0 h1:QO8U2CdOzSn1BBsmXJXduaaW+dY/5QLjfB8svtSzKKE=
Expand Down Expand Up @@ -512,8 +512,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=
Expand Down
10 changes: 6 additions & 4 deletions pkg/archive/tar_opts_windows.go
View file
Open in desktop
Expand Up @@ -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
}
}
39 changes: 18 additions & 21 deletions plugins/diff/windows/cimfs.go
View file
Open in desktop
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/containerd/containerd/v2/core/mount"
"github.com/containerd/containerd/v2/pkg/archive"
"github.com/containerd/containerd/v2/plugins"
winsn "github.com/containerd/containerd/v2/plugins/snapshots/windows"
"github.com/containerd/errdefs"
"github.com/containerd/platforms"
"github.com/containerd/plugin"
Expand Down Expand Up @@ -77,11 +78,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" {
ambarve marked this conversation as resolved.
Show resolved Hide resolved
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 := winsn.GetParentCimPaths(&m)
if err != nil {
return emptyDesc, err
}
cimPath, err := winsn.GetCimPath(&m)
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
Expand All @@ -90,22 +106,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
}
124 changes: 92 additions & 32 deletions plugins/snapshots/windows/cimfs.go
View file
Open in desktop
Expand Up @@ -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"
Expand All @@ -52,6 +51,7 @@ const (
templateVHDName = "blank.vhdx"
vhdMaxSizeInBytes uint64 = 10 * 1024 * 1024 * 1024 // 10 GB
vhdBlockSizeInBytes uint32 = 1 * 1024 * 1024 // 1 MB
labelSnapshotRef = "containerd.io/snapshot.ref"
)

// Composite image FileSystem (CimFS) is a new read-only filesystem (similar to overlayFS on Linux) created
Expand Down Expand Up @@ -95,35 +95,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)
func (s *cimFSSnapshotter) parentIDsToCimPaths(parentIDs []string) []string {
cimPaths := make([]string, 0, len(parentIDs))
for _, id := range parentIDs {
cimPaths = append(cimPaths, s.getLayerCimPath(id))
}
snCimPath := getCimLayerPath(s.cimDir, id)
if _, err := os.Stat(snCimPath); err != nil {
if os.IsNotExist(err) {
return false, nil
}
return false, err
}
return true, nil
return cimPaths
}

func (s *cimFSSnapshotter) Usage(ctx context.Context, key string) (snapshots.Usage, error) {
Expand All @@ -138,15 +132,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are saying not just that committed -> cimfs, but also cimfs -> committed, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, here the comment just suggests that a committed snapshot means we should add the usage of cim files too.

cimUsage, err := cimfs.GetCimUsage(ctx, s.getLayerCimPath(id))
if err != nil {
return snapshots.Usage{}, err
}
Expand Down Expand Up @@ -219,7 +212,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")
}
Expand All @@ -232,7 +225,13 @@ func (s *cimFSSnapshotter) Remove(ctx context.Context, key string) error {
}

func (s *cimFSSnapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string, opts []snapshots.Opt) (_ []mount.Mount, err error) {
var newSnapshot storage.Snapshot
var (
newSnapshot storage.Snapshot
snapshotInfo snapshots.Info
)
for _, o := range opts {
o(&snapshotInfo)
}
err = s.ms.WithTransaction(ctx, true, func(ctx context.Context) (retErr error) {
newSnapshot, err = storage.CreateSnapshot(ctx, kind, key, parent, opts...)
if err != nil {
Expand All @@ -251,7 +250,10 @@ func (s *cimFSSnapshotter) createSnapshot(ctx context.Context, kind snapshots.Ki
}
}()

if strings.Contains(key, snapshots.UnpackKeyPrefix) {
// If we are unpacking an image, unpacker already knows the chain ID for
// this snapshot and it includes that in the labels. Check that to decide
// if we are unpacking an image.
if _, ok := snapshotInfo.Labels[labelSnapshotRef]; ok {
// IO/disk space optimization: Do nothing
//
// We only need one sandbox.vhdx for the container. Skip making one for this
Expand All @@ -266,10 +268,6 @@ func (s *cimFSSnapshotter) createSnapshot(ctx context.Context, kind snapshots.Ki
}

parentLayerPaths := s.parentIDsToParentPaths(newSnapshot.ParentIDs)
var snapshotInfo snapshots.Info
for _, o := range opts {
o(&snapshotInfo)
}

sizeInBytes, err := getRequestedScratchSize(ctx, snapshotInfo)
if err != nil {
Expand Down Expand Up @@ -327,27 +325,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 := 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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to avoid keying more stuff off of UnpackKeyPrefix. Is there any way we can avoid this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated this to use the snapshot digest label that is passed with each Prepare call during image extraction. I think it is slightly better than checking for the key prefix. Let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, there are two other places (Commit & Mounts) where we still need to use the UnpackKeyPrefix. I am trying to figure out if we can easily change it there too. But if not, would you be okay with making this change in a separate PR?

cimPathOption := s.getLayerCimPath(sn.ID)
options = append(options, LayerCimPathFlag+cimPathOption)
}

mounts := []mount.Mount{
{
Source: source,
Type: mountType,
Options: options,
},
}

return mounts
}

Expand Down Expand Up @@ -426,3 +434,55 @@ func createScratchVHDs(ctx context.Context, path string) (err error) {
}
return nil
}

const (
// 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="
)

// getOptionByPrefix finds an option that has the provided prefix, cuts the prefix from
// that option string and return the remaining string. Boolean return is set to true if an
// option is found with the given prefix. It is set to false otherwise.
func getOptionByPrefix(m *mount.Mount, prefix string) (string, bool) {
for _, option := range m.Options {
val, found := strings.CutPrefix(option, prefix)
if found {
return val, true
}
}
return "", false
}

// gets the paths of the parent cims of this mount
func GetParentCimPaths(m *mount.Mount) ([]string, error) {
if m.Type != "CimFS" {
return nil, fmt.Errorf("invalid mount type: '%s'", m.Type)
}
var parentCimPaths []string
val, found := getOptionByPrefix(m, ParentLayerCimPathsFlag)
if !found {
return nil, fmt.Errorf("parent cim paths not found")
}
err := json.Unmarshal([]byte(val), &parentCimPaths)
return parentCimPaths, err
}

// 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 GetCimPath(m *mount.Mount) (string, error) {
if m.Type != "CimFS" {
return "", fmt.Errorf("invalid mount type: '%s'", m.Type)
}
cimPath, found := getOptionByPrefix(m, LayerCimPathFlag)
if !found {
return "", fmt.Errorf("cim path not found")
}
return cimPath, nil

}