engine: Use known versions for logging drivers

agent/app/agent_test.go

@@ -158,6 +158,7 @@ func TestDoStartRegisterContainerInstanceErrorTerminal(t *testing.T) {
 	gomock.InOrder(
 		mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil),
 		dockerClient.EXPECT().SupportedVersions().Return(nil),
+		dockerClient.EXPECT().KnownVersions().Return(nil),
 		client.EXPECT().RegisterContainerInstance(gomock.Any(), gomock.Any()).Return(
 			"", utils.NewAttributeError("error")),
 	)
@@ -185,6 +186,7 @@ func TestDoStartRegisterContainerInstanceErrorNonTerminal(t *testing.T) {
 
 	gomock.InOrder(
 		dockerClient.EXPECT().SupportedVersions().Return(nil),
+		dockerClient.EXPECT().KnownVersions().Return(nil),
 		client.EXPECT().RegisterContainerInstance(gomock.Any(), gomock.Any()).Return(
 			"", errors.New("error")),
 	)

agent/app/agent_unix_test.go

@@ -45,6 +45,7 @@ func TestDoStartHappyPath(t *testing.T) {
 	gomock.InOrder(
 		mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil),
 		dockerClient.EXPECT().SupportedVersions().Return(nil),
+		dockerClient.EXPECT().KnownVersions().Return(nil),
 		client.EXPECT().RegisterContainerInstance(gomock.Any(), gomock.Any()).Return("arn", nil),
 		imageManager.EXPECT().SetSaver(gomock.Any()),
 		dockerClient.EXPECT().ContainerEvents(gomock.Any()).Return(containerChangeEvents, nil),

agent/app/agent_windows_test.go

@@ -48,6 +48,7 @@ func TestDoStartHappyPath(t *testing.T) {
 	gomock.InOrder(
 		mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil),
 		dockerClient.EXPECT().SupportedVersions().Return(nil),
+		dockerClient.EXPECT().KnownVersions().Return(nil),
 		client.EXPECT().RegisterContainerInstance(gomock.Any(), gomock.Any()).Return("arn", nil),
 		imageManager.EXPECT().SetSaver(gomock.Any()),
 		dockerClient.EXPECT().ContainerEvents(gomock.Any()).Return(containerChangeEvents, nil),

agent/ecr/mocks/ecr_mocks.go

@@ -17,8 +17,8 @@
 package mock_ecr
 
 import (
-	ecr0 "github.com/aws/amazon-ecs-agent/agent/ecr"
-	ecr "github.com/aws/amazon-ecs-agent/agent/ecr/model/ecr"
+	ecr "github.com/aws/amazon-ecs-agent/agent/ecr"
+	ecr0 "github.com/aws/amazon-ecs-agent/agent/ecr/model/ecr"
 	gomock "github.com/golang/mock/gomock"
 )
 
@@ -43,9 +43,9 @@ func (_m *MockECRSDK) EXPECT() *_MockECRSDKRecorder {
 	return _m.recorder
 }
 
-func (_m *MockECRSDK) GetAuthorizationToken(_param0 *ecr.GetAuthorizationTokenInput) (*ecr.GetAuthorizationTokenOutput, error) {
+func (_m *MockECRSDK) GetAuthorizationToken(_param0 *ecr0.GetAuthorizationTokenInput) (*ecr0.GetAuthorizationTokenOutput, error) {
 	ret := _m.ctrl.Call(_m, "GetAuthorizationToken", _param0)
-	ret0, _ := ret[0].(*ecr.GetAuthorizationTokenOutput)
+	ret0, _ := ret[0].(*ecr0.GetAuthorizationTokenOutput)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
@@ -75,9 +75,9 @@ func (_m *MockECRFactory) EXPECT() *_MockECRFactoryRecorder {
 	return _m.recorder
 }
 
-func (_m *MockECRFactory) GetClient(_param0 string, _param1 string) ecr0.ECRClient {
+func (_m *MockECRFactory) GetClient(_param0 string, _param1 string) ecr.ECRClient {
 	ret := _m.ctrl.Call(_m, "GetClient", _param0, _param1)
-	ret0, _ := ret[0].(ecr0.ECRClient)
+	ret0, _ := ret[0].(ecr.ECRClient)
 	return ret0
 }
 
@@ -106,9 +106,9 @@ func (_m *MockECRClient) EXPECT() *_MockECRClientRecorder {
 	return _m.recorder
 }
 
-func (_m *MockECRClient) GetAuthorizationToken(_param0 string) (*ecr.AuthorizationData, error) {
+func (_m *MockECRClient) GetAuthorizationToken(_param0 string) (*ecr0.AuthorizationData, error) {
 	ret := _m.ctrl.Call(_m, "GetAuthorizationToken", _param0)
-	ret0, _ := ret[0].(*ecr.AuthorizationData)
+	ret0, _ := ret[0].(*ecr0.AuthorizationData)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
@@ -117,7 +117,7 @@ func (_mr *_MockECRClientRecorder) GetAuthorizationToken(arg0 interface{}) *gomo
 	return _mr.mock.ctrl.RecordCall(_mr.mock, "GetAuthorizationToken", arg0)
 }
 
-func (_m *MockECRClient) IsTokenValid(_param0 *ecr.AuthorizationData) bool {
+func (_m *MockECRClient) IsTokenValid(_param0 *ecr0.AuthorizationData) bool {
 	ret := _m.ctrl.Call(_m, "IsTokenValid", _param0)
 	ret0, _ := ret[0].(bool)
 	return ret0

agent/engine/docker_container_engine.go

@@ -71,25 +71,60 @@ const (
 type DockerClient interface {
 	// SupportedVersions returns a slice of the supported docker versions (or at least supposedly supported).
 	SupportedVersions() []dockerclient.DockerVersion
+
+	// KnownVersions returns a slice of the Docker API versions known to the Docker daemon.
+	KnownVersions() []dockerclient.DockerVersion
+
 	// WithVersion returns a new DockerClient for which all operations will use the given remote api version.
 	// A default version will be used for a client not produced via this method.
 	WithVersion(dockerclient.DockerVersion) DockerClient
+
+	// ContainerEvents returns a channel of DockerContainerChangeEvents. Events are placed into the channel and should
+	// be processed by the listener.
 	ContainerEvents(ctx context.Context) (<-chan DockerContainerChangeEvent, error)
 
+	// PullImage pulls an image. authData should contain authentication data provided by the ECS backend.
 	PullImage(image string, authData *api.RegistryAuthenticationData) DockerContainerMetadata
 
+	// CreateContainer creates a container with the provided docker.Config, docker.HostConfig, and name. A timeout value
+	// should be provided for the request.
 	CreateContainer(*docker.Config, *docker.HostConfig, string, time.Duration) DockerContainerMetadata
+
+	// StartContainer starts the container identified by the name provided. A timeout value should be provided for the
+	// request.
 	StartContainer(string, time.Duration) DockerContainerMetadata
+
+	// StopContainer stops the container identified by the name provided. A timeout value should be provided for the
+	// request.
 	StopContainer(string, time.Duration) DockerContainerMetadata
+
+	// DescribeContainer returns status information about the specified container.
 	DescribeContainer(string) (api.ContainerStatus, DockerContainerMetadata)
+
+	// RemoveContainer removes a container (typically the rootfs, logs, and associated metadata) identified by the name.
+	// A timeout value should be provided for the request.
 	RemoveContainer(string, time.Duration) error
 
+	// InspectContainer returns information about the specified container. A timeout value should be provided for the
+	// request.
 	InspectContainer(string, time.Duration) (*docker.Container, error)
+
+	// ListContainers returns the set of containers known to the Docker daemon. A timeout value should be provided for
+	// the request.
 	ListContainers(bool, time.Duration) ListContainersResponse
+
+	// Stats returns a channel of stat data for the specified container. A context should be provided so the request can
+	// be canceled.
 	Stats(string, context.Context) (<-chan *docker.Stats, error)
 
+	// Version returns the version of the Docker daemon.
 	Version() (string, error)
+
+	// InspectImage returns information about the specified image.
 	InspectImage(string) (*docker.Image, error)
+
+	// RemoveImage removes the metadata associated with an image and may remove the underlying layer data. A timeout
+	// value should be provided for the request.
 	RemoveImage(string, time.Duration) error
 }
 
@@ -789,6 +824,10 @@ func (dg *dockerGoClient) SupportedVersions() []dockerclient.DockerVersion {
 	return dg.clientFactory.FindSupportedAPIVersions()
 }
 
+func (dg *dockerGoClient) KnownVersions() []dockerclient.DockerVersion {
+	return dg.clientFactory.FindKnownAPIVersions()
+}
+
 func (dg *dockerGoClient) Version() (string, error) {
 	client, err := dg.dockerClient()
 	if err != nil {

agent/engine/docker_task_engine.go

@@ -761,15 +761,25 @@ func (engine *DockerTaskEngine) Capabilities() []string {
 	if !engine.cfg.PrivilegedDisabled {
 		capabilities = append(capabilities, capabilityPrefix+"privileged-container")
 	}
-	versions := make(map[dockerclient.DockerVersion]bool)
+
+	supportedVersions := make(map[dockerclient.DockerVersion]bool)
+	// Determine API versions to report as supported. Supported versions are also used for capability-enablement, except
+	// logging drivers.
 	for _, version := range engine.client.SupportedVersions() {
 		capabilities = append(capabilities, capabilityPrefix+"docker-remote-api."+string(version))
-		versions[version] = true
+		supportedVersions[version] = true
+	}
+
+	knownVersions := make(map[dockerclient.DockerVersion]struct{})
+	// Determine known API versions. Known versions are used exclusively for logging-driver enablement, since none of
+	// the structural API elements change.
+	for _, version := range engine.client.KnownVersions() {
+		knownVersions[version] = struct{}{}
 	}
 
 	for _, loggingDriver := range engine.cfg.AvailableLoggingDrivers {
 		requiredVersion := dockerclient.LoggingDriverMinimumVersion[loggingDriver]
-		if _, ok := versions[requiredVersion]; ok {
+		if _, ok := knownVersions[requiredVersion]; ok {
 			capabilities = append(capabilities, capabilityPrefix+"logging-driver."+string(loggingDriver))
 		}
 	}
@@ -781,15 +791,15 @@ func (engine *DockerTaskEngine) Capabilities() []string {
 		capabilities = append(capabilities, capabilityPrefix+"apparmor")
 	}
 
-	if _, ok := versions[dockerclient.Version_1_19]; ok {
+	if _, ok := supportedVersions[dockerclient.Version_1_19]; ok {
 		capabilities = append(capabilities, capabilityPrefix+"ecr-auth")
 	}
 
 	if engine.cfg.TaskIAMRoleEnabled {
 		// The "task-iam-role" capability is supported for docker v1.7.x onwards
 		// Refer https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api.md
-		// to lookup the table of docker versions to API versions
-		if _, ok := versions[dockerclient.Version_1_19]; ok {
+		// to lookup the table of docker supportedVersions to API supportedVersions
+		if _, ok := supportedVersions[dockerclient.Version_1_19]; ok {
 			capabilities = append(capabilities, capabilityPrefix+capabilityTaskIAMRole)
 		} else {
 			seelog.Warn("Task IAM Role not enabled due to unsuppported Docker version")
@@ -798,7 +808,7 @@ func (engine *DockerTaskEngine) Capabilities() []string {
 
 	if engine.cfg.TaskIAMRoleEnabledForNetworkHost {
 		// The "task-iam-role-network-host" capability is supported for docker v1.7.x onwards
-		if _, ok := versions[dockerclient.Version_1_19]; ok {
+		if _, ok := supportedVersions[dockerclient.Version_1_19]; ok {
 			capabilities = append(capabilities, capabilityPrefix+capabilityTaskIAMRoleNetHost)
 		} else {
 			seelog.Warn("Task IAM Role for Host Network not enabled due to unsuppported Docker version")

agent/engine/docker_task_engine_test.go

@@ -984,6 +984,12 @@ func TestCapabilities(t *testing.T) {
 		dockerclient.Version_1_18,
 	})
 
+	client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{
+		dockerclient.Version_1_17,
+		dockerclient.Version_1_18,
+		dockerclient.Version_1_19,
+	})
+
 	capabilities := taskEngine.Capabilities()
 
 	expectedCapabilities := []string{
@@ -992,6 +998,7 @@ func TestCapabilities(t *testing.T) {
 		"com.amazonaws.ecs.capability.docker-remote-api.1.18",
 		"com.amazonaws.ecs.capability.logging-driver.json-file",
 		"com.amazonaws.ecs.capability.logging-driver.syslog",
+		"com.amazonaws.ecs.capability.logging-driver.journald",
 		"com.amazonaws.ecs.capability.selinux",
 		"com.amazonaws.ecs.capability.apparmor",
 	}
@@ -1009,6 +1016,7 @@ func TestCapabilitiesECR(t *testing.T) {
 	client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{
 		dockerclient.Version_1_19,
 	})
+	client.EXPECT().KnownVersions().Return(nil)
 
 	capabilities := taskEngine.Capabilities()
 
@@ -1032,6 +1040,7 @@ func TestCapabilitiesTaskIAMRoleForSupportedDockerVersion(t *testing.T) {
 	client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{
 		dockerclient.Version_1_19,
 	})
+	client.EXPECT().KnownVersions().Return(nil)
 
 	capabilities := taskEngine.Capabilities()
 	capMap := make(map[string]bool)
@@ -1053,6 +1062,7 @@ func TestCapabilitiesTaskIAMRoleForUnSupportedDockerVersion(t *testing.T) {
 	client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{
 		dockerclient.Version_1_18,
 	})
+	client.EXPECT().KnownVersions().Return(nil)
 
 	capabilities := taskEngine.Capabilities()
 	capMap := make(map[string]bool)
@@ -1074,6 +1084,7 @@ func TestCapabilitiesTaskIAMRoleNetworkHostForSupportedDockerVersion(t *testing.
 	client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{
 		dockerclient.Version_1_19,
 	})
+	client.EXPECT().KnownVersions().Return(nil)
 
 	capabilities := taskEngine.Capabilities()
 	capMap := make(map[string]bool)
@@ -1095,6 +1106,7 @@ func TestCapabilitiesTaskIAMRoleNetworkHostForUnSupportedDockerVersion(t *testin
 	client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{
 		dockerclient.Version_1_18,
 	})
+	client.EXPECT().KnownVersions().Return(nil)
 
 	capabilities := taskEngine.Capabilities()
 	capMap := make(map[string]bool)

agent/engine/dockerclient/dockerclientfactory.go

@@ -32,11 +32,21 @@ type Factory interface {
 	// if the client doesn't exist.
 	GetClient(version DockerVersion) (dockeriface.Client, error)
 
-	// FindSupportedAPIVersions tests each agent supported API version
-	// against the Docker daemon and returns a slice of supported
-	// versions. The slice will always be a subset (non-strict)
-	// of agent supported versions.
+	// FindSupportedAPIVersions returns a slice of agent-supported Docker API
+	// versions. Versions are tested by making calls against the Docker daemon
+	// and may occur either at Factory creation time or lazily upon invocation
+	// of this function. The slice represents the intersection of
+	// agent-supported versions and daemon-supported versions.
 	FindSupportedAPIVersions() []DockerVersion
+
+	// FindKnownAPIVersions returns a slice of Docker API versions that are
+	// known to the Docker daemon. Versions are tested by making calls against
+	// the Docker daemon and may occur either at Factory creation time or
+	// lazily upon invocation of this function. The slice represents the
+	// intersection of the API versions that the agent knows exist (but does
+	// not necessarily fully support) and the versions that result in
+	// successful responses by the Docker daemon.
+	FindKnownAPIVersions() []DockerVersion
 }
 
 type factory struct {
@@ -64,16 +74,28 @@ func (f *factory) GetDefaultClient() (dockeriface.Client, error) {
 
 func (f *factory) FindSupportedAPIVersions() []DockerVersion {
 	var supportedVersions []DockerVersion
-	for _, agentVersion := range getAgentVersions() {
-		_, err := f.GetClient(agentVersion)
+	for _, testVersion := range getAgentVersions() {
+		_, err := f.GetClient(testVersion)
 		if err != nil {
 			continue
 		}
-		supportedVersions = append(supportedVersions, agentVersion)
+		supportedVersions = append(supportedVersions, testVersion)
 	}
 	return supportedVersions
 }
 
+func (f *factory) FindKnownAPIVersions() []DockerVersion {
+	var knownVersions []DockerVersion
+	for _, testVersion := range getKnownAPIVersions() {
+		_, err := f.GetClient(testVersion)
+		if err != nil {
+			continue
+		}
+		knownVersions = append(knownVersions, testVersion)
+	}
+	return knownVersions
+}
+
 // getClient returns a client specified by the docker version. Its wrapped
 // by GetClient so that it can do platform-specific magic
 func (f *factory) getClient(version DockerVersion) (dockeriface.Client, error) {

agent/engine/dockerclient/mocks/dockerclient_mocks.go

@@ -43,6 +43,16 @@ func (_m *MockFactory) EXPECT() *_MockFactoryRecorder {
 	return _m.recorder
 }
 
+func (_m *MockFactory) FindKnownAPIVersions() []dockerclient.DockerVersion {
+	ret := _m.ctrl.Call(_m, "FindKnownAPIVersions")
+	ret0, _ := ret[0].([]dockerclient.DockerVersion)
+	return ret0
+}
+
+func (_mr *_MockFactoryRecorder) FindKnownAPIVersions() *gomock.Call {
+	return _mr.mock.ctrl.RecordCall(_mr.mock, "FindKnownAPIVersions")
+}
+
 func (_m *MockFactory) FindSupportedAPIVersions() []dockerclient.DockerVersion {
 	ret := _m.ctrl.Call(_m, "FindSupportedAPIVersions")
 	ret0, _ := ret[0].([]dockerclient.DockerVersion)

agent/engine/engine_mocks.go

@@ -243,6 +243,16 @@ func (_mr *_MockDockerClientRecorder) InspectImage(arg0 interface{}) *gomock.Cal
 	return _mr.mock.ctrl.RecordCall(_mr.mock, "InspectImage", arg0)
 }
 
+func (_m *MockDockerClient) KnownVersions() []dockerclient.DockerVersion {
+	ret := _m.ctrl.Call(_m, "KnownVersions")
+	ret0, _ := ret[0].([]dockerclient.DockerVersion)
+	return ret0
+}
+
+func (_mr *_MockDockerClientRecorder) KnownVersions() *gomock.Call {
+	return _mr.mock.ctrl.RecordCall(_mr.mock, "KnownVersions")
+}
+
 func (_m *MockDockerClient) ListContainers(_param0 bool, _param1 time.Duration) ListContainersResponse {
 	ret := _m.ctrl.Call(_m, "ListContainers", _param0, _param1)
 	ret0, _ := ret[0].(ListContainersResponse)