Adding support for ebs volumes within task handler #3917
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mye956
force-pushed
the
ebstaskhandler
branch
2 times, most recently
from
5e3f264
to
2cda585
Compare
mye956
changed the title
mye956
force-pushed
the
ebstaskhandler
branch
4 times, most recently
from
b285695
to
8481805
Compare
xxx0624
reviewed
Sep 21, 2023
mye956
force-pushed
the
ebstaskhandler
branch
2 times, most recently
from
bd970b3
to
5d94a03
Compare
xxx0624
previously approved these changes
Sep 21, 2023
| "github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs" | ||
| acssession "github.com/aws/amazon-ecs-agent/ecs-agent/acs/session" | ||
| "github.com/aws/amazon-ecs-agent/ecs-agent/acs/session/testconst" | ||
| "github.com/aws/amazon-ecs-agent/ecs-agent/api/eni" | ||
| apira "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource" |
There was a problem hiding this comment.
nit: Should we name it apiresource instead of apira ?
There was a problem hiding this comment.
good call out, that makes it more clearer. thanks Utsa!
mye956
force-pushed
the
ebstaskhandler
branch
from
5d94a03
to
ff5187d
Compare
ubhattacharjya
approved these changes
Sep 22, 2023
xxx0624
approved these changes
Sep 22, 2023
chienhanlin
reviewed
Sep 22, 2023
| if property == nil { | ||
| return nil, fmt.Errorf("failed to parse task ebs attachment, encountered nil property") | ||
| } | ||
| if aws.StringValue(property.Value) == "" { |
There was a problem hiding this comment.
Discussed offline. By design, an initial validation for non-empty property value is done here. The required properties will be validate again in func validateEBSTaskVolumeAttachment.
chienhanlin
approved these changes
Sep 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The goal of this PR is to add functionality to handle and process task payload with EBS volume attachments. Once the payload has been processed, agent will spin up the task docker container and mount the EBS volume within the host instance.
Implementation details
task.gotask_attachment_handler.gohandleTaskAttachments: The functionality will now be parsing the list of attachment that's within the task payload (in the form of the shared Task struct). If there are any attachments of typeAmazonElasticBlockStoragethen we'll parse + validate it into an object of typeEBSTaskVolumeConfigvia theParseEBSTaskVolumeAttachmentfunction. At the end, we'll append each newEBSTaskVolumeConfigobject to the list of volumes for the managed task.taskvolume.gounmarshalEBSVolume: This function will unmarshal raw data (in JSON form) and convert into an object of typeEBSTaskVolumeConfig. This is used for saving to state.dockervolume.go:Source: Returns the complete source host path that's used to bind mount with the task docker containerdockervolume_ebs.gototaskresource/volumeParseEBSTaskVolumeAttachment: Parses and validates an EBS volume attachment from the task payload sent from ACS. Returns a new object of typedEBSTaskVolumeConfigvalidateEBSTaskVolumeAttachment: Validates that the parsedEBSTaskVolumeConfighas VolumeId, VolumeName, and SourceVolumeHostPath set and non-empty which is required for task start and stop.EBSTaskVolumeConfigwhich is used to store all of the attachment information of the EBS volume for a managed taskTesting
Unit tests
New tests cover the changes: Yes
Description for the changelog
Add support to handle task payload with EBS volume attachments.
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.