Fix #1138: Set org.opencontainers.image.created annotation on ModelKit manifests#1139
Fix #1138: Set org.opencontainers.image.created annotation on ModelKit manifests#1139itniuma2026 wants to merge 2 commits intokitops-ml:mainfrom
Conversation
|
Can you add the sign-off messages to your commits. Click on the failing DCO check above for how to instructions. |
There was a problem hiding this comment.
Pull request overview
Adds the standard OCI manifest annotation org.opencontainers.image.created during ModelKit pack so registries/tools can read creation time directly from the manifest metadata.
Changes:
- Import
timein the local storage pack implementation. - Set
org.opencontainers.image.createdon the OCI manifest annotations duringSaveModel.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/lib/filesystem/local-storage.go
Outdated
| if manifest.Annotations == nil { | ||
| manifest.Annotations = map[string]string{} | ||
| } |
There was a problem hiding this comment.
createManifest() already initializes manifest.Annotations (and sets ml.kitops.modelkit.cli-version), so this nil-check/map allocation is currently redundant, and it also makes the later if manifest.Annotations == nil block in the ModelPack annotation section dead code. Consider consolidating all “global annotation” writes (including org.opencontainers.image.created) in createManifest() and removing the duplicated nil checks.
pkg/lib/filesystem/local-storage.go
Outdated
| if manifest.Annotations == nil { | ||
| manifest.Annotations = map[string]string{} | ||
| } | ||
| manifest.Annotations[ocispec.AnnotationCreated] = time.Now().UTC().Format(time.RFC3339) |
There was a problem hiding this comment.
Setting org.opencontainers.image.created to time.Now() makes the packed manifest digest non-deterministic across runs. This will break the existing reproducibility expectation (e.g., testing/pack-unpack_test.go:TestPackReproducibility asserts two consecutive kit pack runs produce the same digest) and can reduce layer/manifest deduplication in storage/registries. Consider sourcing the timestamp deterministically (e.g., from a reproducible build time env var) or gating this annotation so reproducible packs remain stable, and adjust tests accordingly.
|
@itniuma2026 can you fix the DCO failure |
…fests Add the standard OCI annotation `org.opencontainers.image.created` with the current time in RFC 3339 format to ModelKit manifests during the pack operation, alongside the existing `ml.kitops.modelkit.cli-version` annotation. Fixes kitops-ml#1138 Signed-off-by: itniuma <itniuma@proton.me>
itniuma2026
force-pushed
the
fix/issue-1138
branch
from
d7840e0 to
92e85bb
Compare
|
@gorkem Done! I've amended the commit with the required |
pkg/lib/filesystem/local-storage.go
Outdated
| if manifest.Annotations == nil { | ||
| manifest.Annotations = map[string]string{} | ||
| } | ||
| manifest.Annotations[ocispec.AnnotationCreated] = time.Now().UTC().Format(time.RFC3339) | ||
|
|
There was a problem hiding this comment.
createManifest already sets up some annotations (the CLI version) -- it should probably also apply this annotation as part of that process. The reason we set the additional annotation below is because it's optionally included and ModelPack-specific behaviour.
createManifest should be responsible for creating the base ModelKit manifest in as complete a way as possible.
There was a problem hiding this comment.
Good point! Moved the org.opencontainers.image.created annotation into createManifest() alongside the CLI version annotation, and removed the redundant nil-check from SaveModel(). Thanks for the review.
Move org.opencontainers.image.created annotation from SaveModel() into createManifest() alongside the existing CliVersion annotation, as requested by reviewer. Removes redundant nil-check in SaveModel(). Signed-off-by: itniuma <itniuma@proton.me>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| manifest.Annotations[constants.CliVersionAnnotation] = constants.Version | ||
| manifest.Annotations[ocispec.AnnotationCreated] = time.Now().UTC().Format(time.RFC3339) | ||
|
|
There was a problem hiding this comment.
org.opencontainers.image.created is being set using a fresh time.Now() call here, which can diverge from the creation timestamp stored in the config blob (e.g., ModelPack config sets CreatedAt separately and currently uses non-UTC time). This can lead to inconsistent "created" times between manifest and config. Consider generating a single UTC timestamp once during pack and reusing it for both config creation and this manifest annotation (or deriving the manifest value from the config’s CreatedAt when available).
Summary
Add the standard OCI annotation
org.opencontainers.image.createdwith the current time in RFC 3339 format to ModelKit manifests during the pack operation, alongside the existingml.kitops.modelkit.cli-versionannotation.Approach
In the function within
pkg/lib/filesystem/local-storage.gothat constructs the manifest and sets annotations (whereml.kitops.modelkit.cli-versionis already set), add a new annotation entry with keyorg.opencontainers.image.createdand value set totime.Now().UTC().Format(time.RFC3339). This requires importing thetimepackage if not already imported. The change should be placed right next to the existing CLI version annotation assignment to keep annotation logic grouped together.Files Changed
pkg/lib/filesystem/local-storage.goRelated Issue
Fixes #1138
Testing
No tests were added with this change. Happy to add them if needed.