-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BZ 1370045: Fix pushing to GCS storage #10640
Conversation
[test] @smarterclayton review pls. |
Only manual testing here? |
@smarterclayton my first initial test failed :/ (still the same issue) |
@smarterclayton I tried to apply the second fix (distribution/distribution#1703) and still no luck, getting "upload resumed at wrong offest: 508 != 254". |
@smarterclayton ok, with the second fix I'm only getting |
@smarterclayton tested manually and verified it works. |
…mber of buffered bytes if the FileWriter is not closed Also includes openshift#1713. This patch should be dropped when we upgrade to Docker Registry 2.5
Evaluated for origin test up to e7db6ba |
Lgtm [merge]
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8463/) (Image: devenv-rhel7_4932) |
Evaluated for origin merge up to e7db6ba |
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8445/) |
My test still fails: |
I was able to work around the problem doing the following: Added the middleware config as indicated here: Pulled in some upstream patches, I think the only required on is #1817. GOHOME=/tmp/go #Prepare to import Upstream Commits #UPSTREAM: docker/distribution: 1688: Pass through known errors #UPSTREAM: docker/distribution: 1706: Blobwriter: call BlobWriter.Size after BlobWriter.Close #UPSTRAM: docker/distribution: 1782: Update "Accept" header parsing for list values #UPSTREAM: docker/distribution: 1817: Avoid formatting errors with %#v #UPSTREAM: docker/distribution: 1787: Re-add support for non-resumable digests #Clean up $PATCH_TMP #Tag #Build Thank you for your time, Brad |
@bdurrow I tested this patch against GCS and I got my image pushed just fine. I think your fix might be better, but I tested that as well before I ended up with the "quick fix" and I was still seeing the error. |
@bdurrow I tested on the latest OpenShift Docker Registry image (2.4.0) |
@bdurrow you can try it using (also the Blobwriter: call BlobWriter.Size after BlobWriter.Close is 1706 not 1817) |
Upstream PR 1705 is against 2.4 instead of master and my be preferred. I tried the openshift/origin-docker-registry:latest last night and got the failure I mentioned above. The build was from 2 hours earlier so I believe this PR (10640) was included. |
@mfojtik, My test with docker.io/mfojtik/test-registry:v4 just passed. |
No description provided.