Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EFSVolumeConfiguration -> efsVolumeConfiguration #2254

Merged
merged 4 commits into from
Oct 31, 2019
Merged

EFSVolumeConfiguration -> efsVolumeConfiguration #2254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
097955d
EFSVolumeConfiguration -> efsVolumeConfiguration
sparrc Oct 30, 2019
49bc6b7
Fix FileSystemId and write json marshalling unit tests
sparrc Oct 30, 2019
9764e83
unit test PostUnmarshalTask efsVolumeConfiguration behavior
sparrc Oct 30, 2019
dcad3a2
windows unit test fix
sparrc Oct 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion agent/acs/model/api/api-2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@
"type":{"shape":"VolumeType"},
"host":{"shape":"HostVolumeProperties"},
"dockerVolumeConfiguration":{"shape":"DockerVolumeConfiguration"},
"EFSVolumeConfiguration":{"shape":"EFSVolumeConfiguration"}
"efsVolumeConfiguration":{"shape":"EFSVolumeConfiguration"}
}
},
"VolumeFrom":{
Expand Down
2 changes: 1 addition & 1 deletion agent/acs/model/ecsacs/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand Down
2 changes: 1 addition & 1 deletion agent/api/task/task.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
74 changes: 74 additions & 0 deletions agent/api/task/task_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions agent/api/task/taskvolume.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
108 changes: 104 additions & 4 deletions agent/api/task/taskvolume_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package task

import (
"encoding/json"
"fmt"
"runtime"
"testing"

apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container"
Expand Down Expand Up @@ -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"}},
},
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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",
},
},
Expand Down Expand Up @@ -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",
},
},
Expand Down
2 changes: 1 addition & 1 deletion agent/functional_tests/tests/functionaltests_unix_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion agent/taskresource/volume/dockervolume.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down