Rework docker app image ls and add the --digests flag #709
Conversation
Codecov Report
@@ Coverage Diff @@
## master #709 +/- ##
========================================
+ Coverage 71.4% 71.5% +0.1%
========================================
Files 59 59
Lines 3175 3201 +26
========================================
+ Hits 2267 2289 +22
- Misses 602 604 +2
- Partials 306 308 +2
Continue to review full report at Codecov.
|
e2e/images_test.go
Outdated
| a-simple-app:latest <none> simple | ||
| b-simple-app:latest <none> simple | ||
| ` | ||
| expectedOutput := fmt.Sprintf(expected, info.registryAddress+"/c-myapp:latest") |
There was a problem hiding this comment.
Should we add an image by digest to check the digest column has something else than <none>?
There was a problem hiding this comment.
This is what I wanted to pair with you to figure out how to do
There was a problem hiding this comment.
At one point the insertBundle function pushed and pulled an image by digest, you can see it here: https://github.com/docker/app/pull/648/files#diff-d8de3905e4eb053a6e7c6a6bda133499R23
You can reuse this kind of logic to get a digested output.
zappy-shu
force-pushed
the
add-digest-flag-to-ls
branch
3 times, most recently
from
c64589c
to
c5d0f5e
Compare
zappy-shu
changed the title
zappy-shu
force-pushed
the
add-digest-flag-to-ls
branch
7 times, most recently
from
a64a013
to
ea4d7d7
Compare
zappy-shu
changed the title
zappy-shu
force-pushed
the
add-digest-flag-to-ls
branch
from
ea4d7d7
to
4c4943c
Compare
Added the --digests flag to image ls. This makes the image ls command print the image digests (if present) along with the app name and image. Signed-off-by: Nick Adcock <nick.adcock@docker.com>
Removed the temporary directories from being created in the app image ls tests as they are not used. Signed-off-by: Nick Adcock <nick.adcock@docker.com>
Split the app image ls output up so that the REPOSITORY and TAG are separate columns, the same as the standard docker image ls. The APP IMAGE ID column is now a default column, same as the ID column in docker image ls. As this ID is randomly generated the e2e tests have been updated to use regexes when asserting the command output. Signed-off-by: Nick Adcock <nick.adcock@docker.com>
zappy-shu
force-pushed
the
add-digest-flag-to-ls
branch
from
4c4943c
to
24ced43
Compare
|
Is the spacing between the columns the same as for container images? From the following, It looks to me like app has less spaces which makes it tricky to read. |
|
I haven't touched the spacing as part of this PR but the tabwriter for the CLI does have different settings to the one used here: Do we want to update the app tabwriter so the spacing match the CLI? |
Yes. That will make the feel the same.
👍 Please just create a ticket to track |
- WIP
Need to get an e2e or unit test that covers the digest.
- What I did
Reworked the image ls output to better match the
docker image lscommand. The output now has separate columns for REPOSITORY and TAG as well as an APP IMAGE ID column.Added the --digests flag to image ls. This makes the image ls command print the image digests (if present) along with the other columns.
- How I did it
Made the columns list for
image lsdynamic depending on what options are sent.The digest is retrieved by attempted to cast to a
Digestedtype (i.e. the image has been stored by digest rather than by tag or ID). If not then just prints<none>.- How to verify it
Pull an image from hub by digest and then run
app image ls --digest. The image in question should show the digest prefixed by the algorithm type. E.g.- Description for the changelog
Added the
--digestsflag to theapp image lscommand. When present, the command will print the image's digest or<none>if the image does not have a digest, along with other image details.- A picture of a cute animal (not mandatory but encouraged)