{"action":"created","comment":{"url":"https://api.github.com/repos/docker/docker/pulls/comments/80847877","pull_request_review_id":1872365,"id":80847877,"diff_hunk":"@@ -404,55 +458,246 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.\n \tprogress.Update(progressOutput, pd.ID(), \"Pushed\")\n \n \t// Cache mapping from this layer's DiffID to the blobsum\n-\tif err := pd.v2MetadataService.Add(diffID, metadata.V2Metadata{Digest: pushDigest, SourceRepository: pd.repoInfo.FullName()}); err != nil {\n+\tif err := pd.v2MetadataService.TagAndAdd(diffID, pd.hmacKey, metadata.V2Metadata{\n+\t\tDigest:           pushDigest,\n+\t\tSourceRepository: pd.repoInfo.FullName(),\n+\t}); err != nil {\n \t\treturn distribution.Descriptor{}, xfer.DoNotRetry{Err: err}\n \t}\n \n-\tpd.pushState.Lock()\n-\n-\t// If Commit succeeded, that's an indication that the remote registry\n-\t// speaks the v2 protocol.\n-\tpd.pushState.confirmedV2 = true\n-\n-\tdescriptor := distribution.Descriptor{\n+\tdesc := distribution.Descriptor{\n \t\tDigest:    pushDigest,\n \t\tMediaType: schema2.MediaTypeLayer,\n \t\tSize:      nn,\n \t}\n-\tpd.pushState.remoteLayers[diffID] = descriptor\n \n+\tpd.pushState.Lock()\n+\t// If Commit succeeded, that's an indication that the remote registry speaks the v2 protocol.\n+\tpd.pushState.confirmedV2 = true\n+\tpd.pushState.remoteLayers[diffID] = desc\n \tpd.pushState.Unlock()\n \n-\treturn descriptor, nil\n-}\n-\n-func (pd *v2PushDescriptor) SetRemoteDescriptor(descriptor distribution.Descriptor) {\n-\tpd.remoteDescriptor = descriptor\n-}\n-\n-func (pd *v2PushDescriptor) Descriptor() distribution.Descriptor {\n-\treturn pd.remoteDescriptor\n+\treturn desc, nil\n }\n \n-// layerAlreadyExists checks if the registry already know about any of the\n-// metadata passed in the \"metadata\" slice. If it finds one that the registry\n-// knows about, it returns the known digest and \"true\".\n-func layerAlreadyExists(ctx context.Context, metadata []metadata.V2Metadata, repoInfo reference.Named, repo distribution.Repository, pushState *pushState) (distribution.Descriptor, bool, error) {\n-\tfor _, meta := range metadata {\n-\t\t// Only check blobsums that are known to this repository or have an unknown source\n-\t\tif meta.SourceRepository != \"\" && meta.SourceRepository != repoInfo.FullName() {\n+// layerAlreadyExists checks if the registry already knows about any of the metadata passed in the \"metadata\"\n+// slice. If it finds one that the registry knows about, it returns the known digest and \"true\". If\n+// \"checkOtherRepositories\" is true, stat will be performed also with digests mapped to any other repository\n+// (not just the target one).\n+func (pd *v2PushDescriptor) layerAlreadyExists(\n+\tctx context.Context,\n+\tprogressOutput progress.Output,\n+\tdiffID layer.DiffID,\n+\tcheckOtherRepositories bool,\n+\tmaxExistenceCheckAttempts int,\n+\tv2Metadata []metadata.V2Metadata,\n+) (desc distribution.Descriptor, exists bool, err error) {\n+\t// filter the metadata\n+\tcandidates := []metadata.V2Metadata{}\n+\tfor _, meta := range v2Metadata {\n+\t\tif len(meta.SourceRepository) > 0 && !checkOtherRepositories && meta.SourceRepository != pd.repoInfo.FullName() {\n+\t\t\tcontinue\n+\t\t}\n+\t\tcandidates = append(candidates, meta)\n+\t}\n+\t// sort the candidates by similarity\n+\tsortV2MetadataByLikenessAndAge(pd.repoInfo, pd.hmacKey, candidates)\n+\n+\tdigestToMetadata := make(map[digest.Digest]*metadata.V2Metadata)\n+\t// an array of unique blob digests ordered from the best mount candidates to worst\n+\tlayerDigests := []digest.Digest{}\n+\tfor i := 0; i < len(candidates) && len(layerDigests) < maxExistenceCheckAttempts; i++ {\n+\t\tmeta := &candidates[i]\n+\t\tif _, exists := digestToMetadata[meta.Digest]; exists {\n+\t\t\t// keep reference just to the first mapping (the best mount candidate)\n+\t\t\tcontinue","path":"distribution/push_v2.go","position":null,"original_position":347,"commit_id":"301befb621801a627bedafe82725d463e470dd44","original_commit_id":"9f05002b2dd247a6f4b3891ac030ec5104d17f10","user":{"login":"miminar","id":1478803,"avatar_url":"https://avatars.githubusercontent.com/u/1478803?v=3","gravatar_id":"","url":"https://api.github.com/users/miminar","html_url":"https://github.com/miminar","followers_url":"https://api.github.com/users/miminar/followers","following_url":"https://api.github.com/users/miminar/following{/other_user}","gists_url":"https://api.github.com/users/miminar/gists{/gist_id}","starred_url":"https://api.github.com/users/miminar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/miminar/subscriptions","organizations_url":"https://api.github.com/users/miminar/orgs","repos_url":"https://api.github.com/users/miminar/repos","events_url":"https://api.github.com/users/miminar/events{/privacy}","received_events_url":"https://api.github.com/users/miminar/received_events","type":"User","site_admin":false},"body":"It does not, the `i` is not compared against check limits. Nevertheless I broke up the for-loop condition to make it more readable.","created_at":"2016-09-28T06:04:11Z","updated_at":"2016-09-28T06:04:11Z","html_url":"https://github.com/docker/docker/pull/26564#discussion_r80847877","pull_request_url":"https://api.github.com/repos/docker/docker/pulls/26564","_links":{"self":{"href":"https://api.github.com/repos/docker/docker/pulls/comments/80847877"},"html":{"href":"https://github.com/docker/docker/pull/26564#discussion_r80847877"},"pull_request":{"href":"https://api.github.com/repos/docker/docker/pulls/26564"}}},"pull_request":{"url":"https://api.github.com/repos/docker/docker/pulls/26564","id":85215706,"html_url":"https://github.com/docker/docker/pull/26564","diff_url":"https://github.com/docker/docker/pull/26564.diff","patch_url":"https://github.com/docker/docker/pull/26564.patch","issue_url":"https://api.github.com/repos/docker/docker/issues/26564","number":26564,"state":"open","locked":false,"title":"Avoid unnecessary blob uploads","user":{"login":"miminar","id":1478803,"avatar_url":"https://avatars.githubusercontent.com/u/1478803?v=3","gravatar_id":"","url":"https://api.github.com/users/miminar","html_url":"https://github.com/miminar","followers_url":"https://api.github.com/users/miminar/followers","following_url":"https://api.github.com/users/miminar/following{/other_user}","gists_url":"https://api.github.com/users/miminar/gists{/gist_id}","starred_url":"https://api.github.com/users/miminar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/miminar/subscriptions","organizations_url":"https://api.github.com/users/miminar/orgs","repos_url":"https://api.github.com/users/miminar/repos","events_url":"https://api.github.com/users/miminar/events{/privacy}","received_events_url":"https://api.github.com/users/miminar/received_events","type":"User","site_admin":false},"body":"In an environment, where multiple users work with one daemon, build images and push them to authenticated repositories, the cross-mount usually fails because of permission issues.\r\n\r\nHere's an example:\r\n\r\n1. user *bob* builds `reg.local/bob/php-base` image based on `docker.io/php` and pushes it to his private repository\r\n  - Docker daemon remembers assocations between blobs and repositories:\r\n\r\n        P <-> docker.io/php            (layer of php base image)\r\n        P <-> reg.local/bob/php-base   (layer of php base image)\r\n        N1<-> reg.local/bob/php-base   (layer on top of base image)\r\n\r\n2. user *alice* builds its own `reg.local/alice/php-base` image based on `docker.io/php` and pushes it to her private repository\r\n\r\n  1. there's a new **N1** layer on top of **P**\r\n  2. daemon attempts to mount **P** from `reg.local/bob/php-base` and fails because *alice* cannot access *bob*'s private repositories\r\n  3. the association between **P** and `reg.local/bob/php-base` is removed as a consequence\r\n  4. both layers **P** and **N2** get fully uploaded\r\n  5. association between **P** and `reg.local/alice/php-base` is remembered\r\n\r\n3. so far so good, all works as expected\r\n\r\n4. but now *bob* adds one additional layer on top of `reg.local/bob/php-base` to build `reg.local/bob/myapp` and pushes it\r\n\r\n  1. there's a new **M** layer on top of **N1**\r\n  2. during a push, daemon does not attempt to stat the **P** because there's no association between **P** and `reg.local/bob/php-base` (it was removed in step 2)\r\n  3. so it attempts to mount the blob again, this time from `reg.local/alice/php-base` private repository and fails\r\n  4. the association between **P** and `reg.local/alice/php-base` is removed as a consequence\r\n  5. blob **N1** is checked before upload, so it doesn't get re-uploaded\r\n  6. the other blobs (**P** and **M**) get fully uploaded (while **P** already exists in the target repository)\r\n  7. the association between **P** and `reg.local/bob/php-base` is remembered once again\r\n\r\nThis is a big problem on a large scale. In OpenShift, with tens or hundreds of users using the same daemon and a small set of base images, the cross repo mount succeeds very rarely. And due to associations being forgotten, Docker daemon mostly re-uploads all the blobs to the target repository where they already exist.\r\n\r\nThis PR attempts to address the above with following:\r\n\r\n1. add an `AuthConfigHash` attribute to v2 metadata structure\r\n2. forgett blob <-> repo association only if its `AuthConfigHash` matches the current one or it is yet unset\r\n3. blob is checked for presence in target repository in all cases\r\n4. blob mount is attempted several times until:\r\n  - a successful mount\r\n  - a maximum number of attempts is reached\r\n\r\nThe drawback of the 4th item is that it creates redundant blob uploads which are being cancelled shortly afterwards. If the cancellation fails, it relies on the fact that most registries do periodic upload cleanups. The point of it is to raise the chance of hitting a repository the user is authorized to pull from.\r\n\r\nMaybe there's even a better solution. Your ideas and suggestions are very welcome.","created_at":"2016-09-14T12:08:13Z","updated_at":"2016-09-28T06:04:11Z","closed_at":null,"merged_at":null,"merge_commit_sha":"922ffd2702584b8cecadcf69d81ecb63640e8013","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/docker/docker/pulls/26564/commits","review_comments_url":"https://api.github.com/repos/docker/docker/pulls/26564/comments","review_comment_url":"https://api.github.com/repos/docker/docker/pulls/comments{/number}","comments_url":"https://api.github.com/repos/docker/docker/issues/26564/comments","statuses_url":"https://api.github.com/repos/docker/docker/statuses/301befb621801a627bedafe82725d463e470dd44","head":{"label":"miminar:prevent-blob-uploads","ref":"prevent-blob-uploads","sha":"301befb621801a627bedafe82725d463e470dd44","user":{"login":"miminar","id":1478803,"avatar_url":"https://avatars.githubusercontent.com/u/1478803?v=3","gravatar_id":"","url":"https://api.github.com/users/miminar","html_url":"https://github.com/miminar","followers_url":"https://api.github.com/users/miminar/followers","following_url":"https://api.github.com/users/miminar/following{/other_user}","gists_url":"https://api.github.com/users/miminar/gists{/gist_id}","starred_url":"https://api.github.com/users/miminar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/miminar/subscriptions","organizations_url":"https://api.github.com/users/miminar/orgs","repos_url":"https://api.github.com/users/miminar/repos","events_url":"https://api.github.com/users/miminar/events{/privacy}","received_events_url":"https://api.github.com/users/miminar/received_events","type":"User","site_admin":false},"repo":{"id":25812675,"name":"docker","full_name":"miminar/docker","owner":{"login":"miminar","id":1478803,"avatar_url":"https://avatars.githubusercontent.com/u/1478803?v=3","gravatar_id":"","url":"https://api.github.com/users/miminar","html_url":"https://github.com/miminar","followers_url":"https://api.github.com/users/miminar/followers","following_url":"https://api.github.com/users/miminar/following{/other_user}","gists_url":"https://api.github.com/users/miminar/gists{/gist_id}","starred_url":"https://api.github.com/users/miminar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/miminar/subscriptions","organizations_url":"https://api.github.com/users/miminar/orgs","repos_url":"https://api.github.com/users/miminar/repos","events_url":"https://api.github.com/users/miminar/events{/privacy}","received_events_url":"https://api.github.com/users/miminar/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/miminar/docker","description":"Docker - the open-source application container engine","fork":true,"url":"https://api.github.com/repos/miminar/docker","forks_url":"https://api.github.com/repos/miminar/docker/forks","keys_url":"https://api.github.com/repos/miminar/docker/keys{/key_id}","collaborators_url":"https://api.github.com/repos/miminar/docker/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/miminar/docker/teams","hooks_url":"https://api.github.com/repos/miminar/docker/hooks","issue_events_url":"https://api.github.com/repos/miminar/docker/issues/events{/number}","events_url":"https://api.github.com/repos/miminar/docker/events","assignees_url":"https://api.github.com/repos/miminar/docker/assignees{/user}","branches_url":"https://api.github.com/repos/miminar/docker/branches{/branch}","tags_url":"https://api.github.com/repos/miminar/docker/tags","blobs_url":"https://api.github.com/repos/miminar/docker/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/miminar/docker/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/miminar/docker/git/refs{/sha}","trees_url":"https://api.github.com/repos/miminar/docker/git/trees{/sha}","statuses_url":"https://api.github.com/repos/miminar/docker/statuses/{sha}","languages_url":"https://api.github.com/repos/miminar/docker/languages","stargazers_url":"https://api.github.com/repos/miminar/docker/stargazers","contributors_url":"https://api.github.com/repos/miminar/docker/contributors","subscribers_url":"https://api.github.com/repos/miminar/docker/subscribers","subscription_url":"https://api.github.com/repos/miminar/docker/subscription","commits_url":"https://api.github.com/repos/miminar/docker/commits{/sha}","git_commits_url":"https://api.github.com/repos/miminar/docker/git/commits{/sha}","comments_url":"https://api.github.com/repos/miminar/docker/comments{/number}","issue_comment_url":"https://api.github.com/repos/miminar/docker/issues/comments{/number}","contents_url":"https://api.github.com/repos/miminar/docker/contents/{+path}","compare_url":"https://api.github.com/repos/miminar/docker/compare/{base}...{head}","merges_url":"https://api.github.com/repos/miminar/docker/merges","archive_url":"https://api.github.com/repos/miminar/docker/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/miminar/docker/downloads","issues_url":"https://api.github.com/repos/miminar/docker/issues{/number}","pulls_url":"https://api.github.com/repos/miminar/docker/pulls{/number}","milestones_url":"https://api.github.com/repos/miminar/docker/milestones{/number}","notifications_url":"https://api.github.com/repos/miminar/docker/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/miminar/docker/labels{/name}","releases_url":"https://api.github.com/repos/miminar/docker/releases{/id}","deployments_url":"https://api.github.com/repos/miminar/docker/deployments","created_at":"2014-10-27T09:56:35Z","updated_at":"2016-01-12T08:05:04Z","pushed_at":"2016-09-28T06:01:46Z","git_url":"git://github.com/miminar/docker.git","ssh_url":"git@github.com:miminar/docker.git","clone_url":"https://github.com/miminar/docker.git","svn_url":"https://github.com/miminar/docker","homepage":"http://www.docker.com","size":101204,"stargazers_count":1,"watchers_count":1,"language":"Go","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":0,"forks":1,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"docker:master","ref":"master","sha":"9a2f01ef13e4bb54813aa548c2d44a8e3bafecd2","user":{"login":"docker","id":5429470,"avatar_url":"https://avatars.githubusercontent.com/u/5429470?v=3","gravatar_id":"","url":"https://api.github.com/users/docker","html_url":"https://github.com/docker","followers_url":"https://api.github.com/users/docker/followers","following_url":"https://api.github.com/users/docker/following{/other_user}","gists_url":"https://api.github.com/users/docker/gists{/gist_id}","starred_url":"https://api.github.com/users/docker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/docker/subscriptions","organizations_url":"https://api.github.com/users/docker/orgs","repos_url":"https://api.github.com/users/docker/repos","events_url":"https://api.github.com/users/docker/events{/privacy}","received_events_url":"https://api.github.com/users/docker/received_events","type":"Organization","site_admin":false},"repo":{"id":7691631,"name":"docker","full_name":"docker/docker","owner":{"login":"docker","id":5429470,"avatar_url":"https://avatars.githubusercontent.com/u/5429470?v=3","gravatar_id":"","url":"https://api.github.com/users/docker","html_url":"https://github.com/docker","followers_url":"https://api.github.com/users/docker/followers","following_url":"https://api.github.com/users/docker/following{/other_user}","gists_url":"https://api.github.com/users/docker/gists{/gist_id}","starred_url":"https://api.github.com/users/docker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/docker/subscriptions","organizations_url":"https://api.github.com/users/docker/orgs","repos_url":"https://api.github.com/users/docker/repos","events_url":"https://api.github.com/users/docker/events{/privacy}","received_events_url":"https://api.github.com/users/docker/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/docker/docker","description":"Docker - the open-source application container engine","fork":false,"url":"https://api.github.com/repos/docker/docker","forks_url":"https://api.github.com/repos/docker/docker/forks","keys_url":"https://api.github.com/repos/docker/docker/keys{/key_id}","collaborators_url":"https://api.github.com/repos/docker/docker/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/docker/docker/teams","hooks_url":"https://api.github.com/repos/docker/docker/hooks","issue_events_url":"https://api.github.com/repos/docker/docker/issues/events{/number}","events_url":"https://api.github.com/repos/docker/docker/events","assignees_url":"https://api.github.com/repos/docker/docker/assignees{/user}","branches_url":"https://api.github.com/repos/docker/docker/branches{/branch}","tags_url":"https://api.github.com/repos/docker/docker/tags","blobs_url":"https://api.github.com/repos/docker/docker/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/docker/docker/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/docker/docker/git/refs{/sha}","trees_url":"https://api.github.com/repos/docker/docker/git/trees{/sha}","statuses_url":"https://api.github.com/repos/docker/docker/statuses/{sha}","languages_url":"https://api.github.com/repos/docker/docker/languages","stargazers_url":"https://api.github.com/repos/docker/docker/stargazers","contributors_url":"https://api.github.com/repos/docker/docker/contributors","subscribers_url":"https://api.github.com/repos/docker/docker/subscribers","subscription_url":"https://api.github.com/repos/docker/docker/subscription","commits_url":"https://api.github.com/repos/docker/docker/commits{/sha}","git_commits_url":"https://api.github.com/repos/docker/docker/git/commits{/sha}","comments_url":"https://api.github.com/repos/docker/docker/comments{/number}","issue_comment_url":"https://api.github.com/repos/docker/docker/issues/comments{/number}","contents_url":"https://api.github.com/repos/docker/docker/contents/{+path}","compare_url":"https://api.github.com/repos/docker/docker/compare/{base}...{head}","merges_url":"https://api.github.com/repos/docker/docker/merges","archive_url":"https://api.github.com/repos/docker/docker/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/docker/docker/downloads","issues_url":"https://api.github.com/repos/docker/docker/issues{/number}","pulls_url":"https://api.github.com/repos/docker/docker/pulls{/number}","milestones_url":"https://api.github.com/repos/docker/docker/milestones{/number}","notifications_url":"https://api.github.com/repos/docker/docker/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/docker/docker/labels{/name}","releases_url":"https://api.github.com/repos/docker/docker/releases{/id}","deployments_url":"https://api.github.com/repos/docker/docker/deployments","created_at":"2013-01-18T18:10:57Z","updated_at":"2016-09-28T05:11:10Z","pushed_at":"2016-09-28T06:01:48Z","git_url":"git://github.com/docker/docker.git","ssh_url":"git@github.com:docker/docker.git","clone_url":"https://github.com/docker/docker.git","svn_url":"https://github.com/docker/docker","homepage":"http://www.docker.com","size":211167,"stargazers_count":35374,"watchers_count":35374,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":10416,"mirror_url":null,"open_issues_count":1906,"forks":10416,"open_issues":1906,"watchers":35374,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/docker/docker/pulls/26564"},"html":{"href":"https://github.com/docker/docker/pull/26564"},"issue":{"href":"https://api.github.com/repos/docker/docker/issues/26564"},"comments":{"href":"https://api.github.com/repos/docker/docker/issues/26564/comments"},"review_comments":{"href":"https://api.github.com/repos/docker/docker/pulls/26564/comments"},"review_comment":{"href":"https://api.github.com/repos/docker/docker/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/docker/docker/pulls/26564/commits"},"statuses":{"href":"https://api.github.com/repos/docker/docker/statuses/301befb621801a627bedafe82725d463e470dd44"}}}}
{"actor":{"display_login":"miminar"}}