libimage: refine pull-policy enforcement for custom platforms #880
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@rhatdan PTAL. This is much simpler now. |
vrothberg
force-pushed
the
platform-pull-policy
branch
from
de06263 to
ee879f4
Compare
|
LGTM |
When pulling down an image with a user-specified custom platform, we try to make sure that user gets what they are asking for. An inherent issue with multi-arch images is that there are many images in the wild which do not get the platform right (see containers/podman/issues/10682). That means we need to pessimistically assume that the local image is wrong and pull the "correct" one down from the registry; in the worst case that is redundant work but we have a guarantee of correctness. Motivated by containers/podman/issues/12707 I had another look at the code and found some space for optimizations. Previously, we enforced the pull policy to "always" but that may be too aggressive since we may be running in an airgapped environment and the local image is correct. With this change, we enforce the pull policy to "newer" which makes errors non-fatal in case a local image has been found; this seems like a good middleground between making sure we are serving the "correct" image and user friendliness. Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg
force-pushed
the
platform-pull-policy
branch
from
ee879f4 to
6b3823c
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, 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 |
|
/hold |
jwhonce
reviewed
Jan 10, 2022
| } | ||
| // NOTE that this is will even override --pull={false,never}. | ||
| pullPolicy = config.PullPolicyNewer | ||
| logrus.Debugf("Enforcing pull policy to %q to pull custom platform (arch: %q, os: %q, variant: %q) - local image may mistakenly specify wrong platform", pullPolicy, options.Architecture, options.OS, options.Variant) |
There was a problem hiding this comment.
Adding the image in message would aid when reading logs.
There was a problem hiding this comment.
In this specific log, we don't know if there's a local image or not. My intention was just to add some more context/explanation on why we're enforcing it.
|
/hold cancel |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When pulling down an image with a user-specified custom platform, we
try to make sure that user gets what they are asking for. An inherent
issue with multi-arch images is that there are many images in the wild
which do not get the platform right (see containers/podman/issues/10682).
That means we need to pessimistically assume that the local image is
wrong and pull the "correct" one down from the registry; in the worst case
that is redundant work but we have a guarantee of correctness.
Motivated by containers/podman/issues/12707 I had another look at the
code and found some space for optimizations. Previously, we enforced
the pull policy to "always" but that may be too aggressive since we may
be running in an airgapped environment and the local image is correct.
With this change, we enforce the pull policy to "newer" which makes
errors non-fatal in case a local image has been found; this seems like a
good middleground between making sure we are serving the "correct" image
and user friendliness.
Signed-off-by: Valentin Rothberg [email protected]