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_MATCH_ALL` | `true` | When `true`, ECS Agent will compare driver options and labels to make sure volumes are identical. When `false`, Agent will short circuit shared volume comparison if the names match. This is the defualt Docker behavior. If a volume is shared across instances, this should be set to `false`. | `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.SharedVolumeMatchAll, 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(SharedVolumeMatchAll bool, 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(SharedVolumeMatchAll, 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(SharedVolumeMatchAll bool, 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 !SharedVolumeMatchAll {
+		seelog.Infof("initialize volume: Task [%s]: ECS_SHARED_MATCH_VALIDATE_NAME_ONLY 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

@@ -133,6 +133,7 @@ func TestInitializeLocalDockerVolume(t *testing.T) {
 }
 
 func TestInitializeSharedProvisionedVolume(t *testing.T) {
+	SharedVolumeMatchAll := true
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -163,14 +164,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(SharedVolumeMatchAll, 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) {
+	SharedVolumeMatchAll := true
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -201,11 +203,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(SharedVolumeMatchAll, dockerClient, nil)
 	assert.Error(t, err, "volume not found for auto-provisioned resource should cause task to fail")
 }
 
 func TestInitializeSharedNonProvisionedVolume(t *testing.T) {
+	SharedVolumeMatchAll := true
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -241,14 +244,62 @@ func TestInitializeSharedNonProvisionedVolume(t *testing.T) {
 			Labels: map[string]string{"test": "test"},
 		},
 	})
-	err := testTask.initializeDockerVolumes(dockerClient, nil)
+	err := testTask.initializeDockerVolumes(SharedVolumeMatchAll, 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) {
+	SharedVolumeMatchAll := false
+
+	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(SharedVolumeMatchAll, 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) {
+	SharedVolumeMatchAll := true
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -278,13 +329,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(SharedVolumeMatchAll, 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) {
+	SharedVolumeMatchAll := true
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -318,11 +370,12 @@ func TestInitializeSharedAutoprovisionVolumeNotMatchError(t *testing.T) {
 			Labels: map[string]string{"test": "test"},
 		},
 	})
-	err := testTask.initializeDockerVolumes(dockerClient, nil)
+	err := testTask.initializeDockerVolumes(SharedVolumeMatchAll, dockerClient, nil)
 	assert.Error(t, err, "volume resource details not match should cause task fail")
 }
 
 func TestInitializeSharedAutoprovisionVolumeTimeout(t *testing.T) {
+	SharedVolumeMatchAll := true
 	ctrl := gomock.NewController(t)
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 
@@ -354,11 +407,12 @@ 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(SharedVolumeMatchAll, dockerClient, nil)
 	assert.Error(t, err, "volume resource details not match should cause task fail")
 }
 
 func TestInitializeTaskVolume(t *testing.T) {
+	SharedVolumeMatchAll := true
 	testTask := &Task{
 		ResourcesMapUnsafe: make(map[string][]taskresource.TaskResource),
 		Containers: []*apicontainer.Container{
@@ -383,7 +437,7 @@ func TestInitializeTaskVolume(t *testing.T) {
 		},
 	}
 
-	err := testTask.initializeDockerVolumes(nil, nil)
+	err := testTask.initializeDockerVolumes(SharedVolumeMatchAll, 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
+
+	// DefaultSharedVolumeMatchAll is set to false, only requiring shared volumes to match
+	// on names
+	DefaultSharedVolumeMatchAll = false
 )
 
 const (
@@ -420,6 +424,7 @@ func environmentConfig() (Config, error) {
 		CgroupPath:                       os.Getenv("ECS_CGROUP_PATH"),
 		TaskMetadataSteadyStateRate:      steadyStateRate,
 		TaskMetadataBurstRate:            burstRate,
+		SharedVolumeMatchAll:             utils.ParseBool(os.Getenv("ECS_SHARED_VOLUME_MATCH_NAME_ONLY"), false),
 	}, err
 }
 

agent/config/config_unix.go

@@ -68,6 +68,7 @@ func DefaultConfig() Config {
 		CgroupPath:                  defaultCgroupPath,
 		TaskMetadataSteadyStateRate: DefaultTaskMetadataSteadyStateRate,
 		TaskMetadataBurstRate:       DefaultTaskMetadataBurstRate,
+		SharedVolumeMatchAll:        DefaultSharedVolumeMatchAll,
 	}
 }
 

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.SharedVolumeMatchAll, "Default SharedVolumeMatchAll set incorrectly")
 }
 
 // TestConfigFromFile tests the configuration can be read from file

agent/config/config_windows.go

@@ -91,6 +91,7 @@ func DefaultConfig() Config {
 		PlatformVariables:           platformVariables,
 		TaskMetadataSteadyStateRate: DefaultTaskMetadataSteadyStateRate,
 		TaskMetadataBurstRate:       DefaultTaskMetadataBurstRate,
+		SharedVolumeMatchAll:        DefaultSharedVolumeMatchAll,
 	}
 }
 

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.SharedVolumeMatchAll, "Default SharedVolumeMatchAll 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
+
+	// SharedVolumeMatchAll is config option used to short-circuit volume validation against a
+	// provisioned volume, if false (default). If true, we perform deep comparison including driver options
+	// and labels. For comparing shared volume across 2 instances, this should be set to false as docker's
+	// default behavior is to match name only, and does not propagate driver options and labels through volume drivers.
+	SharedVolumeMatchAll bool
 }