add --pull flag for podman create&run #3617

QiWang19 · 2019-07-22T14:00:59Z

Requirement from #3575 (comment)
Fixes: #3734

Added --pull for podman create and pull to match the newly added flag in docker CLI.
missing: default value, podman will pull the image if it does not exist in the local.
always: podman will always pull the image.
never: podman will never pull the image.

Signed-off-by: Qi Wang qiwan@redhat.com

TomSweeneyRedHat · 2019-07-22T14:25:12Z

docs/podman-create.1.md

+**--pull**=*missing*
+
+Pull image before creating ("always"|"missing"|"never") (default "missing").
+       'missing': default value, podman will pull the image if it does not exist in the local.


Suggest: "exist in the local" to "exist locally". If you take the change, make it throughout. That lines this up with the Buildah verbiage.

TomSweeneyRedHat · 2019-07-22T14:30:46Z

pkg/adapter/runtime.go

-func (r *LocalRuntime) New(ctx context.Context, name, signaturePolicyPath, authfile string, writer io.Writer, dockeroptions *image.DockerRegistryOptions, signingoptions image.SigningOptions, forcePull bool, label *string) (*ContainerImage, error) {
-	img, err := r.Runtime.ImageRuntime().New(ctx, name, signaturePolicyPath, authfile, writer, dockeroptions, signingoptions, forcePull, label)
+func (r *LocalRuntime) New(ctx context.Context, name, signaturePolicyPath, authfile string, writer io.Writer, dockeroptions *image.DockerRegistryOptions, signingoptions image.SigningOptions, forcePull bool, label *string, pull string) (*ContainerImage, error) {
+	img, err := r.Runtime.ImageRuntime().New(ctx, name, signaturePolicyPath, authfile, writer, dockeroptions, signingoptions, forcePull, label, pull)


I'd prefer "pullType" for the variable name "pull", just specifies better what the variable is used for and easier to grep for. I'll let @mheon break the tie on the name.

Do we need to keep both focePull and pullType as arguments?

please drop forcePull

TomSweeneyRedHat · 2019-07-22T14:31:33Z

Does this line up with Docker functionality? Or at least it's just added functionality and won't break previous scripts?

QiWang19 · 2019-07-22T14:36:36Z

I think this lines up with Docker functionality. And this hasn't been added to the current docker release. I just tried this functionality with nightly build of the docker CLI https://download.docker.com/linux/static/nightly/x86_64/

TomSweeneyRedHat · 2019-07-22T14:37:14Z

@QiWang19 all kinds of test unhappiness

mheon · 2019-07-22T15:23:43Z

I think we had reservations about the old patches for this from @haircommander but I don't think they're related to the code as written here.

haircommander · 2019-07-22T15:27:21Z

@mheon there were reservations about that patch (#2618) because it was addressing functionality that seemed like it existed in upstream docker, but didn't. this patch is much better, as it actually mirrors docker functionality that made it in.

rh-atomic-bot · 2019-07-29T19:57:33Z

☔ The latest upstream changes (presumably #3656) made this pull request unmergeable. Please resolve the merge conflicts.

mheon · 2019-08-02T23:01:04Z

/approve

Tagging for 1.5.1

openshift-ci-robot · 2019-08-02T23:01:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon, QiWang19

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:

~~OWNERS~~ [mheon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

abitrolly · 2019-08-05T04:16:31Z

@QiWang19

always: podman will always pull the image.

Is there an option to pull only when there is a newer image? (newer of not exist)

QiWang19 · 2019-08-05T13:35:24Z

Is there an option to pull only when there is a newer image? (newer of not exist)

@abitrolly I think this PR https://github.com/containers/libpod/pull/2618/files implemented --update can check the newer image and pull it. But this PR hasn't been merged.
@rhatdan @haircommander do we want to reopen that PR and add --update?

haircommander · 2019-08-05T13:40:10Z

@QiWang19 there were some fundamental problems that @mtrmac found with that PR, and I never found any corresponding functionality in the docker cli. The closest they have is what you're adding here. I am not inclined to reopen #2618 and rebase, but feel free to take it over if you want it :)

rhatdan · 2019-08-07T12:12:53Z

@QiWang19 Lets get this merged, then we can look at Buildah.

rh-atomic-bot · 2019-08-08T12:32:52Z

☔ The latest upstream changes (presumably #3744) made this pull request unmergeable. Please resolve the merge conflicts.

vrothberg · 2019-08-09T06:50:42Z

For now no plan to support --pull("always"|"missing"|"never") for build.
build also has --pull(pull the image if not present (default true))
Do we want to change build? make --pull and --pull-aways to --pull same as create and run in this PR.

I definitely prefer the "new" --pull flag of this PR but it is causing an inconsistency in our CLI since the one for build is a simple boolean. I am certain that this will confuse users if it's being released as is.

@QiWang19 Lets get this merged, then we can look at Buildah.

This is affecting podman-build already. Merging this PR will cause the inconsistency.

I think we can fix the inconsistency by cheating a bit and make podman build --pull understand missing,always,never,true,false ... true,false to be backward compatible.

QiWang19 · 2019-08-09T16:19:20Z

the podman build --pull bool flag is defined in BudResults buildah/pkg/cli/common.go, it should be changed in build so podman build can accept string "missing|always|never"
😨 hard to cheat

Requirement from containers#3575 (comment) Added --pull for podman create and pull to match the newly added flag in docker CLI. `missing`: default value, podman will pull the image if it does not exist in the local. `always`: podman will always pull the image. `never`: podman will never pull the image. Signed-off-by: Qi Wang <qiwan@redhat.com>

mheon · 2019-08-15T16:58:58Z

Code LGTM

QiWang19 · 2019-08-16T19:20:49Z

This patch can get in. Can not change or cheat --pull for podman build or buildah bud in this PR.

rhatdan · 2019-08-17T12:45:31Z

/lgtm

vrothberg · 2019-08-19T06:01:40Z

This patch can get in. Can not change or cheat --pull for podman build or buildah bud in this PR.

I think we should have first applied that change to Buildah, then revendor and change the code here. @QiWang19, can you open a PR at Buildah to address it? I really want this flag to be consistent across commands (and tools).

openshift-ci-robot requested review from mheon and rhatdan July 22, 2019 14:01

openshift-ci-robot added the size/M label Jul 22, 2019

QiWang19 force-pushed the create_pull branch from 7ebbc98 to 7e493fa Compare July 22, 2019 14:13

TomSweeneyRedHat reviewed Jul 22, 2019

View reviewed changes

QiWang19 force-pushed the create_pull branch 7 times, most recently from 3157c7d to 27d9c0c Compare July 25, 2019 18:32

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 29, 2019

QiWang19 force-pushed the create_pull branch from 27d9c0c to 6fb0bc4 Compare August 1, 2019 20:10

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 1, 2019

QiWang19 force-pushed the create_pull branch from 6fb0bc4 to 5e5c7b7 Compare August 2, 2019 13:09

mheon added the Release Notes 1.5.1 label Aug 2, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 2, 2019

openshift-ci-robot added size/L and removed size/M labels Aug 6, 2019

QiWang19 force-pushed the create_pull branch 2 times, most recently from f0d3c61 to 4b98ccc Compare August 6, 2019 20:39

QiWang19 force-pushed the create_pull branch from 4b98ccc to 612d8f1 Compare August 8, 2019 15:27

QiWang19 force-pushed the create_pull branch 2 times, most recently from 8dd99d4 to e6851f1 Compare August 9, 2019 19:19

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2019

QiWang19 force-pushed the create_pull branch from e6851f1 to decfea6 Compare August 9, 2019 19:21

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2019

mheon added Release Notes 1.5.2 and removed Release Notes 1.5.1 labels Aug 15, 2019

openshift-ci-robot assigned rhatdan Aug 17, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2019

openshift-merge-robot merged commit 76f327f into containers:master Aug 17, 2019

rh-atomic-bot mentioned this pull request Aug 17, 2019

add --cert-dir image sign #3833

Merged

QiWang19 mentioned this pull request Aug 19, 2019

change --pull to be consistent with podman containers/buildah#1803

Closed

TomSweeneyRedHat mentioned this pull request Aug 21, 2019

Flag "--pull" of "podman run" not recognized #3867

Closed

QiWang19 deleted the create_pull branch June 26, 2020 15:10

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 24, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add --pull flag for podman create&run #3617

add --pull flag for podman create&run #3617

QiWang19 commented Jul 22, 2019 •

edited

Loading

TomSweeneyRedHat Jul 22, 2019

TomSweeneyRedHat Jul 22, 2019

QiWang19 Jul 25, 2019

giuseppe Jul 30, 2019

TomSweeneyRedHat commented Jul 22, 2019

QiWang19 commented Jul 22, 2019

TomSweeneyRedHat commented Jul 22, 2019

mheon commented Jul 22, 2019

haircommander commented Jul 22, 2019

rh-atomic-bot commented Jul 29, 2019

mheon commented Aug 2, 2019

openshift-ci-robot commented Aug 2, 2019

abitrolly commented Aug 5, 2019

QiWang19 commented Aug 5, 2019

haircommander commented Aug 5, 2019

rhatdan commented Aug 7, 2019

rh-atomic-bot commented Aug 8, 2019

vrothberg commented Aug 9, 2019

QiWang19 commented Aug 9, 2019

mheon commented Aug 15, 2019

QiWang19 commented Aug 16, 2019

rhatdan commented Aug 17, 2019

vrothberg commented Aug 19, 2019

add --pull flag for podman create&run #3617

add --pull flag for podman create&run #3617

Conversation

QiWang19 commented Jul 22, 2019 • edited Loading

TomSweeneyRedHat Jul 22, 2019

Choose a reason for hiding this comment

TomSweeneyRedHat Jul 22, 2019

Choose a reason for hiding this comment

QiWang19 Jul 25, 2019

Choose a reason for hiding this comment

giuseppe Jul 30, 2019

Choose a reason for hiding this comment

TomSweeneyRedHat commented Jul 22, 2019

QiWang19 commented Jul 22, 2019

TomSweeneyRedHat commented Jul 22, 2019

mheon commented Jul 22, 2019

haircommander commented Jul 22, 2019

rh-atomic-bot commented Jul 29, 2019

mheon commented Aug 2, 2019

openshift-ci-robot commented Aug 2, 2019

abitrolly commented Aug 5, 2019

QiWang19 commented Aug 5, 2019

haircommander commented Aug 5, 2019

rhatdan commented Aug 7, 2019

rh-atomic-bot commented Aug 8, 2019

vrothberg commented Aug 9, 2019

QiWang19 commented Aug 9, 2019

mheon commented Aug 15, 2019

QiWang19 commented Aug 16, 2019

rhatdan commented Aug 17, 2019

vrothberg commented Aug 19, 2019

QiWang19 commented Jul 22, 2019 •

edited

Loading