After containers/podman/issues/10682, we decided to always re-pull
images of non-local platforms and match *any* local image. Over time, we
refined this logic to not *always* pull the image but only if there is a
*newer* one. This has slightly changed the semantics and requires to
perform platform checks when looking up a local image. Otherwise, bogus
values would match a local image and mistakenly return it.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
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 <rothberg@redhat.com>
Path-based transports may contain characters that are invalid for a
reference. In such cases, we should pessimistically generate an ID
and not attempt to look at the (possibly path-based) string within the
transport.
This fixes an error when running `podman run dir:/tmp/CapitalChar` and
will prevent the same issue for the upcoming SIF transport.
Extend the tests to make sure we're not going to regress in the future.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Path-based transports may contain characters that are invalid for a
reference. In such cases, we should pessimistically generate an ID
and not attempt to look at the (possibly path-based) string within the
transport.
This fixes an error when running `podman run dir:/tmp/CapitalChar` and
will prevent the same issue for the upcoming SIF transport.
Extend the tests to make sure we're not going to regress in the future.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Archives generated with buildkit have some kind of "hybrid" layout which
is the same for OCI and Docker archives. OCI ones ship with a
manifest.json but set the image's reference in the index.json but in a
custom annotation and not the one the OCI image spec wants. Archives
in the Docker format set the reference in `RepoTags` of the
manifest.json.
To support these archives, simply look for the custom containerd
annotation *and* change the order back to give OCI archives precedence.
Fixes: containers/podman/issues/12560
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Exchange two debug logs in the pull-never policy. Those were probably
not moved along with the code and could lead to confusions when reading
debug logs.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Allow for passing down credentials when comparing a local image with a
remote one. The linked BZ relates to a regression in `podman auto-update`
but while reading the code I noticed it's also impacting pull policies.
BZ: bugzilla.redhat.com/show_bug.cgi?id=2000943
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Pull is a bit of an "one size fits all" API to keep complexity away from
callers and hide everything behind the interface.
Commit 369aaa4178 recently altered the error reporting to when pulling
fromt the `docker-daemon` transport which in turn caused a regression in
Buildah CI when pulling `docker:latest`. Such an input would cause a
parsing error in the `docker:`.
Fix the regression by relaxing the stricter error reporting introduced
by commit 369aaa4178 and make an exception for the `docker:`
transport. Note that invalid input would still be caught a couple of
lines below.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
When copying images into local storage, parse the manifest of the copied
image and then look up the IDs of the matching image.
There's a short period of time, between when we copy the image into
local storage and when we subsequently go to look for it using the name
that we specified for it when we copied it, when the name we wanted to
assign to the image could have been assigned to another image by another
process.
The manifest that we copied as part of the image that we copied will
still be in the right image regardless, and we can use that to find the
image's ID, and from there fill out our own Image structure that we
return to our caller.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Normalize images pulled from the docker-daemon transport to docker.io
and not to localhost. That preserves previous behavior of Podman.
Also fix a parsing bug in the pull code that was revealed while testing
the changes; parsing errors should be returned if there is a matching
transport without falling through to pulling the input string as an
image from a registry.
Context: containers/podman/issues/10998
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Commit 37f6e92ece 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 37f6e92ecef5: 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 <rothberg@redhat.com>
When writing LookupImage, I thought that it's a good idea to always
attempt to match an image against the local (or requested) platform.
The use case I had in mind is multi-arch support:
`$ podman run image` should only match `image` if it matches the local
platform. We may have previously pulled `image` for another
architecture.
The core criteria for these checks is that images set their platform
(arch/os/variant) correctly. As it turned out that is not the case.
We recently performed a number of fixes to better support multi-arch
images and this change should put the last nail in the coffin.
Hence, entirely remove the `IgnorePlatform` option and only perform
platform matches if the arch, os or variant is specified explicitly via
the LookupImageOptions or the runtime's system context (as Buildah likes
to do it).
Note that this is a breaking change, so I need to update Buildah and
Podman.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Stop the whack-a-mole selectively patching multi-arch issues by forcing
all internal image lookups to ignore the platform.
In retrospect, the `IgnorePlatform` options for image lookups was a
mistake and I will remove it soon but for now, let's just patch
something we can backport to the v0.38 branch.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Some users rely on events being written *after* the operation ran.
Hence, defer all event writes.
Context: containers/podman/issues/10812
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Do not use the name of the locally resolved image when pulling an image
with a custom platform. As we recently re-discovered [1], many
multi-arch images in the wild do not adhere to the OCI image spec and
either declare custom or simply wrong platforms (arch, os, variant).
To address such wrong images, we enforce the pull-always policy whenever
a custom arch, os or variant is specified. We have to do that since we
cannot reliably perform platform matches to any image we would find in
the local containers storage.
To complete the fix, we need to ignore any local image and not use the
locally resolved name which we usually have to do (see [2]).
Let's assume we have a local image "localhost/foo" (arch=amd64). If we
perform a `pull --arch=arm64`, we would not attempt to be pulling
`localhost/foo` but use the ordinary short-name resolution and look for
a matching alias or walk the unqualified-search registries.
In other words: short-name resolution of multi-arch images is prone to
errors but we should continue supporting images in the wild.
[1] containers/podman/issues/10682
[2] containers/buildah/issues/2904
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
As it turned out in Podman CI (containers/podman/pull/10739), the policy
is overridden via --arch/os/platform/variant even when the policy is set
to never.
While I think this is a bug, it is a separate one and must tackled
separately.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Enforce the pull policy to always if a custom platform is requested by
the user. Some images ship with invalid platforms which we must
pessimistically assume, see containers/podman/issues/10682.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
We must ignore the platform of a local image when doing lookups. Some
images set an incorrect or even invalid platform (see
containers/podman/issues/10682). Doing the lookup while ignoring the
platform checks prevents redundantly downloading the same image.
Note that this has the consequence that a `--pull-never --arch=hurz` may
chose a local image of another architecture. However, I estimate the
benefit of continuing to allow potentially invalid images higher than
not running them (and breaking workloads).
The changes required to touch the corrupted checks. I used the occasion
to make the corrupted checks a bit cheaper.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Allow for looking up images via customizable arch, os and variant.
This prevents `podman run --arch=xxx` from redundantly pulling down the
image if needed.
Context: containers/podman/issues/10648
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
An image can be corrupted if, for instance, a pull or build operation is
killed (e.g., during commit). In such cases, an image may be listed
even if a layer is missing.
Over time, Podman and Buildah have made various execution paths more
robust to handle such cases gracefully and/or give the users some help
in trying to resolve the issue. So far, the recommended way was to
remove the corrupted image from storage and then pull it.
The linked Bugzilla issue raised the desire to simplify the recovery by
allowing to pull an image even if the local counterpart is corrupted.
This is especially important for installer scenarios, where users may
restart the installer which would attempt to pull the image again.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1966872
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
For the sake of Docker compatibility, support the tag@digest notation.
In that case, the tag is stripped off the reference and the digest is
the sole source of truth.
Add a number of tests to make sure we're behaving as expected.
Context: containers/podman/issues/6721
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Add tests for exercising pushing images to various transports and
attempt to pull from the destinations.
Fix an error determining the storage reference and image name when
pushing to containers-storage.
Fix a bug in `RemoveImages`: leaving `names` empty and specifying no
filters should remove *all* images.
Please note that the tests are currently not exercising pushing to a
registry. That requires a local registry but since CI is currently
running inside a container, we cannot do much just yet. Once CI runs
in another environment, I will go back and extend the tests.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
The recent refactoring introduced a bug yielding a pull from the dir
transport a NOP. I will soon add unit tests for that.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Add an event system to libimage. Callers can opt-in to using events by
requesting an event channel via `(*Runtime).EventChannel()`. The
returned channel has a buffer of size 100 which should be sufficient
even under high loads. But, to be on the safe side, writing an event
will time out after 2 seconds to prevent operations from blocking.
Currently, the only user of such an event system is Podman which will
need to convert the `Event` type to what's used internally in libpod.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Simplify the transports-sensitive dispatcher when pulling images and use
the default case for transports that do not require special casing.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Recently images tagged by 64 chars cannot be pulled when ommiting "docker://"
prefix. This commit fixes this issue.
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
The following changes were not split into smaller commits since the
entire package is still work in progress and I want to keep moving:
* Various small fixes.
* The internal image cache has been removed as it's a recipe for
inconsistencies for longer running processes. This should make
libimage easier to use for CRI-O and a Podman service.
* LookupImage now returns storage.ErrUnknownImage rather than nil.
This simplifies the callers and makes sure we have a consistent
error.
* LookupImage is now able to handle manifests lists. Unless the
platform is explicitly ignored via the options, the matching
image within the manifest list is now returned. This greatly
simplifies the spec generation in Podman; no callers should have
to worry about this kind of detail.
* LookupImage has been refactored into smaller-sized and easier to
read functions.
* RemoveImages has been changed to assemble the data of removed or
untagged images. This comes in handy for pruning images. I am
heavily against having a dedicated API for pruning since the it's
really just a combination of filtering and removing images which
RemoveImages already supports. Hence these changes to satisfy
the needs of `podman image prune`.
Furthermore, it now returns an []error slice rather than a single
error. Again to make Podman happy which needs to inspect *all*
errors for setting the appropriate exit code.
* A rather large refactoring of the removal code along with very
verbose comments. Those were largely absent in the Podman code base
but there many rules and contracts embedded that I partially could
only reconstruct by manually tests and comparing to Docker.
* Add a new `containers={true,false}` filter which allows filtering
images whether they are used by containers (=true) or if no container
is using them (=false). This filter is required for pruning images
in Podman.
* `libimage/types` has been merged into `libimage`. Podman has to do
_a lot of_ massaging for the remote client already and the types
are pretty much nailed down for the remote API. Hence, I prefer to
do some translation between `libimage` types and what Podman needs
rather than splitting `libimage` in half without an obvious reason.
This way the package is self-contained allowing for an easier
navigation and maintenance.
* `libimage.PullPolicy` has been merged into `pkg/config.PullPolicy`
to have _one_ central place to deal with pull policies. The type
system in `pkg/config` sets "always" as the default unfortunately
but I think consistency is more important at that point.
* Added `CopyOptions.DirForceCompress` to enforce layer compression
when copying to a `dir` destination.
* We now use `github.com/disiqueira/gotree` for pretty printing image
trees. That greatly simplifies the code and we don't have to worry
about the logic of printing a tree. Note that trees are now always
printed top down!
* Added a new `libimage.ManifestList` type along with an API for local
lookups and performing certain operations on it to wrap around
`libimage/manifests` as previously done in `libpod/image` and other
places in Podman.
* Correct caching of `(*Image).Inspect`.
* In addition to username, password and credentials, allow for
speciying an identity token for copying images. That's needed for
Podman's remote API.
* Make image removal more tolerant toward corrupted images.
* A new "until=timestamp" filter that can be used by all APIs
supporting filtering.
* An empty string now resolves to PullPolicyMissing.
* `(*Runtime) systemContextCopy()` returns a deep copy of the runtime's
system context. Golang's shallow copies are very dangerous for long
running processes such as Podman's system service. Hence, we need to
make sure that base data is not altered over time. That adds another
external dependency but I do not see a way around that. Long term,
I desire a `(*containers/image/types.SystemContext).Copy()` function.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
The new `libimage` package is an attempt to consolidate the code for
managing container images and performing operations on them such as
pulling, pushing, saving, searching, local lookups, removing etc.
Over time, Buildah, CRI-O and Podman diverged with respect to managing
container images resulting in a high amount of code duplication
rendering the tools harder to maintain (e.g., bug fixes) and harder to
extend (e.g., adding new features) than necessary.
The desire to share all that code in a common library grew and this is
an attempt to address the it.
The changes as they are now pass Buildah CI [1]. Once merged into
Buildah, I expect follow up changes when migrating Podman over to
`libimage`.
Miscellaneous changes:
* Copy `podman/pkg/signal` to `pkg/signal`.
* Copy `buildah/manifests` to `image/manifests`. Note that the unit
tests require root privileges. Skip()'s are added when running
rootless. Currently excluded from linting.
* Copy `buildah/pkg/manifests` to `pkg/manifests`. Currently excluded
from linting.
* Copy `buildah/pkg/supplemented` to `pkg/supplemented`. Currently
excluded from linting.
[1] github.com/containers/buildah/pull/3148
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Valentin Rothberg
OpenShift Merge Robot
Paul Holzinger
Daniel J Walsh
Nalin Dahyabhai
ktock