Skip to content

Commit

Permalink
remotes/docker: Only return "already exists" on push when the upload …
Browse files Browse the repository at this point in the history
…was successful

The `(dockerPusher).Push` method uses a `StatusTracker` to check if an
upload already happened, before repeating the upload. However, there is
no provision for failure handling. If a PUT request returns an error,
the `StatusTracker` will still see the upload as if it happened
successfully. Add a status boolean so that only successful uploads
short-circuit `Push`.

Signed-off-by: Aaron Lehmann <alehmann@netflix.com>
  • Loading branch information
aaronlehmann committed Apr 7, 2021
1 parent dc8af03 commit 4c1fa57
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 3 deletions.
8 changes: 5 additions & 3 deletions remotes/docker/pusher.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
ref := remotes.MakeRefKey(ctx, desc)
status, err := p.tracker.GetStatus(ref)
if err == nil {
if status.Offset == status.Total {
if status.Committed && status.Offset == status.Total {
return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "ref %v", ref)
}
// TODO: Handle incomplete status
Expand Down Expand Up @@ -341,8 +341,6 @@ func (pw *pushWriter) Commit(ctx context.Context, size int64, expected digest.Di
if err := pw.pipe.Close(); err != nil {
return err
}
// TODO: Update status to determine committing

// TODO: timeout waiting for response
resp := <-pw.responseC
if resp.err != nil {
Expand Down Expand Up @@ -379,6 +377,10 @@ func (pw *pushWriter) Commit(ctx context.Context, size int64, expected digest.Di
return errors.Errorf("got digest %s, expected %s", actual, expected)
}

status.Committed = true
status.UpdatedAt = time.Now()
pw.tracker.SetStatus(pw.ref, status)

return nil
}

Expand Down
2 changes: 2 additions & 0 deletions remotes/docker/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
type Status struct {
content.Status

Committed bool

// UploadUUID is used by the Docker registry to reference blob uploads
UploadUUID string
}
Expand Down

0 comments on commit 4c1fa57

Please sign in to comment.