engine: Use known versions for logging drivers

The ECS agent's capability system is used for conditionally-enabling
features based on whether the underlying Docker daemon and the ECS agent
support those features. This is a reasonable approach for many of the
features that Docker supports, especially those that involve structural
elements in the Config and HostConfig objects of the Docker API. It's
particularly important that the agent reports only the versions that
have full structural compatibility as "supported" to the ECS backend, as
some parts of the Config and HostConfig are sent from the backend as
serialized JSON and then are deserialized in the agent into defined
structs. (If we were to deserialize JSON that contained unknown fields,
we'd lose the data in those unknown fields.)

However, there are a few unstructured elements that the Docker daemon
uses, especially for driver selection and driver options. We know that
the agent can deserialize those fields, so there's no risk of losing
data. We've also found that it can be time-consuming to validate that we
have complete structural API representation for new versions, which
complicates the process for adding support for new drivers.

This commit changes the logic to separate "known" API versions from
"supported" API versions.  A "known" version can be used to gate
non-structural API changes (things like logging driver availability)
from API changes that do require structural compatibility. "known" API
versions are not reported in the capability map, but can be used as
feature gates 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)