-
Notifications
You must be signed in to change notification settings - Fork 245
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
feat: Add support for 'VolumeConfigurations' property on both UpdateService and RunTask API call #721
Conversation
… to the action.yml file
… it during subsequent Update Service Calls
@@ -33,6 +33,8 @@ async function runTask(ecs, clusterName, taskDefArn, waitForMinutes, enableECSMa | |||
const assignPublicIP = core.getInput('run-task-assign-public-IP', { required: false }) || 'DISABLED'; | |||
const tags = JSON.parse(core.getInput('run-task-tags', { required: false }) || '[]'); | |||
const capacityProviderStrategy = JSON.parse(core.getInput('run-task-capacity-provider-strategy', { required: false }) || '[]'); | |||
const runTaskManagedEBSVolumeName = core.getInput('run-task-managed-ebs-volume-name', { required: false }) || ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we thought about auto populating this property for customers from the task-definition.json
file provided in the input? Also, would auto-populate break any existing customers or in other words would it be backwards compatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, That is a good point. I think it would be possible to auto-populate the volume name from task-definition if, the task-definition contains a volume with configuredAtLaunch
set to true
. We could parse the Volume property from a pre-existingTaskDefinitionContents
variable.
Backward-compatibility wise - it should be fine, because:
- Task definition MUST have a volume with configuredAtLaunch: true - in-order to configure a volume property with ECS
- This volume MUST match the name used in VolumeConfigurations
Pros of this auto-population I think would be:
- less configuration on user side
- can't break the volume name matching (since it uses exact name from task definition)
Cons might be:
- a bit of a magic/implicit behavior, but can be called-out in the README
…y will be ignored if service or task managed-ebs-volume-name property is not provided
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
*Issue #, if available: n/A
Description of changes:
This PR enhances the
amazon-ecs-deploy-task-definition
GitHub Action to support Amazon ECS Managed EBS Volumes for both RunTask and UpdateService operations. The changes include:run-task-managed-ebs-volume-name
run-task-managed-ebs-volume
service-managed-ebs-volume-name
service-managed-ebs-volume
run-task-managed-ebs-volume-name
and
run-task-managed-ebs-volume
parameters have been set.service-managed-ebs-volume-name
andservice-managed-ebs-volume
parameters have been set.Usage:
To use
VolumeConfigurations
property inrunTask
andUpdateService
update your task definition to support an ECS volume that is configured at launch, and update your workflow file to look like below:For RunTask:
For UpdateService:
Task Definition Example:
Testing:
Manually Tested below use cases:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.