Skip to content
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

docker: always use API version negotiation when initializing clients #24237

Merged
merged 2 commits into from
Oct 17, 2024

Conversation

pkazmierczak
Copy link
Contributor

@pkazmierczak pkazmierczak commented Oct 17, 2024

During a refactoring of the docker driver in #23966 we introduced a bug: API version negotiation option was not passed to every new client call.

Fixes #24212
internal ref: https://hashicorp.atlassian.net/browse/NET-11337

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I realize it's not super realistic for us to exercise the version negotiation in the unit tests, but can you post here how you tested this fixes #24212 just so we have a note on that for the future?

@pkazmierczak
Copy link
Contributor Author

can you post here how you tested this fixes #24212 just so we have a note on that for the future?

I reproduced the exact steps from the ticket. I used an aws instance, installed an older docker version and tested with nomad 1.8.4, 1.9 and then a binary from this branch. It was easy to reproduce the bug and check the solution works.

Automating this is somewhat non-trivial. We already use incompatible dockerd in our e2e, but we never point to an endpoint in our plugin config, so that code path wasn't exercised (as the reporter mentions, if endpoint isn't explicitly configured, API version negotiation works). Changing plugin configuration of our nomad clients on the fly to actually test this is hard. Mocking a simple dockerd—an approach I just tried—is also harder than expected. I might get back to this but it'll take a while.

@pkazmierczak pkazmierczak merged commit 1ac14f4 into main Oct 17, 2024
32 checks passed
@pkazmierczak pkazmierczak deleted the b-docker-api-ver-negotiation branch October 17, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.9.x backport to 1.9.x release line theme/driver/docker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nomad 1.9.0 fails to negotiate docker API version if endpoint argument is set
2 participants