EFSVolumeConfiguration -> efsVolumeConfiguration (#2254)

* EFSVolumeConfiguration -> efsVolumeConfiguration

* Fix FileSystemId and write json marshalling unit tests

* unit test PostUnmarshalTask efsVolumeConfiguration behavior

* windows unit test fix

agent/acs/model/api/api-2.json

@@ -677,7 +677,7 @@
         "type":{"shape":"VolumeType"},
         "host":{"shape":"HostVolumeProperties"},
         "dockerVolumeConfiguration":{"shape":"DockerVolumeConfiguration"},
-        "EFSVolumeConfiguration":{"shape":"EFSVolumeConfiguration"}
+        "efsVolumeConfiguration":{"shape":"EFSVolumeConfiguration"}
       }
     },
     "VolumeFrom":{

agent/acs/model/ecsacs/api.go

@@ -1358,7 +1358,7 @@ type Volume struct {
 
 	DockerVolumeConfiguration *DockerVolumeConfiguration `locationName:"dockerVolumeConfiguration" type:"structure"`
 
-	EFSVolumeConfiguration *EFSVolumeConfiguration `type:"structure"`
+	EfsVolumeConfiguration *EFSVolumeConfiguration `locationName:"efsVolumeConfiguration" type:"structure"`
 
 	Host *HostVolumeProperties `locationName:"host" type:"structure"`
 

agent/api/task/task.go

@@ -547,7 +547,7 @@ func (task *Task) addEFSVolumes(
 	// TODO CN and gov partition logic
 	// These are the NFS options recommended by EFS, see:
 	// https://docs.aws.amazon.com/efs/latest/ug/mounting-fs-mount-cmd-general.html
-	ostr := fmt.Sprintf("addr=%s.efs.%s.amazonaws.com,nfsvers=4.1,rsize=1048576,wsize=1048576,hard,timeo=600,retrans=2,noresvport", efsvol.Filesystem, cfg.AWSRegion)
+	ostr := fmt.Sprintf("addr=%s.efs.%s.amazonaws.com,nfsvers=4.1,rsize=1048576,wsize=1048576,hard,timeo=600,retrans=2,noresvport", efsvol.FileSystemID, cfg.AWSRegion)
 	devstr := fmt.Sprintf(":%s", efsvol.RootDirectory)
 	volumeResource, err := taskresourcevolume.NewVolumeResource(
 		ctx,

agent/api/task/task_test.go

@@ -688,6 +688,80 @@ func TestPostUnmarshalTaskWithDockerVolumes(t *testing.T) {
 	assert.Equal(t, DockerVolumeType, taskVol.Type)
 }
 
+// Test that the PostUnmarshal function properly changes EfsVolumeConfiguration
+// task definitions into a dockerVolumeConfiguration task resource.
+func TestPostUnmarshalTaskWithEFSVolumes(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
+	dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.SDKVolumeResponse{DockerVolume: &types.Volume{}})
+	taskFromACS := ecsacs.Task{
+		Arn:           strptr("myArn"),
+		DesiredStatus: strptr("RUNNING"),
+		Family:        strptr("myFamily"),
+		Version:       strptr("1"),
+		Containers: []*ecsacs.Container{
+			{
+				Name: strptr("myName1"),
+				MountPoints: []*ecsacs.MountPoint{
+					{
+						ContainerPath: strptr("/some/path"),
+						SourceVolume:  strptr("efsvolume"),
+					},
+				},
+			},
+		},
+		Volumes: []*ecsacs.Volume{
+			{
+				Name: strptr("efsvolume"),
+				Type: strptr("efs"),
+				EfsVolumeConfiguration: &ecsacs.EFSVolumeConfiguration{
+					FileSystemId:  strptr("fs-12345"),
+					RootDirectory: strptr("/tmp"),
+				},
+			},
+		},
+	}
+	seqNum := int64(42)
+	task, err := TaskFromACS(&taskFromACS, &ecsacs.PayloadMessage{SeqNum: &seqNum})
+	assert.Nil(t, err, "Should be able to handle acs task")
+	assert.Equal(t, 1, len(task.Containers)) // before PostUnmarshalTask
+	cfg := config.Config{}
+	cfg.AWSRegion = "us-west-2"
+	task.PostUnmarshalTask(&cfg, nil, nil, dockerClient, nil)
+	assert.Equal(t, 1, len(task.Containers), "Should match the number of containers as before PostUnmarshalTask")
+	assert.Equal(t, 1, len(task.Volumes), "Should have 1 volume")
+	taskVol := task.Volumes[0]
+	assert.Equal(t, "efsvolume", taskVol.Name)
+	assert.Equal(t, "efs", taskVol.Type)
+
+	resources := task.GetResources()
+	assert.Len(t, resources, 1)
+	vol, ok := resources[0].(*taskresourcevolume.VolumeResource)
+	require.True(t, ok)
+	dockerVolName := vol.VolumeConfig.DockerVolumeName
+	b, err := json.Marshal(resources[0])
+	require.NoError(t, err)
+	require.JSONEq(t, fmt.Sprintf(`{
+		"name": "efsvolume",
+		"dockerVolumeConfiguration": {
+		  "scope": "task",
+		  "autoprovision": false,
+		  "mountPoint": "",
+		  "driver": "local",
+		  "driverOpts": {
+			"device": ":/tmp",
+			"o": "addr=fs-12345.efs.us-west-2.amazonaws.com,nfsvers=4.1,rsize=1048576,wsize=1048576,hard,timeo=600,retrans=2,noresvport",
+			"type": "nfs"
+		  },
+		  "labels": {},
+		  "dockerVolumeName": "%s"
+		},
+		"createdAt": "0001-01-01T00:00:00Z",
+		"desiredStatus": "NONE",
+		"knownStatus": "NONE"
+	  }`, dockerVolName), string(b))
+}
+
 func TestInitializeContainersV3MetadataEndpoint(t *testing.T) {
 	task := Task{
 		Containers: []*apicontainer.Container{

agent/api/task/taskvolume.go

@@ -66,7 +66,7 @@ func (tv *TaskVolume) UnmarshalJSON(b []byte) error {
 	case DockerVolumeType:
 		return tv.unmarshalDockerVolume(intermediate["dockerVolumeConfiguration"])
 	case EFSVolumeType:
-		return tv.unmarshalEFSVolume(intermediate["EFSVolumeConfiguration"])
+		return tv.unmarshalEFSVolume(intermediate["efsVolumeConfiguration"])
 	default:
 		return errors.Errorf("unrecognized volume type: %q", tv.Type)
 	}
@@ -89,7 +89,7 @@ func (tv *TaskVolume) MarshalJSON() ([]byte, error) {
 	case HostVolumeType:
 		result["host"] = tv.Volume
 	case EFSVolumeType:
-		result["EFSVolumeConfiguration"] = tv.Volume
+		result["efsVolumeConfiguration"] = tv.Volume
 	default:
 		return nil, errors.Errorf("unrecognized volume type: %q", tv.Type)
 	}

agent/api/task/taskvolume_test.go

@@ -17,6 +17,8 @@ package task
 
 import (
 	"encoding/json"
+	"fmt"
+	"runtime"
 	"testing"
 
 	apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container"
@@ -61,14 +63,112 @@ func TestMarshalUnmarshalOldTaskVolumes(t *testing.T) {
 	assert.Equal(t, "", v2.Volume.Source(), "Expected v2 to have 'sourcepath' work correctly")
 }
 
+func TestMarshalTaskVolumesEFS(t *testing.T) {
+	task := &Task{
+		Arn: "test",
+		Volumes: []TaskVolume{
+			{Name: "1", Type: EFSVolumeType, Volume: &taskresourcevolume.EFSVolumeConfig{FileSystemID: "fs-12345", RootDirectory: "/tmp"}},
+		},
+	}
+
+	marshal, err := json.Marshal(task)
+	require.NoError(t, err, "Could not marshal task")
+	expectedTaskDef := `{
+		"Arn": "test",
+		"Family": "",
+		"Version": "",
+		"Containers": null,
+		"associations": null,
+		"resources": null,
+		"volumes": [
+		  {
+			"efsVolumeConfiguration": {
+			  "fileSystemId": "fs-12345",
+			  "rootDirectory": "/tmp",
+			  "dockerVolumeName": ""
+			},
+			"name": "1",
+			"type": "efs"
+		  }
+		],
+		"DesiredStatus": "NONE",
+		"KnownStatus": "NONE",
+		"KnownTime": "0001-01-01T00:00:00Z",
+		"PullStartedAt": "0001-01-01T00:00:00Z",
+		"PullStoppedAt": "0001-01-01T00:00:00Z",
+		"ExecutionStoppedAt": "0001-01-01T00:00:00Z",
+		"SentStatus": "NONE",
+		"StartSequenceNumber": 0,
+		"StopSequenceNumber": 0,
+		"executionCredentialsID": "",
+		"ENI": null,
+		"AppMesh": null,
+		"PlatformFields": %s
+	  }`
+	if runtime.GOOS == "windows" {
+		// windows task defs have a special 'cpu unbounded' field added.
+		// see https://github.com/aws/amazon-ecs-agent/pull/1227
+		expectedTaskDef = fmt.Sprintf(expectedTaskDef, `{"cpuUnbounded": false}`)
+	} else {
+		expectedTaskDef = fmt.Sprintf(expectedTaskDef, "{}")
+	}
+	require.JSONEq(t, expectedTaskDef, string(marshal))
+}
+
+func TestUnmarshalTaskVolumesEFS(t *testing.T) {
+	taskDef := []byte(`{
+		"Arn": "test",
+		"Family": "",
+		"Version": "",
+		"Containers": null,
+		"associations": null,
+		"resources": null,
+		"volumes": [
+		  {
+			"efsVolumeConfiguration": {
+			  "fileSystemId": "fs-12345",
+			  "rootDirectory": "/tmp",
+			  "dockerVolumeName": ""
+			},
+			"name": "1",
+			"type": "efs"
+		  }
+		],
+		"DesiredStatus": "NONE",
+		"KnownStatus": "NONE",
+		"KnownTime": "0001-01-01T00:00:00Z",
+		"PullStartedAt": "0001-01-01T00:00:00Z",
+		"PullStoppedAt": "0001-01-01T00:00:00Z",
+		"ExecutionStoppedAt": "0001-01-01T00:00:00Z",
+		"SentStatus": "NONE",
+		"StartSequenceNumber": 0,
+		"StopSequenceNumber": 0,
+		"executionCredentialsID": "",
+		"ENI": null,
+		"AppMesh": null,
+		"PlatformFields": {}
+	  }`)
+	var task Task
+	err := json.Unmarshal(taskDef, &task)
+	require.NoError(t, err, "Could not unmarshal task")
+
+	require.Len(t, task.Volumes, 1)
+	require.Equal(t, "efs", task.Volumes[0].Type)
+	require.Equal(t, "1", task.Volumes[0].Name)
+	efsConfig, ok := task.Volumes[0].Volume.(*taskresourcevolume.EFSVolumeConfig)
+	require.True(t, ok)
+	require.Equal(t, "fs-12345", efsConfig.FileSystemID)
+	require.Equal(t, "/tmp", efsConfig.RootDirectory)
+}
+
 func TestMarshalUnmarshalTaskVolumes(t *testing.T) {
 	task := &Task{
 		Arn: "test",
 		Volumes: []TaskVolume{
 			{Name: "1", Type: HostVolumeType, Volume: &taskresourcevolume.LocalDockerVolume{}},
 			{Name: "2", Type: HostVolumeType, Volume: &taskresourcevolume.FSHostVolume{FSSourcePath: "/path"}},
 			{Name: "3", Type: DockerVolumeType, Volume: &taskresourcevolume.DockerVolumeConfig{Scope: "task", Driver: "local"}},
-			{Name: "4", Type: EFSVolumeType, Volume: &taskresourcevolume.EFSVolumeConfig{Filesystem: "fs-12345", RootDirectory: "/tmp"}},
+			{Name: "4", Type: EFSVolumeType, Volume: &taskresourcevolume.EFSVolumeConfig{FileSystemID: "fs-12345", RootDirectory: "/tmp"}},
 		},
 	}
 
@@ -109,7 +209,7 @@ func TestMarshalUnmarshalTaskVolumes(t *testing.T) {
 
 	efsVolume, ok := v4.Volume.(*taskresourcevolume.EFSVolumeConfig)
 	assert.True(t, ok, "incorrect EFSVolumeConfig type")
-	assert.Equal(t, "fs-12345", efsVolume.Filesystem)
+	assert.Equal(t, "fs-12345", efsVolume.FileSystemID)
 	assert.Equal(t, "/tmp", efsVolume.RootDirectory)
 }
 
@@ -205,7 +305,7 @@ func TestInitializeEFSVolume(t *testing.T) {
 				Name: "efs-volume-test",
 				Type: "efs",
 				Volume: &taskresourcevolume.EFSVolumeConfig{
-					Filesystem:    "fs-12345",
+					FileSystemID:  "fs-12345",
 					RootDirectory: "/my/root/dir",
 				},
 			},
@@ -284,7 +384,7 @@ func TestInitializeEFSVolume_WrongVolumeType(t *testing.T) {
 				Name: "efs-volume-test",
 				Type: "docker",
 				Volume: &taskresourcevolume.EFSVolumeConfig{
-					Filesystem:    "fs-12345",
+					FileSystemID:  "fs-12345",
 					RootDirectory: "/my/root/dir",
 				},
 			},

agent/functional_tests/tests/functionaltests_unix_test.go

@@ -1364,7 +1364,7 @@ func TestASMSecretsARN(t *testing.T) {
 //   1. creates an EFS filesystem with a mount target.
 //   2. spins up a task to mount and write to the filesystem.
 //   3. spins up a task to mount and read from the filesystem.
-//   4. TODO: do this via EFSVolumeConfiguration instead of via NFS/Docker.
+//   4. TODO: do this via efsVolumeConfiguration instead of via NFS/Docker.
 func TestRunEFSVolumeTask(t *testing.T) {
 	os.Setenv("ECS_FTEST_FORCE_NET_HOST", "true")
 	defer os.Unsetenv("ECS_FTEST_FORCE_NET_HOST")

agent/taskresource/volume/dockervolume.go

@@ -84,7 +84,7 @@ type DockerVolumeConfig struct {
 
 // EFSVolumeConfig represents efs volume configuration.
 type EFSVolumeConfig struct {
-	Filesystem    string `json:"filesystem"`
+	FileSystemID  string `json:"fileSystemId"`
 	RootDirectory string `json:"rootDirectory"`
 	// DockerVolumeName is internal docker name for this volume.
 	DockerVolumeName string `json:"dockerVolumeName"`