Instead of duplicating the hasRegistry logic, just record whether we
did use search or not.
Should not change behavior (but does not add unit tests for all of it).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
Instead, encode it explicitly in pullGoal.pullAllPairs.
Should not change behavior (but does not add unit tests for
all of it).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
This is an intermediate version of pullGoal, which exists basically
only for easier testing without containers-storage: (i.e. root access)
in unit tests.
Like pullGoal, we will add more members to make it useful in the future.
RFC: Unlike pullGoal, the return value is *pullGoalNames, because there are
quite a few (return nil, err) cases which would be more difficult to read
when returning a value.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
The eventual goal is to cleanly capture semantics like "pull all images
for DockerArchive" and "did a search through $registries" without
hard-coding it through; and to allow a pullImage variant where
the caller can pass an imageReference directly.
For now, this just wraps []pullRefPair and should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
We are passing the values, don't really need the pointer sharing semantics,
and the structures are small enough, and the arrays short enough,
that we very likely lose on the indirect accesses more than we save on
quicker copying of the slices when extending them. Value semantics
is safer anyway.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
We are passing the values, don't really need the pointer sharing semantics,
and the structures are small enough, and the arrays short enough,
that we very likely lose on the indirect accesses more than we save on
quicker copying of the slices when extending them. Value semantics
is safer anyway.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
All but two cases returning a []*pullRefName only return a single
item. Introduce a helper for that case, which seems not
worth it now, but the return value will get a bit more complex
and introducing the helper now will minimize code changes in future
commits.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
We will introduce helpers for the "single image" case, and having a separate
return statement will make them applicable here.
(Also allows us to reduce the scope of some variables a bit.)
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
The goal is to be very explicit about which functions try to heuristically
guess what is the expected format of the string. Not quite "shaming"
the users, but making sure they stand out.
RFC:
- Is this at all acceptable? Desirable?
- varlink ExportImage says "destination must have transport type";
should it be using alltransports.ParseImageReference
+ PushImageToReference, then?
(While touching the call in cmd/podman, also remove a commented-out
older version of the call.)
Should not change behavior (but does not add unit tests).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
When the string is formatted including a constant transport name,
just call the transport to create or parse a reference explicitly.
This avoids unnecessary string formatting and parsing.
Then drop image.TarballTransport, which has no remaining users.
Should not change behavior (but does not add unit tests).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
We don't really want to change the names of the CLI options just because
the transport names change (with oci-dir/docker-dir there is no
direct correspondence wanyway), and this removes a dependency.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
To do that, create the relevant ImageReference values directly
by calling ParseReference/NewReference from the relevant transport
subpackages instead of formatting strings to be parsed (and
heuristically re-parsed) by PushImage.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
Instead of creating a reference string and then checking it again
to see which kind of archive it is, just call
imageNameForSaveDestination at the place where we already know
what kind of archive it is because we are making that decision.
This also notably fixes the use of strings.CONTAINS to see
whether the just constructed strings start with one of the
transport names; that would match anywhere in the
path.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
... to make it a tiny bit easier to read.
Should not change behavior (but does not add unit tests).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
... to make their relationship clear, at the very least.
Should not change behavior (but does not add unit tests).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
We will need to call it from two places in the future.
Should not change behavior, the code is pretty unchanged
(down to using confusing parameter names, which we will change
immediately) (but does not add unit tests).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
This should not change behavior; it will only make it
easier to show that future code move does not change it (but
does not add unit tets.)
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
This will allow adding the reference in the OCIArchive/DockerArchive case
in one step, instead of appending it later.
Should not change behavior, except that source-related errors
will now be reported before possible destination-related errors
(but does not add unit tests).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
This retains the existing string parsing heuristic for users
who must continue to use it (notably the varlink API - or is
it still subject to change?), but allows callers who can get
precise references to supply them without having to deal
with string formatting.
Should not change behavior (but does not add unit tests).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
We already have a c/image/docker/reference.Named; no need to
round-trip it through a string. This also eliminates the theoretical
parsing failure, and the unchecked .(reference.Named) cast.
Also add a check for DockerReference() == nil to be extra paranoid,
although that should never happen.
Should not change behavior (but does not add unit tests).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
(... but keep it in DefaultTransport, which remains irregular.)
This makes DockerTransport consistent with the others, and much more importantly,
allows several instances to do
> imgRef.Transport().Name() == DockerTransport
instead of the current
> strings.HasPrefix(DockerTransport, imgRef.Transport().Name())
, which currently works but is pretty nonsensical (it does not check
the "docker://" prefix against the _full reference_, but it checks
the _transport name_ as a prefix of "docker://", i.e. a transport named
"d" would be accepted.
Should not change behavior, because the only currently existing transport
which has a name that is a prefix of "docker://" is c/image/docker.Transport
(but does not add unit tests).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
They are not used anywhere AFAICS, and the underlying idea
that transport-specific image names are reusable across transports
is very dubious anyway. So, drop them instead of documenting
or fixing them.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
This has no ambition to change the design, just to be clear about
what the design is.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
This avoids another "append an only item to an empty array"
pattern, and will allow us to get rid of the "dest" variable
entirely.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
This should not change behavior, only to make future edits
for an early exit easier to review.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
Almost all paths appended to pullNames exactly once; just construct a
single-element array in place and return it.
That way we can add empty lines as separators, and still come out shorter.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
Because srcRef is created by parsing imgName, both hard-code assumptions
about transport-specific formats of the strings, so that is neither better nor worse;
but we do less explicit parsing.
Should not change behavior for dir:, nor for fully-correct docker-archive:.
docker-archive:, though, also supports docker-archive:path:reference, where
the reference is ignored (with a warning) on read; in such cases the previous
code would use the reference only (not the path), the new code uses both
as the path. Neither works, we just change the failure mode (but
"error opening path:reference" is now more suggestive of the correct usage).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
This is a bit more specific as to what "ref" or "list" means,
and consistent with refPairsFromPossiblyUnqualifiedName
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
Again, that makes the core logic independent from Runtime == containers-storage,
and easier to test independently.
So, this also adds tests.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
... and use pullRefPairsFromRefNames to convert to the
desired data structure later.
This will make both getPullRefName, and later the bulk of
getPullListFromRef, independent of the storage, and thus much easier to test.
Then add tests for getPullRefName. (Ideally they should be shorter,
e.g. hopefully the .image member can be eliminated.)
Should not change behavior, except that error messages on invalid
dstName will now include the value.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1176
Approved by: rhatdan
podman umount will currently only unmount file system if not other
process is using it, otherwise the umount decrements the container
storage to indicate that the caller is no longer using the mount
point, once the count gets to 0, the file system is actually unmounted.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #1184
Approved by: TomSweeneyRedHat
refresh() is the only major command we had that did not perform a
sync before running, and thus was not guaranteed to pick up a
good copy of the state. Fix this by updating the state before a
refresh().
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
Closes: #1186
Approved by: rhatdan
Pod and container State structs are now allowed to be empty on
first being retrieved from the database. Rework pod and container
equality functions used in testing to account for this change.
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
Closes: #1186
Approved by: rhatdan
The new state changes are potentially confusing to people writing
API functions on containers or pods. Add comments to the structs
on how to safely use them.
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
Closes: #1186
Approved by: rhatdan
It's not necessary to fill in state immediately, as we'll be
overwriting it on any API call accessing it thanks to
syncContainer(). It is also causing races when we fetch it
without holding the container lock (which syncContainer() does).
As such, just don't retrieve the state on initial pull from the
database with Bolt.
Also, refactor some Linux-specific netns handling functions out
of container_internal_linux.go into boltdb_linux.go.
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
Closes: #1186
Approved by: rhatdan
When removing an image via varlink, we should always return the
ID of the image even in the case where the image has multiple
repository names and one was only untagged.
Reported by jhonce during integration testing.
Signed-off-by: baude <bbaude@redhat.com>
Closes: #1191
Approved by: jwhonce
This is the second round of performance improvements for out
integration tests.
Signed-off-by: baude <bbaude@redhat.com>
Closes: #1190
Approved by: rhatdan
due to how cstorage is designed, we were spewing thousands of fds when
we restored cached images causing unwieldy rlimits. we now use podman
load to restore the images thereby not tripping the issue.
Signed-off-by: baude <bbaude@redhat.com>
Closes: #1188
Approved by: baude
The auto decompression functionality was already vendored in
with containers/image. Adding a test for it.
Signed-off-by: umohnani8 <umohnani@redhat.com>
Closes: #1137
Approved by: rhatdan
Stop all containers with a zero timeout prior to trying to rm -fa. This results
in quicker teardown times by not waiting for timeouts.
Also, with wait tests, no need to wait the full 10 second sleep. 1 will do.
Signed-off-by: baude <bbaude@redhat.com>
Closes: #1181
Approved by: rhatdan
Because our tests are getting so long, we want to be able to audit which tests are taking
the longest to complete. This may indicate a bad test, bad CI, bad code, etc and therefore
should be auditable.
Also, make speed improvements to tests by making sure we only unpack caches images that
actually get used.
Signed-off-by: baude <bbaude@redhat.com>
Closes: #1178
Approved by: mheon
update to version 956a1971694f18fd602b1203c0a2d192e2cc88a1
inherit support for IDs shifting when fuse-overlayfs is used.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Closes: #1177
Approved by: mheon
