Fix CREATED field when listing image if date is not specified #2107

jonatasbaldin · 2019-09-26T03:14:49Z

Signed-off-by: Jonatas Baldin jonatas.baldin@gmail.com

Fixes #2047.

- What I did
When listing images with the docker images command, returns <none> in the CREATED field if the date is not specified during the image build. Without this fix, a 292 years ago string was displayed.

- How I did it
Added a special case in the CLI formatter.

- How to verify it

Get an image that has no created date docker pull nixery.dev/hello

Before the fix:

Run docker images

REPOSITORY                           TAG                 IMAGE ID            CREATED             SIZE
nixery.dev/hello                     latest              866a9896ca80        292 years ago             28.1MB

After the fix:

REPOSITORY                           TAG                 IMAGE ID            CREATED             SIZE
nixery.dev/hello                     latest              866a9896ca80        <none>              28.1MB

- Description for the changelog
Fix CREATED field when listing image if date is not specified.

- A picture of a cute animal (not mandatory but encouraged)
an alpaca an me

jonatasbaldin · 2019-10-01T01:25:07Z

hey there, any comments on this? 🌱

thaJeztah

Thanks! And apologies for the long delay; maintainers have been quite busy, and there's a bit of a backlog in reviewing pull requests. Left a comment; suggestion following 🤗

thaJeztah · 2019-10-25T15:46:27Z

cli/command/formatter/image.go

@@ -235,6 +235,11 @@ func (c *imageContext) Digest() string {

 func (c *imageContext) CreatedSince() string {
 	createdAt := time.Unix(c.i.Created, 0)
+
+	if createdAt.IsZero() {
+		return "<none>"


Thinking of a slightly different approach; I'll write down a comment / suggestion

thaJeztah · 2019-10-25T15:50:48Z

I'm thinking if we should use this special format, or instead to return an empty string. I realise we already have some of the "special" values that we return, and have not been really consistent in that matter (unfortunately; bad choices in the past), e.g. here's an example of docker image ls --format '{{ json . }}' where it's clear we're not really consistent.

{
  "Containers": "N/A",
  "CreatedAt": "2019-08-20 22:19:55 +0200 CEST",
  "CreatedSince": "2 months ago",
  "Digest": "<none>",
  "ID": "961769676411",
  "Repository": "alpine",
  "SharedSize": "N/A",
  "Size": "5.58MB",
  "Tag": "3.10",
  "UniqueSize": "N/A",
  "VirtualSize": "5.582MB"
}

I'm thinking of using an empty string instead. That way, the json output would be closer to what the API returns (and reduce the amount of "presentation" in that format. This would keep the flexibility for users to format it to their liking. For example;

docker image ls -a --format "table {{.ID}}\t{{if .CreatedSince }}{{.CreatedSince}}{{else}}Prehistoric image, may contain dragons{{end}}"

IMAGE ID            CREATED
dc2fed1afb2c        19 hours ago
8c75c34169e5        2 days ago
48479b5deffa        2 days ago
75fed645a5a0        Prehistoric image, may contain dragons
87b2dd3fa15d        3 days ago
3a3b77f619aa        3 days ago
1544ad69a5d2        Prehistoric image, may contain dragons
9614462fec5e        3 days ago
1f61a4270ed5        4 days ago

If we go for this option, I'd be ok with changing the default table format to print <none> or N/A;

cli/cli/command/formatter/image.go

Lines 14 to 15 in 69fdd2a

    
           defaultImageTableFormat           = "table {{.Repository}}\t{{.Tag}}\t{{.ID}}\t{{.CreatedSince}}\t{{.Size}}" 
        
           defaultImageTableFormatWithDigest = "table {{.Repository}}\t{{.Tag}}\t{{.Digest}}\t{{.ID}}\t{{.CreatedSince}}\t{{.Size}}"

I'm slightly leaning towards N/A, but as mentioned, we're already inconsistent, and <none> is also used (for images without tag or digest), so I'm open to suggestions there.

These would then look something like;

	defaultImageTableFormat           = "table {{.Repository}}\t{{.Tag}}\t{{.ID}}\t{{if .CreatedSince }}{{.CreatedSince}}{{else}}N/A{{end}}\t{{.Size}}"
	defaultImageTableFormatWithDigest = "table {{.Repository}}\t{{.Tag}}\t{{.Digest}}\t{{.ID}}\t{{if .CreatedSince }}{{.CreatedSince}}{{else}}N/A{{end}}\t{{.Size}}"

Note: Perhaps as a follow-up, we should consider taking a similar approach for existing fields, but we may need to discuss that, as there could be people relying on the format, even though the cli output format is not a formal API.

thaJeztah · 2019-10-25T15:52:36Z

Happy to hear your thoughts! If you agree with my suggestions, could you update the pull request?

As to the empty values themselves; I think this was a bug in BuildKit (not 100% sure), which should be resolved, but I agree that there may still be images with that issue, so it's nice to take those into account

jonatasbaldin · 2019-10-28T12:54:42Z

Hey there! I don't have strong opinions on this, so I'm good going with an empty string return and N/A.

Will make the changes!

jonatasbaldin · 2019-10-28T13:13:36Z

Did the changes ✨ Here's the outputs:

$ ./build/docker-darwin-amd64  images -f reference='nixery.dev/hello'
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
nixery.dev/hello    latest              866a9896ca80        N/A                 28.1MB

$ ./build/docker-darwin-amd64 images -f reference='nixery.dev/hello' --format '{{ json .}}'      
{
  "Containers": "N/A",
  "CreatedAt": "0000-12-31 20:53:32 -0306 LMT",
  "CreatedSince": "N/A",
  "Digest": "<none>",
  "ID": "866a9896ca80",
  "Repository": "nixery.dev/hello",
  "SharedSize": "N/A",
  "Size": "28.1MB",
  "Tag": "latest",
  "UniqueSize": "N/A",
  "VirtualSize": "28.13MB"
}

GordonTheTurtle · 2019-10-28T13:16:52Z

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "2047-fix-image-createdsince-without-value" git@github.com:jonatasbaldin/cli.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354553256
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

thaJeztah · 2019-10-28T16:40:01Z

cli/command/formatter/image.go

@@ -235,6 +235,11 @@ func (c *imageContext) Digest() string {

 func (c *imageContext) CreatedSince() string {
 	createdAt := time.Unix(c.i.Created, 0)
+
+	if createdAt.IsZero() {
+		return "N/A"


Thanks! Could you change this to return an empty string, so that the --format '{{ json .}}' output will show the empty value as-is?

Oh man, I'm so sorry, shouldn't do any PRs when I'm waking up hahaha Fixed.

LOL no worries! 😂

thaJeztah

changes look good! erm (my bad, forgot to ask); could you squash your commits down to one? (Let me know if you need a hand on that)

jonatasbaldin · 2019-10-29T00:33:40Z

Squashed ✨!

thaJeztah

LGTM 👍

thaJeztah · 2019-10-29T10:49:10Z

ping @vdemeester @silvin-lubecki PTAL

cli/command/formatter/image_test.go

silvin-lubecki

Thanks for this PR @jonatasbaldin ! Just a small change requested 😅

Signed-off-by: Jonatas Baldin <jonatas.baldin@gmail.com>

jonatasbaldin · 2019-10-31T17:33:25Z

Hey there, just a gentle nudge on this! @silvin-lubecki answered your comment in the thread ✨

thaJeztah

LGTM

silvin-lubecki

LGTM

silvin-lubecki · 2019-11-05T08:47:03Z

Thank you @jonatasbaldin !!

GordonTheTurtle added the status/0-triage label Sep 26, 2019

jonatasbaldin mentioned this pull request Sep 26, 2019

"292 years ago" in docker images listing tazjin/nixery#12

Closed

thaJeztah added status/2-code-review and removed status/0-triage labels Oct 25, 2019

thaJeztah requested changes Oct 25, 2019

View reviewed changes

GordonTheTurtle added the dco/no label Oct 28, 2019

thaJeztah reviewed Oct 28, 2019

View reviewed changes

thaJeztah requested changes Oct 28, 2019

View reviewed changes

GordonTheTurtle removed the dco/no label Oct 29, 2019

thaJeztah approved these changes Oct 29, 2019

View reviewed changes

silvin-lubecki reviewed Oct 29, 2019

View reviewed changes

cli/command/formatter/image_test.go Show resolved Hide resolved

silvin-lubecki requested changes Oct 29, 2019

View reviewed changes

GordonTheTurtle added the dco/no label Oct 29, 2019

Return empty string if image createdAt is zero

56729bc

Signed-off-by: Jonatas Baldin <jonatas.baldin@gmail.com>

GordonTheTurtle removed the dco/no label Oct 29, 2019

thaJeztah approved these changes Nov 5, 2019

View reviewed changes

silvin-lubecki approved these changes Nov 5, 2019

View reviewed changes

silvin-lubecki merged commit 9e041dd into docker:master Nov 5, 2019

thaJeztah mentioned this pull request Nov 14, 2019

Added created date to app images docker/app#735

Merged

thaJeztah added this to the 19.03.6 milestone Feb 5, 2020

thaJeztah added the impact/changelog label Feb 5, 2020

thaJeztah modified the milestones: 19.03.6, next Feb 5, 2020

gabrieljones mentioned this pull request May 25, 2021

Deprecate created metadata field that is covered by digest. Create uncovered created moby/moby#42417

Closed

thaJeztah mentioned this pull request May 26, 2023

Use OCI "History" type instead of inventing our own copy moby/moby#45504

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix CREATED field when listing image if date is not specified #2107

Fix CREATED field when listing image if date is not specified #2107

jonatasbaldin commented Sep 26, 2019

jonatasbaldin commented Oct 1, 2019

thaJeztah left a comment

thaJeztah Oct 25, 2019

thaJeztah commented Oct 25, 2019

thaJeztah commented Oct 25, 2019

jonatasbaldin commented Oct 28, 2019

jonatasbaldin commented Oct 28, 2019

GordonTheTurtle commented Oct 28, 2019

thaJeztah Oct 28, 2019

jonatasbaldin Oct 28, 2019

thaJeztah Oct 28, 2019

thaJeztah left a comment

jonatasbaldin commented Oct 29, 2019

thaJeztah left a comment

thaJeztah commented Oct 29, 2019

silvin-lubecki left a comment

jonatasbaldin commented Oct 31, 2019

thaJeztah left a comment

silvin-lubecki left a comment

silvin-lubecki commented Nov 5, 2019

Fix CREATED field when listing image if date is not specified #2107

Fix CREATED field when listing image if date is not specified #2107

Conversation

jonatasbaldin commented Sep 26, 2019

jonatasbaldin commented Oct 1, 2019

thaJeztah left a comment

Choose a reason for hiding this comment

thaJeztah Oct 25, 2019

Choose a reason for hiding this comment

thaJeztah commented Oct 25, 2019

thaJeztah commented Oct 25, 2019

jonatasbaldin commented Oct 28, 2019

jonatasbaldin commented Oct 28, 2019

GordonTheTurtle commented Oct 28, 2019

thaJeztah Oct 28, 2019

Choose a reason for hiding this comment

jonatasbaldin Oct 28, 2019

Choose a reason for hiding this comment

thaJeztah Oct 28, 2019

Choose a reason for hiding this comment

thaJeztah left a comment

Choose a reason for hiding this comment

jonatasbaldin commented Oct 29, 2019

thaJeztah left a comment

Choose a reason for hiding this comment

thaJeztah commented Oct 29, 2019

silvin-lubecki left a comment

Choose a reason for hiding this comment

jonatasbaldin commented Oct 31, 2019

thaJeztah left a comment

Choose a reason for hiding this comment

silvin-lubecki left a comment

Choose a reason for hiding this comment

silvin-lubecki commented Nov 5, 2019