-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add creation timestamp to podman artifacts #27155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Generated by Cursor. |
pkg/libartifact/store/store.go
Outdated
if options.Annotations != nil { | ||
annotations = maps.Clone(options.Annotations) | ||
} | ||
annotations[specV1.AnnotationCreated] = fmt.Sprintf("%d", time.Now().UTC().UnixNano()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong, should be an RFC 3339 string per the OCI spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will tell Cursor to make the change. I would prefer this as an int, so it is easy to convert, But if this is defined in the spec.
test/e2e/artifact_test.go
Outdated
artifact1Name := "localhost/test/artifact1" | ||
|
||
// Record time before creation (with some buffer for slow systems) | ||
beforeCreate := time.Now().UTC().Add(-time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this gonna cause flakes for people?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so - the way it's implemented seems safe
Expect(failSession).Should(ExitWithError(125, "Error: append option is not compatible with type option")) | ||
}) | ||
|
||
It("podman artifact inspect shows created date", func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this be part of another test, maybe one of the more basic ones? not a requirement, just asking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I combined the two tests together. One to check the creation date and one to make sure that appending to the artifact does not change the date.
These tests take almost not time.
This commit implements automatic creation timestamp functionality for artifacts as requested in GitHub issue containers#27081, allowing users to see when artifacts were created. Changes made: - Add org.opencontainers.image.created annotation with Unix nanoseconds timestamp during artifact creation - Preserve original creation timestamp when using --append option - Update artifact inspect and add man pages to document the new functionality - Add comprehensive e2e and system BATS tests to verify creation timestamp behavior - Store timestamp as integer (Unix nanoseconds) for programmatic access The creation timestamp helps users understand artifact freshness, particularly useful for AI models and other time-sensitive artifacts managed by tools like RamaLama. Usage examples: podman artifact add myartifact:latest /path/to/file # Creates with timestamp podman artifact inspect myartifact:latest # Shows created annotation as integer podman artifact add --append myartifact:latest /file2 # Preserves original timestamp Fixes: containers#27081 Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
I'm cool with this ... LGTM |
Sure |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2102673
into
containers:main
This commit implements automatic creation timestamp functionality for artifacts as requested in GitHub issue #27081, allowing users to see when artifacts were created.
Changes made:
The creation timestamp helps users understand artifact freshness, particularly useful for AI models and other time-sensitive artifacts managed by tools like RamaLama.
Usage examples:
podman artifact add myartifact:latest /path/to/file # Creates with timestamp
podman artifact inspect myartifact:latest # Shows created annotation as integer
podman artifact add --append myartifact:latest /file2 # Preserves original timestamp
Fixes: #27081
Does this PR introduce a user-facing change?