pull with custom platform: handle "localhost/" #679
Conversation
|
[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:
Approvers can indicate their approval by writing |
| pullPolicy = config.PullPolicyAlways | ||
| if strings.HasPrefix(resolvedImageName, "localhost/") { | ||
| logrus.Debugf("Enforcing pull policy to %q to support custom platform (arch: %q, os: %q, variant: %q)", "newer", options.Architecture, options.OS, options.Variant) | ||
| pullPolicy = config.PullPolicyNewer |
There was a problem hiding this comment.
Wouldn't this be never? Were is podman going to pull a localhost/* image from?
There was a problem hiding this comment.
"newer" was intended but I am open to change to "never". Let me change something to the following:
- pull("foo") -> resolved to "localhost/foo" -> pull neWer since there may still be an alias or registry
- pull("localhost/foo") -> clearly resolves to a local image -> pull neVer
There was a problem hiding this comment.
@rhatdan updated. Does it make more sense now?
Commit bee8cae enforced the pull policy to "always" when a custom platform was specified. The reason for always pulling is that many multi-arch images are broken; wrong configs, wrong platforms, etc. We cannot perform reliable platform checks. While we may to have to revisit this strategy in the future, it is more important to keep existing workloads running; a bit between a rock and hard place. This change complements commit bee8cae: if attempt to pull an image that resolves to "localhost/", set the pull policy "newer" instead of "always" such that the image may be used instead of erroring out. Ultimately to preserve previous behavior. Context: containers/podman/issues/10914 Signed-off-by: Valentin Rothberg <[email protected]>
|
LGTM |
|
/lgtm |
|
The changes look fine for what you're proposing, but I'm leery. B) This doesn't line up with Docker and prior pull policies, no? Is this going to break people, or at least give unexpected results? |
|
And, |
Skopeo is not using that code. But I agree that we should mention that in the docs.
Can you precise "this"? Just to be sure we're talking about the same thing. Ultimately, I do not think we have much of a choice. Relying on the platform is wrong. One thing I will add to the main branch soon though is to only overwrite the policy if the local image does not match. That will be an optimzation for cases where the specified platform matches the local image. But I am too afraid to do that now so close to the release. Needs some more cooking. |
|
My concern is when a user says "when I pull with Docker, I get this image, but something different when I pull with Buildah/Podman" after this change goes through. |
|
If you pull Pulling a localhost/image makes no sense, so we can either throw an error or ignore the pull. |
Commit bee8cae enforced the pull policy to "always" when a custom
platform was specified. The reason for always pulling is that many
multi-arch images are broken; wrong configs, wrong platforms, etc.
We cannot perform reliable platform checks. While we may to have to
revisit this strategy in the future, it is more important to keep
existing workloads running; a bit between a rock and hard place.
This change complements commit bee8cae: if attempt to pull an image
that resolves to "localhost/", set the pull policy "newer" instead of
"always" such that the image may be used instead of erroring out.
Ultimately to preserve previous behavior.
Context: containers/podman/issues/10914
Signed-off-by: Valentin Rothberg [email protected]