Skip to content
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

Add support for image name history #4568

Merged
merged 1 commit into from
Nov 27, 2019

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Nov 26, 2019

We leverage the containers/storage image history tracking feature to
show the previously used image names when running:
podman images --history

Closes #4566

When building two images locally in sequence, then the output looks like this now:

> sudo ./bin/podman images --history
REPOSITORY       TAG      IMAGE ID       CREATED          SIZE      HISTORY
localhost/test   latest   2ba60be0975e   1 second ago     2.07 kB   <none>
<none>           <none>   79af25cf9b71   21 seconds ago   2.07 kB   localhost/test:latest

We skip the first history entry in the first line, because it is already part of the first image name.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 26, 2019
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of nits. Needs man page changes and bash completion. Other than that, LGTM

cmd/podman/images.go Outdated Show resolved Hide resolved
libpod/image/image.go Show resolved Hide resolved
@saschagrunert saschagrunert force-pushed the history branch 2 times, most recently from a0452da to 83908be Compare November 27, 2019 05:19
@rhatdan
Copy link
Member

rhatdan commented Nov 27, 2019

I think you need to update pkg/varlinkapi/images.go

@saschagrunert saschagrunert force-pushed the history branch 2 times, most recently from a394fa5 to 24bd03f Compare November 27, 2019 11:24
@saschagrunert saschagrunert changed the title WIP: Add support for image name history Add support for image name history Nov 27, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 27, 2019
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one last nit. I really like that we don't need to change the API but do post-processing in the client 👍

docs/source/markdown/podman-images.1.md Outdated Show resolved Hide resolved
We leverage the containers/storage image history tracking feature to
show the previously used image names when running:
`podman images --history`

Signed-off-by: Sascha Grunert <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Nov 27, 2019

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 27, 2019
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, saschagrunert

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 27, 2019
@openshift-merge-robot openshift-merge-robot merged commit 2178875 into containers:master Nov 27, 2019
@saschagrunert saschagrunert deleted the history branch November 27, 2019 14:32
@edsantiago
Copy link
Member

@saschagrunert the new test in 010-images.bats fails upon rerun because the history component is no longer empty. I'm willing to open a PR to fix that -- there are a couple other issues I want to address there -- but I'm not sure how to approach it. Is there a history-clear mechanism? Can you suggest a way to tackle this? TIA.

@saschagrunert
Copy link
Member Author

@saschagrunert the new test in 010-images.bats fails upon rerun because the history component is no longer empty. I'm willing to open a PR to fix that -- there are a couple other issues I want to address there -- but I'm not sure how to approach it. Is there a history-clear mechanism? Can you suggest a way to tackle this? TIA.

Yeah, do you have a test output at hand somewhere in the CI where I can have a look at it?

@edsantiago
Copy link
Member

It's on my own system, not CI (that's why I'm seeing this: CI only runs once, I'm running repeated iterations). This, I believe, demonstrates the problem:

# podman rmi -a                     ! get a fresh start
# podman pull -q alpine
965ea09ff2ebd2b9eeec88cd822ce156f6674c7e99be082c7efac3c62f3ff652
# podman images --format '{{.History}}'
                            <<<< empty, as expected
# podman tag alpine foo
# podman rmi foo
Untagged: localhost/foo:latest
# podman images --format '{{.History}}'
localhost/foo:latest        <<<<<<< no longer empty; and this persists to the next BATS run

@saschagrunert
Copy link
Member Author

Ah I think I got it. Is it because the cleanup function does not remove the image completely after the run? Might this solve the issue already on recurring runs?

@edsantiago
Copy link
Member

Is it because the cleanup function does not remove the image completely

Possibly, but that's intentional: it is critical that these tests pull only once, because quay.io is so unreliable: we need to minimize errors due to flakes, and I choose to do that by having one sane, trusted, unmodified image cached locally. Is there a way to clear/reset this tag history?

@saschagrunert
Copy link
Member Author

No not now, but we could build the test images via an empty Dockerfile ad-hoc, maybe via a little helper function to prepare test images.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discussion] Names history support for images
6 participants