Skip to content

Commit

Permalink
engine/dockerclient: fixup error messages format
Browse files Browse the repository at this point in the history
  • Loading branch information
sharanyad committed Oct 13, 2017
1 parent 331a171 commit e84db1a
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 18 deletions.
24 changes: 11 additions & 13 deletions agent/engine/dockerclient/dockerclientfactory.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@ import (
)

const (
// minDockerAPILinux is the min Docker API version supported by agent
minDockerAPILinux = Version_1_17
// minAPIVersionKey is the docker.Env key for min API version
minAPIVersionKey = "MinAPIVersion"
minAPIVersionKey = "MinAPIVersion"
// apiVersionKey is the docker.Env key for API version
apiVersionKey = "ApiVersion"
apiVersionKey = "ApiVersion"
)
// Factory provides a collection of docker remote clients that include a
// recommended client version as well as a set of alternative supported
Expand Down Expand Up @@ -122,7 +120,7 @@ func findDockerVersions(endpoint string) map[DockerVersion]dockeriface.Client {
// Docker versions
clients, err := findDockerVersionsfromMinMaxVersions(endpoint)
if err != nil {
log.Infof("Error getting Docker clients from min API version: %v", err)
log.Debugf("Unable to get Docker clients from min API version: %v", err)
} else {
return clients
}
Expand All @@ -146,14 +144,14 @@ func findDockerVersions(endpoint string) map[DockerVersion]dockeriface.Client {

func findDockerVersionsfromMinMaxVersions(endpoint string) (map[DockerVersion]dockeriface.Client, error) {
// get a Docker client with the default supported version
client, err := newVersionedClient(endpoint, string(minDockerAPILinux))
client, err := newVersionedClient(endpoint, string(minDockerAPIVersion))
if err != nil {
return nil, errors.Wrap(err, "Error while creating client")
return nil, errors.Wrap(err, "min api version check: error while creating default Docker client")
}

clientVersion, err := client.Version()
if err != nil {
return nil, errors.Wrap(err, "Error while getting client version")
return nil, errors.Wrap(err, "min api version check: error while getting client version")
}

clients := make(map[DockerVersion]dockeriface.Client)
Expand All @@ -164,7 +162,7 @@ func findDockerVersionsfromMinMaxVersions(endpoint string) (map[DockerVersion]do
for _, version := range getKnownAPIVersions() {
dockerClient, err := getDockerClientForVersion(endpoint, string(version), minAPIVersion, apiVersion)
if err != nil {
log.Infof("Error getting client version: %v", err)
log.Debugf("unable to get Docker client for version %s: %v", version, err)
continue
}
clients[version] = dockerClient
Expand All @@ -184,19 +182,19 @@ func getDockerClientForVersion(
minVersionCheck, minErr := utils.Version(version).Matches(lessThanMinCheck)
maxVersionCheck, maxErr := utils.Version(version).Matches(moreThanMaxCheck)
if minErr != nil {
return nil, errors.Wrapf(minErr, "Error while comparing version %s with minAPIVersion %s", version, minAPIVersion)
return nil, errors.Wrapf(minErr, "error while comparing version %s with minAPIVersion %s", version, minAPIVersion)
}
if maxErr != nil {
return nil, errors.Wrapf(minErr, "Error while comparing version %s with apiVersion %s", version, apiVersion)
return nil, errors.Wrapf(minErr, "error while comparing version %s with apiVersion %s", version, apiVersion)
}
// do not add the version when it is less than min api version or greater
// than api version
if minVersionCheck || maxVersionCheck {
return nil, errors.Errorf("Min and API versions comparison check failed for version: %s", version)
return nil, errors.Errorf("min and API versions comparison check failed for version: %s", version)
}
client, err := newVersionedClient(endpoint, string(version))
if err != nil {
return nil, errors.Wrap(err, "Error while creating client")
return nil, errors.Wrapf(err, "unable to create Docker client for version: %s", version)
}
return client, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestGetClientMinimumVersion(t *testing.T) {

newVersionedClient = func(endpoint, version string) (dockeriface.Client, error) {
mockClient := mock_dockeriface.NewMockClient(ctrl)
if version == string(MinDockerAPIWindows) {
if version == string(minDockerAPIVersion) {
mockClient = expectedClient
}
mockClient.EXPECT().Version().Return(&docker.Env{}, nil).AnyTimes()
Expand Down
4 changes: 4 additions & 0 deletions agent/engine/dockerclient/versionsupport_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ import (
"github.com/aws/amazon-ecs-agent/agent/engine/dockeriface"
)

const (
// minDockerAPIVersion is the min Docker API version supported by agent
minDockerAPIVersion = Version_1_17
)
// GetClient on linux will simply return the cached client from the map
func (f *factory) GetClient(version DockerVersion) (dockeriface.Client, error) {
return f.getClient(version)
Expand Down
8 changes: 4 additions & 4 deletions agent/engine/dockerclient/versionsupport_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ import (
"github.com/aws/amazon-ecs-agent/agent/engine/dockeriface"
)

const MinDockerAPIWindows = Version_1_24
const minDockerAPIVersion = Version_1_24

// GetClient will replace some versions of Docker on Windows. We need this because
// agent assumes that it can always call older versions of the docker API.
func (f *factory) GetClient(version DockerVersion) (dockeriface.Client, error) {
for _, v := range getWindowsReplaceableVersions() {
if v == version {
version = MinDockerAPIWindows
version = minDockerAPIVersion
break
}
}
Expand All @@ -48,10 +48,10 @@ func getWindowsReplaceableVersions() []DockerVersion {

// getAgentVersions for Windows should return all of the replaceable versions plus additional versions
func getAgentVersions() []DockerVersion {
return append(getWindowsReplaceableVersions(), MinDockerAPIWindows)
return append(getWindowsReplaceableVersions(), minDockerAPIVersion)
}

// getDefaultVersion returns agent's default version of the Docker API
func getDefaultVersion() DockerVersion {
return MinDockerAPIWindows
return minDockerAPIVersion
}
1 change: 1 addition & 0 deletions agent/utils/compare_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ func parseSemver(version string) (semver, error) {
return result, nil
}

// TODO: remove this logic once non-semver comparator is implemented
func appendIfNoPatch(version string) string {
versionParts := strings.Split(version, ".")
// Only major and minor versions are present
Expand Down

0 comments on commit e84db1a

Please sign in to comment.