Skip to content

Commit 670b82e

Browse files
luke-beamcloudluke-beamcloud
committed
cleaning up
1 parent 7643849 commit 670b82e

File tree

4 files changed

+46
-55
lines changed

4 files changed

+46
-55
lines changed

‎pkg/worker/image.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -417,12 +417,12 @@ func (c *ImageClient) GetSourceImageRef(imageId string) (string, bool) {
417417
return c.v2ImageRefs.Get(imageId)
418418
}
419419

420-
// GetCLIPImageMetadata extracts CLIP image metadata from the archive for a v2 image
420+
// GetImageMetadata extracts CLIP image metadata from the archive for a v2 image
421421
// Returns the CLIP metadata directly from the archive (source of truth)
422-
func (c *ImageClient) GetCLIPImageMetadata(imageId string) (*clipCommon.ImageMetadata, bool) {
422+
func (c *ImageClient) GetImageMetadata(imageId string) (*clipCommon.ImageMetadata, bool) {
423423
// Determine the archive path for this image
424424
archivePath := fmt.Sprintf("/images/%s.%s", imageId, reg.LocalImageFileExtension)
425-
425+
426426
// Check if the archive exists
427427
if _, err := os.Stat(archivePath); os.IsNotExist(err) {
428428
// Try cache path as fallback
@@ -431,27 +431,27 @@ func (c *ImageClient) GetCLIPImageMetadata(imageId string) (*clipCommon.ImageMet
431431
} else {
432432
archivePath = fmt.Sprintf("%s/%s.clip", c.imageCachePath, imageId)
433433
}
434-
434+
435435
if _, err := os.Stat(archivePath); os.IsNotExist(err) {
436436
return nil, false
437437
}
438438
}
439-
439+
440440
// Extract metadata from the CLIP archive
441441
archiver := clip.NewClipArchiver()
442442
meta, err := archiver.ExtractMetadata(archivePath)
443443
if err != nil {
444444
log.Warn().Err(err).Str("image_id", imageId).Msg("failed to extract metadata from clip archive")
445445
return nil, false
446446
}
447-
447+
448448
// Check if this is an OCI archive with metadata
449449
if meta != nil && meta.StorageInfo != nil {
450450
if ociInfo, ok := meta.StorageInfo.(clipCommon.OCIStorageInfo); ok && ociInfo.ImageMetadata != nil {
451451
return ociInfo.ImageMetadata, true
452452
}
453453
}
454-
454+
455455
return nil, false
456456
}
457457

‎pkg/worker/lifecycle.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -386,8 +386,7 @@ func (s *Worker) readBundleConfig(request *types.ContainerRequest) (*specs.Spec,
386386
// This is ONLY used for v2 images where we don't have an unpacked bundle with config.json.
387387
// V1 images always have a config.json, so if we're here, it's a v2 image.
388388
func (s *Worker) deriveSpecFromV2Image(request *types.ContainerRequest) (*specs.Spec, error) {
389-
// Extract metadata from CLIP archive
390-
clipMeta, ok := s.imageClient.GetCLIPImageMetadata(request.ImageId)
389+
metadata, ok := s.imageClient.GetImageMetadata(request.ImageId)
391390
if !ok {
392391
log.Warn().
393392
Str("image_id", request.ImageId).
@@ -398,41 +397,42 @@ func (s *Worker) deriveSpecFromV2Image(request *types.ContainerRequest) (*specs.
398397
log.Info().
399398
Str("image_id", request.ImageId).
400399
Msg("using metadata from v2 clip archive")
401-
402-
return s.buildSpecFromCLIPMetadata(clipMeta), nil
403-
}
404400

401+
return s.buildSpecFromMetadata(metadata), nil
402+
}
405403

406-
// buildSpecFromCLIPMetadata constructs an OCI spec from CLIP image metadata
404+
// buildSpecFromMetadata constructs an OCI spec from image metadata
407405
// This is the primary path for v2 images with embedded metadata
408-
func (s *Worker) buildSpecFromCLIPMetadata(clipMeta *clipCommon.ImageMetadata) *specs.Spec {
406+
func (s *Worker) buildSpecFromMetadata(metadata *clipCommon.ImageMetadata) *specs.Spec {
409407
spec := specs.Spec{
410408
Process: &specs.Process{
411409
Env: []string{},
412410
},
413411
}
414412

415413
// CLIP metadata has a flat structure with all fields at the top level
416-
if len(clipMeta.Env) > 0 {
417-
spec.Process.Env = clipMeta.Env
414+
if len(metadata.Env) > 0 {
415+
spec.Process.Env = metadata.Env
418416
}
419-
if clipMeta.WorkingDir != "" {
420-
spec.Process.Cwd = clipMeta.WorkingDir
417+
418+
if metadata.WorkingDir != "" {
419+
spec.Process.Cwd = metadata.WorkingDir
421420
}
422-
if clipMeta.User != "" {
423-
spec.Process.User.Username = clipMeta.User
421+
422+
if metadata.User != "" {
423+
spec.Process.User.Username = metadata.User
424424
}
425+
425426
// Combine Entrypoint and Cmd, or use Cmd alone
426-
if len(clipMeta.Entrypoint) > 0 {
427-
spec.Process.Args = append(clipMeta.Entrypoint, clipMeta.Cmd...)
428-
} else if len(clipMeta.Cmd) > 0 {
429-
spec.Process.Args = clipMeta.Cmd
427+
if len(metadata.Entrypoint) > 0 {
428+
spec.Process.Args = append(metadata.Entrypoint, metadata.Cmd...)
429+
} else if len(metadata.Cmd) > 0 {
430+
spec.Process.Args = metadata.Cmd
430431
}
431432

432433
return &spec
433434
}
434435

435-
436436
// Generate a runc spec from a given request
437437
func (s *Worker) specFromRequest(request *types.ContainerRequest, options *ContainerOptions) (*specs.Spec, error) {
438438
os.MkdirAll(filepath.Join(baseConfigPath, request.ContainerId), os.ModePerm)

‎pkg/worker/lifecycle_test.go‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func TestV2ImageEnvironmentFlow(t *testing.T) {
6565
// Without a real CLIP archive, readBundleConfig returns nil gracefully
6666
initialSpec, err := worker.readBundleConfig(request)
6767
require.NoError(t, err)
68-
68+
6969
// Spec will be nil without archive (real archives tested in integration tests)
7070
assert.Nil(t, initialSpec, "Should return nil when CLIP archive is not present")
7171
t.Logf("✅ V2 image correctly attempts to extract from CLIP archive")
@@ -215,8 +215,8 @@ func TestCachedImageMetadata(t *testing.T) {
215215
t.Run("UsesCachedMetadata", func(t *testing.T) {
216216
// Note: In real use, metadata would be extracted from the CLIP archive on-demand.
217217
// Since we don't have actual archives in tests, this test verifies the fallback path.
218-
// For v2 images with metadata, GetCLIPImageMetadata() would extract it from the archive.
219-
218+
// For v2 images with metadata, GetImageMetadata() would extract it from the archive.
219+
220220
imageId := "v2-cached-image-123"
221221
request := &types.ContainerRequest{
222222
ContainerId: "test-container-cached",
@@ -250,20 +250,6 @@ func TestCachedImageMetadata(t *testing.T) {
250250
})
251251
}
252252

253-
// TestGetCLIPImageMetadata tests the CLIP image metadata extraction from archives
254-
func TestGetCLIPImageMetadata(t *testing.T) {
255-
t.Run("ExtractsFromArchiveWhenPresent", func(t *testing.T) {
256-
// GetCLIPImageMetadata extracts metadata on-demand from CLIP archives.
257-
// This test would require creating an actual CLIP archive file.
258-
// In real usage:
259-
// 1. The archive is downloaded/cached at /images/{imageId}.clip
260-
// 2. GetCLIPImageMetadata() extracts OCIStorageInfo.ImageMetadata from it
261-
// 3. The metadata is used to build the OCI spec
262-
// Integration tests verify this with actual archives.
263-
t.Skip("Requires actual CLIP archive file - tested in integration tests")
264-
})
265-
}
266-
267253
// Get a base test spec
268254
func getTestBaseSpec() specs.Spec {
269255
return specs.Spec{

‎pkg/worker/runc_server.go‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -341,24 +341,27 @@ func (s *RunCServer) writeInitialSpecFromImage(ctx context.Context, instance *Co
341341
spec := s.baseConfigSpec
342342

343343
// First try to get cached metadata from CLIP archive (v2 images)
344-
if clipMeta, ok := s.imageClient.GetCLIPImageMetadata(instance.Request.ImageId); ok {
344+
if metadata, ok := s.imageClient.GetImageMetadata(instance.Request.ImageId); ok {
345345
log.Info().Str("image_id", instance.Request.ImageId).Msg("using cached image metadata from clip archive for initial spec")
346-
346+
347347
// CLIP metadata has a flat structure with all fields at the top level
348-
if len(clipMeta.Env) > 0 {
349-
spec.Process.Env = append(spec.Process.Env, clipMeta.Env...)
348+
if len(metadata.Env) > 0 {
349+
spec.Process.Env = append(spec.Process.Env, metadata.Env...)
350350
}
351-
if clipMeta.WorkingDir != "" {
352-
spec.Process.Cwd = clipMeta.WorkingDir
351+
352+
if metadata.WorkingDir != "" {
353+
spec.Process.Cwd = metadata.WorkingDir
353354
}
354-
if clipMeta.User != "" {
355-
spec.Process.User.Username = clipMeta.User
355+
356+
if metadata.User != "" {
357+
spec.Process.User.Username = metadata.User
356358
}
359+
357360
// Set default args from Cmd if Entrypoint is not set, or combine them
358-
if len(clipMeta.Entrypoint) > 0 {
359-
spec.Process.Args = append(clipMeta.Entrypoint, clipMeta.Cmd...)
360-
} else if len(clipMeta.Cmd) > 0 {
361-
spec.Process.Args = clipMeta.Cmd
361+
if len(metadata.Entrypoint) > 0 {
362+
spec.Process.Args = append(metadata.Entrypoint, metadata.Cmd...)
363+
} else if len(metadata.Cmd) > 0 {
364+
spec.Process.Args = metadata.Cmd
362365
}
363366
} else {
364367
// Fallback: use runtime inspection for v1 images or when metadata is not cached
@@ -367,6 +370,7 @@ func (s *RunCServer) writeInitialSpecFromImage(ctx context.Context, instance *Co
367370
if instance.Request.BuildOptions.SourceImage != nil {
368371
imageRef = *instance.Request.BuildOptions.SourceImage
369372
}
373+
370374
creds = instance.Request.BuildOptions.SourceImageCreds
371375

372376
if imageRef != "" {
@@ -375,7 +379,7 @@ func (s *RunCServer) writeInitialSpecFromImage(ctx context.Context, instance *Co
375379
log.Warn().Str("image_ref", imageRef).Err(err).Msg("failed to inspect image for initial spec; proceeding with base config only")
376380
} else {
377381
log.Info().Str("image_ref", imageRef).Msg("using runtime inspection for initial spec")
378-
382+
379383
// Apply env from skopeo output if available
380384
if len(imgMeta.Env) > 0 {
381385
spec.Process.Env = append(spec.Process.Env, imgMeta.Env...)
@@ -388,6 +392,7 @@ func (s *RunCServer) writeInitialSpecFromImage(ctx context.Context, instance *Co
388392
if err != nil {
389393
return err
390394
}
395+
391396
return os.WriteFile(destPath, b, 0644)
392397
}
393398

0 commit comments

Comments
 (0)