-
Notifications
You must be signed in to change notification settings - Fork 185
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
image list: add creation date #2611
Conversation
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.
Looking good, Just some minor comments!
6f9c028
to
9af5373
Compare
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.
Looks good, I tested it and it seems to be working fine for locally built images.
I noticed if I pull the image and print it, I see an empty value:
REPOSITORY TAG DIGEST CREATED
test latest 2772850011db 4 days ago
trace_open latest 42832ffe4547
Is that expected?
It depends if the image you pulled has the annotation with the creation date. |
Actually, there seems to be a bug there... |
When pulling the image from the remote repository with However, the annotations from the manifests are preserved. Two possible solutions:
I tried to implement the first solution, but I didn't get it to work: --- a/pkg/oci/oci.go
+++ b/pkg/oci/oci.go
@@ -165,11 +165,31 @@ func pullGadgetImageToStore(ctx context.Context, imageStore oras.Target, image s
return nil, fmt.Errorf("copying to remote repository: %w", err)
}
+ // oras.Copy() ignores annotations from the index. Retrieve annotations from manifest.
+ manifest, err := getManifestForHost(ctx, imageStore, targetImage.String())
+ if err != nil {
+ return nil, fmt.Errorf("reading manifest: %w", err)
+ }
+
+ desc.Annotations = manifest.Annotations
+
+ var manifestDesc ocispec.Descriptor
+ manifestBytes, err := getContentBytesFromDescriptor(ctx, repo, manifestDesc)
+ if err != nil {
+ return nil, fmt.Errorf("getting manifest from descriptor: %w", err)
+ }
+ repo.Push(ctx, desc, bytes.NewReader(manifestBytes))
+ err = repo.Tag(ctx, desc, targetImage.String())
+ if err != nil {
+ return nil, fmt.Errorf("tagging image with annotations: %w", err)
+ }
+
imageDesc := &GadgetImageDesc{
Repository: targetImage.Name(),
Digest: desc.Digest.String(),
- Created: getTimeFromAnnotations(desc.Annotations),
+ Created: getTimeFromAnnotations(manifest.Annotations),
}
+ fmt.Printf("Image %s:%s@%s created at %+v\n", imageDesc.Repository, imageDesc.Tag, imageDesc.Digest, imageDesc.Created)
if ref, ok := targetImage.(reference.Tagged); ok {
imageDesc.Tag = ref.Tag()
} The Printf prints the correct date, but it is not saved in See also this long discussion oras-project/oras-go#22 (comment) about annotations in descriptor versus annotations in the manifest itself. IIUC, the discussion suggests we should not rely on the remote registry to keep the annotations in the index, but instead we should use the annotations in the manifest. I suggest to merge this PR, and find a way to fix it in a follow-up PR. Wdyt? |
9af5373
to
7912246
Compare
The OCI image spec defines the creation date annotation: https://github.com/opencontainers/image-spec/blob/main/annotations.md org.opencontainers.image.created date and time on which the image was built, conforming to RFC 3339. This patch ensures that the annotation is set with the same value for both amd64 and arm64. Then, the date is printed in the image list command: ``` $ sudo -E ig image list INFO[0000] Experimental features enabled REPOSITORY TAG DIGEST CREATED trace_dns latest d98eea3ede9e 4 minutes ago ``` Note: the creation date will also be useful for the future Artifact Hub patch because the 'createdAt' field is mandatory in artifacthub-pkg.yml: https://github.com/artifacthub/hub/blob/master/docs/metadata/artifacthub-pkg.yml ``` createdAt: The date this package was created (RFC3339 layout) (required) ``` Signed-off-by: Alban Crequy <albancrequy@linux.microsoft.com>
7912246
to
e1e4433
Compare
I fixed it with solution 2. It should work with pulled images too now. |
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.
LGTM, works fine for pulled images as well!
image list: add creation date
The OCI image spec defines the creation date annotation: https://github.com/opencontainers/image-spec/blob/main/annotations.md
This patch ensures that the annotation is set with the same value for both amd64 and arm64.
Then, the date is printed in the image list command:
Note: the creation date will also be useful for the future Artifact Hub patch because the 'createdAt' field is mandatory in artifacthub-pkg.yml:
https://github.com/artifacthub/hub/blob/master/docs/metadata/artifacthub-pkg.yml
How to use
Testing done