Skip to content

Commit 5691fa6

Browse files
committed
Fix snapshot storage bugs: prevent panic on uninitialized chunker and propagate header wait failures
- Bug 1 (7a08b411): Add nil check in StorageDiff.FileSize to prevent panic when chunker is not initialized - Bug 2 (33d2a3b3): Change NewSnapshotSchedulingMetadata to return error, failing pause/checkpoint when header resolution fails instead of silently returning nil metadata
1 parent 6233209 commit 5691fa6

3 files changed

Lines changed: 16 additions & 5 deletions

File tree

packages/orchestrator/pkg/sandbox/build/storage_diff.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ func (b *StorageDiff) CachePath(context.Context) (string, error) {
128128
}
129129

130130
func (b *StorageDiff) FileSize(ctx context.Context) (int64, error) {
131+
if b.chunker == nil {
132+
return 0, fmt.Errorf("chunker not initialized")
133+
}
134+
131135
return b.chunker.FileSize(ctx)
132136
}
133137

packages/orchestrator/pkg/sandbox/sandbox.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1186,7 +1186,10 @@ func (s *Sandbox) Pause(
11861186
cleanup.AddNoContext(ctx, rootfsDiff.Close)
11871187

11881188
rootfsDiffHeader := NewResolvedDiffHeader(rootfsHeader)
1189-
schedulingMetadata := NewSnapshotSchedulingMetadata(ctx, memfileDiffHeader, rootfsDiffHeader)
1189+
schedulingMetadata, err := NewSnapshotSchedulingMetadata(ctx, memfileDiffHeader, rootfsDiffHeader)
1190+
if err != nil {
1191+
return nil, fmt.Errorf("failed to resolve scheduling metadata: %w", err)
1192+
}
11901193

11911194
metadataFileLink := template.NewLocalFileLink(cachePaths.CacheMetadata())
11921195
cleanup.AddNoContext(ctx, metadataFileLink.Close)

packages/orchestrator/pkg/sandbox/snapshot.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,16 @@ func (s *Snapshot) Close(ctx context.Context) error {
5959

6060
// NewSnapshotSchedulingMetadata derives the metadata from the new snapshot's
6161
// diff headers so the freshly created build and its contributions are included.
62-
func NewSnapshotSchedulingMetadata(ctx context.Context, memfileDiffHeader, rootfsDiffHeader *DiffHeader) *orchestrator.SchedulingMetadata {
62+
func NewSnapshotSchedulingMetadata(ctx context.Context, memfileDiffHeader, rootfsDiffHeader *DiffHeader) (*orchestrator.SchedulingMetadata, error) {
6363
memfileHeader, memfileErr := memfileDiffHeader.WaitWithContext(ctx)
64+
if memfileErr != nil {
65+
return nil, fmt.Errorf("memfile header wait failed: %w", memfileErr)
66+
}
67+
6468
rootfsHeader, rootfsErr := rootfsDiffHeader.WaitWithContext(ctx)
65-
if memfileErr != nil || rootfsErr != nil {
66-
return nil
69+
if rootfsErr != nil {
70+
return nil, fmt.Errorf("rootfs header wait failed: %w", rootfsErr)
6771
}
6872

69-
return scheduling.FromHeaders(memfileHeader, rootfsHeader)
73+
return scheduling.FromHeaders(memfileHeader, rootfsHeader), nil
7074
}

0 commit comments

Comments
 (0)