Propagate container image size lookup errors

dan-stowell · dan-stowell · commit b5fd10c2c355 · 2026-04-21T13:53:49.000Z
diff --git a/enterprise/server/remote_execution/container/container.go b/enterprise/server/remote_execution/container/container.go
@@ -469,7 +469,7 @@ type CommandContainer interface {
 	// ImageSizeBytes returns the estimated on-disk size of the pulled
 	// container image in bytes. Should be called after PullImage.
 	// Returns 0 if unknown or not applicable.
-	ImageSizeBytes(ctx context.Context) int64
+	ImageSizeBytes(ctx context.Context) (int64, error)
 }
 
 // StatsRecorder is an optional interface implemented by a [CommandContainer]
@@ -723,7 +723,7 @@ func (t *TracedCommandContainer) IsolationType() string {
 	return t.Delegate.IsolationType()
 }
 
-func (t *TracedCommandContainer) ImageSizeBytes(ctx context.Context) int64 {
+func (t *TracedCommandContainer) ImageSizeBytes(ctx context.Context) (int64, error) {
 	return t.Delegate.ImageSizeBytes(ctx)
 }
 
diff --git a/enterprise/server/remote_execution/container/container_test.go b/enterprise/server/remote_execution/container/container_test.go
@@ -28,6 +28,8 @@ type FakeContainer struct {
 	RequiredPullCredentials oci.Credentials
 	PullCount               int
 	pullDelay               time.Duration
+	imageSizeBytes          int64
+	imageSizeErr            error
 }
 
 func (c *FakeContainer) IsolationType() string {
@@ -62,7 +64,9 @@ func (c *FakeContainer) Unpause(ctx context.Context) error { return nil }
 func (c *FakeContainer) Stats(context.Context) (*repb.UsageStats, error) {
 	return &repb.UsageStats{}, nil
 }
-func (c *FakeContainer) ImageSizeBytes(context.Context) int64 { return 0 }
+func (c *FakeContainer) ImageSizeBytes(context.Context) (int64, error) {
+	return c.imageSizeBytes, c.imageSizeErr
+}
 
 func userCtx(t *testing.T, ta *testauth.TestAuthenticator, userID string) context.Context {
 	ctx, err := ta.WithAuthenticatedUser(context.Background(), userID)
@@ -421,10 +425,12 @@ func TestUsageStats_Timeseries(t *testing.T) {
 	assert.Equal(t, []int64{0, 500, 400}, memKBSamples, "memory kb samples")
 }
 
-func TestTracedCommandContainer_ImageSizeBytes(t *testing.T) {
-	delegate := &FakeContainer{}
+func TestTracedCommandContainer_ImageSizeBytes_Error(t *testing.T) {
+	delegate := &FakeContainer{imageSizeErr: status.InternalError("image inspect failed")}
 	traced := container.NewTracedCommandContainer(delegate)
-	assert.Equal(t, int64(0), traced.ImageSizeBytes(context.Background()))
+	_, err := traced.ImageSizeBytes(context.Background())
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "image inspect failed")
 }
 
 func makePSI(someTotal, fullTotal int64) *repb.PSI {
diff --git a/enterprise/server/remote_execution/containers/bare/bare.go b/enterprise/server/remote_execution/containers/bare/bare.go
@@ -193,4 +193,4 @@ func (c *bareCommandContainer) Stats(ctx context.Context) (*repb.UsageStats, err
 	return nil, nil
 }
 
-func (c *bareCommandContainer) ImageSizeBytes(ctx context.Context) int64 { return 0 }
+func (c *bareCommandContainer) ImageSizeBytes(ctx context.Context) (int64, error) { return 0, nil }
diff --git a/enterprise/server/remote_execution/containers/docker/docker.go b/enterprise/server/remote_execution/containers/docker/docker.go
@@ -202,12 +202,12 @@ func (c *dockerCommandContainer) IsolationType() string {
 	return "docker"
 }
 
-func (c *dockerCommandContainer) ImageSizeBytes(ctx context.Context) int64 {
+func (c *dockerCommandContainer) ImageSizeBytes(ctx context.Context) (int64, error) {
 	ii, err := c.client.ImageInspect(ctx, c.image, dockerclient.ImageInspectWithRawResponse(nil))
 	if err != nil {
-		return 0
+		return 0, err
 	}
-	return ii.Size
+	return ii.Size, nil
 }
 
 func (r *dockerCommandContainer) Run(ctx context.Context, command *repb.Command, workDir string, creds oci.Credentials) *interfaces.CommandResult {
diff --git a/enterprise/server/remote_execution/containers/firecracker/firecracker.go b/enterprise/server/remote_execution/containers/firecracker/firecracker.go
@@ -2102,19 +2102,22 @@ func (c *FirecrackerContainer) IsolationType() string {
 	return "firecracker"
 }
 
-func (c *FirecrackerContainer) ImageSizeBytes(ctx context.Context) int64 {
+func (c *FirecrackerContainer) ImageSizeBytes(ctx context.Context) (int64, error) {
 	if !c.pulled {
-		return 0
+		return 0, nil
 	}
 	imagePath, err := ociconv.CachedDiskImagePath(ctx, c.executorConfig.CacheRoot, c.containerImage)
-	if err != nil || imagePath == "" {
-		return 0
+	if err != nil {
+		return 0, err
+	}
+	if imagePath == "" {
+		return 0, nil
 	}
 	info, err := os.Stat(imagePath)
 	if err != nil {
-		return 0
+		return 0, err
 	}
-	return info.Size()
+	return info.Size(), nil
 }
 
 // Run the given command within the container and remove the container after
diff --git a/enterprise/server/remote_execution/containers/ociruntime/ociruntime.go b/enterprise/server/remote_execution/containers/ociruntime/ociruntime.go
@@ -629,15 +629,15 @@ func (c *ociContainer) IsolationType() string {
 	return "oci" // TODO: make const in platform.go
 }
 
-func (c *ociContainer) ImageSizeBytes(ctx context.Context) int64 {
+func (c *ociContainer) ImageSizeBytes(ctx context.Context) (int64, error) {
 	if c.lockedImage == nil {
-		return 0
+		return 0, nil
 	}
 	var total int64
 	for _, layer := range c.lockedImage.Layers {
 		total += layer.EstimatedDiskUsageBytes
 	}
-	return total
+	return total, nil
 }
 
 func (c *ociContainer) IsImageCached(ctx context.Context) (bool, error) {
diff --git a/enterprise/server/remote_execution/containers/podman/podman.go b/enterprise/server/remote_execution/containers/podman/podman.go
@@ -368,17 +368,20 @@ func (c *podmanCommandContainer) IsolationType() string {
 	return "podman"
 }
 
-func (c *podmanCommandContainer) ImageSizeBytes(ctx context.Context) int64 {
+func (c *podmanCommandContainer) ImageSizeBytes(ctx context.Context) (int64, error) {
 	stdout := &bytes.Buffer{}
 	res := c.runPodman(ctx, "image", &interfaces.Stdio{Stdout: stdout}, "inspect", c.image, "--format", "{{.Size}}")
-	if res.Error != nil || res.ExitCode != 0 {
-		return 0
+	if res.Error != nil {
+		return 0, res.Error
+	}
+	if res.ExitCode != 0 {
+		return 0, status.UnknownErrorf("podman image inspect exited with code %d", res.ExitCode)
 	}
 	size, err := strconv.ParseInt(strings.TrimSpace(stdout.String()), 10, 64)
 	if err != nil {
-		return 0
+		return 0, status.WrapError(err, "parse podman image size")
 	}
-	return size
+	return size, nil
 }
 
 func (c *podmanCommandContainer) Run(ctx context.Context, command *repb.Command, workDir string, creds oci.Credentials) *interfaces.CommandResult {
diff --git a/enterprise/server/remote_execution/containers/sandbox/sandbox.go b/enterprise/server/remote_execution/containers/sandbox/sandbox.go
@@ -348,4 +348,4 @@ func (c *sandbox) Unpause(ctx context.Context) error                          {
 func (c *sandbox) Stats(ctx context.Context) (*repb.UsageStats, error) {
 	return nil, nil
 }
-func (c *sandbox) ImageSizeBytes(ctx context.Context) int64 { return 0 }
+func (c *sandbox) ImageSizeBytes(ctx context.Context) (int64, error) { return 0, nil }
diff --git a/enterprise/server/remote_execution/executor/executor.go b/enterprise/server/remote_execution/executor/executor.go
@@ -329,7 +329,11 @@ func (s *Executor) ExecuteTaskAndStreamResults(ctx context.Context, st *repb.Sch
 	}
 
 	// Record container image metadata after pulling.
-	auxMetadata.ContainerImageRef, auxMetadata.ContainerImageSizeBytes = r.ContainerImageInfo(ctx)
+	var imageSizeErr error
+	auxMetadata.ContainerImageRef, auxMetadata.ContainerImageSizeBytes, imageSizeErr = r.ContainerImageInfo(ctx)
+	if imageSizeErr != nil {
+		log.CtxWarningf(ctx, "Failed to get container image size: %s", imageSizeErr)
+	}
 
 	md.InputFetchStartTimestamp = timestamppb.New(s.env.GetClock().Now())
 
diff --git a/enterprise/server/remote_execution/executor/executor_test.go b/enterprise/server/remote_execution/executor/executor_test.go
@@ -570,8 +570,8 @@ type imageSizingContainer struct {
 	imageSize int64
 }
 
-func (c *imageSizingContainer) ImageSizeBytes(ctx context.Context) int64 {
-	return c.imageSize
+func (c *imageSizingContainer) ImageSizeBytes(ctx context.Context) (int64, error) {
+	return c.imageSize, nil
 }
 
 type imageSizingContainerProvider struct {
diff --git a/enterprise/server/remote_execution/runner/runner.go b/enterprise/server/remote_execution/runner/runner.go
@@ -239,8 +239,9 @@ func (r *taskRunner) Metadata() *espb.RunnerMetadata {
 	return r.metadata.CloneVT()
 }
 
-func (r *taskRunner) ContainerImageInfo(ctx context.Context) (ref string, sizeBytes int64) {
-	return r.PlatformProperties.ContainerImage, r.Container.ImageSizeBytes(ctx)
+func (r *taskRunner) ContainerImageInfo(ctx context.Context) (ref string, sizeBytes int64, err error) {
+	sizeBytes, err = r.Container.ImageSizeBytes(ctx)
+	return r.PlatformProperties.ContainerImage, sizeBytes, err
 }
 
 func (r *taskRunner) String() string {
diff --git a/enterprise/server/remote_execution/runner/runner_test.go b/enterprise/server/remote_execution/runner/runner_test.go
@@ -139,7 +139,7 @@ func (c *fakeContainer) Pause(ctx context.Context) error {
 func (c *fakeContainer) Unpause(ctx context.Context) error {
 	return nil
 }
-func (c *fakeContainer) ImageSizeBytes(ctx context.Context) int64 { return 0 }
+func (c *fakeContainer) ImageSizeBytes(ctx context.Context) (int64, error) { return 0, nil }
 
 // fakeFirecrackerContainer behaves like a bare container except it returns
 // 0 mem / CPU resources, like Firecracker.
@@ -158,11 +158,12 @@ func (*fakeFirecrackerContainer) Stats(context.Context) (*repb.UsageStats, error
 // fakeImageSizingContainer is a bare container that also reports an image size.
 type fakeImageSizingContainer struct {
 	container.CommandContainer
-	imageSize int64
+	imageSize    int64
+	imageSizeErr error
 }
 
-func (c *fakeImageSizingContainer) ImageSizeBytes(ctx context.Context) int64 {
-	return c.imageSize
+func (c *fakeImageSizingContainer) ImageSizeBytes(ctx context.Context) (int64, error) {
+	return c.imageSize, c.imageSizeErr
 }
 
 func (c *fakeImageSizingContainer) Stats(context.Context) (*repb.UsageStats, error) {
@@ -1267,23 +1268,37 @@ func TestContainerImageInfo_WithImageSizer(t *testing.T) {
 	r, err := pool.Get(ctx, task)
 	require.NoError(t, err)
 
-	ref, sizeBytes := r.(*taskRunner).ContainerImageInfo(ctx)
+	ref, sizeBytes, err := r.(*taskRunner).ContainerImageInfo(ctx)
+	require.NoError(t, err)
 	// Bare runner normalizes container-image to "".
 	assert.Equal(t, "", ref)
 	assert.Equal(t, expectedSize, sizeBytes)
 }
 
-func TestContainerImageInfo_WithoutImageSizer(t *testing.T) {
+func TestContainerImageInfo_ImageSizeError(t *testing.T) {
 	env := newTestEnv(t)
-	pool := newRunnerPool(t, env, noLimitsCfg())
+	pool := newRunnerPool(t, env, &RunnerPoolOptions{
+		PoolOptions: &PoolOptions{
+			ContainerProvider: &ImageSizingContainerProvider{
+				ImageSize: 0,
+			},
+		},
+		MaxRunnerCount:            unlimited,
+		MaxRunnerDiskSizeBytes:    unlimited,
+		MaxRunnerMemoryUsageBytes: unlimited,
+	})
 	ctx := withAuthenticatedUser(t, context.Background(), env, "US1")
 
-	// bare container does not implement ImageSizer
 	task := newTask()
 	r, err := pool.Get(ctx, task)
 	require.NoError(t, err)
 
-	ref, sizeBytes := r.(*taskRunner).ContainerImageInfo(ctx)
+	imageSizingContainer := r.(*taskRunner).Container.Delegate.(*fakeImageSizingContainer)
+	imageSizingContainer.imageSizeErr = status.InternalError("image size unavailable")
+
+	ref, sizeBytes, err := r.(*taskRunner).ContainerImageInfo(ctx)
+	require.Error(t, err)
 	assert.Equal(t, "", ref)
 	assert.Equal(t, int64(0), sizeBytes)
+	assert.Contains(t, err.Error(), "image size unavailable")
 }
diff --git a/server/interfaces/interfaces.go b/server/interfaces/interfaces.go
@@ -1136,7 +1136,7 @@ type Runner interface {
 	// ContainerImageInfo returns the container image ref used for the
 	// execution, and its estimated on-disk size in bytes.
 	// Should be called after PrepareForTask.
-	ContainerImageInfo(ctx context.Context) (ref string, sizeBytes int64)
+	ContainerImageInfo(ctx context.Context) (ref string, sizeBytes int64, err error)
 }
 
 type CacheRoutingService interface {

Original file line number	Diff line number	Diff line change
`@@ -469,7 +469,7 @@ type CommandContainer interface {`
`469`	`469`	`// ImageSizeBytes returns the estimated on-disk size of the pulled`
`470`	`470`	`// container image in bytes. Should be called after PullImage.`
`471`	`471`	`// Returns 0 if unknown or not applicable.`
`472`		`- ImageSizeBytes(ctx context.Context) int64`
	`472`	`+ ImageSizeBytes(ctx context.Context) (int64, error)`
`473`	`473`	`}`
`474`	`474`
`475`	`475`	`// StatsRecorder is an optional interface implemented by a [CommandContainer]`
`@@ -723,7 +723,7 @@ func (t *TracedCommandContainer) IsolationType() string {`
`723`	`723`	`return t.Delegate.IsolationType()`
`724`	`724`	`}`
`725`	`725`
`726`		`-func (t *TracedCommandContainer) ImageSizeBytes(ctx context.Context) int64 {`
	`726`	`+func (t *TracedCommandContainer) ImageSizeBytes(ctx context.Context) (int64, error) {`
`727`	`727`	`return t.Delegate.ImageSizeBytes(ctx)`
`728`	`728`	`}`
`729`	`729`
Original file line number	Diff line number	Diff line change
`@@ -193,4 +193,4 @@ func (c bareCommandContainer) Stats(ctx context.Context) (repb.UsageStats, err`
`193`	`193`	`return nil, nil`
`194`	`194`	`}`
`195`	`195`
`196`		`-func (c *bareCommandContainer) ImageSizeBytes(ctx context.Context) int64 { return 0 }`
	`196`	`+func (c *bareCommandContainer) ImageSizeBytes(ctx context.Context) (int64, error) { return 0, nil }`
Original file line number	Diff line number	Diff line change
`@@ -202,12 +202,12 @@ func (c *dockerCommandContainer) IsolationType() string {`
`202`	`202`	`return "docker"`
`203`	`203`	`}`
`204`	`204`
`205`		`-func (c *dockerCommandContainer) ImageSizeBytes(ctx context.Context) int64 {`
	`205`	`+func (c *dockerCommandContainer) ImageSizeBytes(ctx context.Context) (int64, error) {`
`206`	`206`	`ii, err := c.client.ImageInspect(ctx, c.image, dockerclient.ImageInspectWithRawResponse(nil))`
`207`	`207`	`if err != nil {`
`208`		`- return 0`
	`208`	`+ return 0, err`
`209`	`209`	`}`
`210`		`- return ii.Size`
	`210`	`+ return ii.Size, nil`
`211`	`211`	`}`
`212`	`212`
`213`	`213`	`func (r dockerCommandContainer) Run(ctx context.Context, command repb.Command, workDir string, creds oci.Credentials) *interfaces.CommandResult {`
Original file line number	Diff line number	Diff line change
`@@ -2102,19 +2102,22 @@ func (c *FirecrackerContainer) IsolationType() string {`
`2102`	`2102`	`return "firecracker"`
`2103`	`2103`	`}`
`2104`	`2104`
`2105`		`-func (c *FirecrackerContainer) ImageSizeBytes(ctx context.Context) int64 {`
	`2105`	`+func (c *FirecrackerContainer) ImageSizeBytes(ctx context.Context) (int64, error) {`
`2106`	`2106`	`if !c.pulled {`
`2107`		`- return 0`
	`2107`	`+ return 0, nil`
`2108`	`2108`	`}`
`2109`	`2109`	`imagePath, err := ociconv.CachedDiskImagePath(ctx, c.executorConfig.CacheRoot, c.containerImage)`
`2110`		`- if err != nil \|\| imagePath == "" {`
`2111`		`- return 0`
	`2110`	`+ if err != nil {`
	`2111`	`+ return 0, err`
	`2112`	`+ }`
	`2113`	`+ if imagePath == "" {`
	`2114`	`+ return 0, nil`
`2112`	`2115`	`}`
`2113`	`2116`	`info, err := os.Stat(imagePath)`
`2114`	`2117`	`if err != nil {`
`2115`		`- return 0`
	`2118`	`+ return 0, err`
`2116`	`2119`	`}`
`2117`		`- return info.Size()`
	`2120`	`+ return info.Size(), nil`
`2118`	`2121`	`}`
`2119`	`2122`
`2120`	`2123`	`// Run the given command within the container and remove the container after`
Original file line number	Diff line number	Diff line change
`@@ -629,15 +629,15 @@ func (c *ociContainer) IsolationType() string {`
`629`	`629`	`return "oci" // TODO: make const in platform.go`
`630`	`630`	`}`
`631`	`631`
`632`		`-func (c *ociContainer) ImageSizeBytes(ctx context.Context) int64 {`
	`632`	`+func (c *ociContainer) ImageSizeBytes(ctx context.Context) (int64, error) {`
`633`	`633`	`if c.lockedImage == nil {`
`634`		`- return 0`
	`634`	`+ return 0, nil`
`635`	`635`	`}`
`636`	`636`	`var total int64`
`637`	`637`	`for _, layer := range c.lockedImage.Layers {`
`638`	`638`	`total += layer.EstimatedDiskUsageBytes`
`639`	`639`	`}`
`640`		`- return total`
	`640`	`+ return total, nil`
`641`	`641`	`}`
`642`	`642`
`643`	`643`	`func (c *ociContainer) IsImageCached(ctx context.Context) (bool, error) {`
Original file line number	Diff line number	Diff line change
`@@ -368,17 +368,20 @@ func (c *podmanCommandContainer) IsolationType() string {`
`368`	`368`	`return "podman"`
`369`	`369`	`}`
`370`	`370`
`371`		`-func (c *podmanCommandContainer) ImageSizeBytes(ctx context.Context) int64 {`
	`371`	`+func (c *podmanCommandContainer) ImageSizeBytes(ctx context.Context) (int64, error) {`
`372`	`372`	`stdout := &bytes.Buffer{}`
`373`	`373`	`res := c.runPodman(ctx, "image", &interfaces.Stdio{Stdout: stdout}, "inspect", c.image, "--format", "{{.Size}}")`
`374`		`- if res.Error != nil \|\| res.ExitCode != 0 {`
`375`		`- return 0`
	`374`	`+ if res.Error != nil {`
	`375`	`+ return 0, res.Error`
	`376`	`+ }`
	`377`	`+ if res.ExitCode != 0 {`
	`378`	`+ return 0, status.UnknownErrorf("podman image inspect exited with code %d", res.ExitCode)`
`376`	`379`	`}`
`377`	`380`	`size, err := strconv.ParseInt(strings.TrimSpace(stdout.String()), 10, 64)`
`378`	`381`	`if err != nil {`
`379`		`- return 0`
	`382`	`+ return 0, status.WrapError(err, "parse podman image size")`
`380`	`383`	`}`
`381`		`- return size`
	`384`	`+ return size, nil`
`382`	`385`	`}`
`383`	`386`
`384`	`387`	`func (c podmanCommandContainer) Run(ctx context.Context, command repb.Command, workDir string, creds oci.Credentials) *interfaces.CommandResult {`
Original file line number	Diff line number	Diff line change
`@@ -348,4 +348,4 @@ func (c *sandbox) Unpause(ctx context.Context) error {`
`348`	`348`	`func (c sandbox) Stats(ctx context.Context) (repb.UsageStats, error) {`
`349`	`349`	`return nil, nil`
`350`	`350`	`}`
`351`		`-func (c *sandbox) ImageSizeBytes(ctx context.Context) int64 { return 0 }`
	`351`	`+func (c *sandbox) ImageSizeBytes(ctx context.Context) (int64, error) { return 0, nil }`
Original file line number	Diff line number	Diff line change
`@@ -570,8 +570,8 @@ type imageSizingContainer struct {`
`570`	`570`	`imageSize int64`
`571`	`571`	`}`
`572`	`572`
`573`		`-func (c *imageSizingContainer) ImageSizeBytes(ctx context.Context) int64 {`
`574`		`- return c.imageSize`
	`573`	`+func (c *imageSizingContainer) ImageSizeBytes(ctx context.Context) (int64, error) {`
	`574`	`+ return c.imageSize, nil`
`575`	`575`	`}`
`576`	`576`
`577`	`577`	`type imageSizingContainerProvider struct {`