Skip to content

metadata: add comments about Image.CreatedAt#8225

Merged
kzys merged 1 commit intocontainerd:mainfrom
AkihiroSuda:comment-image-created-at
Mar 7, 2023
Merged

metadata: add comments about Image.CreatedAt#8225
kzys merged 1 commit intocontainerd:mainfrom
AkihiroSuda:comment-image-created-at

Conversation

@AkihiroSuda
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda commented Mar 7, 2023 •
edited
Loading

The value of image.CreatedAt passed from the caller is discarded in func (s *imageStore) Create(ctx context.Context, image images.Image) (images.Image, error).

Ideally we should return an error when the value is already set.

However, as image.CreatedAt is defined as a non-pointer time.Time, we can't compare it to nil.
And we can't compare it to time.Time{} either, as time.Time{} is a proper timestamp (1970-01-01 00:00:00).

@AkihiroSuda AkihiroSuda mentioned this pull request Mar 7, 2023
Merged
@AkihiroSuda AkihiroSuda changed the title metadata: add comments about Image.CreatedAt metadata: add comments about Image.CreatedAt Mar 7, 2023
@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Mar 7, 2023

Ideally, we should set image.CreatedAt in *imageStore.Create() only if it is "unset".

I don't think that is necessarily true. The CreatedAt in the image store database is referring to when that entry was created in the image store database, not when an image was built. Any build or other related metadata should be stored either on the image config object or as label metadata on the image store entry. I understand the confusion with how it ends up getting used for end clients, but that doesn't change what the value in the database means. It's possible we should have found a better solution for the source date stuff rather than overwriting this value.

@AkihiroSuda AkihiroSuda force-pushed the comment-image-created-at branch from a00c390 to 36d8038 Compare March 7, 2023 18:13
@AkihiroSuda
Copy link
Copy Markdown
Member Author

AkihiroSuda commented Mar 7, 2023

Reworded

@AkihiroSuda
metadata: add comments about Image.CreatedAt
ff4acdc 
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda AkihiroSuda force-pushed the comment-image-created-at branch from 36d8038 to ff4acdc Compare March 7, 2023 18:23
@kzys kzys merged commit 13bd938 into containerd:main Mar 7, 2023
@AkihiroSuda AkihiroSuda mentioned this pull request Mar 29, 2023
Open
@AkihiroSuda
Copy link
Copy Markdown
Member Author

AkihiroSuda commented Mar 29, 2023

Opened an issue for continuing the discussion:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kzys kzys kzys approved these changes

@dmcgowan dmcgowan dmcgowan approved these changes

Assignees

No one assigned

Labels

kind/docs

Projects

Pull Request Review
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AkihiroSuda @dmcgowan @kzys