Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ require (
github.com/mdlayher/vsock v1.3.0
github.com/moby/locker v1.0.1
github.com/moby/sys/mountinfo v0.7.2
github.com/moby/sys/sequential v0.7.0
github.com/moby/sys/signal v0.7.1
github.com/moby/sys/symlink v0.3.0
github.com/moby/sys/user v0.4.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,6 @@ github.com/moby/sys/capability v0.4.0 h1:4D4mI6KlNtWMCM1Z/K0i7RV1FkX+DBDHKVJpCnd
github.com/moby/sys/capability v0.4.0/go.mod h1:4g9IK291rVkms3LKCDOoYlnV8xKwoDTpIrNEE35Wq0I=
github.com/moby/sys/mountinfo v0.7.2 h1:1shs6aH5s4o5H2zQLn796ADW1wMrIwHsyJ2v9KouLrg=
github.com/moby/sys/mountinfo v0.7.2/go.mod h1:1YOa8w8Ih7uW0wALDUgT1dTTSBrZ+HiBLGws92L2RU4=
github.com/moby/sys/sequential v0.7.0 h1:ASQNGNROJSuOO6LL6bPHbKvuZu6NU8P4ldPWk31zj/8=
github.com/moby/sys/sequential v0.7.0/go.mod h1:NfSTAp6V3fw4tmkD62PEcOKeZKquXT8VKCkf7aVR79o=
github.com/moby/sys/signal v0.7.1 h1:PrQxdvxcGijdo6UXXo/lU/TvHUWyPhj7UOpSo8tuvk0=
github.com/moby/sys/signal v0.7.1/go.mod h1:Se1VGehYokAkrSQwL4tDzHvETwUZlnY7S5XtQ50mQp8=
github.com/moby/sys/symlink v0.3.0 h1:GZX89mEZ9u53f97npBy4Rc3vJKj7JBDj/PN2I22GrNU=
Expand Down
45 changes: 29 additions & 16 deletions internal/cri/server/images/image_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,14 @@ func (c *CRIImageService) PullImage(ctx context.Context, name string, credential
//
// Transfer service does not currently support all the CRI image config options.
// TODO: Add support for DisableSnapshotAnnotations, DiscardUnpackedLayers, ImagePullWithSyncFs and unpackDuplicationSuppressor
var image containerd.Image
var (
image containerd.Image
bytesPulled uint64
)
if c.config.UseLocalImagePull {
image, err = c.pullImageWithLocalPull(ctx, ref, credentials, snapshotter, labels, imagePullProgressTimeout)
image, bytesPulled, err = c.pullImageWithLocalPull(ctx, ref, credentials, snapshotter, labels, imagePullProgressTimeout)
} else {
image, err = c.pullImageWithTransferService(ctx, ref, credentials, snapshotter, labels, imagePullProgressTimeout)
image, bytesPulled, err = c.pullImageWithTransferService(ctx, ref, credentials, snapshotter, labels, imagePullProgressTimeout)
}

if err != nil {
Expand Down Expand Up @@ -216,13 +219,13 @@ func (c *CRIImageService) PullImage(ctx context.Context, name string, credential
}
}

const mbToByte = 1024 * 1024
size, _ := image.Size(ctx)
imagePullingSpeed := float64(size) / mbToByte / time.Since(startTime).Seconds()
imagePullThroughput.Observe(imagePullingSpeed)
elapsed := time.Since(startTime)
recordImagePullThroughput(imagePullThroughput, bytesPulled, elapsed)
recordImagePullThroughput(imagePullThroughputMiBps, bytesPulled, elapsed)

size, _ := image.Size(ctx)
log.G(ctx).Infof("Pulled image %q with image id %q, repo tag %q, repo digest %q, size %q in %s", name, imageID,
repoTag, repoDigest, strconv.FormatInt(size, 10), time.Since(startTime))
repoTag, repoDigest, strconv.FormatInt(size, 10), elapsed)
// NOTE(random-liu): the actual state in containerd is the source of truth, even we maintain
// in-memory image store, it's only for in-memory indexing. The image could be removed
// by someone else anytime, before/during/after we create the metadata. We should always
Expand All @@ -232,14 +235,18 @@ func (c *CRIImageService) PullImage(ctx context.Context, name string, credential
}

// pullImageWithLocalPull handles image pulling using the local client.
//
// The returned bytesPulled is the number of bytes actually fetched from the
// registry — cached layers are not counted because they never trigger an
// HTTP request.
func (c *CRIImageService) pullImageWithLocalPull(
ctx context.Context,
ref string,
credentials func(string) (string, string, error),
snapshotter string,
labels map[string]string,
imagePullProgressTimeout time.Duration,
) (containerd.Image, error) {
) (containerd.Image, uint64, error) {
pctx, pcancel := context.WithCancel(ctx)
defer pcancel()
pullReporter := newPullProgressReporter(ref, pcancel, imagePullProgressTimeout)
Expand Down Expand Up @@ -279,20 +286,25 @@ func (c *CRIImageService) pullImageWithLocalPull(
image, err := c.client.Pull(pctx, ref, pullOpts...)
pcancel()
if err != nil {
return nil, fmt.Errorf("failed to pull and unpack image %q: %w", ref, err)
return nil, 0, fmt.Errorf("failed to pull and unpack image %q: %w", ref, err)
}
return image, nil
_, bytesPulled := pullReporter.reqReporter.status()
return image, bytesPulled, nil
}

// pullImageWithTransferService handles image pulling using the transfer service.
//
// The returned bytesPulled is the number of bytes actually fetched from the
// registry, accumulated from transfer progress events; cached layers emit an
// "already exists" event that does not increment the counter.
func (c *CRIImageService) pullImageWithTransferService(
ctx context.Context,
ref string,
credentials func(string) (string, string, error),
snapshotter string,
labels map[string]string,
imagePullProgressTimeout time.Duration,
) (containerd.Image, error) {
) (containerd.Image, uint64, error) {
log.G(ctx).Debugf("PullImage %q with snapshotter %s using transfer service", ref, snapshotter)
rctx, rcancel := context.WithCancel(ctx)
defer rcancel()
Expand All @@ -318,23 +330,24 @@ func (c *CRIImageService) pullImageWithTransferService(

reg, err := registry.NewOCIRegistry(ctx, ref, opts...)
if err != nil {
return nil, fmt.Errorf("failed to create OCI registry: %w", err)
return nil, 0, fmt.Errorf("failed to create OCI registry: %w", err)
}

transferProgressReporter.start(rctx)
log.G(ctx).Debugf("Calling cri transfer service")
err = c.transferrer.Transfer(rctx, reg, is, transfer.WithProgress(transferProgressReporter.createProgressFunc(rctx)))
rcancel()
if err != nil {
return nil, fmt.Errorf("failed to pull and unpack image %q: %w", ref, err)
return nil, 0, fmt.Errorf("failed to pull and unpack image %q: %w", ref, err)
}

// Image should be pulled, unpacked and present in containerd image store at this moment
image, err := c.client.GetImage(ctx, ref)
if err != nil {
return nil, fmt.Errorf("failed to get image %q from containerd image store: %w", ref, err)
return nil, 0, fmt.Errorf("failed to get image %q from containerd image store: %w", ref, err)
}
return image, nil
_, bytesPulled := transferProgressReporter.reqReporter.status()
return image, bytesPulled, nil
}

// ParseAuth parses AuthConfig and returns username and password/secret required by containerd.
Expand Down
63 changes: 63 additions & 0 deletions internal/cri/server/images/image_pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -834,3 +834,66 @@ func TestPullProgressReporter(t *testing.T) {
}
})
}

// fakeObserver records every Observe call so tests can assert both the
// presence and the value of observations without touching the real histogram.
type fakeObserver struct {
samples []float64
}

func (f *fakeObserver) Observe(v float64) {
f.samples = append(f.samples, v)
}

func TestRecordImagePullThroughput(t *testing.T) {
for _, tc := range []struct {
name string
bytesPulled uint64
duration time.Duration
wantSamples int
wantValue float64 // only checked when wantSamples == 1
}{
{
name: "fully cached pull is not observed",
bytesPulled: 0,
duration: 2 * time.Second,
wantSamples: 0,
},
{
name: "zero duration is not observed",
bytesPulled: 10 * mibToByte,
duration: 0,
wantSamples: 0,
},
{
name: "cold pull observes MiB/s from fetched bytes",
bytesPulled: 10 * mibToByte,
duration: 2 * time.Second,
wantSamples: 1,
wantValue: 5.0,
},
{
name: "partial cache hit observes only fetched bytes",
// 200 MiB image, 150 MiB cached, 50 MiB actually fetched over 1s.
bytesPulled: 50 * mibToByte,
duration: 1 * time.Second,
wantSamples: 1,
wantValue: 50.0,
},
{
name: "sub-second pull observes correctly",
bytesPulled: 25 * mibToByte,
duration: 500 * time.Millisecond,
wantSamples: 1,
wantValue: 50.0,
},
} {
t.Run(tc.name, func(t *testing.T) {
obs := &fakeObserver{}
recordImagePullThroughput(obs, tc.bytesPulled, tc.duration)
if assert.Len(t, obs.samples, tc.wantSamples) && tc.wantSamples == 1 {
assert.InDelta(t, tc.wantValue, obs.samples[0], 0.001)
}
})
}
}
48 changes: 46 additions & 2 deletions internal/cri/server/images/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,29 @@
package images

import (
"time"

"github.com/docker/go-metrics"
prom "github.com/prometheus/client_golang/prometheus"
)

const mibToByte = 1024 * 1024

var (
imagePulls metrics.LabeledCounter
inProgressImagePulls metrics.Gauge
// image size in MB / image pull duration in seconds
// imagePullThroughput observes one MiB/s sample per image pull: bytes
// actually fetched from the registry over end-to-end pull duration.
// Fully-cached pulls are skipped.
//
// Deprecated: use imagePullThroughputMiBps instead. The new metric has
// the same semantics but more useful buckets — DefBuckets tops out at
// 10 MiB/s and saturates almost immediately on real hardware.
imagePullThroughput prom.Histogram
// imagePullThroughputMiBps observes one MiB/s sample per image pull (not
// per layer): bytes actually fetched from the registry over end-to-end
// pull duration (includes extraction). Fully-cached pulls are skipped.
imagePullThroughputMiBps prom.Histogram
)

func init() {
Expand All @@ -44,10 +58,40 @@ func init() {
Namespace: namespace,
Subsystem: subsystem,
Name: "image_pulling_throughput",
Help: "image pull throughput",
Help: "Deprecated: use image_pulling_throughput_mibps instead. Image pull throughput in MiB/s (fetched bytes / pull duration; cached layers excluded).",
Buckets: prom.DefBuckets,
},
)
imagePullThroughputMiBps = prom.NewHistogram(
prom.HistogramOpts{
Namespace: namespace,
Subsystem: subsystem,
Name: "image_pulling_throughput_mibps",
Help: "image pull throughput in MiB/s (fetched bytes / pull duration; cached layers excluded)",
// Buckets cover everything from slow registry pulls (0.1 MiB/s)
// to 4000 MiB/s (~31 Gbps), enough for modern 10G+ NICs. The
// top end is intentionally generous: past ~4 GiB/s, disk write
// throughput becomes the bottleneck rather than the network, so
// we don't need finer buckets above that.
Buckets: []float64{0.1, 1, 10, 25, 75, 150, 300, 500, 1000, 2000, 4000},
},
)
ns.Add(imagePullThroughput)
ns.Add(imagePullThroughputMiBps)
metrics.Register(ns)
}

// recordImagePullThroughput observes one MiB/s sample on obs (intended for
// imagePullThroughputMiBps). bytesPulled is bytes actually fetched from the
// registry — cached layers never trigger a fetch, so they are excluded.
// duration is the end-to-end pull duration (includes extraction, not just
// network transfer).
//
// Pulls that fetched nothing (fully cached) are skipped rather than recorded
// as zero or near-infinite samples, which would pollute the histogram.
func recordImagePullThroughput(obs prom.Observer, bytesPulled uint64, duration time.Duration) {
if bytesPulled == 0 || duration <= 0 {
return
}
obs.Observe(float64(bytesPulled) / mibToByte / duration.Seconds())
}
14 changes: 11 additions & 3 deletions pkg/archive/tar_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"os"
"strings"

"github.com/moby/sys/sequential"
"golang.org/x/sys/windows"
)

// chmodTarEntry is used to adjust the file permissions used in tar header based
Expand All @@ -44,13 +44,21 @@ func setHeaderForSpecialDevice(*tar.Header, string, os.FileInfo) error {
func open(p string) (*os.File, error) {
// We use sequential file access to avoid depleting the standby list on
// Windows.
return sequential.Open(p)
//
// Refer to the [Win32 API documentation] for details on sequential file access.
//
// [Win32 API documentation]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#FILE_FLAG_SEQUENTIAL_SCAN
return os.OpenFile(p, os.O_RDONLY|windows.O_FILE_FLAG_SEQUENTIAL_SCAN, 0)
}

func openFile(name string, flag int, perm os.FileMode) (*os.File, error) {
// Source is regular file. We use sequential file access to avoid depleting
// the standby list on Windows.
return sequential.OpenFile(name, flag, perm)
//
// Refer to the [Win32 API documentation] for details on sequential file access.
//
// [Win32 API documentation]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#FILE_FLAG_SEQUENTIAL_SCAN
return os.OpenFile(name, flag|windows.O_FILE_FLAG_SEQUENTIAL_SCAN, perm)
}

func mkdir(path string, perm os.FileMode) error {
Expand Down
10 changes: 9 additions & 1 deletion pkg/oci/spec_opts_user_bounds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import (

// TestOpenUserFileCapsReads asserts the boundary behavior of the read cap:
// well below, ending exactly at, and past maxUserFileBytes.
//
// Regression test for CVE-2026-47262 / GHSA-jpcc-p29g-p8mq
func TestOpenUserFileCapsReads(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -60,7 +62,13 @@ func TestOpenUserFileCapsReads(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

data := append(bytes.Repeat([]byte{0}, tc.padBytes), beyond...)
pattern := []byte("# padding\n")
pad := bytes.Repeat(pattern, (tc.padBytes+len(pattern)-1)/len(pattern))[:tc.padBytes]
if len(pad) > 0 {
pad[len(pad)-1] = '\n'
}

data := append(pad, beyond...)
fsys := fstest.MapFS{
"etc/group": &fstest.MapFile{Data: data, Mode: 0o644},
}
Expand Down
7 changes: 6 additions & 1 deletion plugins/snapshots/devmapper/snapshotter.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,8 +428,13 @@ func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
errs = append(errs, sErr)
}

poolStatus := "unavailable"
if status != nil {
poolStatus = status.RawOutput
}

// Rollback thin device creation if mkfs failed
log.G(ctx).WithError(errors.Join(errs...)).Errorf("failed to initialize thin device %q for snapshot %s pool status %s", deviceName, snap.ID, status.RawOutput)
log.G(ctx).WithError(errors.Join(errs...)).Errorf("failed to initialize thin device %q for snapshot %s pool status %s", deviceName, snap.ID, poolStatus)
return nil, errors.Join(append(errs, s.pool.RemoveDevice(ctx, deviceName))...)
}
} else {
Expand Down
Loading
Loading