Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Limit v1 protocol fallbacks #18590

Merged
merged 2 commits into from Dec 17, 2015
Merged

Limit v1 protocol fallbacks #18590

merged 2 commits into from Dec 17, 2015

Conversation

aaronlehmann
Copy link
Contributor

Two commits:

Do not fall back to the V1 protocol when we know we are talking to a V2 registry

If we detect a Docker-Distribution-Api-Version header indicating that
the registry speaks the V2 protocol, no fallback to V1 should take
place.

The same applies if a V2 registry operation succeeds while attempting a
push or pull.

and:

Make v1 pull/push output consistent with v2

- Use layer DiffIDs for progress output in v1 push. This makes the
  output consistent with v2 pushes, which means that a fallback to v1
  won't start progress bars for a different set of IDs.

- Change wording used in v1 status updates to be consistent with v2.

@aaronlehmann aaronlehmann force-pushed the limit-v1-fallbacks branch 2 times, most recently from 73ddd0c to d178cf8 Compare December 11, 2015 02:41
@estesp
Copy link
Contributor

estesp commented Dec 11, 2015

I've looked through the code earlier today and seems reasonable. One question--is there anything user facing (other than the messages being updated in the 2nd commit) that needs to be mentioned in docs? Is behavior changing in any way that will be different than any docs we have on registry interactions? I'm being lazy by not looking myself, but that's what came to mind when I was reviewing.

@aaronlehmann
Copy link
Contributor Author

I don't believe the fallback logic (or pull/push in general for that matter) is documented in any significant detail. When I looked at the CLI documentation for docker pull and docker push recently, it didn't mention protocol versions at all. But this is a good point, and if anyone knows a place in the documentation that would be affected, please point it out.

@aaronlehmann
Copy link
Contributor Author

Rebased.

dmcgowan
dmcgowan reviewed Dec 14, 2015
View reviewed changes
distribution/pull_v2.go
@@ -78,6 +80,10 @@ func (p *v2Puller) pullV2Repository(ctx context.Context, ref reference.Named) (e
return err
}

// If theis call succeeded, we can be confident that the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

@dmcgowan
Copy link
Member

LGTM

@tonistiigi
Copy link
Member

LGTM

@aaronlehmann
Do not fall back to the V1 protocol when we know we are talking to a …
a57478d 
…V2 registry

If we detect a Docker-Distribution-Api-Version header indicating that
the registry speaks the V2 protocol, no fallback to V1 should take
place.

The same applies if a V2 registry operation succeeds while attempting a
push or pull.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@aaronlehmann
Make v1 pull/push output consistent with v2
891bbc1 
- Use layer DiffIDs for progress output in v1 push. This makes the
  output consistent with v2 pushes, which means that a fallback to v1
  won't start progress bars for a different set of IDs.

- Change wording used in v1 status updates to be consistent with v2.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@aaronlehmann
Copy link
Contributor Author

Rebased.

@runcom
Copy link
Member

runcom commented Dec 17, 2015

LGTM

runcom added a commit that referenced this pull request Dec 17, 2015
@runcom
Merge pull request #18590 from aaronlehmann/limit-v1-fallbacks
98be580 
Limit v1 protocol fallbacks
@runcom runcom merged commit 98be580 into moby:master Dec 17, 2015
@aaronlehmann aaronlehmann mentioned this pull request Dec 17, 2015
Merged
@aaronlehmann aaronlehmann deleted the limit-v1-fallbacks branch December 17, 2015 18:49
@thaJeztah thaJeztah added this to the 1.10 milestone Dec 21, 2015
@aaronlehmann aaronlehmann mentioned this pull request Dec 21, 2015
Closed
@mattmoor
Copy link
Contributor

@aaronlehmann This breaks compatibility at Docker head in ways that weren't discussed as part of the deprecation plan.

@aaronlehmann aaronlehmann mentioned this pull request Dec 21, 2015
Merged
aaronlehmann added a commit to aaronlehmann/docker that referenced this pull request Dec 22, 2015
@aaronlehmann
When a manifest is not found, allow fallback to v1
9d6acbe 
PR moby#18590 caused compatibility issues with registries such as gcr.io
which support both the v1 and v2 protocols, but do not provide the same
set of images over both protocols. After moby#18590, pulls from these
registries would never use the v1 protocol, because of the
Docker-Distribution-Api-Version header indicating that v2 was supported.

Fix the problem by making an exception for the case where a manifest is
not found. This should allow fallback to v1 in case that image is
exposed over the v1 protocol but not the v2 protocol.

This avoids the overly aggressive fallback behavior before moby#18590 which
would allow protocol fallback after almost any error, but restores
interoperability with mixed v1/v2 registry setups.

Fixes moby#18832

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@aaronlehmann aaronlehmann mentioned this pull request Jan 7, 2016
Closed
aditirajagopal pushed a commit to aditirajagopal/docker that referenced this pull request Feb 8, 2016
@aaronlehmann
When a manifest is not found, allow fallback to v1
e719669 
PR moby#18590 caused compatibility issues with registries such as gcr.io
which support both the v1 and v2 protocols, but do not provide the same
set of images over both protocols. After moby#18590, pulls from these
registries would never use the v1 protocol, because of the
Docker-Distribution-Api-Version header indicating that v2 was supported.

Fix the problem by making an exception for the case where a manifest is
not found. This should allow fallback to v1 in case that image is
exposed over the v1 protocol but not the v2 protocol.

This avoids the overly aggressive fallback behavior before moby#18590 which
would allow protocol fallback after almost any error, but restores
interoperability with mixed v1/v2 registry setups.

Fixes moby#18832

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
area/distribution impact/changelog status/2-code-review
Projects
None yet
Milestone
1.10.0
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@aaronlehmann @estesp @dmcgowan @tonistiigi @runcom @mattmoor @wzheng2310 @calavera @thaJeztah @GordonTheTurtle