-
Notifications
You must be signed in to change notification settings - Fork 18.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
Porting libnetwork fixes #29004
Porting libnetwork fixes #29004
Conversation
Signed-off-by: Alessandro Boch <[email protected]>
@aboch looks like issues |
Signed-off-by: Tom Denham <[email protected]>
Not sure about vendor failure:
It looks like it is looking for the |
experimental is failing because in 1.12, plugins are still in experimental. The media type for plugins was changed in 1.13 to That's why the tests are failing. We have 2 options:
I prefer (1) |
For the plugin mediatype changes, we need 970b23d |
An easier option is to simply add the new media type here https://github.com/aboch/docker/blob/76d2f1f29bbc11c92ab20489fc5eee2593b6d879/plugin/distribution/types.go#L16. Then use both the new and the old type for checking in https://github.com/aboch/docker/blob/76d2f1f29bbc11c92ab20489fc5eee2593b6d879/plugin/distribution/pull.go#L146 |
@johnstep can you please try this one, since there is no CI for 1.12.x on Windows |
LGTM on Windows. It built successfully on Windows (for Windows) and all unit tests passed. I rebuilt and tested again with the resulting binaries, and that succeeded. |
@@ -143,7 +143,8 @@ func Pull(name string, rs registry.Service, metaheader http.Header, authConfig * | |||
logrus.Debugf("pull.go: error in json.Unmarshal(): %v", err) | |||
return nil, err | |||
} | |||
if m.Config.MediaType != MediaTypeConfig { | |||
if m.Config.MediaType != MediaTypeConfig && | |||
m.Config.MediaType != "application/vnd.docker.plugin.v0+json" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this check is updated as follows, we should be good:
if m.Config.MediaType != MediaTypeConfig &&
m.Config.MediaType != "application/vnd.docker.plugin.v0+json" &&
m.Config.MediaType != "application/vnd.docker.plugin.image.v0+json"{
...
}
All experimental tests are failing due to the known plugin mediatype issue. It can be fixed by updating the code to accommodate to the expected media type of the sample plugin. Or the sample plugin can be re-pushed by @tiborvass In either case, this PR can be merged, since the failing tests are unrelated to the PR. |
LGTM ping @LK4D4 @tonistiigi |
@anusha-ragunathan Thanks for your help in trying to solve the experimental failure on 1.12.x. |
@tonistiigi windows is expected to fail |
#29089 handles the experimental failures. LGTM |
Brings to 1.12.x the fixes for the following issues: