podman build: correct default pull policy

[vrothberg]

The default pull policy is "missing" not "always".

Fixes: #20125

Does this PR introduce a user-facing change?

Fix a bug in the podman-build man pages and correct the default value of --pull to "missing".

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vrothberg

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 [vrothberg]

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

[Luap99]

podman build --help says the default is true which according to the man page equals always, can you fix it there as well?

[vrothberg]

podman build --help says the default is true which according to the man page equals always, can you fix it there as well?

That needs fixing in buildah.

EDIT: oh, actually I should be able to update it in Podman.

[flouthoc]

Is true equal to always ? i remember conversation on this in past. If they are equal I think some correction is needed here as well https://github.com/containers/buildah/blob/main/docs/buildah-build.1.md

[vrothberg]

Is true equal to always ? i remember conversation on this in past. If they are equal I think some correction is needed here as well https://github.com/containers/buildah/blob/main/docs/buildah-build.1.md

Yes, true equals always.

[Luap99]

LGTM

[edsantiago]

Suggestion, should you feel like re-pushing:

diff --git a/test/system/070-build.bats b/test/system/070-build.bats
index d780a2e2d..56da8ef32 100644
--- a/test/system/070-build.bats
+++ b/test/system/070-build.bats
@@ -24,6 +24,10 @@ EOF
     PODMAN_TIMEOUT=240 run_podman build -t build_test --format=docker $tmpdir
     is "$output" ".*COMMIT" "COMMIT seen in log"
 
+    # $IMAGE is preloaded, so we should never re-pull
+    assert "$output" !~ "Trying to pull" "Default pull policy should be 'missing'"
+    assert "$output" !~ "Writing manifest" "Default pull policy should be 'missing'"
+
     run_podman run --rm build_test cat /$rand_filename
     is "$output"   "$rand_content"   "reading generated file in image"
 

[vrothberg]

Updated and repushed, thanks @edsantiago !

[vrothberg]

Note: close #20125 once merged. I didn't add that to the commit message.

[edsantiago]

/lgtm

[TomSweeneyRedHat]

Unless I'm mistaken, this is different than Docker, which does a pull=true by default.

[TomSweeneyRedHat]

This change worries me. It seems like every year we decide on different functionality/behavior for pull and its options. For instance, in March 2020, I posted this blog: https://www.redhat.com/sysadmin/podman-image-pulling.

In my book, especially if we want to match Docker, --pull=true != --pull=always. IMO, true should only pull the image if it is not in the local storage, or if the version on the registry is different than the one in local storage. A failure should be thrown if the image can't be found in either the registry or local storage. The always variant should always pull, regardless of what's in the local storage. If the image can't be pulled, an error should be thrown, even if there's a copy in local storage.

This appears to be different than what @vrothberg says where true=always.

I think the default pull should be pull=true, as I defined above, so that we can emulate Docker. However, I think I made this argument once before and it was shot down.

Regardless, I'd like someone to write a document that we can all agree with, and that, going forward, we can use it as the point of truth for what pull and its variety of options should and should not do. I'd be happy to start the initial document if people would like. Please let's discuss this in a scrum soon.

[vrothberg]

This change worries me. It seems like every year we decide on different functionality/behavior for pull and its options. For instance, in March 2020, I posted this blog: https://www.redhat.com/sysadmin/podman-image-pulling.

Did you read the conversation above (#20124 (comment))? The only thing that changes in this PR is the displayed default in the help message, nothing else.

The default behavior is "missing" but --pull (without a string value) is the same as --pull=true. That's exactly how Docker behaves: only with --pull local images will be repulled.

In my book, especially if we want to match Docker, --pull=true != --pull=always.

That's not how it's been implemented. true always mapped to always and that is how Docker behaves as well.

This appears to be different than what @vrothberg says where true==always.

The code and the docs say it, not me ;-)

I think the default pull should be pull=true, as I defined above, so that we can emulate Docker. However, I think I made this argument once before and it was shot down.

It behaves like Docker at the moment already. Again, the only the changes in this PR is to display the correct default.

Regardless, I'd like someone to write a document that we can all agree with, and that, going forward, we can use it as the point of truth for what pull and its variety of options should and should not do. I'd be happy to start the initial document if people would like. Please let's discuss this in a scrum soon.

I honestly think you started reviewing this PR on the wrong foot. The last time we wrestled through these details, the behavior was written down in the docs already which clearly state that true == always.