-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Change default Docker client API version from 1.22 to 1.24 #30927
Change default Docker client API version from 1.22 to 1.24 #30927
Conversation
39aff7b
to
5bd4bff
Compare
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!
5bd4bff
to
a85df5e
Compare
29e5e90
to
ed42415
Compare
I filed #31005 for the failing CI/CD test, unrelated to this change. |
@crobert-1 please rebase |
@samiura 🙂 |
ae8821b
to
9a11337
Compare
sorry for the delay, rebased. |
9f2d62c
to
b070900
Compare
b070900
to
6c1c545
Compare
@@ -18,7 +18,7 @@ The Docker observer extension is a [Receiver Creator](../../../receiver/receiver | |||
container endpoints discovered through the Docker API. Only containers that are in the state of `Running` and not `Paused` will emit endpoints. | |||
This observer watches the Docker engine's stream of events to dynamically create, update, and remove endpoints as events are processed. | |||
|
|||
Requires Docker API Version 1.22+. | |||
Requires Docker API Version 1.24+. |
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.
Curious why this line is needed if it's configurable. Seems confusing. Feel free to remove in a separate PR if it makes sense
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.
this is meant to say to require a minimum of 1.24. A better wording would be Requires Docker API with a version of 1.24 or higher.
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.
I thought it could be configured with a lower version. Maybe I misunderstood
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.
The issue has a link to an explanation as a commit message to the moby project:
moby/moby@08e4e88
The daemon currently provides support for API versions all the way back
to v1.12, which is the version of the API that shipped with docker 1.0. On
Windows, the minimum supported version is v1.24.
Such old versions of the client are rare, and supporting older API versions
has accumulated significant amounts of code to remain backward-compatible
(which is largely untested, and a "best-effort" at most).
This patch updates the minimum API version to v1.24, which is the fallback
API version used when API-version negotiation fails. The intent is to start
deprecating older API versions, but no code is removed yet as part of this
patch, and a DOCKER_MIN_API_VERSION environment variable is added, which
allows overriding the minimum version (to allow restoring the behavior from
before this patch).
With this patch the daemon defaults to API v1.24 as minimum
Description: Docker Client API default has been upgraded from 1.22 to 1.24. The details of the requirements to this change has been given in the accompanying issue with docker upstream PRs.
Link to tracking Issue:
Fixes #30900
Testing: Updated unit tests accordingly and ran integration tests to make sure everything works as it should.
Documentation: