summaryrefslogtreecommitdiff
path: root/libpod
diff options
context:
space:
mode:
authorMiloslav Trmač <mitr@redhat.com>2018-07-28 06:25:23 +0200
committerAtomic Bot <atomic-devel@projectatomic.io>2018-08-01 18:22:59 +0000
commit5eac0740c3d57150af39a3da26fb680744213bd1 (patch)
treebdcdfe5a151a2ec25a9ca1127bb97440358d8bbe /libpod
parent86491efea0fcf5ab9e76668b1dadf004a74c62e0 (diff)
downloadpodman-5eac0740c3d57150af39a3da26fb680744213bd1.tar.gz
podman-5eac0740c3d57150af39a3da26fb680744213bd1.tar.bz2
podman-5eac0740c3d57150af39a3da26fb680744213bd1.zip
Eliminate the "DockerArchive means pull all refPairs" special case
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
Diffstat (limited to 'libpod')
-rw-r--r--libpod/image/pull.go28
-rw-r--r--libpod/image/pull_test.go30
2 files changed, 43 insertions, 15 deletions
diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index 0d1299c81..26f18691f 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -48,7 +48,7 @@ var (
DefaultLocalRepo = "localhost"
)
-// pullRefPair records a pair of prepared image references to try to pull (if not DockerArchive) or to pull all (if DockerArchive)
+// pullRefPair records a pair of prepared image references to pull.
type pullRefPair struct {
image string
srcRef types.ImageReference
@@ -57,10 +57,11 @@ type pullRefPair struct {
// pullGoal represents the prepared image references and decided behavior to be executed by imagePull
type pullGoal struct {
- refPairs []pullRefPair
+ refPairs []pullRefPair
+ pullAllPairs bool // Pull all refPairs instead of stopping on first success.
}
-// pullRefName records a prepared source reference and a destination name to try to pull (if not DockerArchive) or to pull all (if DockerArchive)
+// pullRefName records a prepared source reference and a destination name to pull.
type pullRefName struct {
image string
srcRef types.ImageReference
@@ -69,11 +70,15 @@ type pullRefName struct {
// pullGoalNames is an intermediate variant of pullGoal which uses pullRefName instead of pullRefPair.
type pullGoalNames struct {
- refNames []pullRefName
+ refNames []pullRefName
+ pullAllPairs bool // Pull all refNames instead of stopping on first success.
}
func singlePullRefNameGoal(rn pullRefName) *pullGoalNames {
- return &pullGoalNames{refNames: []pullRefName{rn}}
+ return &pullGoalNames{
+ refNames: []pullRefName{rn},
+ pullAllPairs: false, // Does not really make a difference.
+ }
}
func getPullRefName(srcRef types.ImageReference, destName string) pullRefName {
@@ -137,7 +142,8 @@ func pullGoalNamesFromImageReference(ctx context.Context, srcRef types.ImageRefe
res = append(res, pullInfo)
}
return &pullGoalNames{
- refNames: res,
+ refNames: res,
+ pullAllPairs: true,
}, nil
case OCIArchive:
@@ -241,7 +247,7 @@ func (i *Image) pullImage(ctx context.Context, writer io.Writer, authfile, signa
io.WriteString(writer, "Failed\n")
}
} else {
- if imageInfo.srcRef.Transport().Name() != DockerArchive {
+ if !goal.pullAllPairs {
return []string{imageInfo.image}, nil
}
images = append(images, imageInfo.image)
@@ -325,7 +331,8 @@ func pullGoalNamesFromPossiblyUnqualifiedName(inputName string) (*pullGoalNames,
pullNames = append(pullNames, ps)
}
return &pullGoalNames{
- refNames: pullNames,
+ refNames: pullNames,
+ pullAllPairs: false,
}, nil
}
@@ -357,5 +364,8 @@ func (ir *Runtime) pullGoalFromGoalNames(goalNames *pullGoalNames) (pullGoal, er
dstRef: destRef,
}
}
- return pullGoal{refPairs: res}, nil
+ return pullGoal{
+ refPairs: res,
+ pullAllPairs: goalNames.pullAllPairs,
+ }, nil
}
diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go
index 910319de6..af8bf14a5 100644
--- a/libpod/image/pull_test.go
+++ b/libpod/image/pull_test.go
@@ -81,24 +81,28 @@ func TestGetPullRefName(t *testing.T) {
func TestPullGoalNamesFromImageReference(t *testing.T) {
type expected struct{ image, dstName string }
for _, c := range []struct {
- srcName string
- expected []expected
+ srcName string
+ expected []expected
+ expectedPullAllPairs bool
}{
// == docker-archive:
- {"docker-archive:/dev/this-does-not-exist", nil}, // Input does not exist.
- {"docker-archive:/dev/null", nil}, // Input exists but does not contain a manifest.
+ {"docker-archive:/dev/this-does-not-exist", nil, false}, // Input does not exist.
+ {"docker-archive:/dev/null", nil, false}, // Input exists but does not contain a manifest.
// FIXME: The implementation has extra code for len(manifest) == 0?! That will fail in getImageDigest anyway.
{ // RepoTags is empty
"docker-archive:testdata/docker-unnamed.tar.xz",
[]expected{{"@ec9293436c2e66da44edb9efb8d41f6b13baf62283ebe846468bc992d76d7951", "@ec9293436c2e66da44edb9efb8d41f6b13baf62283ebe846468bc992d76d7951"}},
+ false,
},
{ // RepoTags is a [docker.io/library/]name:latest, normalized to the short format.
"docker-archive:testdata/docker-name-only.tar.xz",
[]expected{{"localhost/pretty-empty:latest", "localhost/pretty-empty:latest"}},
+ true,
},
{ // RepoTags is a registry/name:latest
"docker-archive:testdata/docker-registry-name.tar.xz",
[]expected{{"example.com/empty:latest", "example.com/empty:latest"}},
+ true,
},
{ // RepoTags has multiple items for a single image
"docker-archive:testdata/docker-two-names.tar.xz",
@@ -106,33 +110,39 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
{"localhost/pretty-empty:latest", "localhost/pretty-empty:latest"},
{"example.com/empty:latest", "example.com/empty:latest"},
},
+ true,
},
{ // FIXME: Two images in a single archive - only the "first" one (whichever it is) is returned
// (and docker-archive: then refuses to read anything when the manifest has more than 1 item)
"docker-archive:testdata/docker-two-images.tar.xz",
[]expected{{"example.com/empty:latest", "example.com/empty:latest"}},
// "example.com/empty/but:different" exists but is ignored
+ true,
},
// == oci-archive:
- {"oci-archive:/dev/this-does-not-exist", nil}, // Input does not exist.
- {"oci-archive:/dev/null", nil}, // Input exists but does not contain a manifest.
+ {"oci-archive:/dev/this-does-not-exist", nil, false}, // Input does not exist.
+ {"oci-archive:/dev/null", nil, false}, // Input exists but does not contain a manifest.
// FIXME: The remaining tests are commented out for now, because oci-archive: does not work unprivileged.
// { // No name annotation
// "oci-archive:testdata/oci-unnamed.tar.gz",
// []expected{{"@5c8aca8137ac47e84c69ae93ce650ce967917cc001ba7aad5494073fac75b8b6", "@5c8aca8137ac47e84c69ae93ce650ce967917cc001ba7aad5494073fac75b8b6"}},
+ // false,
// },
// { // Name is a name:latest (no normalization is defined).
// "oci-archive:testdata/oci-name-only.tar.gz",
// []expected{{"localhost/pretty-empty:latest", "localhost/pretty-empty:latest"}},
+ // false,
// },
// { // Name is a registry/name:latest
// "oci-archive:testdata/oci-registry-name.tar.gz",
// []expected{{"example.com/empty:latest", "example.com/empty:latest"}},
+ // false,
// },
// { // Name exists, but is an invalid Docker reference; such names are passed through, and will fail when intepreting dstName.
// "oci-archive:testdata/oci-non-docker-name.tar.gz",
// []expected{{"UPPERCASE-IS-INVALID", "UPPERCASE-IS-INVALID"}},
+ // false,
// },
// Maybe test support of two images in a single archive? It should be transparently handled by adding a reference to srcRef.
@@ -140,16 +150,19 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
{ // Absolute path
"dir:/dev/this-does-not-exist",
[]expected{{"localhost/dev/this-does-not-exist", "localhost/dev/this-does-not-exist"}},
+ false,
},
{ // Relative path, single element.
// FIXME? Note the :latest difference in .image. (In .dstName as well, but it has the same semantics in there.)
"dir:this-does-not-exist",
[]expected{{"localhost/this-does-not-exist:latest", "localhost/this-does-not-exist:latest"}},
+ false,
},
{ // Relative path, multiple elements.
// FIXME: This does not add localhost/, and dstName is parsed as docker.io/testdata.
"dir:testdata/this-does-not-exist",
[]expected{{"testdata/this-does-not-exist", "testdata/this-does-not-exist"}},
+ false,
},
// == Others, notably:
@@ -157,10 +170,12 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
{ // Fully-specified input
"docker://docker.io/library/busybox:latest",
[]expected{{"docker://docker.io/library/busybox:latest", "docker.io/library/busybox:latest"}},
+ false,
},
{ // Minimal form of the input
"docker://busybox",
[]expected{{"docker://busybox", "docker.io/library/busybox:latest"}},
+ false,
},
// === tarball: (as an example of what happens when ImageReference.DockerReference is nil).
@@ -168,6 +183,7 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
// (This is NOT an API promise that the results will continue to be this way.)
"tarball:/dev/null",
[]expected{{"tarball:/dev/null", "tarball:/dev/null"}},
+ false,
},
} {
srcRef, err := alltransports.ParseImageName(c.srcName)
@@ -182,6 +198,7 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
for i, e := range c.expected {
assert.Equal(t, pullRefName{image: e.image, srcRef: srcRef, dstName: e.dstName}, res.refNames[i], fmt.Sprintf("%s #%d", c.srcName, i))
}
+ assert.Equal(t, c.expectedPullAllPairs, res.pullAllPairs, c.srcName)
}
}
}
@@ -296,6 +313,7 @@ func TestPullGoalNamesFromPossiblyUnqualifiedName(t *testing.T) {
}
}
assert.Equal(t, c.expected, strings, c.input)
+ assert.False(t, res.pullAllPairs, c.input)
}
}
}