From 132d6a576484dd5461b978782a02ce87eec86125 Mon Sep 17 00:00:00 2001 From: Eric Sun Date: Tue, 3 Dec 2019 16:12:31 -0800 Subject: [PATCH] Load pause container image by default on linux. This commits addresses the issue on [#2290](https://github.com/aws/amazon-ecs-agent/issues/2290). Load pause container image by default. Enforce pid/ipc capabilities append to check for existence of pause container image --- agent/app/agent.go | 5 ++ agent/app/agent_capability_unix.go | 10 +++ agent/app/agent_capability_unix_test.go | 96 +++++++++++++++++++++++++ agent/app/agent_unix.go | 17 ++++- agent/app/agent_unix_test.go | 5 +- agent/app/agent_unspecified.go | 4 ++ agent/app/agent_windows.go | 4 ++ agent/eni/pause/load.go | 1 + agent/eni/pause/mocks/load_mocks.go | 28 ++++++-- agent/eni/pause/pause_linux.go | 16 +++++ agent/eni/pause/pause_linux_test.go | 72 ++++++++++++++++++- agent/eni/pause/pause_unsupported.go | 6 ++ 12 files changed, 251 insertions(+), 13 deletions(-) diff --git a/agent/app/agent.go b/agent/app/agent.go index eeea01ec852..e0797da16be 100644 --- a/agent/app/agent.go +++ b/agent/app/agent.go @@ -260,6 +260,11 @@ func (agent *ecsAgent) doStart(containerChangeEventStream *eventstream.EventStre return exitcodes.ExitTerminal } + err = agent.loadPauseContainer() + if err != nil { + seelog.Error("Failed to load pause container: %v", err) + } + var vpcSubnetAttributes []*ecs.Attribute // Check if Task ENI is enabled if agent.cfg.TaskENIEnabled { diff --git a/agent/app/agent_capability_unix.go b/agent/app/agent_capability_unix.go index b86f7e08793..144dcebe293 100644 --- a/agent/app/agent_capability_unix.go +++ b/agent/app/agent_capability_unix.go @@ -111,6 +111,16 @@ func (agent *ecsAgent) appendBranchENIPluginVersionAttribute(capabilities []*ecs } func (agent *ecsAgent) appendPIDAndIPCNamespaceSharingCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { + if agent.pauseLoader == nil { + seelog.Error("Pause Loader is not initialized") + return capabilities + } + + isLoaded, err := agent.pauseLoader.IsLoaded(agent.dockerClient) + if !isLoaded || err != nil { + seelog.Warnf("Pause container is not loaded, did not append PID and IPC capabilities: %v", err) + return capabilities + } return appendNameOnlyAttribute(capabilities, attributePrefix+capabiltyPIDAndIPCNamespaceSharing) } diff --git a/agent/app/agent_capability_unix_test.go b/agent/app/agent_capability_unix_test.go index af9429b815c..ca93993883c 100644 --- a/agent/app/agent_capability_unix_test.go +++ b/agent/app/agent_capability_unix_test.go @@ -17,6 +17,8 @@ package app import ( "context" + "errors" + mock_pause "github.com/aws/amazon-ecs-agent/agent/eni/pause/mocks" "os" "path/filepath" "testing" @@ -466,10 +468,12 @@ func TestPIDAndIPCNamespaceSharingCapabilitiesUnix(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) + mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ PrivilegedDisabled: true, } + mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil) gomock.InOrder( client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, @@ -532,6 +536,92 @@ func TestPIDAndIPCNamespaceSharingCapabilitiesUnix(t *testing.T) { ctx: ctx, cfg: conf, dockerClient: client, + pauseLoader: mockPauseLoader, + 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 TestPIDAndIPCNamespaceSharingCapabilitiesNoPauseContainer(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) + mockPauseLoader := mock_pause.NewMockLoader(ctrl) + conf := &config.Config{ + PrivilegedDisabled: true, + } + + mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(false, errors.New("mock error")) + 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 + capabilityFullTaskSync), + }, + }...) + ctx, cancel := context.WithCancel(context.TODO()) + // Cancel the context to cancel async routines + defer cancel() + agent := &ecsAgent{ + ctx: ctx, + cfg: conf, + dockerClient: client, + pauseLoader: mockPauseLoader, credentialProvider: aws_credentials.NewCredentials(mockCredentialsProvider), mobyPlugins: mockMobyPlugins, } @@ -551,10 +641,12 @@ func TestAppMeshCapabilitiesUnix(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) + mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ PrivilegedDisabled: true, } + mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil) gomock.InOrder( client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, @@ -620,6 +712,7 @@ func TestAppMeshCapabilitiesUnix(t *testing.T) { ctx: ctx, cfg: conf, dockerClient: client, + pauseLoader: mockPauseLoader, credentialProvider: aws_credentials.NewCredentials(mockCredentialsProvider), mobyPlugins: mockMobyPlugins, } @@ -730,10 +823,12 @@ func TestAWSLoggingDriverAndLogRouterCapabilitiesUnix(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) + mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ PrivilegedDisabled: true, } + mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil) gomock.InOrder( client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, @@ -808,6 +903,7 @@ func TestAWSLoggingDriverAndLogRouterCapabilitiesUnix(t *testing.T) { ctx: ctx, cfg: conf, dockerClient: client, + pauseLoader: mockPauseLoader, credentialProvider: aws_credentials.NewCredentials(mockCredentialsProvider), mobyPlugins: mockMobyPlugins, } diff --git a/agent/app/agent_unix.go b/agent/app/agent_unix.go index 1201fe3d425..b7f7d131edd 100644 --- a/agent/app/agent_unix.go +++ b/agent/app/agent_unix.go @@ -17,6 +17,7 @@ package app import ( "fmt" + "github.com/aws/amazon-ecs-agent/agent/eni/pause" asmfactory "github.com/aws/amazon-ecs-agent/agent/asm/factory" "github.com/aws/amazon-ecs-agent/agent/config" @@ -25,7 +26,6 @@ import ( "github.com/aws/amazon-ecs-agent/agent/ecscni" "github.com/aws/amazon-ecs-agent/agent/engine" "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate" - "github.com/aws/amazon-ecs-agent/agent/eni/pause" "github.com/aws/amazon-ecs-agent/agent/eni/udevwrapper" "github.com/aws/amazon-ecs-agent/agent/eni/watcher" "github.com/aws/amazon-ecs-agent/agent/gpu" @@ -86,8 +86,7 @@ func (agent *ecsAgent) initializeTaskENIDependencies(state dockerstate.TaskEngin return err, true } - // Load the pause container's image from the 'disk' - if _, err := agent.pauseLoader.LoadImage(agent.ctx, agent.cfg, agent.dockerClient); err != nil { + if isLoaded, err := agent.pauseLoader.IsLoaded(agent.dockerClient); err != nil || !isLoaded { if pause.IsNoSuchFileError(err) || pause.UnsupportedPlatform(err) { // If the pause container's image tarball doesn't exist or if the // invocation is done for an unsupported platform, we cannot recover. @@ -251,3 +250,15 @@ func (agent *ecsAgent) getPlatformDevices() []*ecs.PlatformDevice { } return nil } + +func (agent *ecsAgent) loadPauseContainer() error { + + if agent.pauseLoader == nil { + return errors.New("Pause Loader is not initialized") + } + + // Load the pause container's image from the 'disk' + _, err := agent.pauseLoader.LoadImage(agent.ctx, agent.cfg, agent.dockerClient) + + return err +} diff --git a/agent/app/agent_unix_test.go b/agent/app/agent_unix_test.go index 089fa43437f..11134724cde 100644 --- a/agent/app/agent_unix_test.go +++ b/agent/app/agent_unix_test.go @@ -173,6 +173,7 @@ func TestDoStartTaskENIHappyPath(t *testing.T) { mockMetadata.EXPECT().OutpostARN().Return("", nil) gomock.InOrder( + mockPauseLoader.EXPECT().LoadImage(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil), mockOS.EXPECT().Getpid().Return(10), mockMetadata.EXPECT().PrimaryENIMAC().Return(mac, nil), mockMetadata.EXPECT().VPCID(mac).Return(vpcID, nil), @@ -182,7 +183,7 @@ func TestDoStartTaskENIHappyPath(t *testing.T) { cniClient.EXPECT().Capabilities(ecscni.ECSIPAMPluginName).Return(cniCapabilities, nil), cniClient.EXPECT().Capabilities(ecscni.ECSAppMeshPluginName).Return(cniCapabilities, nil), cniClient.EXPECT().Capabilities(ecscni.ECSBranchENIPluginName).Return(cniCapabilities, nil), - mockPauseLoader.EXPECT().LoadImage(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil), + mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil), state.EXPECT().ENIByMac(gomock.Any()).Return(nil, false).AnyTimes(), mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil), dockerClient.EXPECT().SupportedVersions().Return(nil), @@ -495,7 +496,7 @@ func TestInitializeTaskENIDependenciesPauseLoaderError(t *testing.T) { cniClient.EXPECT().Capabilities(ecscni.ECSBridgePluginName).Return(cniCapabilities, nil), cniClient.EXPECT().Capabilities(ecscni.ECSIPAMPluginName).Return(cniCapabilities, nil), cniClient.EXPECT().Capabilities(ecscni.ECSAppMeshPluginName).Return(cniCapabilities, nil), - mockPauseLoader.EXPECT().LoadImage(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, loadErr), + mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(false, loadErr), ) cfg := getTestConfig() agent := &ecsAgent{ diff --git a/agent/app/agent_unspecified.go b/agent/app/agent_unspecified.go index 155d38d8a71..48350f3164b 100644 --- a/agent/app/agent_unspecified.go +++ b/agent/app/agent_unspecified.go @@ -49,3 +49,7 @@ func (agent *ecsAgent) initializeGPUManager() error { func (agent *ecsAgent) getPlatformDevices() []*ecs.PlatformDevice { return nil } + +func (agent *ecsAgent) loadPauseContainer() error { + return nil +} diff --git a/agent/app/agent_windows.go b/agent/app/agent_windows.go index 1b74ae27c07..28ef524baf4 100644 --- a/agent/app/agent_windows.go +++ b/agent/app/agent_windows.go @@ -277,3 +277,7 @@ func (agent *ecsAgent) initializeGPUManager() error { func (agent *ecsAgent) getPlatformDevices() []*ecs.PlatformDevice { return nil } + +func (agent *ecsAgent) loadPauseContainer() error { + return nil +} \ No newline at end of file diff --git a/agent/eni/pause/load.go b/agent/eni/pause/load.go index 87921e315f8..1a1174c2dd3 100644 --- a/agent/eni/pause/load.go +++ b/agent/eni/pause/load.go @@ -25,6 +25,7 @@ import ( // to facilitate mocking and testing of the LoadImage method type Loader interface { LoadImage(ctx context.Context, cfg *config.Config, dockerClient dockerapi.DockerClient) (*types.ImageInspect, error) + IsLoaded(dockerClient dockerapi.DockerClient) (bool, error) } type loader struct{} diff --git a/agent/eni/pause/mocks/load_mocks.go b/agent/eni/pause/mocks/load_mocks.go index d7fe182682f..41e961631dc 100644 --- a/agent/eni/pause/mocks/load_mocks.go +++ b/agent/eni/pause/mocks/load_mocks.go @@ -13,19 +13,18 @@ // // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/aws/amazon-ecs-agent/agent/eni/pause (interfaces: Loader) +// Source: load.go // Package mock_pause is a generated GoMock package. package mock_pause import ( context "context" - reflect "reflect" - config "github.com/aws/amazon-ecs-agent/agent/config" dockerapi "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi" types "github.com/docker/docker/api/types" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockLoader is a mock of Loader interface @@ -52,16 +51,31 @@ func (m *MockLoader) EXPECT() *MockLoaderMockRecorder { } // LoadImage mocks base method -func (m *MockLoader) LoadImage(arg0 context.Context, arg1 *config.Config, arg2 dockerapi.DockerClient) (*types.ImageInspect, error) { +func (m *MockLoader) LoadImage(ctx context.Context, cfg *config.Config, dockerClient dockerapi.DockerClient) (*types.ImageInspect, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "LoadImage", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "LoadImage", ctx, cfg, dockerClient) ret0, _ := ret[0].(*types.ImageInspect) ret1, _ := ret[1].(error) return ret0, ret1 } // LoadImage indicates an expected call of LoadImage -func (mr *MockLoaderMockRecorder) LoadImage(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockLoaderMockRecorder) LoadImage(ctx, cfg, dockerClient interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadImage", reflect.TypeOf((*MockLoader)(nil).LoadImage), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadImage", reflect.TypeOf((*MockLoader)(nil).LoadImage), ctx, cfg, dockerClient) +} + +// IsLoaded mocks base method +func (m *MockLoader) IsLoaded(dockerClient dockerapi.DockerClient) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsLoaded", dockerClient) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 } + +// IsLoaded indicates an expected call of IsLoaded +func (mr *MockLoaderMockRecorder) IsLoaded(dockerClient interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsLoaded", reflect.TypeOf((*MockLoader)(nil).IsLoaded), dockerClient) +} \ No newline at end of file diff --git a/agent/eni/pause/pause_linux.go b/agent/eni/pause/pause_linux.go index bca153d0112..0710a2551f4 100644 --- a/agent/eni/pause/pause_linux.go +++ b/agent/eni/pause/pause_linux.go @@ -40,6 +40,22 @@ func (*loader) LoadImage(ctx context.Context, cfg *config.Config, dockerClient d config.DefaultPauseContainerImageName, config.DefaultPauseContainerTag, dockerClient) } +func (*loader) IsLoaded(dockerClient dockerapi.DockerClient) (bool, error) { + image, err := getPauseContainerImage( + config.DefaultPauseContainerImageName, config.DefaultPauseContainerTag, dockerClient) + + if err != nil { + return false, errors.Wrapf(err, + "pause container inspect: failed to inspect image: %s", config.DefaultPauseContainerImageName) + } + + if image == nil || image.ID == "" { + return false, nil + } + + return true, nil +} + func loadFromFile(ctx context.Context, path string, dockerClient dockerapi.DockerClient, fs os.FileSystem) error { pauseContainerReader, err := fs.Open(path) if err != nil { diff --git a/agent/eni/pause/pause_linux_test.go b/agent/eni/pause/pause_linux_test.go index a72851ef37a..ffc4dc77645 100644 --- a/agent/eni/pause/pause_linux_test.go +++ b/agent/eni/pause/pause_linux_test.go @@ -1,4 +1,4 @@ -// +build linux,unit +//+build linux,unit // Copyright 2017-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. // @@ -154,3 +154,73 @@ func TestGetPauseContainerHappyPath(t *testing.T) { _, err = getPauseContainerImage(pauseName, pauseTag, client) assert.NoError(t, err) } + +func TestIsLoadedHappyPath(t *testing.T) { + ctrl := gomock.NewController(t) + pauseLoader := New() + defer ctrl.Finish() + + // Docker SDK tests + mockDockerSDK := mock_sdkclient.NewMockClient(ctrl) + mockDockerSDK.EXPECT().Ping(gomock.Any()).Return(types.Ping{}, nil) + sdkFactory := mock_sdkclientfactory.NewMockFactory(ctrl) + sdkFactory.EXPECT().GetDefaultClient().AnyTimes().Return(mockDockerSDK, nil) + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + client, err := dockerapi.NewDockerGoClient(sdkFactory, &defaultConfig, ctx) + assert.NoError(t, err) + mockDockerSDK.EXPECT().ImageInspectWithRaw(gomock.Any(), gomock.Any()).Return(types.ImageInspect{ID: "test123"}, nil, nil) + + isLoaded, err := pauseLoader.IsLoaded(client) + assert.NoError(t, err) + assert.True(t, isLoaded) +} + +func TestIsLoadedNotLoaded(t *testing.T) { + ctrl := gomock.NewController(t) + pauseLoader := New() + defer ctrl.Finish() + + // Docker SDK tests + mockDockerSDK := mock_sdkclient.NewMockClient(ctrl) + mockDockerSDK.EXPECT().Ping(gomock.Any()).Return(types.Ping{}, nil) + sdkFactory := mock_sdkclientfactory.NewMockFactory(ctrl) + sdkFactory.EXPECT().GetDefaultClient().AnyTimes().Return(mockDockerSDK, nil) + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + client, err := dockerapi.NewDockerGoClient(sdkFactory, &defaultConfig, ctx) + assert.NoError(t, err) + mockDockerSDK.EXPECT().ImageInspectWithRaw(gomock.Any(), gomock.Any()).Return(types.ImageInspect{}, nil, nil) + + isLoaded, err := pauseLoader.IsLoaded(client) + assert.NoError(t, err) + assert.False(t, isLoaded) +} + +func TestIsLoadedError(t *testing.T) { + ctrl := gomock.NewController(t) + pauseLoader := New() + defer ctrl.Finish() + + // Docker SDK tests + mockDockerSDK := mock_sdkclient.NewMockClient(ctrl) + mockDockerSDK.EXPECT().Ping(gomock.Any()).Return(types.Ping{}, nil) + sdkFactory := mock_sdkclientfactory.NewMockFactory(ctrl) + sdkFactory.EXPECT().GetDefaultClient().AnyTimes().Return(mockDockerSDK, nil) + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + client, err := dockerapi.NewDockerGoClient(sdkFactory, &defaultConfig, ctx) + assert.NoError(t, err) + mockDockerSDK.EXPECT().ImageInspectWithRaw(gomock.Any(), gomock.Any()).Return( + types.ImageInspect{}, nil, errors.New("error")) + + isLoaded, err := pauseLoader.IsLoaded(client) + assert.Error(t, err) + assert.False(t, isLoaded) +} diff --git a/agent/eni/pause/pause_unsupported.go b/agent/eni/pause/pause_unsupported.go index 0c96eedee6b..3125ca13795 100644 --- a/agent/eni/pause/pause_unsupported.go +++ b/agent/eni/pause/pause_unsupported.go @@ -31,3 +31,9 @@ func (*loader) LoadImage(ctx context.Context, cfg *config.Config, dockerClient d "pause container load: unsupported platform: %s/%s", runtime.GOOS, runtime.GOARCH)) } + +func (*loader) IsLoaded(dockerClient dockerapi.DockerClient) (bool, error) { + return false, NewUnsupportedPlatformError(errors.Errorf( + "pause container isloaded: unsupported platform: %s/%s", + runtime.GOOS, runtime.GOARCH)) +}