-
Notifications
You must be signed in to change notification settings - Fork 678
Fix docker api proxy smoke-tests after docker-py change (on 1.0 branch) #1367
Conversation
I have no idea why particular test is failing on 1.0 and not on master:
|
So that's in test_valid_no_config_specified, which was introduced recently. |
@paulbellamy can you look into that "breaks on 1.0, fine on master" issue? I've spent a few minutes checking whether anything significant has changed between 1.0 and master in how we manipulate the HostConfig (which is where the log config resides) on create, but it all looks pretty similar. |
I believe it is because in the To completely avoid this we would have to refrain from parsing the request using the go docker client, and just parse it as a set of maps (which would all-around be safer anyway). However that is a bigger change than should go in 1.0. |
That's exactly the kind of thing I was looking for in the proxy code, but afaict we do that in both 1.0 and master, so it doesn't explain the difference in behaviour. |
There is no change. It also fails on master, but master's test matching regex is wrong. |
File an issue.
Is that what docker itself does? I do rather wonder whether the test is too strict, or even just wrong. Note that it actually sends |
Even when you say |
Edit: disregard this, was not what was happening. |
So how do the docker client and docker itself differ in decoding the request? |
Hold on actually. Needs more research.. |
This is a temporary fix for a failure in docker-py tests: "test_valid_no_config_specified".
Ok, have added a temporary fix for these tests, where we don't replace the request body unless we modify it. Opened #1371 to deal with the underlying issue, which is a bigger change than I would like for 1.0. |
@@ -5,6 +5,7 @@ | |||
start_suite "Run docker-py test suite against the proxy" | |||
|
|||
docker_on $HOST1 pull joffrey/docker-py >/dev/null | |||
docker_on $HOST1 pull busybox >/dev/null |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@paulbellamy @bboreham I have made a couple of small tweaks. Please check they are ok. Then squash and merge and do the 1.0->master merge. All LGTM :) |
Fix docker api proxy smoke-tests after docker-py change
Docker-py was changed upstream to run using a test harness Pytest.
Also I have seen some failures due to the 'busybox' image not being found on the test host.