Skip to content

Commit

Permalink
Load pause container image by default on linux.
Browse files Browse the repository at this point in the history
This commits addresses the issue on [aws#2290](aws#2290). Load pause container image by default. Enforce pid/ipc capabilities append to check for existence of pause container image
  • Loading branch information
suneyz committed Dec 4, 2019
1 parent 9e40df4 commit 132d6a5
Show file tree
Hide file tree
Showing 12 changed files with 251 additions and 13 deletions.
5 changes: 5 additions & 0 deletions agent/app/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions agent/app/agent_capability_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
96 changes: 96 additions & 0 deletions agent/app/agent_capability_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
Expand All @@ -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,
Expand Down Expand Up @@ -620,6 +712,7 @@ func TestAppMeshCapabilitiesUnix(t *testing.T) {
ctx: ctx,
cfg: conf,
dockerClient: client,
pauseLoader: mockPauseLoader,
credentialProvider: aws_credentials.NewCredentials(mockCredentialsProvider),
mobyPlugins: mockMobyPlugins,
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -808,6 +903,7 @@ func TestAWSLoggingDriverAndLogRouterCapabilitiesUnix(t *testing.T) {
ctx: ctx,
cfg: conf,
dockerClient: client,
pauseLoader: mockPauseLoader,
credentialProvider: aws_credentials.NewCredentials(mockCredentialsProvider),
mobyPlugins: mockMobyPlugins,
}
Expand Down
17 changes: 14 additions & 3 deletions agent/app/agent_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
5 changes: 3 additions & 2 deletions agent/app/agent_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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{
Expand Down
4 changes: 4 additions & 0 deletions agent/app/agent_unspecified.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,7 @@ func (agent *ecsAgent) initializeGPUManager() error {
func (agent *ecsAgent) getPlatformDevices() []*ecs.PlatformDevice {
return nil
}

func (agent *ecsAgent) loadPauseContainer() error {
return nil
}
4 changes: 4 additions & 0 deletions agent/app/agent_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,7 @@ func (agent *ecsAgent) initializeGPUManager() error {
func (agent *ecsAgent) getPlatformDevices() []*ecs.PlatformDevice {
return nil
}

func (agent *ecsAgent) loadPauseContainer() error {
return nil
}
1 change: 1 addition & 0 deletions agent/eni/pause/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
28 changes: 21 additions & 7 deletions agent/eni/pause/mocks/load_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions agent/eni/pause/pause_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 132d6a5

Please sign in to comment.