Eliminate duplicate determination whether to use search registries

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

libpod/image/pull.go

@@ -57,8 +57,9 @@ 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.
+	pullAllPairs         bool // Pull all refPairs instead of stopping on first success.
+	usedSearchRegistries bool // refPairs construction has depended on registries.GetRegistries()
 }
 
 // pullRefName records a prepared source reference and a destination name to pull.
@@ -70,14 +71,16 @@ 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.
+	pullAllPairs         bool // Pull all refNames instead of stopping on first success.
+	usedSearchRegistries bool // refPairs construction has depended on registries.GetRegistries()
 }
 
 func singlePullRefNameGoal(rn pullRefName) *pullGoalNames {
 	return &pullGoalNames{
-		refNames:     []pullRefName{rn},
+		refNames:             []pullRefName{rn},
-		pullAllPairs: false, // Does not really make a difference.
+		pullAllPairs:         false, // Does not really make a difference.
+		usedSearchRegistries: false,
 	}
 }
 
@@ -142,8 +145,9 @@ func pullGoalNamesFromImageReference(ctx context.Context, srcRef types.ImageRefe
 			res = append(res, pullInfo)
 		}
 		return &pullGoalNames{
-			refNames:     res,
+			refNames:             res,
-			pullAllPairs: true,
+			pullAllPairs:         true,
+			usedSearchRegistries: false,
 		}, nil
 
 	case OCIArchive:
@@ -260,11 +264,7 @@ func (i *Image) pullImage(ctx context.Context, writer io.Writer, authfile, signa
 		if err != nil {
 			return nil, err
 		}
-		hasRegistryInName, err := i.hasRegistry()
+		if goal.usedSearchRegistries && len(searchRegistries) == 0 {
-		if err != nil {
-			return nil, err
-		}
-		if !hasRegistryInName && len(searchRegistries) == 0 {
 			return nil, errors.Errorf("image name provided is a short name and no search registries are defined in %s.", registryPath)
 		}
 		return nil, errors.Errorf("unable to find image in the registries defined in %q", registryPath)
@@ -331,8 +331,9 @@ func pullGoalNamesFromPossiblyUnqualifiedName(inputName string) (*pullGoalNames,
 		pullNames = append(pullNames, ps)
 	}
 	return &pullGoalNames{
-		refNames:     pullNames,
+		refNames:             pullNames,
-		pullAllPairs: false,
+		pullAllPairs:         false,
+		usedSearchRegistries: true,
 	}, nil
 }
 
@@ -365,7 +366,8 @@ func (ir *Runtime) pullGoalFromGoalNames(goalNames *pullGoalNames) (pullGoal, er
 		}
 	}
 	return pullGoal{
-		refPairs:     res,
+		refPairs:             res,
-		pullAllPairs: goalNames.pullAllPairs,
+		pullAllPairs:         goalNames.pullAllPairs,
+		usedSearchRegistries: goalNames.usedSearchRegistries,
 	}, nil
 }

libpod/image/pull_test.go

@@ -199,6 +199,7 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
 				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)
+			assert.False(t, res.usedSearchRegistries, c.srcName)
 		}
 	}
 }
@@ -232,37 +233,43 @@ func TestPullGoalNamesFromPossiblyUnqualifiedName(t *testing.T) {
 	os.Setenv("REGISTRIES_CONFIG_PATH", registriesConf.Name())
 
 	for _, c := range []struct {
-		input    string
+		input                        string
-		expected []pullRefStrings
+		expected                     []pullRefStrings
+		expectedUsedSearchRegistries bool
 	}{
-		{"#", nil}, // Clearly invalid.
+		{"#", nil, false}, // Clearly invalid.
 		{ // Fully-explicit docker.io, name-only.
 			"docker.io/library/busybox",
 			// (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".)
 			[]pullRefStrings{{"docker.io/library/busybox", "docker://busybox:latest", "docker.io/library/busybox"}},
+			false,
 		},
 		{ // docker.io with implied /library/, name-only.
 			"docker.io/busybox",
 			// (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".)
 			// The .dstName fields differ for the explicit/implicit /library/ cases, but StorageTransport.ParseStoreReference normalizes that.
 			[]pullRefStrings{{"docker.io/busybox", "docker://busybox:latest", "docker.io/busybox"}},
+			false,
 		},
 		{ // Qualified example.com, name-only.
 			"example.com/ns/busybox",
 			[]pullRefStrings{{"example.com/ns/busybox", "docker://example.com/ns/busybox:latest", "example.com/ns/busybox"}},
+			false,
 		},
 		{ // Qualified example.com, name:tag.
 			"example.com/ns/busybox:notlatest",
 			[]pullRefStrings{{"example.com/ns/busybox:notlatest", "docker://example.com/ns/busybox:notlatest", "example.com/ns/busybox:notlatest"}},
+			false,
 		},
 		{ // Qualified example.com, name@digest.
 			"example.com/ns/busybox" + digestSuffix,
 			[]pullRefStrings{{"example.com/ns/busybox" + digestSuffix, "docker://example.com/ns/busybox" + digestSuffix,
 				// FIXME?! Why is .dstName dropping the digest, and adding :none?!
 				"example.com/ns/busybox:none"}},
+			false,
 		},
 		// Qualified example.com, name:tag@digest.  This code is happy to try, but .srcRef parsing currently rejects such input.
-		{"example.com/ns/busybox:notlatest" + digestSuffix, nil},
+		{"example.com/ns/busybox:notlatest" + digestSuffix, nil, false},
 		{ // Unqualified, single-name, name-only
 			"busybox",
 			[]pullRefStrings{
@@ -270,6 +277,7 @@ func TestPullGoalNamesFromPossiblyUnqualifiedName(t *testing.T) {
 				// (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".)
 				{"docker.io/busybox:latest", "docker://busybox:latest", "docker.io/busybox:latest"},
 			},
+			true,
 		},
 		{ // Unqualified, namespaced, name-only
 			"ns/busybox",
@@ -278,6 +286,7 @@ func TestPullGoalNamesFromPossiblyUnqualifiedName(t *testing.T) {
 			[]pullRefStrings{
 				{"ns/busybox", "docker://ns/busybox:latest", "ns/busybox"},
 			},
+			false,
 		},
 		{ // Unqualified, name:tag
 			"busybox:notlatest",
@@ -286,6 +295,7 @@ func TestPullGoalNamesFromPossiblyUnqualifiedName(t *testing.T) {
 				// (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".)
 				{"docker.io/busybox:notlatest", "docker://busybox:notlatest", "docker.io/busybox:notlatest"},
 			},
+			true,
 		},
 		{ // Unqualified, name@digest
 			"busybox" + digestSuffix,
@@ -295,9 +305,10 @@ func TestPullGoalNamesFromPossiblyUnqualifiedName(t *testing.T) {
 				// (The docker:// representation is shortened by c/image/docker.Reference but it refers to "docker.io/library".)
 				{"docker.io/busybox:none", "docker://busybox" + digestSuffix, "docker.io/busybox:none"},
 			},
+			true,
 		},
 		// Unqualified, name:tag@digest. This code is happy to try, but .srcRef parsing currently rejects such input.
-		{"busybox:notlatest" + digestSuffix, nil},
+		{"busybox:notlatest" + digestSuffix, nil, false},
 	} {
 		res, err := pullGoalNamesFromPossiblyUnqualifiedName(c.input)
 		if len(c.expected) == 0 {
@@ -314,6 +325,7 @@ func TestPullGoalNamesFromPossiblyUnqualifiedName(t *testing.T) {
 			}
 			assert.Equal(t, c.expected, strings, c.input)
 			assert.False(t, res.pullAllPairs, c.input)
+			assert.Equal(t, c.expectedUsedSearchRegistries, res.usedSearchRegistries, c.input)
 		}
 	}
 }