Remove EBS Attach for RFS in GovCloud #1216
Conversation
Signed-off-by: Andre Kurait <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1216 +/- ##
=========================================
Coverage 80.49% 80.49%
Complexity 3079 3079
=========================================
Files 421 421
Lines 15650 15650
Branches 1057 1057
=========================================
Hits 12598 12598
Misses 2408 2408
Partials 644 644
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
AndreKurait
requested review from
chelma,
gregschohn,
lewijacn,
mikaylathompson,
peternied and
sumobrian
as code owners
| @@ -61,7 +61,7 @@ export class MigrationServiceCore extends Stack { | |||
| props.taskRolePolicies?.forEach(policy => this.serviceTaskRole.addToPolicy(policy)) | |||
|
|
|||
| const serviceTaskDef = new FargateTaskDefinition(this, "ServiceTaskDef", { | |||
| ephemeralStorageGiB: props.ephemeralStorageGiB ? props.ephemeralStorageGiB : 75, | |||
| ephemeralStorageGiB: Math.max(props.ephemeralStorageGiB ? props.ephemeralStorageGiB : 75, 21), // valid values 21 - 200 | |||
There was a problem hiding this comment.
How does this change in behavior affect the user experience? Can you briefly list all the scenarios and the before/after outcome? I ask because I'm not entirely sure it does change things.
There was a problem hiding this comment.
This also ensures for RFS GovCloud we don't attempt to set this less than 21GB which will otherwise result in an deployment error
There was a problem hiding this comment.
Got it - so we're preventing a deploy time error by overriding a too-small value with the actual minimum. Thanks!
| 1 | ||
| ) | ||
|
|
||
| if (volumeSizeGB > 16000) { | ||
| if (shardVolumeSizeGiB > (16*1024)) { |
There was a problem hiding this comment.
Thanks for cleaning up this logic a bit
| // ECS EBS attachment is not supported in GovCloud | ||
| // https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-ecs.html#govcloud-ecs-diffs | ||
| // Use Ephemeral Storage instead, adding size for shard | ||
| ephemeralStorageGiB = Math.ceil(shardVolumeSizeGiB + systemStorageGiB) |
There was a problem hiding this comment.
Nit: Lots of math, that logic is easier to test if it was refactored into a stand-alone function(s)
Description
We introduced support for larger shards by using ECS EBS Attach on the tasks, this is not supported in govcloud:
This pr introduces a short-term fix (before EKS) to go back to ephemeral storage for GovCloud. This will limit our max shard size back to 80gb.
Issues Resolved
MIGRATIONS-2293
Testing
Added unit tests for govcloud expected behavior
Deployed to govcloud successfully
Verified the error message is appropriate
Check List
Documentation PR:
Add supported regions for Migration Assistant documentation-website#9017
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.