Windows: [TP3] Move netmode validation to server #14530
Conversation
fc52126 to
b690152
Compare
|
I'm sure someone will ask why I changed container_config_1_14.json. I went back to Docker v1.2.0 which is API version 1.14. Docker 1.2 has netmode present in HostConfig, but it wasn't present in the fixture. A an empty networkmode is not valid and it's validated in runconfig rather than the client, the tests on runconfig would otherwise fail. |
99b67ec to
46e28d4
Compare
46e28d4 to
632b8bb
Compare
7978e40 to
59fa7ce
Compare
|
Added tests to integration-cli (reworked from runconfig); rebased; waiting for Jenkins |
|
A few failures. |
59fa7ce to
2c1da0b
Compare
|
@cpuguy83 - yes, there were a few lazily written tests with incomplete hostconfigs. And a further fix I had to put into runconfig. All the tests should (famous last words) run successfully now. Worked locally. |
|
Now needs an update to docker-py to fix missing netmode in their integration tests.... |
2c1da0b to
877b176
Compare
|
This should pass Jenkins. But do not merge in it's current form - it has a hack in Dockerfile to pull docker-py from a GitHub branch which has a change which has not yet been PR'd to docker-py. (Will revert this change once Jenkins validates the fix). Strike that - looks like after not having updated docker-py in docker CI for some 6 months, there are some unrelated things which have broken. Investigating.... |
0fea7da to
3897966
Compare
cfdd928 to
6e5422c
Compare
There was a problem hiding this comment.
I don't quite understand this change. Can you please help understand what this is about ?
the "default" implicit mode is introduced recently. The default used to be "bridge".
I assume this is required for backward compatibility check. But isnt it true that the current code should work without the need for this particular change ?
There was a problem hiding this comment.
You may be right now with the latest updates. Let me check.
There was a problem hiding this comment.
Yes, you're right. Reverting.
727bdc3 to
6af5cf5
Compare
|
@mavenugo Updated. Can you verify that my update for your last comment is correct. Running integration-cli locally, and now waiting on Jenkins. |
|
@jhowardmsft Thanks for addressing all the comments. 👍 LGTM. |
|
Thank you so much, @mavenugo! |
There was a problem hiding this comment.
@jhowardmsft curious to know why you need to bump docker py commit?
|
The tests fail when I run them in windows against a linux daemon: |
Signed-off-by: John Howard <jhoward@microsoft.com>
6af5cf5 to
f6ed590
Compare
|
@tiborvass Verified tests work locally Windows to Linux. Waiting for Jenkins. https://gist.github.com/jhowardmsft/50ba1d79f10b1fb81447 |
|
LGTM |
|
I've never been so happy to see status/4-merge on a PR so far!!!!! Thank you all for the reviews and comments :) |
Windows: [TP3] Move netmode validation to server
Signed-off-by: John Howard jhoward@microsoft.com
@swernli
This PR is part of the proposal described in docker/docker issue 10662 to port the docker daemon to Windows. This PR moves the validation of --netmode to the daemon. As the CLI could be pointing to multiple platforms which support different netmodes, it's impossible for the CLI to be deterministic and validate the supplied value.
Fixes #14615