Add netmode to tests #691
Add netmode to tests #691
Conversation
lowenna
force-pushed
the
14530-netmode
branch
3 times, most recently
from
6f3c40e to
74a3b39
Compare
|
@shin- @mpetazzoni @aanand - Guys, any chance of getting a review on this? It's needed as a pre-req for a docker PR which we need for the Windows daemon to support the upcoming TP3 docker release. Thanks :) |
docker/utils/utils.py
Outdated
There was a problem hiding this comment.
Shouldn't it be default instead to maintain backwards compatibility?
There was a problem hiding this comment.
Just looked at v1.6.0 docker\docker runconfig\parse.go, func parseNetMode L445. (Sorry, can't work out how to get a link directly to a previous version). The valid cases are bridge, none, host and container. Looks like default didn't arrive until later.
There was a problem hiding this comment.
But since the pre-condition here is that API version is >= 1.20, Docker 1.6 shouldn't be a concern.
There was a problem hiding this comment.
Using none is (IMO) cleaner as it pulls in "less" for the test on the daemon side (bearing in mind networking isn't needed for these tests).
There was a problem hiding this comment.
Yeah, but that's the default value you're assigning not just for the tests but every other project using docker-py. It should be default.
There was a problem hiding this comment.
Sure. I should have used none in the others lol. I'll update :)
|
Can you address those few comments? Thanks! |
lowenna
force-pushed
the
14530-netmode
branch
5 times, most recently
from
72f4485 to
e211e45
Compare
Signed-off-by: John Howard <jhoward@microsoft.com>
|
@shin- I have it passing again, but I'm not sure how to address the remainder of your comments. (Python is new to me entirely). |
|
Thanks, that's helpful - I'll take it from here. |
|
Ping @shin-: can we move this forward please? |
|
See #698 |
Signed-off-by: John Howard jhoward@microsoft.com
@swernli @cpuguy83
This PR relates to moby/moby#10662 which is for the docker daemon port to Windows. Part of the daemon port requires the checking of the network mode setting to be moved from the CLI to the daemon, as network modes are platform specific. That PR in docker is moby/moby#14530.
However, for CI on docker-py to pass in docker, it requires updates to docker-py to pass the netmode (which the docker CLI itself would normally do - just the tests here are generally written with the smallest amount of config settings necessary for validation).
Hence this PR adds the network mode where required to the tests. After this is merged, I can update the Dockerfile in 14530 to point to the right docker-py commit.
Validation performed: I have validated docker-py with these changes passes "make binary test-docker-py" against docker/docker master as of this morning. I have also validated that docker-py running against docker with the 14530 changes passes Jenkins CI in docker.