From 632a9abd944aa8816dba81615d4aebee09aa3ab7 Mon Sep 17 00:00:00 2001 From: Meghna Srivastav Date: Tue, 28 May 2019 15:36:37 -0700 Subject: [PATCH] Fixing Agent reports incorrect capabilities on windows (#2035) --- agent/app/agent_capability.go | 19 +- agent/app/agent_capability_test.go | 9 - agent/app/agent_capability_unix.go | 12 + agent/app/agent_capability_unix_test.go | 266 +++++++++++++++++++++- agent/app/agent_capability_unspecified.go | 12 + agent/app/agent_capability_windows.go | 12 + 6 files changed, 301 insertions(+), 29 deletions(-) diff --git a/agent/app/agent_capability.go b/agent/app/agent_capability.go index 158aecce5f4..83c460ef5d0 100644 --- a/agent/app/agent_capability.go +++ b/agent/app/agent_capability.go @@ -119,7 +119,6 @@ func (agent *ecsAgent) capabilities() ([]*ecs.Attribute, error) { capabilities = agent.appendTaskENICapabilities(capabilities) capabilities = agent.appendENITrunkingCapabilities(capabilities) - capabilities = agent.appendDockerDependentCapabilities(capabilities, supportedVersions) // TODO: gate this on docker api version when ecs supported docker includes @@ -140,10 +139,6 @@ func (agent *ecsAgent) capabilities() ([]*ecs.Attribute, error) { // ecs agent version 1.27.0 supports ecs secrets for logging drivers capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+capabilitySecretLogDriverSSM) - // ecs agent version 1.22.0 supports sharing PID namespaces and IPC resource namespaces - // with host EC2 instance and among containers within the task - capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+capabiltyPIDAndIPCNamespaceSharing) - if agent.cfg.GPUSupportEnabled { capabilities = agent.appendNvidiaDriverVersionAttribute(capabilities) } @@ -156,14 +151,18 @@ func (agent *ecsAgent) capabilities() ([]*ecs.Attribute, error) { // ecs agent version 1.27.0 supports ecs secrets for logging drivers capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+capabilitySecretLogDriverASM) + // support container ordering in agent + capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+capabilityContainerOrdering) + + // ecs agent version 1.22.0 supports sharing PID namespaces and IPC resource namespaces + // with host EC2 instance and among containers within the task + capabilities = agent.appendPIDAndIPCNamespaceSharingCapabilities(capabilities) + // ecs agent version 1.26.0 supports aws-appmesh cni plugin - capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+appMeshAttributeSuffix) + capabilities = agent.appendAppMeshCapabilities(capabilities) // support elastic inference in agent - capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+taskEIAAttributeSuffix) - - // support container ordering in agent - capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+capabilityContainerOrdering) + capabilities = agent.appendTaskEIACapabilities(capabilities) return capabilities, nil } diff --git a/agent/app/agent_capability_test.go b/agent/app/agent_capability_test.go index 1644882dda3..8ca0de52345 100644 --- a/agent/app/agent_capability_test.go +++ b/agent/app/agent_capability_test.go @@ -117,9 +117,6 @@ func TestCapabilities(t *testing.T) { { Name: aws.String(attributePrefix + capabilitySecretLogDriverSSM), }, - { - Name: aws.String(attributePrefix + capabiltyPIDAndIPCNamespaceSharing), - }, { Name: aws.String(attributePrefix + capabilityECREndpoint), }, @@ -129,12 +126,6 @@ func TestCapabilities(t *testing.T) { { Name: aws.String(attributePrefix + capabilitySecretLogDriverASM), }, - { - Name: aws.String(attributePrefix + appMeshAttributeSuffix), - }, - { - Name: aws.String(attributePrefix + taskEIAAttributeSuffix), - }, { Name: aws.String(attributePrefix + capabilityContainerOrdering), }, diff --git a/agent/app/agent_capability_unix.go b/agent/app/agent_capability_unix.go index 60f81181e17..24a18e946a3 100644 --- a/agent/app/agent_capability_unix.go +++ b/agent/app/agent_capability_unix.go @@ -102,3 +102,15 @@ func (agent *ecsAgent) appendBranchENIPluginVersionAttribute(capabilities []*ecs Value: aws.String(version), }) } + +func (agent *ecsAgent) appendPIDAndIPCNamespaceSharingCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + return appendNameOnlyAttribute(capabilities, attributePrefix+capabiltyPIDAndIPCNamespaceSharing) +} + +func (agent *ecsAgent) appendAppMeshCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + return appendNameOnlyAttribute(capabilities, attributePrefix+appMeshAttributeSuffix) +} + +func (agent *ecsAgent) appendTaskEIACapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + return appendNameOnlyAttribute(capabilities, attributePrefix+taskEIAAttributeSuffix) +} diff --git a/agent/app/agent_capability_unix_test.go b/agent/app/agent_capability_unix_test.go index dac22126301..810ae74a0e6 100644 --- a/agent/app/agent_capability_unix_test.go +++ b/agent/app/agent_capability_unix_test.go @@ -188,9 +188,6 @@ func TestNvidiaDriverCapabilitiesUnix(t *testing.T) { { Name: aws.String(attributePrefix + capabilitySecretLogDriverSSM), }, - { - Name: aws.String(attributePrefix + capabiltyPIDAndIPCNamespaceSharing), - }, // nvidia driver version capability { Name: aws.String(attributePrefix + "nvidia-driver-version.396.44"), @@ -270,9 +267,6 @@ func TestEmptyNvidiaDriverCapabilitiesUnix(t *testing.T) { { Name: aws.String(attributePrefix + capabilitySecretLogDriverSSM), }, - { - Name: aws.String(attributePrefix + capabiltyPIDAndIPCNamespaceSharing), - }, }...) ctx, cancel := context.WithCancel(context.TODO()) @@ -363,9 +357,6 @@ func TestENITrunkingCapabilitiesUnix(t *testing.T) { { Name: aws.String(attributePrefix + capabilitySecretLogDriverSSM), }, - { - Name: aws.String(attributePrefix + capabiltyPIDAndIPCNamespaceSharing), - }, }...) ctx, cancel := context.WithCancel(context.TODO()) @@ -444,11 +435,178 @@ func TestNoENITrunkingCapabilitiesUnix(t *testing.T) { { Name: aws.String(attributePrefix + capabilitySecretLogDriverSSM), }, + }...) + + ctx, cancel := context.WithCancel(context.TODO()) + // Cancel the context to cancel async routines + defer cancel() + agent := &ecsAgent{ + ctx: ctx, + cfg: conf, + dockerClient: client, + cniClient: cniClient, + credentialProvider: aws_credentials.NewCredentials(mockCredentialsProvider), + mobyPlugins: mockMobyPlugins, + } + capabilities, err := agent.capabilities() + assert.NoError(t, err) + + for i, expected := range expectedCapabilities { + assert.Equal(t, aws.StringValue(expected.Name), aws.StringValue(capabilities[i].Name)) + assert.Equal(t, aws.StringValue(expected.Value), aws.StringValue(capabilities[i].Value)) + } +} + +func TestPIDAndIPCNamespaceSharingCapabilitiesUnix(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + client := mock_dockerapi.NewMockDockerClient(ctrl) + mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) + mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) + conf := &config.Config{ + PrivilegedDisabled: true, + GPUSupportEnabled: true, + } + + gomock.InOrder( + client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ + dockerclient.Version_1_17, + }), + client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ + dockerclient.Version_1_17, + }), + mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), + client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), + gomock.Any()).AnyTimes().Return([]string{}, nil), + ) + + expectedCapabilityNames := []string{ + "com.amazonaws.ecs.capability.docker-remote-api.1.17", + } + + var expectedCapabilities []*ecs.Attribute + for _, name := range expectedCapabilityNames { + expectedCapabilities = append(expectedCapabilities, + &ecs.Attribute{Name: aws.String(name)}) + } + expectedCapabilities = append(expectedCapabilities, + []*ecs.Attribute{ + // linux specific capabilities + { + Name: aws.String("ecs.capability.docker-plugin.local"), + }, + { + Name: aws.String(attributePrefix + capabilityPrivateRegistryAuthASM), + }, + { + Name: aws.String(attributePrefix + capabilitySecretEnvSSM), + }, + { + Name: aws.String(attributePrefix + capabilitySecretLogDriverSSM), + }, + { + Name: aws.String(attributePrefix + capabilityECREndpoint), + }, + { + Name: aws.String(attributePrefix + capabilitySecretEnvASM), + }, + { + Name: aws.String(attributePrefix + capabilitySecretLogDriverASM), + }, + { + Name: aws.String(attributePrefix + capabilityContainerOrdering), + }, { Name: aws.String(attributePrefix + capabiltyPIDAndIPCNamespaceSharing), }, }...) + ctx, cancel := context.WithCancel(context.TODO()) + // Cancel the context to cancel async routines + defer cancel() + agent := &ecsAgent{ + ctx: ctx, + cfg: conf, + dockerClient: client, + credentialProvider: aws_credentials.NewCredentials(mockCredentialsProvider), + mobyPlugins: mockMobyPlugins, + } + capabilities, err := agent.capabilities() + assert.NoError(t, err) + + for i, expected := range expectedCapabilities { + assert.Equal(t, aws.StringValue(expected.Name), aws.StringValue(capabilities[i].Name)) + assert.Equal(t, aws.StringValue(expected.Value), aws.StringValue(capabilities[i].Value)) + } +} + +func TestAppMeshCapabilitiesUnix(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + client := mock_dockerapi.NewMockDockerClient(ctrl) + mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) + mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) + conf := &config.Config{ + PrivilegedDisabled: true, + GPUSupportEnabled: true, + } + + gomock.InOrder( + client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ + dockerclient.Version_1_17, + }), + client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ + dockerclient.Version_1_17, + }), + mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), + client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), + gomock.Any()).AnyTimes().Return([]string{}, nil), + ) + + expectedCapabilityNames := []string{ + "com.amazonaws.ecs.capability.docker-remote-api.1.17", + } + + var expectedCapabilities []*ecs.Attribute + for _, name := range expectedCapabilityNames { + expectedCapabilities = append(expectedCapabilities, + &ecs.Attribute{Name: aws.String(name)}) + } + expectedCapabilities = append(expectedCapabilities, + []*ecs.Attribute{ + // linux specific capabilities + { + Name: aws.String("ecs.capability.docker-plugin.local"), + }, + { + Name: aws.String(attributePrefix + capabilityPrivateRegistryAuthASM), + }, + { + Name: aws.String(attributePrefix + capabilitySecretEnvSSM), + }, + { + Name: aws.String(attributePrefix + capabilitySecretLogDriverSSM), + }, + { + Name: aws.String(attributePrefix + capabilityECREndpoint), + }, + { + Name: aws.String(attributePrefix + capabilitySecretEnvASM), + }, + { + Name: aws.String(attributePrefix + capabilitySecretLogDriverASM), + }, + { + Name: aws.String(attributePrefix + capabilityContainerOrdering), + }, + { + Name: aws.String(attributePrefix + capabiltyPIDAndIPCNamespaceSharing), + }, + { + Name: aws.String(attributePrefix + appMeshAttributeSuffix), + }, + }...) ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines defer cancel() @@ -456,7 +614,6 @@ func TestNoENITrunkingCapabilitiesUnix(t *testing.T) { ctx: ctx, cfg: conf, dockerClient: client, - cniClient: cniClient, credentialProvider: aws_credentials.NewCredentials(mockCredentialsProvider), mobyPlugins: mockMobyPlugins, } @@ -468,3 +625,92 @@ func TestNoENITrunkingCapabilitiesUnix(t *testing.T) { assert.Equal(t, aws.StringValue(expected.Value), aws.StringValue(capabilities[i].Value)) } } + +func TestTaskEIACapabilitiesUnix(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + client := mock_dockerapi.NewMockDockerClient(ctrl) + mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) + mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) + conf := &config.Config{ + PrivilegedDisabled: true, + GPUSupportEnabled: true, + } + + gomock.InOrder( + client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ + dockerclient.Version_1_17, + }), + client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ + dockerclient.Version_1_17, + }), + mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), + client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), + gomock.Any()).AnyTimes().Return([]string{}, nil), + ) + + expectedCapabilityNames := []string{ + "com.amazonaws.ecs.capability.docker-remote-api.1.17", + } + + var expectedCapabilities []*ecs.Attribute + for _, name := range expectedCapabilityNames { + expectedCapabilities = append(expectedCapabilities, + &ecs.Attribute{Name: aws.String(name)}) + } + expectedCapabilities = append(expectedCapabilities, + []*ecs.Attribute{ + // linux specific capabilities + { + Name: aws.String("ecs.capability.docker-plugin.local"), + }, + { + Name: aws.String(attributePrefix + capabilityPrivateRegistryAuthASM), + }, + { + Name: aws.String(attributePrefix + capabilitySecretEnvSSM), + }, + { + Name: aws.String(attributePrefix + capabilitySecretLogDriverSSM), + }, + { + Name: aws.String(attributePrefix + capabilityECREndpoint), + }, + { + Name: aws.String(attributePrefix + capabilitySecretEnvASM), + }, + { + Name: aws.String(attributePrefix + capabilitySecretLogDriverASM), + }, + { + Name: aws.String(attributePrefix + capabilityContainerOrdering), + }, + { + Name: aws.String(attributePrefix + capabiltyPIDAndIPCNamespaceSharing), + }, + { + Name: aws.String(attributePrefix + appMeshAttributeSuffix), + }, + { + Name: aws.String(attributePrefix + taskEIAAttributeSuffix), + }, + }...) + ctx, cancel := context.WithCancel(context.TODO()) + // Cancel the context to cancel async routines + defer cancel() + agent := &ecsAgent{ + ctx: ctx, + cfg: conf, + dockerClient: client, + credentialProvider: aws_credentials.NewCredentials(mockCredentialsProvider), + mobyPlugins: mockMobyPlugins, + } + capabilities, err := agent.capabilities() + assert.NoError(t, err) + + for i, expected := range expectedCapabilities { + assert.Equal(t, aws.StringValue(expected.Name), aws.StringValue(capabilities[i].Name)) + assert.Equal(t, aws.StringValue(expected.Value), aws.StringValue(capabilities[i].Value)) + } +} \ No newline at end of file diff --git a/agent/app/agent_capability_unspecified.go b/agent/app/agent_capability_unspecified.go index 8247d25ef73..95c202c05ea 100644 --- a/agent/app/agent_capability_unspecified.go +++ b/agent/app/agent_capability_unspecified.go @@ -73,3 +73,15 @@ func (agent *ecsAgent) appendNvidiaDriverVersionAttribute(capabilities []*ecs.At func (agent *ecsAgent) appendENITrunkingCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { return capabilities } + +func (agent *ecsAgent) appendPIDAndIPCNamespaceSharingCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + return capabilities +} + +func (agent *ecsAgent) appendAppMeshCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + return capabilities +} + +func (agent *ecsAgent) appendTaskEIACapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + return capabilities +} diff --git a/agent/app/agent_capability_windows.go b/agent/app/agent_capability_windows.go index 8b608270646..e43b4fa55fd 100644 --- a/agent/app/agent_capability_windows.go +++ b/agent/app/agent_capability_windows.go @@ -32,3 +32,15 @@ func (agent *ecsAgent) appendNvidiaDriverVersionAttribute(capabilities []*ecs.At func (agent *ecsAgent) appendENITrunkingCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { return capabilities } + +func (agent *ecsAgent) appendPIDAndIPCNamespaceSharingCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + return capabilities +} + +func (agent *ecsAgent) appendAppMeshCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + return capabilities +} + +func (agent *ecsAgent) appendTaskEIACapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + return capabilities +}