Use a types.ImageReference instead of (transport, name) strings in pu…

…llImage etc. Use a typed value, to hopefully decrease further temptation to process strings manually, and to avoid the unnecessary alltransports.ParseImageName which resolveImage has already called. This may change the strings used in some error/debug messages, which now use transports.ImageName instead of the original input; the strings should by definition have the same semantics. Signed-off-by: Miloslav Trmač <[email protected]> Closes: #1361 Approved by: rhatdan
containers · Feb 27, 2019 · 5946d06 · 5946d06
1 parent f5f2cb1
commit 5946d06
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 24 deletions.
diff --git a/new.go b/new.go
@@ -24,21 +24,21 @@ const (
 	BaseImageFakeName = imagebuilder.NoBaseImageSpecifier
 )
 
-func pullAndFindImage(ctx context.Context, store storage.Store, transport string, imageName string, options BuilderOptions, sc *types.SystemContext) (*storage.Image, types.ImageReference, error) {
+func pullAndFindImage(ctx context.Context, store storage.Store, srcRef types.ImageReference, options BuilderOptions, sc *types.SystemContext) (*storage.Image, types.ImageReference, error) {
 	pullOptions := PullOptions{
 		ReportWriter:  options.ReportWriter,
 		Store:         store,
 		SystemContext: options.SystemContext,
 		BlobDirectory: options.PullBlobDirectory,
 	}
-	ref, err := pullImage(ctx, store, transport, imageName, pullOptions, sc)
+	ref, err := pullImage(ctx, store, srcRef, pullOptions, sc)
 	if err != nil {
-		logrus.Debugf("error pulling image %q: %v", imageName, err)
+		logrus.Debugf("error pulling image %q: %v", transports.ImageName(srcRef), err)
 		return nil, nil, err
 	}
 	img, err := is.Transport.GetStoreImage(store, ref)
 	if err != nil {
-		logrus.Debugf("error reading pulled image %q: %v", imageName, err)
+		logrus.Debugf("error reading pulled image %q: %v", transports.ImageName(srcRef), err)
 		return nil, nil, errors.Wrapf(err, "error locating image %q in local storage", transports.ImageName(ref))
 	}
 	return img, ref, nil
@@ -137,7 +137,7 @@ func resolveImage(ctx context.Context, systemContext *types.SystemContext, store
 		}
 
 		if options.PullPolicy == PullAlways {
-			pulledImg, pulledReference, err := pullAndFindImage(ctx, store, transport, image, options, systemContext)
+			pulledImg, pulledReference, err := pullAndFindImage(ctx, store, srcRef, options, systemContext)
 			if err != nil {
 				logrus.Debugf("unable to pull and read image %q: %v", image, err)
 				failures = append(failures, failure{resolvedImageName: image, err: err})
@@ -172,7 +172,7 @@ func resolveImage(ctx context.Context, systemContext *types.SystemContext, store
 			continue
 		}
 
-		pulledImg, pulledReference, err := pullAndFindImage(ctx, store, transport, image, options, systemContext)
+		pulledImg, pulledReference, err := pullAndFindImage(ctx, store, srcRef, options, systemContext)
 		if err != nil {
 			logrus.Debugf("unable to pull and read image %q: %v", image, err)
 			failures = append(failures, failure{resolvedImageName: image, err: err})

diff --git a/pull.go b/pull.go
@@ -190,10 +190,14 @@ func Pull(ctx context.Context, imageName string, options PullOptions) error {
 				errs = multierror.Append(errs, err)
 				continue
 			}
+			taggedRef, err := docker.NewReference(tagged)
+			if err != nil {
+				return errors.Wrapf(err, "internal error creating docker.Transport reference for %s", tagged.String())
+			}
 			if options.ReportWriter != nil {
 				options.ReportWriter.Write([]byte("Pulling " + tagged.String() + "\n"))
 			}
-			ref, err := pullImage(ctx, options.Store, transport, tagged.String(), options, systemContext)
+			ref, err := pullImage(ctx, options.Store, taggedRef, options, systemContext)
 			if err != nil {
 				errs = multierror.Append(errs, err)
 				continue
@@ -212,21 +216,7 @@ func Pull(ctx context.Context, imageName string, options PullOptions) error {
 	return errs.ErrorOrNil()
 }
 
-func pullImage(ctx context.Context, store storage.Store, transport string, imageName string, options PullOptions, sc *types.SystemContext) (types.ImageReference, error) {
-	if transport == "" {
-		transport = util.DefaultTransport
-	} else {
-		if transport != util.DefaultTransport {
-			transport = transport + ":"
-		}
-	}
-	spec := transport + imageName
-	srcRef, err := alltransports.ParseImageName(spec)
-	if err != nil {
-		return nil, errors.Wrapf(err, "error parsing image name %q", spec)
-	}
-	logrus.Debugf("parsed image name %q", spec)
-
+func pullImage(ctx context.Context, store storage.Store, srcRef types.ImageReference, options PullOptions, sc *types.SystemContext) (types.ImageReference, error) {
 	blocked, err := isReferenceBlocked(srcRef, sc)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error checking if pulling from registry for %q is blocked", transports.ImageName(srcRef))
@@ -272,9 +262,9 @@ func pullImage(ctx context.Context, store storage.Store, transport string, image
 		}
 	}()
 
-	logrus.Debugf("copying %q to %q", spec, destName)
+	logrus.Debugf("copying %q to %q", transports.ImageName(srcRef), destName)
 	if _, err := cp.Image(ctx, policyContext, maybeCachedDestRef, srcRef, getCopyOptions(options.ReportWriter, srcRef, sc, maybeCachedDestRef, nil, "")); err != nil {
-		logrus.Debugf("error copying src image [%q] to dest image [%q] err: %v", spec, destName, err)
+		logrus.Debugf("error copying src image [%q] to dest image [%q] err: %v", transports.ImageName(srcRef), destName, err)
 		return nil, err
 	}
 	return destRef, nil