-
Notifications
You must be signed in to change notification settings - Fork 1.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
tests/exec: expect 127 exit code for missing executable #3290
tests/exec: expect 127 exit code for missing executable #3290
Conversation
PR to fix the breaking docker-py test: #3290 I called it out there, but we'll need to TAL at the dependencies between the engine and docker-py. Docker-py runs integration tests against the engine, and the engine uses docker-py to run tests, so changing it in either place first is a bit annoying. We might need to skip the test in docker-py -> change the engine behavior -> unskip+fix the test in docker-py, and update the engine version it runs against. |
3235d5c
to
a252402
Compare
Docker Engine has always returned `126` when starting an exec fails due to a missing binary, but this was due to a bug in the daemon causing the correct exit code to be overwritten in some cases – see: moby/moby#45795 Change tests to expect correct exit code (`127`). Signed-off-by: Laura Brehm <laurabrehm@hey.com>
a252402
to
cd2d263
Compare
Temporarily skip the exec run failed exit code test in `docker-py` – https://github.com/docker/docker-py/blob/a3652028b1ead708bd9191efb286f909ba6c2a49/tests/integration/models_containers_test.py#L356-L363 We can reenable this after the PR fixing the expected exit code in that test is merged/released/included – docker/docker-py#3290 Signed-off-by: Laura Brehm <laurabrehm@hey.com>
128ae9a
to
f0048c1
Compare
Execs should return the exit code of the exec'd process, if it started. Signed-off-by: Laura Brehm <laurabrehm@hey.com>
f0048c1
to
b126547
Compare
It's passing now @thaJeztah 🎉 |
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.
LGTM, thanks!
The Docker Engine has always returned
126
when starting an exec fails due to a missing binary, but this was due to a bug in the daemon causing the correct exit code to be overwritten in some cases – see: moby/moby#45795Change tests to expect correct exit code (
127
).Note: Must take some care to what's the best way to fix this without breaking engine or vice-versa, due to the engine running tests against docker-py and docker-py running tests against the engine. It might be necessary to 1st. skip the test, then fix the engine, then fix/unskip the test.