Change podman build --pull=true to PullIfMissing #8301
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TomSweeneyRedHat 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 |
openshift-ci-robot
added
the
approved
| image from the registry only if the image is not present locally. Raise an | ||
| error if the image is not found in the registries. | ||
|
|
||
| If the option is not specified, pull the image from the registry if it is not | ||
| present locally or if there is a newer version on the registry. Raise an |
There was a problem hiding this comment.
This is technically incorrect. It pulls if the registry version is different, not necessarily newer.
Image I create a container image and push it to a registry as :latest, later I find there is a big bug in my version, and remove it, causing the previous version to be :latest. Now if someone had pulled in my buggy image, and later pulls again they will revert back to the older image, even though they were PullIfNewer.
Container/Image has no way to determine whether or not one image is "NEWER" then another image, it can only check if the digest is different. We recently received a bug on this misunderstanding from a user, so we should not be documenting it incorrectly in the man pages and other docs.
There was a problem hiding this comment.
@vrothberg I thought you had recently changed the check to pull only if the image had a newer date stamp, not that it had changed. Am I misremembering?
There was a problem hiding this comment.
I don't know, but as @mtrmac pointed out in a different issue/pr this is a bad idea, since it will not remove a bad image unless someone pushes a "new" version. As opposed to removing the old "new" image.
There was a problem hiding this comment.
@vrothberg I thought you had recently changed the check to pull only if the image had a newer date stamp, not that it had changed. Am I misremembering?
An earlier version of the shortnames PR for Buildah had that but I reverted that part based on the feedback. A digest check is really the way forward. Some images claim to be have been born before me which I doubt :^)
|
LGTM |
One last tweak to the man page for 'build --pull' and after further testing against Docker, one slight change to the pull policy. First I changed `--pull=false` from PullNever to PullIfMissing. This matches Docker and will pull the image if it's not present rather than erroring. We've the `--pull-never` option if someone wants the pull to not do an actual pull and to error if the image isn't local. Then for the man page, I'd a much bigger change, in the initial PR, I've backed most of that out and just added a tweak. Hopefully this puts this portion of the pull work behind us for a while. Signed-off-by: TomSweeneyRedHat <[email protected]>
TomSweeneyRedHat
changed the title
|
Please re-review from the start. Much smaller change, plus I've added a Wiki page to explain the pull policies if you're interested in more depth. |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
One last tweak to the man page for 'build --pull' and after
further testing against Docker, one slight change to the
pull policy. First I changed
--pull=falsefrom PullNeverto PullIfMissing. This matches Docker and will pull the
image if it's not present rather than erroring. We've
the
--pull-neveroption if someone wants the pull tonot do an actual pull and to error if the image isn't
local.
Then for the man page, I'd a much bigger change, in the
initial PR, I've backed most of that out and just
added a tweak.
Hopefully, this puts this portion of the pull work behind
us for a while.
Signed-off-by: TomSweeneyRedHat [email protected]