diff --git a/diff/windows/windows.go b/diff/windows/windows.go index d59ef1cff..b694b4d67 100644 --- a/diff/windows/windows.go +++ b/diff/windows/windows.go @@ -320,10 +320,13 @@ func mountsToLayerAndParents(mounts []mount.Mount) (string, []string, error) { return "", nil, fmt.Errorf("number of mounts should always be 1 for Windows layers: %w", errdefs.ErrInvalidArgument) } mnt := mounts[0] - if mnt.Type != "windows-layer" { + + if mnt.Type != "windows-layer" && mnt.Type != "bind" { // This is a special case error. When this is received the diff service // will attempt the next differ in the chain which for Windows is the // lcow differ that we want. + // TODO: Is there any situation where we actually wanted a "bind" mount to + // fall through to the lcow differ? return "", nil, fmt.Errorf("windowsDiff does not support layer type %s: %w", mnt.Type, errdefs.ErrNotImplemented) } @@ -332,6 +335,30 @@ func mountsToLayerAndParents(mounts []mount.Mount) (string, []string, error) { return "", nil, err } + isView := false + for _, o := range mnt.Options { + if o == "ro" { + isView = true + break + } + } + + if isView { + if mnt.Type == "bind" && len(parentLayerPaths) != 0 { + return "", nil, fmt.Errorf("unexpected bind-mount View with parents: %w", errdefs.ErrInvalidArgument) + } else if mnt.Type == "bind" { + // rootfs.CreateDiff creates a new, empty View to diff against, + // when diffing something with no parent. + // This makes perfect sense for a walking Diff, but for WCOW, + // we have to recognise this as "diff against nothing" + return "", nil, nil + } else if len(parentLayerPaths) == 0 { + return "", nil, fmt.Errorf("unexpected windows-layer View with no parent: %w", errdefs.ErrInvalidArgument) + } + // Ignore the dummy sandbox. + return parentLayerPaths[0], parentLayerPaths[1:], nil + } + return mnt.Source, parentLayerPaths, nil } @@ -346,8 +373,16 @@ func mountPairToLayerStack(lower, upper []mount.Mount) ([]string, error) { return nil, fmt.Errorf("Upper mount invalid: %w", err) } + lowerLayer, lowerParentLayerPaths, err := mountsToLayerAndParents(lower) + if errdefs.IsNotImplemented(err) { + // Upper was a windows-layer or bind, lower is not. We can't handle that. + return nil, fmt.Errorf("windowsDiff cannot diff a windows-layer against a non-windows-layer: %w", errdefs.ErrInvalidArgument) + } else if err != nil { + return nil, fmt.Errorf("Lower mount invalid: %w", err) + } + // Trivial case, diff-against-nothing - if len(lower) == 0 { + if lowerLayer == "" { if len(upperParentLayerPaths) != 0 { return nil, fmt.Errorf("windowsDiff cannot diff a layer with parents against a null layer: %w", errdefs.ErrInvalidArgument) } @@ -358,14 +393,6 @@ func mountPairToLayerStack(lower, upper []mount.Mount) ([]string, error) { return nil, fmt.Errorf("windowsDiff cannot diff a layer with no parents against another layer: %w", errdefs.ErrInvalidArgument) } - lowerLayer, lowerParentLayerPaths, err := mountsToLayerAndParents(lower) - if errdefs.IsNotImplemented(err) { - // Upper was a windows-layer, lower is not. We can't handle that. - return nil, fmt.Errorf("windowsDiff cannot diff a windows-layer against a non-windows-layer: %w", errdefs.ErrInvalidArgument) - } else if err != nil { - return nil, fmt.Errorf("Lower mount invalid: %w", err) - } - if upperParentLayerPaths[0] != lowerLayer { return nil, fmt.Errorf("windowsDiff cannot diff a layer against a layer other than its own parent: %w", errdefs.ErrInvalidArgument) } diff --git a/snapshots/windows/windows.go b/snapshots/windows/windows.go index 310e4380b..cec364070 100644 --- a/snapshots/windows/windows.go +++ b/snapshots/windows/windows.go @@ -187,7 +187,7 @@ func (s *snapshotter) Mounts(ctx context.Context, key string) (_ []mount.Mount, return nil, err } - return s.mounts(snapshot), nil + return s.mounts(snapshot, key), nil } func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snapshots.Opt) (retErr error) { @@ -208,7 +208,11 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap // If (windowsDiff).Apply was used to populate this layer, then it's already in the 'committed' state. // See createSnapshot below for more details if !strings.Contains(key, snapshots.UnpackKeyPrefix) { - if err := s.convertScratchToReadOnlyLayer(ctx, snapshot, path); err != nil { + if len(snapshot.ParentIDs) == 0 { + if err = hcsshim.ConvertToBaseLayer(path); err != nil { + return err + } + } else if err := s.convertScratchToReadOnlyLayer(ctx, snapshot, path); err != nil { return err } } @@ -299,11 +303,9 @@ func (s *snapshotter) Close() error { return s.ms.Close() } -func (s *snapshotter) mounts(sn storage.Snapshot) []mount.Mount { +func (s *snapshotter) mounts(sn storage.Snapshot, key string) []mount.Mount { var ( - roFlag string - source string - parentLayerPaths []string + roFlag string ) if sn.Kind == snapshots.KindView { @@ -312,12 +314,19 @@ func (s *snapshotter) mounts(sn storage.Snapshot) []mount.Mount { roFlag = "rw" } - if len(sn.ParentIDs) == 0 || sn.Kind == snapshots.KindActive { - source = s.getSnapshotDir(sn.ID) - parentLayerPaths = s.parentIDsToParentPaths(sn.ParentIDs) - } else { - source = s.getSnapshotDir(sn.ParentIDs[0]) - parentLayerPaths = s.parentIDsToParentPaths(sn.ParentIDs[1:]) + source := s.getSnapshotDir(sn.ID) + parentLayerPaths := s.parentIDsToParentPaths(sn.ParentIDs) + + mountType := "windows-layer" + + if len(sn.ParentIDs) == 0 { + // A mount of a parentless snapshot is a bind-mount. + mountType = "bind" + // If not being extracted into, then the bind-target is the + // "Files" subdirectory. + if !strings.Contains(key, snapshots.UnpackKeyPrefix) { + source = filepath.Join(source, "Files") + } } // error is not checked here, as a string array will never fail to Marshal @@ -327,7 +336,7 @@ func (s *snapshotter) mounts(sn storage.Snapshot) []mount.Mount { var mounts []mount.Mount mounts = append(mounts, mount.Mount{ Source: source, - Type: "windows-layer", + Type: mountType, Options: []string{ roFlag, parentLayersOption, @@ -360,13 +369,22 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k return err } - // IO/disk space optimization - // - // We only need one sandbox.vhdx for the container. Skip making one for this - // snapshot if this isn't the snapshot that just houses the final sandbox.vhd - // that will be mounted as the containers scratch. Currently the key for a snapshot - // where a layer will be extracted to will have the string `extract-` in it. - if !strings.Contains(key, snapshots.UnpackKeyPrefix) { + if strings.Contains(key, snapshots.UnpackKeyPrefix) { + // IO/disk space optimization: Do nothing + // + // We only need one sandbox.vhdx for the container. Skip making one for this + // snapshot if this isn't the snapshot that just houses the final sandbox.vhd + // that will be mounted as the containers scratch. Currently the key for a snapshot + // where a layer will be extracted to will have the string `extract-` in it. + } else if len(newSnapshot.ParentIDs) == 0 { + // A parentless snapshot is just a bind-mount to a directory named + // "Files". When committed, there'll be some post-processing to fill in the rest + // of the metadata. + filesDir := filepath.Join(snDir, "Files") + if err := os.MkdirAll(filesDir, 0700); err != nil { + return err + } + } else { parentLayerPaths := s.parentIDsToParentPaths(newSnapshot.ParentIDs) var snapshotInfo snapshots.Info @@ -375,22 +393,28 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k } var sizeInBytes uint64 - if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok { - log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel) + if kind == snapshots.KindActive { + if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok { + log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel) - sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32) - if err != nil { - return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err) + sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32) + if err != nil { + return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err) + } + sizeInBytes = sizeInGB * 1024 * 1024 * 1024 } - sizeInBytes = sizeInGB * 1024 * 1024 * 1024 - } - // Prefer the newer label in bytes over the deprecated Windows specific GB variant. - if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok { - sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64) - if err != nil { - return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err) + // Prefer the newer label in bytes over the deprecated Windows specific GB variant. + if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok { + sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64) + if err != nil { + return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err) + } } + } else { + // A view is just a read-write snapshot with a _really_ small sandbox, since we cannot actually + // make a read-only mount or junction point. https://superuser.com/q/881544/112473 + sizeInBytes = 1024 * 1024 * 1024 } var makeUVMScratch bool @@ -415,7 +439,7 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k return nil, err } - return s.mounts(newSnapshot), nil + return s.mounts(newSnapshot, key), nil } func (s *snapshotter) parentIDsToParentPaths(parentIDs []string) []string {