[pull] main from containerd:main

core/mount/mount.go

@@ -93,10 +93,35 @@ func CanonicalizePath(path string) (string, error) {
 	return filepath.EvalSymlinks(path)
 }
 
-// ReadOnly returns a boolean value indicating whether this mount has the "ro"
-// option set.
+// ReadOnly reports whether this mount is read-only, deriving it from the mount
+// type where the options alone don't say so.
 func (m *Mount) ReadOnly() bool {
-	return slices.Contains(m.Options, "ro")
+	typ := m.Type
+	// The mount type may carry "/"-separated modifiers meaningful only to the
+	// mount manager (e.g. "format/mkdir/overlay"), so only its last segment is
+	// considered.
+	if i := strings.LastIndex(typ, "/"); i >= 0 {
+		typ = typ[i+1:]
+	}
+	switch typ {
+	case "erofs":
+		// Read-only by construction, whatever the options say.
+		return true
+	case "overlay":
+		// Writable only through an upperdir, which a snapshotter signals by
+		// setting it rather than by setting "rw". An element may be a
+		// comma-joined fragment ("lowerdir=a,upperdir=b"), so split first.
+		options := strings.Split(strings.Join(m.Options, ","), ",")
+		// An explicit "ro" wins over an upperdir.
+		if slices.Contains(options, "ro") {
+			return true
+		}
+		return !slices.ContainsFunc(options, func(o string) bool {
+			return strings.HasPrefix(o, "upperdir=")
+		})
+	default:
+		return slices.Contains(m.Options, "ro")
+	}
 }
 
 // Mount to the provided target path.

core/mount/mount_test.go

@@ -149,6 +149,71 @@ func TestReadonlyMounts(t *testing.T) {
 	}
 }
 
+func TestMountReadOnly(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		mount    Mount
+		expected bool
+	}{
+		{
+			desc:     "erofs is always read-only",
+			mount:    Mount{Type: "erofs", Source: "/path/to/layer.erofs", Options: []string{"loop"}},
+			expected: true,
+		},
+		{
+			desc:     "overlay without upperdir is read-only",
+			mount:    Mount{Type: "overlay", Source: "overlay", Options: []string{"lowerdir=/lower"}},
+			expected: true,
+		},
+		{
+			desc:     "overlay with upperdir is writable",
+			mount:    Mount{Type: "overlay", Source: "overlay", Options: []string{"lowerdir=/lower", "upperdir=/upper"}},
+			expected: false,
+		},
+		{
+			desc:     "overlay with upperdir packed into a comma-joined options string is writable",
+			mount:    Mount{Type: "overlay", Source: "overlay", Options: []string{"lowerdir=/lower,upperdir=/upper,workdir=/work"}},
+			expected: false,
+		},
+		{
+			desc:     "type modifiers are stripped before matching overlay",
+			mount:    Mount{Type: "format/mkdir/overlay", Source: "overlay", Options: []string{"lowerdir=/lower"}},
+			expected: true,
+		},
+		{
+			desc:     "type modifiers are stripped, overlay with upperdir still writable",
+			mount:    Mount{Type: "format/mkdir/overlay", Source: "overlay", Options: []string{"upperdir=/upper"}},
+			expected: false,
+		},
+		{
+			desc:     "overlay with an explicit `ro` option is read-only despite upperdir",
+			mount:    Mount{Type: "overlay", Source: "overlay", Options: []string{"lowerdir=/lower", "upperdir=/upper", "ro"}},
+			expected: true,
+		},
+		{
+			desc:     "overlay `ro` packed into a comma-joined options string is read-only",
+			mount:    Mount{Type: "overlay", Source: "overlay", Options: []string{"lowerdir=/lower,upperdir=/upper,ro"}},
+			expected: true,
+		},
+		{
+			desc:     "other types are read-only only with the `ro` option",
+			mount:    Mount{Type: "bind", Source: "/path", Options: []string{"ro", "rbind"}},
+			expected: true,
+		},
+		{
+			desc:     "other types are writable without the `ro` option",
+			mount:    Mount{Type: "bind", Source: "/path", Options: []string{"rbind"}},
+			expected: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		if got := tc.mount.ReadOnly(); got != tc.expected {
+			t.Errorf("%s: ReadOnly() = %v, want %v", tc.desc, got, tc.expected)
+		}
+	}
+}
+
 func TestRemoveVolatileTempMount(t *testing.T) {
 	testCases := []struct {
 		desc     string

core/snapshots/snapshotter.go

@@ -58,6 +58,12 @@ const (
 	// Ignoring is not a failure — callers that require enforcement must
 	// pick a snapshotter that supports it.
 	LabelSnapshotMaxSize = "containerd.io/snapshot/max-size"
+
+	// RebaseCap is a snapshotter capability (advertised via the plugin's metadata)
+	// indicating that an active snapshot may be committed with a parent supplied at
+	// Commit time (via WithParent). It lets the unpacker prepare and apply layers in
+	// parallel and rebase the chain into place at commit.
+	RebaseCap = "rebase"
 )
 
 // Kind identifies the kind of snapshot.

core/unpack/unpacker.go

@@ -404,7 +404,11 @@ func (u *Unpacker) unpack(
 		var (
 			key    string
 			mounts []mount.Mount
-			opts   = append(unpack.SnapshotOpts, snapshots.WithLabels(snapshotLabels))
+			// Clone before appending: topHalf runs concurrently per layer in
+			// parallel mode, and appending directly to unpack.SnapshotOpts could
+			// write into its shared backing array from multiple goroutines.
+			opts   = append(slices.Clone(unpack.SnapshotOpts), snapshots.WithLabels(snapshotLabels))
+			staged bool
 		)
 
 		for try := 1; try <= 3; try++ {
@@ -435,13 +439,75 @@ func (u *Unpacker) unpack(
 			return nil, fmt.Errorf("unable to prepare extraction snapshot: %w", err)
 		}
 
+		if isStaged(mounts) {
+			// The snapshotter staged the layer content into the active snapshot
+			// as read-only (e.g. a layer content cache hit). Skip fetch+apply,
+			// but still commit it below (which applies the parent).
+			staged = true
+		}
+
 		// Abort the snapshot if commit does not happen
 		abort := func(ctx context.Context) {
 			if err := sn.Remove(ctx, key); err != nil {
 				log.G(ctx).WithError(err).Errorf("failed to cleanup %q", key)
 			}
 		}
 
+		// commitF is the bottom half shared by normal and staged layers: it rebases
+		// in the real parent (parallel mode) and commits the snapshot. Staged layers
+		// have no fetched content, so they skip the post-apply uncompressed label.
+		commitF := func(shouldAbort bool) error {
+			defer unlock()
+			if shouldAbort {
+				cleanup.Do(ctx, abort)
+				return nil
+			}
+
+			if i > 0 && parallel {
+				opts = append(opts, snapshots.WithParent(chainIDs[i-1].String()))
+			}
+			if err := sn.Commit(ctx, chainID, key, opts...); err != nil {
+				cleanup.Do(ctx, abort)
+				if errdefs.IsAlreadyExists(err) {
+					return nil
+				}
+				return fmt.Errorf("failed to commit snapshot %s: %w", key, err)
+			}
+
+			if staged {
+				// No layer was fetched, so there is no content to label.
+				return nil
+			}
+
+			// Set the uncompressed label after the uncompressed
+			// digest has been verified through apply.
+			cinfo := content.Info{
+				Digest: desc.Digest,
+				Labels: map[string]string{
+					labels.LabelUncompressed: diffIDs[i].String(),
+				},
+			}
+			if _, err := cs.Update(ctx, cinfo, "labels."+labels.LabelUncompressed); err != nil {
+				return err
+			}
+			return nil
+		}
+
+		if staged {
+			// Content is already staged in the active snapshot; there is nothing to
+			// fetch or apply. Emit a status that runs commitF in the (serialized)
+			// bottom half so the parent is rebased in and the chain is linked.
+			resCh := make(chan *unpackStatus, 1)
+			resCh <- &unpackStatus{
+				desc:    desc,
+				span:    span,
+				startAt: startAt,
+				bottomF: commitF,
+			}
+			close(resCh)
+			return resCh, nil
+		}
+
 		if fetchErr == nil {
 			fetchOffset = i
 			n := len(layers) - fetchOffset
@@ -478,38 +544,7 @@ func (u *Unpacker) unpack(
 				desc:    desc,
 				span:    span,
 				startAt: startAt,
-				bottomF: func(shouldAbort bool) error {
-					defer unlock()
-					if shouldAbort {
-						cleanup.Do(ctx, abort)
-						return nil
-					}
-
-					if i > 0 && parallel {
-						parent = chainIDs[i-1].String()
-						opts = append(opts, snapshots.WithParent(parent))
-					}
-					if err = sn.Commit(ctx, chainID, key, opts...); err != nil {
-						cleanup.Do(ctx, abort)
-						if errdefs.IsAlreadyExists(err) {
-							return nil
-						}
-						return fmt.Errorf("failed to commit snapshot %s: %w", key, err)
-					}
-
-					// Set the uncompressed label after the uncompressed
-					// digest has been verified through apply.
-					cinfo := content.Info{
-						Digest: desc.Digest,
-						Labels: map[string]string{
-							labels.LabelUncompressed: diffIDs[i].String(),
-						},
-					}
-					if _, err := cs.Update(ctx, cinfo, "labels."+labels.LabelUncompressed); err != nil {
-						return err
-					}
-					return nil
-				},
+				bottomF: commitF,
 			}
 
 			select {
@@ -746,7 +781,7 @@ func (u *Unpacker) supportParallel(unpack *Platform) bool {
 	if u.unpackLimiter == nil {
 		return false
 	}
-	if !slices.Contains(unpack.SnapshotterCapabilities, "rebase") {
+	if !slices.Contains(unpack.SnapshotterCapabilities, snapshots.RebaseCap) {
 		log.L.Infof("snapshotter does not support rebase capability, unpacking will be sequential")
 		return false
 	}
@@ -761,6 +796,24 @@ func uniquePart() string {
 	return fmt.Sprintf("%d-%s", t.Nanosecond(), base64.URLEncoding.EncodeToString(b[:]))
 }
 
+// isStaged reports whether a successful Prepare has already staged the
+// layer's content into the active snapshot instead of returning a normal,
+// writable active snapshot (e.g. a snapshotter serving the layer from a local
+// content cache). There is nothing to write into a staged snapshot, so the
+// caller should skip fetching and applying the layer, and just Commit the
+// snapshot as-is (applying the real parent at Commit time).
+//
+// Only the last mount in the slice is inspected: earlier entries are inputs
+// consumed by mount templating (e.g. "{{ mount 0 }}" in an overlay's
+// lowerdir) rather than the mount that is actually stacked on top, so they
+// carry no information about writability.
+func isStaged(mounts []mount.Mount) bool {
+	if len(mounts) == 0 {
+		return false
+	}
+	return mounts[len(mounts)-1].ReadOnly()
+}
+
 // TODO: this is a temporary workaround until #13053 lands.
 func bindToOverlay(mounts []mount.Mount) []mount.Mount {
 	if len(mounts) != 1 || mounts[0].Type != "bind" {