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.

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

Added EFSVolumeConfiguration #2234

Merged
merged 9 commits into from
Oct 11, 2019
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
13 changes: 11 additions & 2 deletions agent/acs/model/api/api-2.json
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,13 @@
}
},

"EFSVolumeConfiguration": {
"type":"structure",
"members":{
"filesystemId":{"shape":"String"},
Copy link
Contributor

@yumex93 yumex93 Oct 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our PRFAQ, this attribute is defined as filesystem, ACS will keep the consistency and use filesystem. Just a reminder that we may want to wait for ACS model get finalized and regenerate the file again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, the exact shape of this is subject to change, we'll keep this code in a feature branch until the ACS model is finalized

"rootDirectory":{"shape":"String"}
}
},
"ElasticNetworkInterface":{
"type":"structure",
"members":{
Expand Down Expand Up @@ -669,7 +676,8 @@
"name":{"shape":"String"},
"type":{"shape":"VolumeType"},
sparrc marked this conversation as resolved.
Show resolved Hide resolved
"host":{"shape":"HostVolumeProperties"},
"dockerVolumeConfiguration":{"shape":"DockerVolumeConfiguration"}
"dockerVolumeConfiguration":{"shape":"DockerVolumeConfiguration"},
"EFSVolumeConfiguration":{"shape":"EFSVolumeConfiguration"}
}
},
"VolumeFrom":{
Expand All @@ -691,7 +699,8 @@
"type":"string",
"enum":[
"host",
"docker"
"docker",
"efs"
]
}
}
Expand Down
20 changes: 20 additions & 0 deletions agent/acs/model/ecsacs/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,24 @@ func (s ECRAuthData) GoString() string {
return s.String()
}

type EFSVolumeConfiguration struct {
_ struct{} `type:"structure"`

FilesystemId *string `locationName:"filesystemId" type:"string"`

RootDirectory *string `locationName:"rootDirectory" type:"string"`
}

// String returns the string representation
func (s EFSVolumeConfiguration) String() string {
return awsutil.Prettify(s)
}

// GoString returns the string representation
func (s EFSVolumeConfiguration) GoString() string {
return s.String()
}

type ElasticNetworkInterface struct {
_ struct{} `type:"structure"`

Expand Down Expand Up @@ -1340,6 +1358,8 @@ type Volume struct {

DockerVolumeConfiguration *DockerVolumeConfiguration `locationName:"dockerVolumeConfiguration" type:"structure"`

EFSVolumeConfiguration *EFSVolumeConfiguration `type:"structure"`

Host *HostVolumeProperties `locationName:"host" type:"structure"`

Name *string `locationName:"name" type:"string"`
Expand Down
88 changes: 81 additions & 7 deletions agent/api/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,22 @@ func TaskFromACS(acsTask *ecsacs.Task, envelope *ecsacs.PayloadMessage) (*Task,
return task, nil
}

func (task *Task) initializeVolumes(cfg *config.Config, dockerClient dockerapi.DockerClient, ctx context.Context) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this had to be factored out to make gocyclo happy

err := task.initializeDockerLocalVolumes(dockerClient, ctx)
if err != nil {
return apierrors.NewResourceInitError(task.Arn, err)
}
err = task.initializeDockerVolumes(cfg.SharedVolumeMatchFullConfig, dockerClient, ctx)
if err != nil {
return apierrors.NewResourceInitError(task.Arn, err)
}
err = task.initializeEFSVolumes(cfg, dockerClient, ctx)
if err != nil {
return apierrors.NewResourceInitError(task.Arn, err)
}
return nil
}

// PostUnmarshalTask is run after a task has been unmarshalled, but before it has been
// run. It is possible it will be subsequently called after that and should be
// able to handle such an occurrence appropriately (e.g. behave idempotently).
Expand Down Expand Up @@ -326,14 +342,10 @@ func (task *Task) PostUnmarshalTask(cfg *config.Config,
task.initializeASMSecretResource(credentialsManager, resourceFields)
}

err = task.initializeDockerLocalVolumes(dockerClient, ctx)
if err != nil {
return apierrors.NewResourceInitError(task.Arn, err)
}
err = task.initializeDockerVolumes(cfg.SharedVolumeMatchFullConfig, dockerClient, ctx)
if err != nil {
return apierrors.NewResourceInitError(task.Arn, err)
if err := task.initializeVolumes(cfg, dockerClient, ctx); err != nil {
return err
}

if cfg.GPUSupportEnabled {
err = task.addGPUResource()
if err != nil {
Expand Down Expand Up @@ -500,6 +512,68 @@ func (task *Task) initializeDockerVolumes(sharedVolumeMatchFullConfig bool, dock
return nil
}

// initializeEFSVolumes inspects the volume definitions in the task definition.
// If it finds EFS volumes in the task definition, then it converts it to a docker
// volume definition.
func (task *Task) initializeEFSVolumes(cfg *config.Config, dockerClient dockerapi.DockerClient, ctx context.Context) error {
for i, vol := range task.Volumes {
// No need to do this for non-efs volume, eg: host bind/empty volume
if vol.Type != EFSVolumeType {
continue
}

efsvol, ok := vol.Volume.(*taskresourcevolume.EFSVolumeConfig)
if !ok {
return errors.New("task volume: volume configuration does not match the type 'efs'")
}

err := task.addEFSVolumes(ctx, cfg, dockerClient, &task.Volumes[i], efsvol)
if err != nil {
return err
}
}
return nil
}

// addEFSVolumes converts the EFS task definition into an internal docker 'local' volume
// mounted with NFS struct and updates container dependency
func (task *Task) addEFSVolumes(
ctx context.Context,
cfg *config.Config,
dockerClient dockerapi.DockerClient,
vol *TaskVolume,
efsvol *taskresourcevolume.EFSVolumeConfig,
) error {
// 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)
sparrc marked this conversation as resolved.
Show resolved Hide resolved
devstr := fmt.Sprintf(":%s", efsvol.RootDirectory)
volumeResource, err := taskresourcevolume.NewVolumeResource(
ctx,
vol.Name,
task.volumeName(vol.Name),
"task",
false,
"local",
map[string]string{
"type": "nfs",
"device": devstr,
"o": ostr,
},
map[string]string{},
dockerClient,
)
if err != nil {
return err
}

vol.Volume = &volumeResource.VolumeConfig
task.AddResource(resourcetype.DockerVolumeKey, volumeResource)
task.updateContainerVolumeDependency(vol.Name)
return nil
}

// addTaskScopedVolumes adds the task scoped volume into task resources and updates container dependency
func (task *Task) addTaskScopedVolumes(ctx context.Context, dockerClient dockerapi.DockerClient,
vol *TaskVolume) error {
Expand Down
21 changes: 20 additions & 1 deletion agent/api/task/taskvolume.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
const (
HostVolumeType = "host"
DockerVolumeType = "docker"
EFSVolumeType = "efs"
)

// TaskVolume is a definition of all the volumes available for containers to
Expand Down Expand Up @@ -64,8 +65,10 @@ func (tv *TaskVolume) UnmarshalJSON(b []byte) error {
return tv.unmarshalHostVolume(intermediate["host"])
case DockerVolumeType:
return tv.unmarshalDockerVolume(intermediate["dockerVolumeConfiguration"])
case EFSVolumeType:
return tv.unmarshalEFSVolume(intermediate["EFSVolumeConfiguration"])
default:
return errors.Errorf("invalid Volume: type must be docker or host, got %q", tv.Type)
return errors.Errorf("unrecognized volume type: %q", tv.Type)
}
}

Expand All @@ -85,6 +88,8 @@ func (tv *TaskVolume) MarshalJSON() ([]byte, error) {
result["dockerVolumeConfiguration"] = tv.Volume
case HostVolumeType:
result["host"] = tv.Volume
case EFSVolumeType:
result["EFSVolumeConfiguration"] = tv.Volume
default:
return nil, errors.Errorf("unrecognized volume type: %q", tv.Type)
}
Expand All @@ -106,6 +111,20 @@ func (tv *TaskVolume) unmarshalDockerVolume(data json.RawMessage) error {
return nil
}

func (tv *TaskVolume) unmarshalEFSVolume(data json.RawMessage) error {
if data == nil {
return errors.New("invalid volume: empty volume configuration")
}
var efsVolumeConfig taskresourcevolume.EFSVolumeConfig
err := json.Unmarshal(data, &efsVolumeConfig)
if err != nil {
return err
}

tv.Volume = &efsVolumeConfig
return nil
}

func (tv *TaskVolume) unmarshalHostVolume(data json.RawMessage) error {
if data == nil {
return errors.New("invalid volume: empty volume configuration")
Expand Down
Loading