-
Notifications
You must be signed in to change notification settings - Fork 618
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
ecs-init support for old docker engines (pre docker 17.x) and future docker engines (when api 1.25 is deprecated). #4080
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -30,14 +30,13 @@ import ( | |||
) | ||||
|
||||
const ( | ||||
// dockerClientAPIVersion specifies the minimum docker client API version | ||||
// required by ECS Init | ||||
// Version 1.25 is required for setting Init to true when constructing | ||||
// the HostConfig for creating the ECS Agent to enable the Task | ||||
// Networking with ENI capability | ||||
dockerClientAPIVersion = "1.25" | ||||
// enableTaskENIDockerClientAPIVersion is the docker version at which we enable | ||||
// the ECS_ENABLE_TASK_ENI env var and set agent's "Init" field to true. | ||||
enableTaskENIDockerClientAPIVersion = "1.25" | ||||
) | ||||
|
||||
var dockerAPIVersion string | ||||
|
||||
type dockerclient interface { | ||||
ListImages(opts godocker.ListImagesOptions) ([]godocker.APIImages, error) | ||||
LoadImage(opts godocker.LoadImageOptions) error | ||||
|
@@ -65,13 +64,98 @@ func (client godockerClientFactory) NewVersionedClient(endpoint string, apiVersi | |||
return godocker.NewVersionedClient(endpoint, apiVersionString) | ||||
} | ||||
|
||||
// backupAPIVersions are API versions that have been deprecated on docker engine 25, | ||||
// they are only checked if none of the preferredAPIVersions are available. | ||||
// This list can be updated with time as docker engine deprecates more API versions. | ||||
func backupAPIVersions() []string { | ||||
return []string{ | ||||
"1.21", | ||||
"1.22", | ||||
"1.23", | ||||
"1.24", | ||||
} | ||||
} | ||||
|
||||
// preferredAPIVersions returns the preferred list of docker client API versions. | ||||
// When establishing the docker client for ecs-init to use, we will try these API version | ||||
// numbers first. | ||||
func preferredAPIVersions() []string { | ||||
return []string{ | ||||
"1.25", | ||||
"1.26", | ||||
"1.27", | ||||
"1.28", | ||||
"1.29", | ||||
"1.30", | ||||
"1.31", | ||||
"1.32", | ||||
"1.33", | ||||
"1.34", | ||||
"1.35", | ||||
"1.36", | ||||
"1.37", | ||||
"1.38", | ||||
"1.39", | ||||
"1.40", | ||||
"1.41", | ||||
"1.42", | ||||
"1.43", | ||||
"1.44", | ||||
} | ||||
Comment on lines
+82
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have a plan to maintain this list and "backup versions" list going forwards? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No plan in particular, this more just matches what we're doing in ecs-agent:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So deprecation of older API versions would be the forcing function for us to update this list? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes I guess deprecation of older versions, or potentially if a new ECS feature comes along that is only supported in a newer version of the docker API (though I think that's fairly unlikely at this point). |
||||
} | ||||
|
||||
// knownAPIVersions returns all known docker API versions, in order from | ||||
// oldest to newest. | ||||
func knownAPIVersions() []string { | ||||
return append(backupAPIVersions(), preferredAPIVersions()...) | ||||
} | ||||
|
||||
// dockerVersionCompare returns 0 if lhs and rhs are equal. | ||||
// returns 1 if lhs > rhs | ||||
// returns -1 if lhs < rhs | ||||
func dockerVersionCompare(lhs, rhs string) int { | ||||
if lhs == rhs { | ||||
return 0 | ||||
} | ||||
var lhsI int = -1 | ||||
var rhsI int = -1 | ||||
for i, v := range knownAPIVersions() { | ||||
if lhs == v { | ||||
lhsI = i | ||||
} | ||||
if rhs == v { | ||||
rhsI = i | ||||
} | ||||
} | ||||
if lhsI < rhsI { | ||||
return -1 | ||||
} | ||||
return 1 | ||||
} | ||||
|
||||
func newDockerClient(dockerClientFactory dockerClientFactory, pingBackoff backoff.Backoff) (dockerclient, error) { | ||||
var dockerClient dockerclient | ||||
var err error | ||||
allAPIVersions := append(preferredAPIVersions(), backupAPIVersions()...) | ||||
for _, apiVersion := range allAPIVersions { | ||||
dockerClient, err = newVersionedDockerClient(apiVersion, dockerClientFactory, pingBackoff) | ||||
if err == nil { | ||||
log.Infof("Successfully created docker client with API version %s", apiVersion) | ||||
dockerAPIVersion = apiVersion | ||||
break | ||||
} | ||||
log.Infof("Could not create docker client with API version %s: %s", apiVersion, err) | ||||
} | ||||
return dockerClient, err | ||||
} | ||||
|
||||
func newVersionedDockerClient(apiVersion string, dockerClientFactory dockerClientFactory, pingBackoff backoff.Backoff) (dockerclient, error) { | ||||
dockerUnixSocketSourcePath, fromEnv := config.DockerUnixSocket() | ||||
if !fromEnv { | ||||
dockerUnixSocketSourcePath = "/var/run/docker.sock" | ||||
} | ||||
client, err := dockerClientFactory.NewVersionedClient( | ||||
config.UnixSocketPrefix+dockerUnixSocketSourcePath, dockerClientAPIVersion) | ||||
config.UnixSocketPrefix+dockerUnixSocketSourcePath, apiVersion) | ||||
if err != nil { | ||||
return nil, err | ||||
} | ||||
|
@@ -157,6 +241,9 @@ func isNetworkError(err error) bool { | |||
func isRetryablePingError(err error) bool { | ||||
godockerError, ok := err.(*godocker.Error) | ||||
if ok { | ||||
if godockerError.Status == http.StatusBadRequest { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when provided with an unsupported docker api version, docker returns "Bad Request" (status code 400), so don't get into a retry loop if the API version is not supported (client errors in general probably shouldn't be retried at all, but just sticking with 400 for now to avoid changing behavior more than necessary). |
||||
return false | ||||
} | ||||
return godockerError.Status != http.StatusOK | ||||
} | ||||
return false | ||||
|
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.
IIUC before this change ecs-init was not compatible with environments with Docker engines that do not support API versions >= 1.25. But this PR makes ecs-init compatible with such environments. So, does that mean the older API versions were artificially made incompatible with ecs-init?
I understand that Task ENI is not supported on API versions < 1.25, but ecs-init doesn't actually make any Docker API calls that are not supported on API versions < 1.25?
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.
Not by default, but I will check on the various flags that ecs-init can add during startup to see if there are any that are not supported before docker 1.25.