Introduce struct pullGoalNames

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

2 files changed, 37 insertions, 25 deletions

diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index 9fe565650..0d1299c81 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -67,8 +67,13 @@ type pullRefName struct {
 	dstName string
 }
 
-func singlePullRefNameGoal(rn pullRefName) []pullRefName {
-	return []pullRefName{rn}
+// pullGoalNames is an intermediate variant of pullGoal which uses pullRefName instead of pullRefPair.
+type pullGoalNames struct {
+	refNames []pullRefName
+}
+
+func singlePullRefNameGoal(rn pullRefName) *pullGoalNames {
+	return &pullGoalNames{refNames: []pullRefName{rn}}
 }
 
 func getPullRefName(srcRef types.ImageReference, destName string) pullRefName {
@@ -91,8 +96,8 @@ func getPullRefName(srcRef types.ImageReference, destName string) pullRefName {
 	}
 }
 
-// refNamesFromImageReference returns a list of pullRefName for a single ImageReference, depending on the used transport.
-func refNamesFromImageReference(ctx context.Context, srcRef types.ImageReference, imgName string, sc *types.SystemContext) ([]pullRefName, error) {
+// pullGoalNamesFromImageReference returns a pullGoalNames for a single ImageReference, depending on the used transport.
+func pullGoalNamesFromImageReference(ctx context.Context, srcRef types.ImageReference, imgName string, sc *types.SystemContext) (*pullGoalNames, error) {
 	// supports pulling from docker-archive, oci, and registries
 	switch srcRef.Transport().Name() {
 	case DockerArchive:
@@ -131,7 +136,9 @@ func refNamesFromImageReference(ctx context.Context, srcRef types.ImageReference
 			pullInfo := getPullRefName(srcRef, dst)
 			res = append(res, pullInfo)
 		}
-		return res, nil
+		return &pullGoalNames{
+			refNames: res,
+		}, nil
 
 	case OCIArchive:
 		// retrieve the manifest from index.json to access the image name
@@ -171,12 +178,12 @@ func refNamesFromImageReference(ctx context.Context, srcRef types.ImageReference
 
 // pullGoalFromImageReference returns a pull goal for a single ImageReference, depending on the used transport.
 func (ir *Runtime) pullGoalFromImageReference(ctx context.Context, srcRef types.ImageReference, imgName string, sc *types.SystemContext) (pullGoal, error) {
-	refNames, err := refNamesFromImageReference(ctx, srcRef, imgName, sc)
+	goalNames, err := pullGoalNamesFromImageReference(ctx, srcRef, imgName, sc)
 	if err != nil {
 		return pullGoal{}, err
 	}
 
-	return ir.pullGoalFromRefNames(refNames)
+	return ir.pullGoalFromGoalNames(goalNames)
 }
 
 // pullImage pulls an image from configured registries
@@ -265,9 +272,9 @@ func hasShaInInputName(inputName string) bool {
 	return strings.Contains(inputName, "@sha256:")
 }
 
-// refNamesFromPossiblyUnqualifiedName looks at a decomposed image and determines the possible
+// pullGoalNamesFromPossiblyUnqualifiedName looks at a decomposed image and determines the possible
 // image names to try pulling in combination with the registries.conf file as well
-func refNamesFromPossiblyUnqualifiedName(inputName string) ([]pullRefName, error) {
+func pullGoalNamesFromPossiblyUnqualifiedName(inputName string) (*pullGoalNames, error) {
 	decomposedImage, err := decompose(inputName)
 	if err != nil {
 		return nil, err
@@ -317,24 +324,29 @@ func refNamesFromPossiblyUnqualifiedName(inputName string) ([]pullRefName, error
 		ps.dstName = ps.image
 		pullNames = append(pullNames, ps)
 	}
-	return pullNames, nil
+	return &pullGoalNames{
+		refNames: pullNames,
+	}, nil
 }
 
 // pullGoalFromPossiblyUnqualifiedName looks at a decomposed image and determines the possible
 // image references to try pulling in combination with the registries.conf file as well
 func (i *Image) pullGoalFromPossiblyUnqualifiedName() (pullGoal, error) {
-	refNames, err := refNamesFromPossiblyUnqualifiedName(i.InputName)
+	goalNames, err := pullGoalNamesFromPossiblyUnqualifiedName(i.InputName)
 	if err != nil {
 		return pullGoal{}, err
 	}
-	return i.imageruntime.pullGoalFromRefNames(refNames)
+	return i.imageruntime.pullGoalFromGoalNames(goalNames)
 }
 
-// pullRefPairsFromNames converts a []pullRefName to a pullGoal
-func (ir *Runtime) pullGoalFromRefNames(refNames []pullRefName) (pullGoal, error) {
+// pullGoalFromGoalNames converts a pullGoalNames to a pullGoal
+func (ir *Runtime) pullGoalFromGoalNames(goalNames *pullGoalNames) (pullGoal, error) {
+	if goalNames == nil { // The value is a pointer only to make (return nil, err) possible in callers; they should never return nil on success
+		return pullGoal{}, errors.New("internal error: pullGoalFromGoalNames(nil)")
+	}
 	// Here we construct the destination references
-	res := make([]pullRefPair, len(refNames))
-	for i, rn := range refNames {
+	res := make([]pullRefPair, len(goalNames.refNames))
+	for i, rn := range goalNames.refNames {
 		destRef, err := is.Transport.ParseStoreReference(ir.store, rn.dstName)
 		if err != nil {
 			return pullGoal{}, errors.Wrapf(err, "error parsing dest reference name %#v", rn.dstName)
diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go
index 11525539b..910319de6 100644
--- a/libpod/image/pull_test.go
+++ b/libpod/image/pull_test.go
@@ -78,7 +78,7 @@ func TestGetPullRefName(t *testing.T) {
 	}
 }
 
-func TestRefNamesFromImageReference(t *testing.T) {
+func TestPullGoalNamesFromImageReference(t *testing.T) {
 	type expected struct{ image, dstName string }
 	for _, c := range []struct {
 		srcName  string
@@ -173,14 +173,14 @@ func TestRefNamesFromImageReference(t *testing.T) {
 		srcRef, err := alltransports.ParseImageName(c.srcName)
 		require.NoError(t, err, c.srcName)
 
-		res, err := refNamesFromImageReference(context.Background(), srcRef, c.srcName, nil)
+		res, err := pullGoalNamesFromImageReference(context.Background(), srcRef, c.srcName, nil)
 		if len(c.expected) == 0 {
 			assert.Error(t, err, c.srcName)
 		} else {
 			require.NoError(t, err, c.srcName)
-			require.Len(t, res, len(c.expected), c.srcName)
+			require.Len(t, res.refNames, len(c.expected), c.srcName)
 			for i, e := range c.expected {
-				assert.Equal(t, pullRefName{image: e.image, srcRef: srcRef, dstName: e.dstName}, res[i], fmt.Sprintf("%s #%d", c.srcName, i))
+				assert.Equal(t, pullRefName{image: e.image, srcRef: srcRef, dstName: e.dstName}, res.refNames[i], fmt.Sprintf("%s #%d", c.srcName, i))
 			}
 		}
 	}
@@ -190,11 +190,11 @@ const registriesConfWithSearch = `[registries.search]
 registries = ['example.com', 'docker.io']
 `
 
-func TestRefNamesFromPossiblyUnqualifiedName(t *testing.T) {
+func TestPullGoalNamesFromPossiblyUnqualifiedName(t *testing.T) {
 	const digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
 	type pullRefStrings struct{ image, srcRef, dstName string } // pullRefName with string data only
 
-	registriesConf, err := ioutil.TempFile("", "TestRefNamesFromPossiblyUnqualifiedName")
+	registriesConf, err := ioutil.TempFile("", "TestPullGoalNamesFromPossiblyUnqualifiedName")
 	require.NoError(t, err)
 	defer registriesConf.Close()
 	defer os.Remove(registriesConf.Name())
@@ -282,13 +282,13 @@ func TestRefNamesFromPossiblyUnqualifiedName(t *testing.T) {
 		// Unqualified, name:tag@digest. This code is happy to try, but .srcRef parsing currently rejects such input.
 		{"busybox:notlatest" + digestSuffix, nil},
 	} {
-		res, err := refNamesFromPossiblyUnqualifiedName(c.input)
+		res, err := pullGoalNamesFromPossiblyUnqualifiedName(c.input)
 		if len(c.expected) == 0 {
 			assert.Error(t, err, c.input)
 		} else {
 			assert.NoError(t, err, c.input)
-			strings := make([]pullRefStrings, len(res))
-			for i, rn := range res {
+			strings := make([]pullRefStrings, len(res.refNames))
+			for i, rn := range res.refNames {
 				strings[i] = pullRefStrings{
 					image:   rn.image,
 					srcRef:  transports.ImageName(rn.srcRef),