Integrate OCI image metadata into index #1485
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change introduces caching for image metadata extracted from CLIP archives. When available, this cached metadata is used to derive OCI specs, avoiding the need for runtime inspection via skopeo. This improves performance and reduces external dependencies during container startup. Co-authored-by: luke <[email protected]>
|
Cursor Agent can help with this pull request. Just |
Remove conversion of CLIP metadata to beta9 format. Store and retrieve CLIP metadata directly. Co-authored-by: luke <[email protected]>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 4 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="pkg/worker/lifecycle.go">
<violation number="1" location="pkg/worker/lifecycle.go:447">
Rule violated: **Prevent Redundant Code Duplication**
`buildSpecFromCLIPMetadata` reimplements the same spec population logic that already exists in `buildSpecFromImageMetadata`. Please extract the shared spec-building steps into a helper so both callers reuse it, avoiding divergent behavior when future fields are added.</violation>
<violation number="2" location="pkg/worker/lifecycle.go:456">
GetCLIPImageMetadata hands out the cached metadata pointer, so this assigns the shared Env slice. `getContainerEnvironment` later appends onto `options.InitialSpec.Process.Env`, which writes into that same backing array. With multiple containers we now race on the cached metadata and corrupt it. Please clone the env slice before storing it (and do the same for args so the metadata stays read-only).</violation>
<violation number="3" location="pkg/worker/lifecycle.go:466">
`clipMeta.Entrypoint` is the shared slice from the cached metadata. Appending to it here writes into that backing array, so concurrent requests will race and mutate the cached metadata. Please copy the entrypoint before appending.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
|
|
||
| // CLIP metadata has a flat structure with all fields at the top level | ||
| if len(clipMeta.Env) > 0 { | ||
| spec.Process.Env = clipMeta.Env |
There was a problem hiding this comment.
GetCLIPImageMetadata hands out the cached metadata pointer, so this assigns the shared Env slice. getContainerEnvironment later appends onto options.InitialSpec.Process.Env, which writes into that same backing array. With multiple containers we now race on the cached metadata and corrupt it. Please clone the env slice before storing it (and do the same for args so the metadata stays read-only).
Prompt for AI agents
Address the following comment on pkg/worker/lifecycle.go at line 456:
<comment>GetCLIPImageMetadata hands out the cached metadata pointer, so this assigns the shared Env slice. `getContainerEnvironment` later appends onto `options.InitialSpec.Process.Env`, which writes into that same backing array. With multiple containers we now race on the cached metadata and corrupt it. Please clone the env slice before storing it (and do the same for args so the metadata stays read-only).</comment>
<file context>
@@ -433,7 +442,37 @@ func (s *Worker) getSourceImageInfo(request *types.ContainerRequest) (string, st
+
+ // CLIP metadata has a flat structure with all fields at the top level
+ if len(clipMeta.Env) > 0 {
+ spec.Process.Env = clipMeta.Env
+ }
+ if clipMeta.WorkingDir != "" {
</file context>
Remove unused cache for CLIP image metadata. Extract metadata on-demand from archives. Co-authored-by: luke <[email protected]>
Co-authored-by: luke <[email protected]>
There was a problem hiding this comment.
Reviewed changes from recent commits (found 2 issues).
2 issues found across 4 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="pkg/worker/lifecycle.go">
<violation number="1" location="pkg/worker/lifecycle.go:484">
Dropping the Config-based assignments leaves InitialSpec without Process.Args/Cwd/User, so containers that rely on the image defaults now get an empty process.args and fail to start when runc requires at least one command.</violation>
</file>
<file name="pkg/worker/runc_server.go">
<violation number="1" location="pkg/worker/runc_server.go:380">
Restoring only Env from skopeo inspect drops the image’s Cmd/Entrypoint/WorkingDir/User, so v1 images without CLIP metadata boot with the base defaults instead of their intended process. Please keep applying the full Config fields when CLIP metadata is missing.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
pkg/worker/lifecycle.go
Outdated
| } | ||
| } else if len(imgMeta.Env) > 0 { | ||
| // Use Env field from skopeo output | ||
| if len(imgMeta.Env) > 0 { |
There was a problem hiding this comment.
Dropping the Config-based assignments leaves InitialSpec without Process.Args/Cwd/User, so containers that rely on the image defaults now get an empty process.args and fail to start when runc requires at least one command.
Prompt for AI agents
Address the following comment on pkg/worker/lifecycle.go at line 484:
<comment>Dropping the Config-based assignments leaves InitialSpec without Process.Args/Cwd/User, so containers that rely on the image defaults now get an empty process.args and fail to start when runc requires at least one command.</comment>
<file context>
@@ -480,24 +480,8 @@ func (s *Worker) buildSpecFromImageMetadata(imgMeta *common.ImageMetadata) *spec
- }
- } else if len(imgMeta.Env) > 0 {
+ // Use Env field from skopeo output
+ if len(imgMeta.Env) > 0 {
spec.Process.Env = imgMeta.Env
}
</file context>
✅ Addressed in 670b82e
pkg/worker/runc_server.go
Outdated
| log.Info().Str("image_ref", imageRef).Msg("using runtime inspection for initial spec") | ||
|
|
||
| // Apply env from skopeo output if available | ||
| if len(imgMeta.Env) > 0 { |
There was a problem hiding this comment.
Restoring only Env from skopeo inspect drops the image’s Cmd/Entrypoint/WorkingDir/User, so v1 images without CLIP metadata boot with the base defaults instead of their intended process. Please keep applying the full Config fields when CLIP metadata is missing.
Prompt for AI agents
Address the following comment on pkg/worker/runc_server.go at line 380:
<comment>Restoring only Env from skopeo inspect drops the image’s Cmd/Entrypoint/WorkingDir/User, so v1 images without CLIP metadata boot with the base defaults instead of their intended process. Please keep applying the full Config fields when CLIP metadata is missing.</comment>
<file context>
@@ -376,25 +376,8 @@ func (s *RunCServer) writeInitialSpecFromImage(ctx context.Context, instance *Co
- } else if len(imgMeta.Env) > 0 {
- // Fallback to legacy Env field if Config is not available
+ // Apply env from skopeo output if available
+ if len(imgMeta.Env) > 0 {
spec.Process.Env = append(spec.Process.Env, imgMeta.Env...)
}
</file context>
This commit refactors the logic for deriving OCI specs for v2 images. Instead of relying on inspecting the source image via skopeo, it now prioritizes extracting metadata directly from the CLIP archive embedded within the v2 image. This change simplifies the process for v2 images, making it more efficient and reliable by leveraging the image's own metadata. The tests have been updated to reflect this new behavior, ensuring that skopeo is no longer called for v2 images when CLIP metadata is available. Co-authored-by: luke <[email protected]>
luke-lombardi
deleted the
cursor/integrate-oci-image-metadata-into-index-2dfe
branch
There was a problem hiding this comment.
Reviewed changes from recent commits (found 1 issue).
1 issue found across 6 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="pkg/worker/lifecycle_test.go">
<violation number="1" location="pkg/worker/lifecycle_test.go:218">
The updated comment refers to GetImageMetadata(), but the actual client method remains GetCLIPImageMetadata(), so this misleads readers about the API name. Please update the comment to reference the correct method name.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| t.Run("UsesCachedMetadata", func(t *testing.T) { | ||
| // Note: In real use, metadata would be extracted from the CLIP archive on-demand. | ||
| // Since we don't have actual archives in tests, this test verifies the fallback path. | ||
| // For v2 images with metadata, GetImageMetadata() would extract it from the archive. |
There was a problem hiding this comment.
The updated comment refers to GetImageMetadata(), but the actual client method remains GetCLIPImageMetadata(), so this misleads readers about the API name. Please update the comment to reference the correct method name.
Prompt for AI agents
Address the following comment on pkg/worker/lifecycle_test.go at line 218:
<comment>The updated comment refers to GetImageMetadata(), but the actual client method remains GetCLIPImageMetadata(), so this misleads readers about the API name. Please update the comment to reference the correct method name.</comment>
<file context>
@@ -215,8 +215,8 @@ func TestCachedImageMetadata(t *testing.T) {
// Since we don't have actual archives in tests, this test verifies the fallback path.
- // For v2 images with metadata, GetCLIPImageMetadata() would extract it from the archive.
-
+ // For v2 images with metadata, GetImageMetadata() would extract it from the archive.
+
imageId := "v2-cached-image-123"
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request contains changes generated by a Cursor Cloud Agent
Summary by cubic
Use CLIP image metadata from the archive to build container specs and initial_config.json for v2 images, removing runtime skopeo inspections and speeding up startup.
Written for commit 35be922. Summary will update automatically on new commits.