Close with error in all interfaces

This allows us to provide in the image interfaces a method of providing
an error at close time. This is only currently used in a few situations.

Signed-off-by: Erik Hollensbe <github@hollensbe.org>

image/copy/copy.go

@@ -100,8 +100,14 @@ type Options struct {
 	Progress         chan types.ProgressProperties // Reported to when ProgressInterval has arrived for a single artifact+offset.
 }
 
-// Image copies image from srcRef to destRef, using policyContext to validate source image admissibility.
-func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageReference, options *Options) error {
+// Image copies image from srcRef to destRef, using policyContext to validate
+// source image admissibility.
+func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageReference, options *Options) (retErr error) {
+	// NOTE this function uses an output parameter for the error return value.
+	// Setting this and returning is the ideal way to return an error.
+	//
+	// the defers in this routine will wrap the error return with its own errors
+	// which can be valuable context in the middle of a multi-streamed copy.
 	if options == nil {
 		options = &Options{}
 	}
@@ -120,7 +126,12 @@ func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageRe
 	if err != nil {
 		return errors.Wrapf(err, "Error initializing destination %s", transports.ImageName(destRef))
 	}
-	defer dest.Close()
+	defer func() {
+		if err := dest.Close(); err != nil {
+			retErr = errors.Wrapf(retErr, " (dest: %v)", err)
+		}
+	}()
+
 	destSupportedManifestMIMETypes := dest.SupportedManifestMIMETypes()
 
 	rawSource, err := srcRef.NewImageSource(options.SourceCtx, destSupportedManifestMIMETypes)
@@ -130,7 +141,9 @@ func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageRe
 	unparsedImage := image.UnparsedFromSource(rawSource)
 	defer func() {
 		if unparsedImage != nil {
-			unparsedImage.Close()
+			if err := unparsedImage.Close(); err != nil {
+				retErr = errors.Wrapf(retErr, " (unparsed: %v)", err)
+			}
 		}
 	}()
 
@@ -140,10 +153,14 @@ func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageRe
 	}
 	src, err := image.FromUnparsedImage(unparsedImage)
 	if err != nil {
-		return errors.Wrapf(err, "Error initializing image from source %s", transports.ImageName(srcRef))
+		retErr = errors.Wrapf(err, "Error initializing image from source %s", transports.ImageName(srcRef))
 	}
 	unparsedImage = nil
-	defer src.Close()
+	defer func() {
+		if err := src.Close(); err != nil {
+			retErr = errors.Wrapf(retErr, " (source: %v)", err)
+		}
+	}()
 
 	if src.IsMultiImage() {
 		return errors.Errorf("can not copy %s: manifest contains multiple images", transports.ImageName(srcRef))

image/directory/directory_dest.go

@@ -26,7 +26,8 @@ func (d *dirImageDestination) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized ImageDestination, if any.
-func (d *dirImageDestination) Close() {
+func (d *dirImageDestination) Close() error {
+	return nil
 }
 
 func (d *dirImageDestination) SupportedManifestMIMETypes() []string {

image/directory/directory_src.go

@@ -28,7 +28,8 @@ func (s *dirImageSource) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized ImageSource, if any.
-func (s *dirImageSource) Close() {
+func (s *dirImageSource) Close() error {
+	return nil
 }
 
 // GetManifest returns the image's manifest along with its MIME type (which may be empty when it can't be determined but the manifest is available).

image/docker/daemon/daemon_dest.go

@@ -91,7 +91,7 @@ func imageLoadGoroutine(ctx context.Context, c *client.Client, reader *io.PipeRe
 }
 
 // Close removes resources associated with an initialized ImageDestination, if any.
-func (d *daemonImageDestination) Close() {
+func (d *daemonImageDestination) Close() error {
 	if !d.committed {
 		logrus.Debugf("docker-daemon: Closing tar stream to abort loading")
 		// In principle, goroutineCancel() should abort the HTTP request and stop the process from continuing.
@@ -107,6 +107,8 @@ func (d *daemonImageDestination) Close() {
 		d.writer.CloseWithError(errors.New("Aborting upload, daemonImageDestination closed without a previous .Commit()"))
 	}
 	d.goroutineCancel()
+
+	return nil
 }
 
 func (d *daemonImageDestination) Reference() types.ImageReference {

image/docker/daemon/daemon_src.go

@@ -92,8 +92,8 @@ func (s *daemonImageSource) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized ImageSource, if any.
-func (s *daemonImageSource) Close() {
-	_ = os.Remove(s.tarCopyPath)
+func (s *daemonImageSource) Close() error {
+	return os.Remove(s.tarCopyPath)
 }
 
 // tarReadCloser is a way to close the backing file of a tar.Reader when the user no longer needs the tar component.

image/docker/docker_image_dest.go

@@ -59,7 +59,8 @@ func (d *dockerImageDestination) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized ImageDestination, if any.
-func (d *dockerImageDestination) Close() {
+func (d *dockerImageDestination) Close() error {
+	return nil
 }
 
 func (d *dockerImageDestination) SupportedManifestMIMETypes() []string {

image/docker/docker_image_src.go

@@ -65,7 +65,8 @@ func (s *dockerImageSource) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized ImageSource, if any.
-func (s *dockerImageSource) Close() {
+func (s *dockerImageSource) Close() error {
+	return nil
 }
 
 // simplifyContentType drops parameters from a HTTP media type (see https://tools.ietf.org/html/rfc7231#section-3.1.1.1)

image/image/docker_schema2_test.go

@@ -25,7 +25,7 @@ type unusedImageSource struct{}
 func (f unusedImageSource) Reference() types.ImageReference {
 	panic("Unexpected call to a mock function")
 }
-func (f unusedImageSource) Close() {
+func (f unusedImageSource) Close() error {
 	panic("Unexpected call to a mock function")
 }
 func (f unusedImageSource) GetManifest() ([]byte, string, error) {
@@ -346,7 +346,7 @@ type memoryImageDest struct {
 func (d *memoryImageDest) Reference() types.ImageReference {
 	return refImageReferenceMock{d.ref}
 }
-func (d *memoryImageDest) Close() {
+func (d *memoryImageDest) Close() error {
 	panic("Unexpected call to a mock function")
 }
 func (d *memoryImageDest) SupportedManifestMIMETypes() []string {

image/image/memory.go

@@ -32,7 +32,8 @@ func (i *memoryImage) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized UnparsedImage, if any.
-func (i *memoryImage) Close() {
+func (i *memoryImage) Close() error {
+	return nil
 }
 
 // Size returns the size of the image as stored, if known, or -1 if not.

image/image/unparsed.go

@@ -36,8 +36,8 @@ func (i *UnparsedImage) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized UnparsedImage, if any.
-func (i *UnparsedImage) Close() {
-	i.src.Close()
+func (i *UnparsedImage) Close() error {
+	return i.src.Close()
 }
 
 // Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need.

image/oci/layout/oci_dest.go

@@ -31,7 +31,8 @@ func (d *ociImageDestination) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized ImageDestination, if any.
-func (d *ociImageDestination) Close() {
+func (d *ociImageDestination) Close() error {
+	return nil
 }
 
 func (d *ociImageDestination) SupportedManifestMIMETypes() []string {

image/oci/layout/oci_src.go

@@ -26,7 +26,8 @@ func (s *ociImageSource) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized ImageSource, if any.
-func (s *ociImageSource) Close() {
+func (s *ociImageSource) Close() error {
+	return nil
 }
 
 // GetManifest returns the image's manifest along with its MIME type (which may be empty when it can't be determined but the manifest is available).

image/openshift/openshift.go

@@ -191,11 +191,15 @@ func (s *openshiftImageSource) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized ImageSource, if any.
-func (s *openshiftImageSource) Close() {
+func (s *openshiftImageSource) Close() error {
 	if s.docker != nil {
-		s.docker.Close()
+		err := s.docker.Close()
 		s.docker = nil
+
+		return err
 	}
+
+	return nil
 }
 
 func (s *openshiftImageSource) GetTargetManifest(digest digest.Digest) ([]byte, string, error) {
@@ -329,8 +333,8 @@ func (d *openshiftImageDestination) Reference() types.ImageReference {
 }
 
 // Close removes resources associated with an initialized ImageDestination, if any.
-func (d *openshiftImageDestination) Close() {
-	d.docker.Close()
+func (d *openshiftImageDestination) Close() error {
+	return d.docker.Close()
 }
 
 func (d *openshiftImageDestination) SupportedManifestMIMETypes() []string {

image/signature/policy_reference_match_test.go

@@ -57,7 +57,7 @@ type refImageMock struct{ reference.Named }
 func (ref refImageMock) Reference() types.ImageReference {
 	return refImageReferenceMock{ref.Named}
 }
-func (ref refImageMock) Close() {
+func (ref refImageMock) Close() error {
 	panic("unexpected call to a mock function")
 }
 func (ref refImageMock) Manifest() ([]byte, string, error) {
@@ -322,7 +322,7 @@ type forbiddenImageMock struct{}
 func (ref forbiddenImageMock) Reference() types.ImageReference {
 	panic("unexpected call to a mock function")
 }
-func (ref forbiddenImageMock) Close() {
+func (ref forbiddenImageMock) Close() error {
 	panic("unexpected call to a mock function")
 }
 func (ref forbiddenImageMock) Manifest() ([]byte, string, error) {

image/storage/storage_image.go

@@ -118,10 +118,12 @@ func (s storageImageDestination) Reference() types.ImageReference {
 	return s.imageRef
 }
 
-func (s storageImageSource) Close() {
+func (s storageImageSource) Close() error {
+	return nil
 }
 
-func (s storageImageDestination) Close() {
+func (s storageImageDestination) Close() error {
+	return nil
 }
 
 func (s storageImageDestination) ShouldCompressLayers() bool {

image/types/types.go

@@ -110,7 +110,7 @@ type ImageSource interface {
 	// (not as the image itself, or its underlying storage, claims).  This can be used e.g. to determine which public keys are trusted for this image.
 	Reference() ImageReference
 	// Close removes resources associated with an initialized ImageSource, if any.
-	Close()
+	Close() error
 	// GetManifest returns the image's manifest along with its MIME type (which may be empty when it can't be determined but the manifest is available).
 	// It may use a remote (= slow) service.
 	GetManifest() ([]byte, string, error)
@@ -138,7 +138,7 @@ type ImageDestination interface {
 	// e.g. it should use the public hostname instead of the result of resolving CNAMEs or following redirects.
 	Reference() ImageReference
 	// Close removes resources associated with an initialized ImageDestination, if any.
-	Close()
+	Close() error
 
 	// SupportedManifestMIMETypes tells which manifest mime types the destination supports
 	// If an empty slice or nil it's returned, then any mime type can be tried to upload
@@ -184,7 +184,7 @@ type UnparsedImage interface {
 	// (not as the image itself, or its underlying storage, claims).  This can be used e.g. to determine which public keys are trusted for this image.
 	Reference() ImageReference
 	// Close removes resources associated with an initialized UnparsedImage, if any.
-	Close()
+	Close() error
 	// Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need.
 	Manifest() ([]byte, string, error)
 	// Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.