Adding ecs config to flag name only or driver options and label compa…

…rison for shared volumees

README.md

@@ -183,6 +183,7 @@ additional details on each available environment variable.
 | `ECS_CGROUP_PATH` | `/sys/fs/cgroup` | The root cgroup path that is expected by the ECS agent. This is the path that accessible from the agent mount. | `/sys/fs/cgroup` | Not applicable |
 | `ECS_ENABLE_CPU_UNBOUNDED_WINDOWS_WORKAROUND` | `true` | When `true`, ECS will allow CPU unbounded(CPU=`0`) tasks to run along with CPU bounded tasks in Windows. | Not applicable | `false` |
 | `ECS_TASK_METADATA_RPS_LIMIT` | `100,150` | Comma separated integer values for steady state and burst throttle limits for task metadata endpoint | `40,60` | `40,60` |
+| `ECS_SHARED_VOLUME_VALIDATE_NAME_ONLY` | `true` | When `true`, ECS will short circuit shared volume match after name. When `false`, both driver options and labels are compared. If a volume is shared across instances, this should be set to `true`. | `false` | `false`|
 
 ### Persistence
 

agent/api/task/task.go

@@ -226,7 +226,7 @@ func (task *Task) PostUnmarshalTask(cfg *config.Config,
 	if err != nil {
 		return apierrors.NewResourceInitError(task.Arn, err)
 	}
-	err = task.initializeDockerVolumes(dockerClient, ctx)
+	err = task.initializeDockerVolumes(cfg, dockerClient, ctx)
 	if err != nil {
 		return apierrors.NewResourceInitError(task.Arn, err)
 	}
@@ -285,7 +285,7 @@ func (task *Task) volumeName(name string) string {
 
 // initializeDockerVolumes checks the volume resource in the task to determine if the agent
 // should create the volume before creating the container
-func (task *Task) initializeDockerVolumes(dockerClient dockerapi.DockerClient, ctx context.Context) error {
+func (task *Task) initializeDockerVolumes(cfg *config.Config, dockerClient dockerapi.DockerClient, ctx context.Context) error {
 	for i, vol := range task.Volumes {
 		// No need to do this for non-docker volume, eg: host bind/empty volume
 		if vol.Type != DockerVolumeType {
@@ -304,7 +304,7 @@ func (task *Task) initializeDockerVolumes(dockerClient dockerapi.DockerClient, c
 			}
 		} else {
 			// Agent needs to create shared volume if that's auto provisioned
-			err := task.addSharedVolumes(ctx, dockerClient, &task.Volumes[i])
+			err := task.addSharedVolumes(cfg, ctx, dockerClient, &task.Volumes[i])
 			if err != nil {
 				return err
 			}
@@ -336,7 +336,7 @@ func (task *Task) addTaskScopedVolumes(ctx context.Context, dockerClient dockera
 }
 
 // addSharedVolumes adds shared volume into task resources and updates container dependency
-func (task *Task) addSharedVolumes(ctx context.Context, dockerClient dockerapi.DockerClient,
+func (task *Task) addSharedVolumes(cfg *config.Config, ctx context.Context, dockerClient dockerapi.DockerClient,
 	vol *TaskVolume) error {
 
 	volumeConfig := vol.Volume.(*taskresourcevolume.DockerVolumeConfig)
@@ -379,6 +379,11 @@ func (task *Task) addSharedVolumes(ctx context.Context, dockerClient dockerapi.D
 	}
 
 	seelog.Infof("initialize volume: Task [%s]: volume [%s] already exists", task.Arn, volumeConfig.DockerVolumeName)
+	if cfg.SharedVolumeValidateNameOnly {
+		seelog.Infof("initialize volume: Task [%s]: SharedVolumeValidateNameOnly is set to true and volume with name [%s] is found", task.Arn, volumeConfig.DockerVolumeName)
+		return nil
+	}
+
 	// validate all the volume metadata fields match to the configuration
 	if len(volumeMetadata.DockerVolume.Labels) == 0 && len(volumeMetadata.DockerVolume.Labels) == len(volumeConfig.Labels) {
 		seelog.Infof("labels are both empty or null: Task [%s]: volume [%s]", task.Arn, volumeConfig.DockerVolumeName)

agent/api/task/taskvolume_test.go

@@ -21,6 +21,7 @@ import (
 
 	apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container"
 	apicontainerstatus "github.com/aws/amazon-ecs-agent/agent/api/container/status"
+	"github.com/aws/amazon-ecs-agent/agent/config"
 	"github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi"
 	"github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi/mocks"
 	"github.com/aws/amazon-ecs-agent/agent/taskresource"
@@ -133,6 +134,7 @@ func TestInitializeLocalDockerVolume(t *testing.T) {
 }
 
 func TestInitializeSharedProvisionedVolume(t *testing.T) {
+	cfg := config.Config{}
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -163,14 +165,15 @@ func TestInitializeSharedProvisionedVolume(t *testing.T) {
 
 	// Expect the volume already exists on the instance
 	dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.VolumeResponse{})
-	err := testTask.initializeDockerVolumes(dockerClient, nil)
+	err := testTask.initializeDockerVolumes(&cfg, dockerClient, nil)
 
 	assert.NoError(t, err)
 	assert.Len(t, testTask.ResourcesMapUnsafe, 0, "no volume resource should be provisioned by agent")
 	assert.Len(t, testTask.Containers[0].TransitionDependenciesMap, 0, "resource already exists")
 }
 
 func TestInitializeSharedProvisionedVolumeError(t *testing.T) {
+	cfg := config.Config{}
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -201,11 +204,12 @@ func TestInitializeSharedProvisionedVolumeError(t *testing.T) {
 
 	// Expect the volume does not exists on the instance
 	dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.VolumeResponse{Error: errors.New("volume not exist")})
-	err := testTask.initializeDockerVolumes(dockerClient, nil)
+	err := testTask.initializeDockerVolumes(&cfg, dockerClient, nil)
 	assert.Error(t, err, "volume not found for auto-provisioned resource should cause task to fail")
 }
 
 func TestInitializeSharedNonProvisionedVolume(t *testing.T) {
+	cfg := config.Config{}
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -241,14 +245,63 @@ func TestInitializeSharedNonProvisionedVolume(t *testing.T) {
 			Labels: map[string]string{"test": "test"},
 		},
 	})
-	err := testTask.initializeDockerVolumes(dockerClient, nil)
+	err := testTask.initializeDockerVolumes(&cfg, dockerClient, nil)
+
+	assert.NoError(t, err)
+	assert.Len(t, testTask.ResourcesMapUnsafe, 0, "no volume resource should be provisioned by agent")
+	assert.Len(t, testTask.Containers[0].TransitionDependenciesMap, 0, "resource already exists")
+}
+
+func TestInitializeSharedNonProvisionedVolumeValidateNameOnly(t *testing.T) {
+	cfg := config.Config{}
+	cfg.SharedVolumeValidateNameOnly = true
+
+	ctrl := gomock.NewController(t)
+	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
+
+	testTask := &Task{
+		ResourcesMapUnsafe: make(map[string][]taskresource.TaskResource),
+		Containers: []*apicontainer.Container{
+			{
+				MountPoints: []apicontainer.MountPoint{
+					{
+						SourceVolume:  "shared-volume-test",
+						ContainerPath: "/ecs",
+					},
+				},
+				TransitionDependenciesMap: make(map[apicontainerstatus.ContainerStatus]apicontainer.TransitionDependencySet),
+			},
+		},
+		Volumes: []TaskVolume{
+			{
+				Name: "shared-volume-test",
+				Type: "docker",
+				Volume: &taskresourcevolume.DockerVolumeConfig{
+					Scope:         "shared",
+					Autoprovision: true,
+					DriverOpts:    map[string]string{"type": "tmpfs"},
+					Labels:        map[string]string{"domain": "test"},
+				},
+			},
+		},
+	}
+
+	// Expect the volume already exists on the instance
+	dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.VolumeResponse{
+		DockerVolume: &docker.Volume{
+			Options: map[string]string{},
+			Labels:  nil,
+		},
+	})
+	err := testTask.initializeDockerVolumes(&cfg, dockerClient, nil)
 
 	assert.NoError(t, err)
 	assert.Len(t, testTask.ResourcesMapUnsafe, 0, "no volume resource should be provisioned by agent")
 	assert.Len(t, testTask.Containers[0].TransitionDependenciesMap, 0, "resource already exists")
 }
 
 func TestInitializeSharedAutoprovisionVolumeNotFoundError(t *testing.T) {
+	cfg := config.Config{}
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -278,13 +331,14 @@ func TestInitializeSharedAutoprovisionVolumeNotFoundError(t *testing.T) {
 	}
 
 	dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.VolumeResponse{Error: errors.New("not found")})
-	err := testTask.initializeDockerVolumes(dockerClient, nil)
+	err := testTask.initializeDockerVolumes(&cfg, dockerClient, nil)
 	assert.NoError(t, err)
 	assert.Len(t, testTask.ResourcesMapUnsafe, 1, "volume resource should be provisioned by agent")
 	assert.Len(t, testTask.Containers[0].TransitionDependenciesMap, 1, "volume resource should be in the container dependency map")
 }
 
 func TestInitializeSharedAutoprovisionVolumeNotMatchError(t *testing.T) {
+	cfg := config.Config{}
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -318,11 +372,12 @@ func TestInitializeSharedAutoprovisionVolumeNotMatchError(t *testing.T) {
 			Labels: map[string]string{"test": "test"},
 		},
 	})
-	err := testTask.initializeDockerVolumes(dockerClient, nil)
+	err := testTask.initializeDockerVolumes(&cfg, dockerClient, nil)
 	assert.Error(t, err, "volume resource details not match should cause task fail")
 }
 
 func TestInitializeSharedAutoprovisionVolumeTimeout(t *testing.T) {
+	cfg := config.Config{}
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -354,7 +409,7 @@ func TestInitializeSharedAutoprovisionVolumeTimeout(t *testing.T) {
 	dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.VolumeResponse{
 		Error: &dockerapi.DockerTimeoutError{},
 	})
-	err := testTask.initializeDockerVolumes(dockerClient, nil)
+	err := testTask.initializeDockerVolumes(&cfg, dockerClient, nil)
 	assert.Error(t, err, "volume resource details not match should cause task fail")
 }
 
@@ -383,7 +438,7 @@ func TestInitializeTaskVolume(t *testing.T) {
 		},
 	}
 
-	err := testTask.initializeDockerVolumes(nil, nil)
+	err := testTask.initializeDockerVolumes(nil, nil, nil)
 	assert.NoError(t, err)
 	assert.Len(t, testTask.ResourcesMapUnsafe, 1, "expect the resource map has an empty volume resource")
 	assert.Len(t, testTask.Containers[0].TransitionDependenciesMap, 1, "expect a volume resource as the container dependency")

agent/config/config.go

@@ -104,6 +104,10 @@ const (
 
 	// DefaultTaskMetadataBurstRate is set to handle 60 burst requests at once
 	DefaultTaskMetadataBurstRate = 60
+
+	// DefaultSharedVolumeValidateNameOnly is set to true, requiring shared volumes to validate
+	// across name, driver options, and labels
+	DefaultSharedVolumeValidateNameOnly = false
 )
 
 const (
@@ -420,6 +424,7 @@ func environmentConfig() (Config, error) {
 		CgroupPath:                       os.Getenv("ECS_CGROUP_PATH"),
 		TaskMetadataSteadyStateRate:      steadyStateRate,
 		TaskMetadataBurstRate:            burstRate,
+		SharedVolumeValidateNameOnly:     utils.ParseBool(os.Getenv("ECS_SHARED_VOLUME_VALIDATE_NAME_ONLY"), false),
 	}, err
 }
 

agent/config/config_unix.go

@@ -41,33 +41,34 @@ const (
 // DefaultConfig returns the default configuration for Linux
 func DefaultConfig() Config {
 	return Config{
-		DockerEndpoint:              "unix:///var/run/docker.sock",
-		ReservedPorts:               []uint16{SSHPort, DockerReservedPort, DockerReservedSSLPort, AgentIntrospectionPort, AgentCredentialsPort},
-		ReservedPortsUDP:            []uint16{},
-		DataDir:                     "/data/",
-		DataDirOnHost:               "/var/lib/ecs",
-		DisableMetrics:              false,
-		ReservedMemory:              0,
-		AvailableLoggingDrivers:     []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver},
-		TaskCleanupWaitDuration:     DefaultTaskCleanupWaitDuration,
-		DockerStopTimeout:           defaultDockerStopTimeout,
-		ContainerStartTimeout:       defaultContainerStartTimeout,
-		CredentialsAuditLogFile:     defaultCredentialsAuditLogFile,
-		CredentialsAuditLogDisabled: false,
-		ImageCleanupDisabled:        false,
-		MinimumImageDeletionAge:     DefaultImageDeletionAge,
-		ImageCleanupInterval:        DefaultImageCleanupTimeInterval,
-		NumImagesToDeletePerCycle:   DefaultNumImagesToDeletePerCycle,
-		CNIPluginsPath:              defaultCNIPluginsPath,
-		PauseContainerTarballPath:   pauseContainerTarballPath,
-		PauseContainerImageName:     DefaultPauseContainerImageName,
-		PauseContainerTag:           DefaultPauseContainerTag,
-		AWSVPCBlockInstanceMetdata:  false,
-		ContainerMetadataEnabled:    false,
-		TaskCPUMemLimit:             DefaultEnabled,
-		CgroupPath:                  defaultCgroupPath,
-		TaskMetadataSteadyStateRate: DefaultTaskMetadataSteadyStateRate,
-		TaskMetadataBurstRate:       DefaultTaskMetadataBurstRate,
+		DockerEndpoint:               "unix:///var/run/docker.sock",
+		ReservedPorts:                []uint16{SSHPort, DockerReservedPort, DockerReservedSSLPort, AgentIntrospectionPort, AgentCredentialsPort},
+		ReservedPortsUDP:             []uint16{},
+		DataDir:                      "/data/",
+		DataDirOnHost:                "/var/lib/ecs",
+		DisableMetrics:               false,
+		ReservedMemory:               0,
+		AvailableLoggingDrivers:      []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver},
+		TaskCleanupWaitDuration:      DefaultTaskCleanupWaitDuration,
+		DockerStopTimeout:            defaultDockerStopTimeout,
+		ContainerStartTimeout:        defaultContainerStartTimeout,
+		CredentialsAuditLogFile:      defaultCredentialsAuditLogFile,
+		CredentialsAuditLogDisabled:  false,
+		ImageCleanupDisabled:         false,
+		MinimumImageDeletionAge:      DefaultImageDeletionAge,
+		ImageCleanupInterval:         DefaultImageCleanupTimeInterval,
+		NumImagesToDeletePerCycle:    DefaultNumImagesToDeletePerCycle,
+		CNIPluginsPath:               defaultCNIPluginsPath,
+		PauseContainerTarballPath:    pauseContainerTarballPath,
+		PauseContainerImageName:      DefaultPauseContainerImageName,
+		PauseContainerTag:            DefaultPauseContainerTag,
+		AWSVPCBlockInstanceMetdata:   false,
+		ContainerMetadataEnabled:     false,
+		TaskCPUMemLimit:              DefaultEnabled,
+		CgroupPath:                   defaultCgroupPath,
+		TaskMetadataSteadyStateRate:  DefaultTaskMetadataSteadyStateRate,
+		TaskMetadataBurstRate:        DefaultTaskMetadataBurstRate,
+		SharedVolumeValidateNameOnly: DefaultSharedVolumeValidateNameOnly,
 	}
 }
 

agent/config/config_unix_test.go

@@ -64,6 +64,7 @@ func TestConfigDefault(t *testing.T) {
 		"Default TaskMetadataSteadyStateRate is set incorrectly")
 	assert.Equal(t, DefaultTaskMetadataBurstRate, cfg.TaskMetadataBurstRate,
 		"Default TaskMetadataBurstRate is set incorrectly")
+	assert.False(t, cfg.SharedVolumeValidateNameOnly, "Default SharedVolumeValidateNameOnly set incorrectly")
 }
 
 // TestConfigFromFile tests the configuration can be read from file

agent/config/config_windows.go

@@ -74,23 +74,24 @@ func DefaultConfig() Config {
 		DataDir:          dataDir,
 		// DataDirOnHost is identical to DataDir for Windows because we do not
 		// run as a container
-		DataDirOnHost:               dataDir,
-		ReservedMemory:              0,
-		AvailableLoggingDrivers:     []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver, dockerclient.AWSLogsDriver},
-		TaskCleanupWaitDuration:     DefaultTaskCleanupWaitDuration,
-		DockerStopTimeout:           defaultDockerStopTimeout,
-		ContainerStartTimeout:       defaultContainerStartTimeout,
-		CredentialsAuditLogFile:     filepath.Join(ecsRoot, defaultCredentialsAuditLogFile),
-		CredentialsAuditLogDisabled: false,
-		ImageCleanupDisabled:        false,
-		MinimumImageDeletionAge:     DefaultImageDeletionAge,
-		ImageCleanupInterval:        DefaultImageCleanupTimeInterval,
-		NumImagesToDeletePerCycle:   DefaultNumImagesToDeletePerCycle,
-		ContainerMetadataEnabled:    false,
-		TaskCPUMemLimit:             ExplicitlyDisabled,
-		PlatformVariables:           platformVariables,
-		TaskMetadataSteadyStateRate: DefaultTaskMetadataSteadyStateRate,
-		TaskMetadataBurstRate:       DefaultTaskMetadataBurstRate,
+		DataDirOnHost:                dataDir,
+		ReservedMemory:               0,
+		AvailableLoggingDrivers:      []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver, dockerclient.AWSLogsDriver},
+		TaskCleanupWaitDuration:      DefaultTaskCleanupWaitDuration,
+		DockerStopTimeout:            defaultDockerStopTimeout,
+		ContainerStartTimeout:        defaultContainerStartTimeout,
+		CredentialsAuditLogFile:      filepath.Join(ecsRoot, defaultCredentialsAuditLogFile),
+		CredentialsAuditLogDisabled:  false,
+		ImageCleanupDisabled:         false,
+		MinimumImageDeletionAge:      DefaultImageDeletionAge,
+		ImageCleanupInterval:         DefaultImageCleanupTimeInterval,
+		NumImagesToDeletePerCycle:    DefaultNumImagesToDeletePerCycle,
+		ContainerMetadataEnabled:     false,
+		TaskCPUMemLimit:              ExplicitlyDisabled,
+		PlatformVariables:            platformVariables,
+		TaskMetadataSteadyStateRate:  DefaultTaskMetadataSteadyStateRate,
+		TaskMetadataBurstRate:        DefaultTaskMetadataBurstRate,
+		SharedVolumeValidateNameOnly: DefaultSharedVolumeValidateNameOnly,
 	}
 }
 

agent/config/config_windows_test.go

@@ -58,6 +58,7 @@ func TestConfigDefault(t *testing.T) {
 		"Default TaskMetadataSteadyStateRate is set incorrectly")
 	assert.Equal(t, DefaultTaskMetadataBurstRate, cfg.TaskMetadataBurstRate,
 		"Default TaskMetadataBurstRate is set incorrectly")
+	assert.False(t, cfg.SharedVolumeValidateNameOnly, "Default SharedVolumeValidateNameOnly set incorrectly")
 }
 
 func TestConfigIAMTaskRolesReserves80(t *testing.T) {

agent/config/types.go

@@ -218,4 +218,10 @@ type Config struct {
 
 	// TaskMetadataBurstRate specifies the burst rate throttle for the task metadata endpoint
 	TaskMetadataBurstRate int
+
+	// SharedVolumeValidateNameOnly is config option used to short-circuit volume validation against a
+	// provisioned volume, if true. If false, we perform deep comparison including driver options and labels.
+	// For comparing shared volume across 2 instances, this should be set to true as docker does not propagate
+	// driver options and labels through volume drivers.
+	SharedVolumeValidateNameOnly bool
 }