This changeset addresses intermittent internal server errors encountered during
pushes. The root cause has been isolated to layers that result in identical,
empty filesystems but may have some path declarations (imaginge "./"),
resulting in different tarsums. The main error message reported during these
upload problems was a 500 error, which was not correct. Further investigation
showed the errors to be rooted in digest verification when finishing uploads.
Inspection of the surrounding code also identified a few issues. PutLayerChunk
was slightly refactered into PutLayerUploadComplete. Helper methods were
avoided to make handler less confusing. This simplification leveraged an
earlier change in the spec that moved non-complete chunk uploads to the PATCH
method. Simple logging was also added in the unknown error case that should
help to avoid mysterious 500 errors in the future.
At the same time, the glaring omission of a proper layer upload cancel method
was rectified. This has been added in this change so it is not missed in the
future.
In the future, we may want to refactor the handler code to be more
straightforward, hopefully letting us avoid these problems in the future.
Added test cases that reproduce these errors and drove these changes include
the following:
1. Push a layer with an empty body results in invalid blob upload.
2. Push a layer with a different tarsum (in this case, empty tar)
3. Deleting a layer upload works.
4. Getting status on a deleted layer upload returns 404.
Common functionality was grouped into shared functions to remove repitition.
The API tests will still require future love.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
Setting a header for all responses can help clients better determine
if the server speaks the legacy v1 API or the v2 API. It is important
that the header be set *BEFORE* routing the request.
Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)
Because we guarded the error check, nil Upload on the handler was getting
through to unexpected branches. This directly handles the missing upload
ensuring its set as expected.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
Most of this change follows from the modifications to the storage api. The
driving factor is the separation of layerUploadState from the storage backend,
leaving it to the web application to store and update it. As part of the
updates to meet changes in the storage api, support for the size parameter has
been completely removed.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
This refactors the hmac state token to take control of the layerUploadState
json message, which has been removed from the storage backend. It also moves
away from the concept of a LayerUploadStateStore callback object, which was
short-lived. This allows for upload offset to be managed by the web application
logic in the face of an inconsistent backend. By controlling the upload offset
externally, we reduce the possibility of misreporting upload state to a client.
We may still want to modify the way this works after getting production
experience.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
Since the repo is no longer just the registry, we are moving the registry web
application package out of the repo root into a sub-package. We may break down
the registry package further to separate webapp components and bring the client
package under it. This change accomplishes the task of freeing up the repo root
for a distribution-oriented package. A stub doc.go file is left in place to
declare intent.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
