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>
An authfile path set by `podman search --authfile` is passed in
`SearchOptions`. This fix uses this authfile for accessing
registries.
Signed-off-by: Hironori Shiina <shiina.hironori@jp.fujitsu.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>
Add a LockerForImage() function that allocates and returns a file-based
lock for an image record that we're using to hold a manifest list.
Without some locking mechanism, two processes that wish to update a list
at roughly the same time can't do so without risk of accidentally
discarding one or the other set of changes.
Multiple processes that merely want to read the list can already safely
do so; only cases with multiple writers that need to bother with using
this function.
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>
Fix a bug where not all removed images were actually reported as such.
A regression test will be added to Podman.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Handle images without physical layers when constructing the layer tree
and store them in a dedicated slice that can later on be consulted when
computing parent/child relations.
Such "empty" images can be built with, for instance, with the following
Dockerfile:
```
FROM scratch
ENV test1=test1
ENV test2=test2
```
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
As discussed in github.com/containers/podman/issues/10832 the definition
of a "dangling" image in Podman has historically been incorrect. While
the Docker docs describe a dangling image as an image without a tag, and
Podman implemented the filters as such, Docker actually implemented the
filters for images without a tag and without children.
Refine the dangling filters and hence `IsDangling()` to only return true
if an image is untagged and has no children.
Also correct the comments of `IsIntermediate()`.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
When it was first introduced, the blob info cache's location didn't
change from the system-wide default location when we were running in
rootless mode, so we started setting its location ourselves to avoid
triggering permissions errors when updating it.
The image library has since started taking into account that it was
running in rootless mode, but its hardwired default isn't the same as
the one we were setting, so we ended up creating a second cache file.
Stop doing that.
Signed-off-by: Nalin Dahyabhai <nalin@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>
(*Image)Tree() hit a nil deref when traversing the children of an image
without a physical layer.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
When importing, first create the image and tag it afterwards. This also
makes sure that an imported image *without* a tag is correctly listed as
"<none>". Previously, such images were tagged as
"docker.io/library/sha256:$ID" (inherited from older Podman code).
Context: containers/podman/issues/10854
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>
Add a method to the libimage runtime to access (a copy of) its
types.SystemContext. That can be helpful for callers which may need to
access the system context.
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>
When removing an image by name, do not remove the image and all its
tags, even if force is set. Instead, just untag the specified name.
Note: adjust the load test to preserve the order in the untagged field.
Also vendor in the latest HEAD in containers/image to fix a bug revealed
in Podman CI.
Context: containers/podman/issues/10685
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Currently if the caller does not specify the BigFilesTemporaryDir,
Podman and Buildah users expect this to default TMPDIR environment
variable or /var/tmp if not set.
Moving to libimage caused a regression in this functionality.
Signed-off-by: Daniel J Walsh <dwalsh@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>
Commit 7f038138c3 introduced a regression to Exists() which would
return an error if the image does not exist.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
While various execution paths in libimage already handle corrupted
images, `(*Runtime).Exists()` did not and would list an image to exist
in the storage even if it is corrupted.
Some corruptions can only be detected when accessing the individual
data. A reliable way of accessing such data is to inspect an image.
Hence, an image will only be listed to exist if a) it has been found
and b) can be inspected. If the inspection fails, the image will be
reported to not exists but without an error. That allows for users
of libimage to properly recover and repull.
Further, add a new unit tests that forces a data corruption and
gradually recovers from it.
Podman will now behave as follows:
```
$ ./bin/podman run -d --rm nginx ls
ERRO[0000] Image nginx exists in local storage but may be corrupted: layer not known
ERRO[0000] Looking up nginx in local storage: layer not known
Resolved "nginx" as an alias (/home/vrothberg/.cache/containers/short-name-aliases.conf)
Trying to pull docker.io/library/nginx:latest...
Getting image source signatures
Copying blob 596b1d696923 skipped: already exists
Copying blob 30afc0b18f67 skipped: already exists
Copying blob febe5bd23e98 skipped: already exists
Copying blob 69692152171a skipped: already exists
Copying blob 8283eee92e2f skipped: already exists
Copying blob 351ad75a6cfa done
Copying config d1a364dc54 done
Writing manifest to image destination
Storing signatures
56b65883c3c32b67277bcc173bd9f26c27cbbdbc6d3aacf6c552be796eb7a337
```
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 unit tests for exercising all kinds of function of an `Image`
object.
Also remove an unused, redundant (and incomplete) `MountPoint` function.
`Mountpoint` is used by Podman instead.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Recent changes in the image-lookup logic will, in many cases, yield a
check whether an image is a manifest list. This had caused a regression
in Podman's test/system/330-corrupt-images.bats system tests where we're
attempting to delete a corrupted image with a missing manifest. Since
the manifest is missing, the manifest list check fails.
To make the image lookups more tolerant towards this specific error
case, we need to ignore the error but emit a warning, similar to what
we're already doing in the parent-child checks.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Commit 724e7c92b5 fixed an issue when pushing images from of a
platform different than the current machine. That required to disable
the platform matching logic when looking up the image before pushing it.
It also required some restructuring of the code such that manifest lists
are resolved and their instances looked up.
The restructuring in turn introduced a regression when looking up bare
manifest lists. To fix the regression and keep the code simple,
introduce an internal field in the LookupImageOptions that indicates
whether we're looking up a bare manifest list or not.
Now we have clearer separation of concerns between looking up images or
manfifests and whether the looked up image needs to match the current
platform or not.
Add some unit tests exercising the manifest-list code to make sure we're
not regressing again.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
When comparing the digests of a local image and a remote image, we've
been reading the manifest from the remote image using NewImage(), which
may or may not return a list, and have been comparing the digest of that
manifest to only one of the local image's manifests.
Start checking if the remote reference points to a manifest list, and if
it is, find the image in the list that we'd choose to pull, and use its
manifest digest for the comparison.
When looking up the digest of the local image to compare to the remote
image, consider them to be the same image if any of the manifests in the
image record has the same digest as the remote manifest, which is now
known to not be a list.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When pushing an image, make sure to ignore the platform of the image to
push exactly what the user wishes to. Add a test to make sure we're not
regressing in the future.
To preserve previous behaviour with respect to attempting to push a
manifest list, move the platform check below resolving to a manifest
list.
Fixes: #containers/podman/issues/10344
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Add a unit test exercising image import. It is not extensive but will
do the work of catching regressions. More tests to increase coverage
can be added later.
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>
Valentin Rothberg
Hironori Shiina
Daniel J Walsh
Nalin Dahyabhai
OpenShift Merge Robot
cdoern