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

Update docker client library to latest #3557

Closed
wants to merge 5 commits into from

Conversation

sparrc
Copy link
Contributor

@sparrc sparrc commented Feb 2, 2023

Summary

Ran the following go module update commands:

go get github.com/docker/docker@latest
go get github.com/microsoft/hcsshim@latest
go mod tidy
go mod vendor

The microsoft library was updated to fix a build issue in the latest docker library with the version of hcsshim we were using. See commit message:

this is to fix a dependency in the windows docker library which depends
on this file and it's Build() function:
https://github.com/microsoft/hcsshim/blob/v0.8.15/osversion/osversion_windows.go

without bumping this we get windows build failures like this:

+ go build -ldflags ' -s' -o out/amazon-ecs-agent.exe ./agent/
Error: agent/vendor/github.com/docker/docker/pkg/system/lcow.go:21:31: undefined: osversion.Build
make: *** [Makefile:60: xplatform-build] Error 2

Testing

verified long functional tests ran on AL2 platform

manually verified agent builds, runs, and runs tasks on an AMI with an older docker version (19.03.13)

Description for the changelog

Enhancement: update docker client library to latest

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

this is to fix a dependency in the windows docker library which depends
on this file and it's Build() function:
https://github.com/microsoft/hcsshim/blob/v0.8.15/osversion/osversion_windows.go

without bumping this we get windows build failures like this:

+ go build -ldflags ' -s' -o out/amazon-ecs-agent.exe ./agent/
Error: agent/vendor/github.com/docker/docker/pkg/system/lcow.go:21:31: undefined: osversion.Build
make: *** [Makefile:60: xplatform-build] Error 2
@sparrc sparrc requested a review from a team as a code owner February 2, 2023 19:20
@sparrc sparrc added the bot/test label Feb 2, 2023
Passing nil to ContainerCreate means that the client library will ignore
this arg and not pass it in the list of arguments it submits to the
docker API.

see https://github.com/moby/moby/blob/4c0d75bc8e8311407040ae7c930f5a2c2c9c03aa/client/container_create.go#L49

We don't want to pass the platform arg because we are not using it and
this is the simplest way to ensure continued support of older docker
versions.
@sparrc sparrc force-pushed the update-docker-library-v2 branch from adf1973 to 1692d11 Compare February 2, 2023 22:18
@sparrc sparrc changed the title [wip] Update docker library v2 Update docker client library to latest Feb 2, 2023
@@ -574,7 +574,7 @@ func (dg *dockerGoClient) createContainer(ctx context.Context,
return DockerContainerMetadata{Error: CannotGetDockerClientError{version: dg.version, err: err}}
}

dockerContainer, err := client.ContainerCreate(ctx, config, hostConfig, &network.NetworkingConfig{}, name)
dockerContainer, err := client.ContainerCreate(ctx, config, hostConfig, &network.NetworkingConfig{}, nil, name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passing nil to ContainerCreate means that the client library will ignore
this arg and not pass it in the list of arguments it submits to the
docker API.

see https://github.com/moby/moby/blob/4c0d75bc8e8311407040ae7c930f5a2c2c9c03aa/client/container_create.go#L49

We don't want to pass the platform arg because we are not using it and
this is the simplest way to ensure continued support of older docker
versions.

@@ -141,7 +141,7 @@ VERBOSE=-v -cover
# provide false positives when running integ tests, so we err on the side of
# caution. See `go help test`
# unit tests include the coverage profile
GOTEST=${GO_EXECUTABLE} test -count=1 ${VERBOSE}
GOTEST=${GO_EXECUTABLE} test -count=1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unrelated fix for test-silent make target

@sparrc
Copy link
Contributor Author

sparrc commented Mar 2, 2023

closing in favor of #3598

@sparrc sparrc closed this Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants