RFC WIP: Define the three return states of HaveBlob as true/false/error

copy/copy.go

@@ -354,7 +354,7 @@ type diffIDResult struct {
 func (ic *imageCopier) copyLayer(srcInfo types.BlobInfo) (types.BlobInfo, digest.Digest, error) {
 	// Check if we already have a blob with this digest
 	haveBlob, extantBlobSize, err := ic.dest.HasBlob(srcInfo)
-	if err != nil && err != types.ErrBlobNotFound {
+	if err != nil {
 		return types.BlobInfo{}, "", errors.Wrapf(err, "Error checking for blob %s at destination", srcInfo.Digest)
 	}
 	// If we already have a cached diffID for this blob, we don't need to compute it

directory/directory_dest.go

@@ -95,14 +95,18 @@ func (d *dirImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobInfo
 	return types.BlobInfo{Digest: computedDigest, Size: size}, nil
 }
 
+// HasBlob returns true iff the image destination already contains a blob with the matching digest which can be reapplied using ReapplyBlob.
+// Unlike PutBlob, the digest can not be empty.  If HasBlob returns true, the size of the blob must also be returned.
+// If the destination does not contain the blob, or it is unknown, HasBlob ordinarily returns (false, -1, nil);
+// it returns a non-nil error only on an unexpected failure.
 func (d *dirImageDestination) HasBlob(info types.BlobInfo) (bool, int64, error) {
 	if info.Digest == "" {
 		return false, -1, errors.Errorf(`"Can not check for a blob with unknown digest`)
 	}
 	blobPath := d.ref.layerPath(info.Digest)
 	finfo, err := os.Stat(blobPath)
 	if err != nil && os.IsNotExist(err) {
-		return false, -1, types.ErrBlobNotFound
+		return false, -1, nil
 	}
 	if err != nil {
 		return false, -1, err

docker/daemon/daemon_dest.go

@@ -151,7 +151,11 @@ func (d *daemonImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobI
 		return types.BlobInfo{}, errors.Errorf(`Can not stream a blob with unknown digest to "docker-daemon:"`)
 	}
 
-	if ok, size, err := d.HasBlob(inputInfo); err == nil && ok {
+	ok, size, err := d.HasBlob(inputInfo)
+	if err != nil {
+		return types.BlobInfo{}, err
+	}
+	if ok {
 		return types.BlobInfo{Digest: inputInfo.Digest, Size: size}, nil
 	}
 
@@ -186,14 +190,18 @@ func (d *daemonImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobI
 	return types.BlobInfo{Digest: digester.Digest(), Size: inputInfo.Size}, nil
 }
 
+// HasBlob returns true iff the image destination already contains a blob with the matching digest which can be reapplied using ReapplyBlob.
+// Unlike PutBlob, the digest can not be empty.  If HasBlob returns true, the size of the blob must also be returned.
+// If the destination does not contain the blob, or it is unknown, HasBlob ordinarily returns (false, -1, nil);
+// it returns a non-nil error only on an unexpected failure.
 func (d *daemonImageDestination) HasBlob(info types.BlobInfo) (bool, int64, error) {
 	if info.Digest == "" {
 		return false, -1, errors.Errorf(`"Can not check for a blob with unknown digest`)
 	}
 	if blob, ok := d.blobs[info.Digest]; ok {
 		return true, blob.Size, nil
 	}
-	return false, -1, types.ErrBlobNotFound
+	return false, -1, nil
 }
 
 func (d *daemonImageDestination) ReapplyBlob(info types.BlobInfo) (types.BlobInfo, error) {

docker/docker_image_dest.go

@@ -113,11 +113,10 @@ func (c *sizeCounter) Write(p []byte) (n int, err error) {
 func (d *dockerImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobInfo) (types.BlobInfo, error) {
 	if inputInfo.Digest.String() != "" {
 		haveBlob, size, err := d.HasBlob(inputInfo)
-		if err != nil && err != types.ErrBlobNotFound {
+		if err != nil {
 			return types.BlobInfo{}, err
 		}
-		// Now err == nil || err == types.ErrBlobNotFound
-		if err == nil && haveBlob {
+		if haveBlob {
 			return types.BlobInfo{Digest: inputInfo.Digest, Size: size}, nil
 		}
 	}
@@ -175,6 +174,10 @@ func (d *dockerImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobI
 	return types.BlobInfo{Digest: computedDigest, Size: sizeCounter.size}, nil
 }
 
+// HasBlob returns true iff the image destination already contains a blob with the matching digest which can be reapplied using ReapplyBlob.
+// Unlike PutBlob, the digest can not be empty.  If HasBlob returns true, the size of the blob must also be returned.
+// If the destination does not contain the blob, or it is unknown, HasBlob ordinarily returns (false, -1, nil);
+// it returns a non-nil error only on an unexpected failure.
 func (d *dockerImageDestination) HasBlob(info types.BlobInfo) (bool, int64, error) {
 	if info.Digest == "" {
 		return false, -1, errors.Errorf(`"Can not check for a blob with unknown digest`)
@@ -196,7 +199,7 @@ func (d *dockerImageDestination) HasBlob(info types.BlobInfo) (bool, int64, erro
 		return false, -1, errors.Errorf("not authorized to read from destination repository %s", reference.Path(d.ref.ref))
 	case http.StatusNotFound:
 		logrus.Debugf("... not present")
-		return false, -1, types.ErrBlobNotFound
+		return false, -1, nil
 	default:
 		return false, -1, errors.Errorf("failed to read from destination repository %s: %v", reference.Path(d.ref.ref), http.StatusText(res.StatusCode))
 	}

oci/layout/oci_dest.go

@@ -112,6 +112,10 @@ func (d *ociImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobInfo
 	return types.BlobInfo{Digest: computedDigest, Size: size}, nil
 }
 
+// HasBlob returns true iff the image destination already contains a blob with the matching digest which can be reapplied using ReapplyBlob.
+// Unlike PutBlob, the digest can not be empty.  If HasBlob returns true, the size of the blob must also be returned.
+// If the destination does not contain the blob, or it is unknown, HasBlob ordinarily returns (false, -1, nil);
+// it returns a non-nil error only on an unexpected failure.
 func (d *ociImageDestination) HasBlob(info types.BlobInfo) (bool, int64, error) {
 	if info.Digest == "" {
 		return false, -1, errors.Errorf(`"Can not check for a blob with unknown digest`)
@@ -122,7 +126,7 @@ func (d *ociImageDestination) HasBlob(info types.BlobInfo) (bool, int64, error)
 	}
 	finfo, err := os.Stat(blobPath)
 	if err != nil && os.IsNotExist(err) {
-		return false, -1, types.ErrBlobNotFound
+		return false, -1, nil
 	}
 	if err != nil {
 		return false, -1, err

openshift/openshift.go

@@ -371,6 +371,10 @@ func (d *openshiftImageDestination) PutBlob(stream io.Reader, inputInfo types.Bl
 	return d.docker.PutBlob(stream, inputInfo)
 }
 
+// HasBlob returns true iff the image destination already contains a blob with the matching digest which can be reapplied using ReapplyBlob.
+// Unlike PutBlob, the digest can not be empty.  If HasBlob returns true, the size of the blob must also be returned.
+// If the destination does not contain the blob, or it is unknown, HasBlob ordinarily returns (false, -1, nil);
+// it returns a non-nil error only on an unexpected failure.
 func (d *openshiftImageDestination) HasBlob(info types.BlobInfo) (bool, int64, error) {
 	return d.docker.HasBlob(info)
 }

storage/storage_image.go

@@ -290,6 +290,10 @@ func (s *storageImageDestination) PutBlob(stream io.Reader, blobinfo types.BlobI
 	return s.putBlob(stream, blobinfo, true)
 }
 
+// HasBlob returns true iff the image destination already contains a blob with the matching digest which can be reapplied using ReapplyBlob.
+// Unlike PutBlob, the digest can not be empty.  If HasBlob returns true, the size of the blob must also be returned.
+// If the destination does not contain the blob, or it is unknown, HasBlob ordinarily returns (false, -1, nil);
+// it returns a non-nil error only on an unexpected failure.
 func (s *storageImageDestination) HasBlob(blobinfo types.BlobInfo) (bool, int64, error) {
 	if blobinfo.Digest == "" {
 		return false, -1, errors.Errorf(`"Can not check for a blob with unknown digest`)
@@ -299,7 +303,7 @@ func (s *storageImageDestination) HasBlob(blobinfo types.BlobInfo) (bool, int64,
 			return true, blob.Size, nil
 		}
 	}
-	return false, -1, types.ErrBlobNotFound
+	return false, -1, nil
 }
 
 func (s *storageImageDestination) ReapplyBlob(blobinfo types.BlobInfo) (types.BlobInfo, error) {

types/types.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/containers/image/docker/reference"
 	"github.com/opencontainers/go-digest"
-	"github.com/pkg/errors"
 )
 
 // ImageTransport is a top-level namespace for ways to to store/load an image.
@@ -160,7 +159,10 @@ type ImageDestination interface {
 	// to any other readers for download using the supplied digest.
 	// If stream.Read() at any time, ESPECIALLY at end of input, returns an error, PutBlob MUST 1) fail, and 2) delete any data stored so far.
 	PutBlob(stream io.Reader, inputInfo BlobInfo) (BlobInfo, error)
-	// HasBlob returns true iff the image destination already contains a blob with the matching digest which can be reapplied using ReapplyBlob.  Unlike PutBlob, the digest can not be empty.  If HasBlob returns true, the size of the blob must also be returned.  A false result will often be accompanied by an ErrBlobNotFound error.
+	// HasBlob returns true iff the image destination already contains a blob with the matching digest which can be reapplied using ReapplyBlob.
+	// Unlike PutBlob, the digest can not be empty.  If HasBlob returns true, the size of the blob must also be returned.
+	// If the destination does not contain the blob, or it is unknown, HasBlob ordinarily returns (false, -1, nil);
+	// it returns a non-nil error only on an unexpected failure.
 	HasBlob(info BlobInfo) (bool, int64, error)
 	// ReapplyBlob informs the image destination that a blob for which HasBlob previously returned true would have been passed to PutBlob if it had returned false.  Like HasBlob and unlike PutBlob, the digest can not be empty.  If the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree.
 	ReapplyBlob(info BlobInfo) (BlobInfo, error)
@@ -300,8 +302,3 @@ type ProgressProperties struct {
 	Artifact BlobInfo
 	Offset   uint64
 }
-
-var (
-	// ErrBlobNotFound can be returned by an ImageDestination's HasBlob() method
-	ErrBlobNotFound = errors.New("no such blob present")
-)