fit get status issue

1, return the right upload offset for client when asks.
2, do not call ResumeBlobUpload on getting status.
3, return 416 rather than 404 on failed to patch chunk blob.
4, add the missing upload close

Signed-off-by: Wang Yan <wangyan@vmware.com>

registry/handlers/api_test.go

@@ -687,6 +687,17 @@ func testBlobAPI(t *testing.T, env *testEnv, args blobArgs) *testEnv {
 	layerFile.Seek(0, 0)
 	uploadURLBase, _ = startPushLayer(t, env, imageName)
 	uploadURLBase, dgst := pushChunk(t, env.builder, imageName, uploadURLBase, layerFile, layerLength)
+
+	// -----------------------------------------
+	// Check the chunk upload status
+	_, end, err := getUploadStatus(uploadURLBase)
+	if err != nil {
+		t.Fatalf("unexpected error doing chunk upload check: %v", err)
+	}
+	if end+1 != layerLength {
+		t.Fatalf("getting wrong chunk upload status: %d", end)
+	}
+
 	finishUpload(t, env.builder, imageName, uploadURLBase, dgst)
 
 	// -----------------------------------------
@@ -2487,6 +2498,27 @@ func pushLayer(t *testing.T, ub *v2.URLBuilder, name reference.Named, dgst diges
 	return resp.Header.Get("Location")
 }
 
+func getUploadStatus(location string) (string, int64, error) {
+	req, err := http.NewRequest(http.MethodGet, location, nil)
+	if err != nil {
+		return location, -1, err
+	}
+
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return location, -1, err
+	}
+
+	defer resp.Body.Close()
+
+	_, end, err := parseContentRange(resp.Header.Get("Range"))
+	if err != nil {
+		return location, -1, err
+	}
+
+	return resp.Header.Get("Location"), end, nil
+}
+
 func finishUpload(t *testing.T, ub *v2.URLBuilder, name reference.Named, uploadURLBase string, dgst digest.Digest) string {
 	resp, err := doPushLayer(t, ub, name, dgst, uploadURLBase, nil)
 	if err != nil {

registry/handlers/blobupload.go

@@ -37,6 +37,9 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler {
 	}
 
 	if buh.UUID != "" {
+		if r.Method == http.MethodGet || r.Method == http.MethodHead {
+			return handler
+		}
 		if h := buh.ResumeBlobUpload(ctx, r); h != nil {
 			return h
 		}
@@ -92,7 +95,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req
 
 	buh.Upload = upload
 
-	if err := buh.blobUploadResponse(w, r, true); err != nil {
+	if err := buh.blobUploadResponse(w, r); err != nil {
 		buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
 		return
 	}
@@ -104,19 +107,25 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req
 // GetUploadStatus returns the status of a given upload, identified by id.
 func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Request) {
 	if buh.Upload == nil {
-		buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown)
-		return
+		blobs := buh.Repository.Blobs(buh)
+		upload, err := blobs.Resume(buh, buh.UUID)
+		if err != nil {
+			if err == distribution.ErrBlobUploadUnknown {
+				buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown.WithDetail(err))
+			} else {
+				buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
+			}
+			return
+		}
+
+		buh.Upload = upload
 	}
 
-	// TODO(dmcgowan): Set last argument to false in blobUploadResponse when
-	// resumable upload is supported. This will enable returning a non-zero
-	// range for clients to begin uploading at an offset.
-	if err := buh.blobUploadResponse(w, r, true); err != nil {
+	if err := buh.blobUploadResponse(w, r); err != nil {
 		buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
 		return
 	}
 
-	w.Header().Set("Docker-Upload-UUID", buh.UUID)
 	w.WriteHeader(http.StatusNoContent)
 }
 
@@ -163,7 +172,7 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	if err := buh.blobUploadResponse(w, r, false); err != nil {
+	if err := buh.blobUploadResponse(w, r); err != nil {
 		buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
 		return
 	}
@@ -181,6 +190,7 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht
 		buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown)
 		return
 	}
+	defer buh.Upload.Close()
 
 	dgstStr := r.FormValue("digest") // TODO(stevvooe): Support multiple digest parameters!
 
@@ -251,6 +261,7 @@ func (buh *blobUploadHandler) CancelBlobUpload(w http.ResponseWriter, r *http.Re
 		buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown)
 		return
 	}
+	defer buh.Upload.Close()
 
 	w.Header().Set("Docker-Upload-UUID", buh.UUID)
 	if err := buh.Upload.Cancel(buh); err != nil {
@@ -302,11 +313,9 @@ func (buh *blobUploadHandler) ResumeBlobUpload(ctx *Context, r *http.Request) ht
 	buh.Upload = upload
 
 	if size := upload.Size(); size != buh.State.Offset {
-		defer upload.Close()
 		dcontext.GetLogger(ctx).Errorf("upload resumed at wrong offset: %d != %d", size, buh.State.Offset)
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err))
-			upload.Cancel(buh)
+			buh.Errors = append(buh.Errors, v2.ErrorCodeRangeInvalid.WithDetail(err))
 		})
 	}
 	return nil
@@ -314,10 +323,8 @@ func (buh *blobUploadHandler) ResumeBlobUpload(ctx *Context, r *http.Request) ht
 
 // blobUploadResponse provides a standard request for uploading blobs and
 // chunk responses. This sets the correct headers but the response status is
-// left to the caller. The fresh argument is used to ensure that new blob
-// uploads always start at a 0 offset. This allows disabling resumable push by
-// always returning a 0 offset on check status.
-func (buh *blobUploadHandler) blobUploadResponse(w http.ResponseWriter, r *http.Request, fresh bool) error {
+// left to the caller.
+func (buh *blobUploadHandler) blobUploadResponse(w http.ResponseWriter, r *http.Request) error {
 	// TODO(stevvooe): Need a better way to manage the upload state automatically.
 	buh.State.Name = buh.Repository.Named().Name()
 	buh.State.UUID = buh.Upload.ID()