feat(deadline): change SEP construct to use launch templates instead of launch specifications #513

jericht · 2021-07-27T20:46:31Z

Notes

Refactors the code around the Spot Event Plugin construct to use Launch Templates instead of Launch Specifications.

Testing

Updated unit tests
Deployed the SEP example app and verified SEP works as intended by:
- Submitting jobs in Deadline to the configured SEP group
- Verifying a new Spot Fleet Request was spun up using the Launch Template created by the SEP construct
Verified the SFRs created by SEP use the latest version of the Launch Template, without requiring a redeploy, by first rendering a job with SEP, terminating the old SEP instance, updating the Launch Template in the AWS console, then submitting a new job and ensured the Worker spun up used the new launch template version.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gdday

How feasible would it be to prevent tying the Spot Event Plugin to the Launch Template version that was created and, instead, using the latest version of the Launch Template?

That would let users edit the Launch Template in the AWS Console rather than making their changes in their CDK application.

jericht · 2021-08-09T19:01:09Z

@gdday So Spot does support using a special value $Latest for Launch Templates that would achieve this, however it looks like it's unfortunately not supported by CloudFormation:

The version number of the launch template. You must specify a version number. AWS CloudFormation does not support specifying $Latest or $Default for the template version number.

Maybe there's a way to do this with a custom resource that would update that value via calling the appropriate AWS APIs though, I'll look into seeing what that would look like

gdday · 2021-08-09T19:55:48Z

Maybe there's a way to do this with a custom resource that would update that value via calling the appropriate AWS APIs though, I'll look into seeing what that would look like

Right, I think you could do it via the describe-launch-templates API in Lambda. I don't know how much additional work that would be, but I do think that would be useful for customers.

gdday

Thank you for adding support for $Latest. This will be very helpful for customers who want to quickly modify the Launch Template without having to redeploy.

This looks good to me, I don't see any issues!

gdday · 2021-09-01T22:51:39Z

packages/aws-rfdk/lib/deadline/lib/configure-spot-event-plugin.ts

@@ -522,51 +521,52 @@ export class ConfigureSpotEventPlugin extends Construct {
 * Each congiguration is a mapping between one Deadline Group and one Spot Fleet Request Configuration.


You didn't change this but... congiguration?

Nice catch, thanks! Fixed

jusiskin

This is great work, @jericht. The three primary concerns are:

Move the launch template to be created in the SpotEvenPluginFleet if possible
Make sure the required tags are added to launch templates for the Deadline Resource Tracker to function properly
Map each SpotEventPluginFleet subnet to an individual LaunchTemplateConfig instead of passing an invalid comma-separated list of subnet IDs

Otherwise, there are a few minor semantic/polish suggestions.

examples/deadline/All-In-AWS-Infrastructure-SEP/python/package/config.py

packages/aws-rfdk/lib/deadline/lib/configure-spot-event-plugin.ts

packages/aws-rfdk/lib/deadline/test/configure-spot-event-plugin.test.ts

packages/aws-rfdk/lib/lambdas/nodejs/configure-spot-event-plugin/test/conversion.test.ts

packages/aws-rfdk/lib/lambdas/nodejs/configure-spot-event-plugin/conversion.ts

packages/aws-rfdk/lib/lambdas/nodejs/configure-spot-event-plugin/test/handler.test.ts

jericht · 2021-09-09T23:08:30Z

@jusiskin @gdday I've retested this end-to-end and verified SEP is still working (as well as the Resource Tracker tags being applied to the spot instances). The CI build is failing because something is up with the CI runners that is causing an expected crash. I'll look into getting those fixed, but in the meantime this is ready for re-review

jusiskin

Just a few minor suggestions here, but this is shaping up really nicely

examples/deadline/All-In-AWS-Infrastructure-SEP/python/package/app.py

packages/aws-rfdk/lib/deadline/lib/spot-event-plugin-fleet.ts

packages/aws-rfdk/lib/lambdas/nodejs/configure-spot-event-plugin/test/conversion.test.ts

packages/aws-rfdk/lib/lambdas/nodejs/configure-spot-event-plugin/test/handler.test.ts

packages/aws-rfdk/lib/deadline/lib/spot-event-plugin-fleet.ts

packages/aws-rfdk/lib/deadline/test/configure-spot-event-plugin.test.ts

examples/deadline/All-In-AWS-Infrastructure-SEP/python/package/app.py

packages/aws-rfdk/lib/deadline/lib/spot-event-plugin-fleet.ts

packages/aws-rfdk/lib/lambdas/nodejs/configure-spot-event-plugin/conversion.ts

…of launch specifications BREAKING CHANGE: SpotEventPluginFleet now uses EC2 Launch Templates instead of Launch Specifications. See the [RFDK 0.39.x upgrade documentation](https://github.com/aws/aws-rfdk/blob/v0.39.0/packages/aws-rfdk/docs/upgrade/upgrading-0.39.md) for more details and guidance on how to upgrade.

…ral cleanup

jericht · 2021-11-10T00:25:03Z

@horsmand @jusiskin I also changed the initial commit message to have a BREAKING CHANGE section and added some upgrade documentation since old SFRs will be orphaned if users try to redeploy while they have active SFRs that don't use launch templates.

I also did a final E2E test (deploy SEP example and render a cmd line job with SEP), so if everything looks good we can merge this in.

horsmand · 2021-11-15T20:33:47Z

Is there any easy way to preview the diagram?

jericht · 2021-11-15T22:35:23Z

Is there any easy way to preview the diagram?

@horsmand Yep, you can click the rich diff button in the top right of the file panel for the svg files

jusiskin

Looks good to me! This will be a really great enhancement for RFDK users. Thanks for your hard work, @jericht

jericht force-pushed the jericht/disable_imdsv1 branch 2 times, most recently from b350484 to 7323f60 Compare July 27, 2021 22:14

jericht marked this pull request as ready for review July 29, 2021 21:54

jusiskin added the contribution/core This is a PR that came from AWS. label Aug 2, 2021

gdday suggested changes Aug 9, 2021

View reviewed changes

jericht force-pushed the jericht/disable_imdsv1 branch from 7323f60 to 9683e08 Compare August 16, 2021 15:07

jericht changed the title ~~feat: add ability to disable imdsv1~~ feat(deadline): change SEP construct to use launch templates instead of launch specifications Aug 16, 2021

jericht force-pushed the jericht/disable_imdsv1 branch 2 times, most recently from 37da9cd to 3b5a75a Compare August 16, 2021 21:23

jericht force-pushed the jericht/disable_imdsv1 branch 3 times, most recently from d11f144 to 91994f6 Compare August 24, 2021 18:50

gdday previously approved these changes Sep 2, 2021

View reviewed changes

jusiskin requested changes Sep 2, 2021

View reviewed changes

jericht dismissed gdday’s stale review via 2a4e80e September 8, 2021 23:46

jericht force-pushed the jericht/disable_imdsv1 branch 3 times, most recently from c07ff41 to d777283 Compare September 9, 2021 22:58

jericht requested review from jusiskin and gdday September 9, 2021 23:06

jericht force-pushed the jericht/disable_imdsv1 branch from d777283 to c4503c3 Compare October 12, 2021 23:02

jericht force-pushed the jericht/disable_imdsv1 branch 3 times, most recently from eb195bc to 44be32e Compare November 1, 2021 23:57

jusiskin requested changes Nov 5, 2021

View reviewed changes

jusiskin reviewed Nov 5, 2021

View reviewed changes

packages/aws-rfdk/lib/deadline/test/configure-spot-event-plugin.test.ts Outdated Show resolved Hide resolved

horsmand self-requested a review November 9, 2021 16:21

horsmand suggested changes Nov 9, 2021

View reviewed changes

jericht added 3 commits November 10, 2021 00:14

update example app flag description, correct subnet ID code, and gene…

6b3cfb7

…ral cleanup

update render queue SEP policy

ec959cd

jericht force-pushed the jericht/disable_imdsv1 branch from 44be32e to c86c5bd Compare November 10, 2021 00:19

add upgrade documentation and general cleanup

5d85d5d

jericht force-pushed the jericht/disable_imdsv1 branch from c86c5bd to 5d85d5d Compare November 10, 2021 00:32

jericht requested review from jusiskin and horsmand November 10, 2021 01:46

horsmand previously approved these changes Nov 15, 2021

View reviewed changes

jericht dismissed horsmand’s stale review via 6125a71 November 15, 2021 19:50

jericht force-pushed the jericht/disable_imdsv1 branch 3 times, most recently from 521fc2b to a8bd154 Compare November 15, 2021 22:30

jericht force-pushed the jericht/disable_imdsv1 branch 3 times, most recently from e66ec42 to 37a8355 Compare November 15, 2021 23:47

update architecture diagram for SpotEventPluginFleet

5b3a123

jericht force-pushed the jericht/disable_imdsv1 branch from 37a8355 to 5b3a123 Compare November 16, 2021 18:03

horsmand approved these changes Nov 16, 2021

View reviewed changes

jusiskin approved these changes Nov 16, 2021

View reviewed changes

jusiskin added the pr/do-not-merge This PR should not be merged at this time. label Nov 16, 2021

jericht removed the pr/do-not-merge This PR should not be merged at this time. label Nov 22, 2021

jericht merged commit 7c61c18 into aws:mainline Nov 22, 2021

jericht deleted the jericht/disable_imdsv1 branch November 22, 2021 19:40

jericht mentioned this pull request Nov 22, 2021

chore(docs): fix upgrade documentation version to 0.40 #638

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(deadline): change SEP construct to use launch templates instead of launch specifications #513

feat(deadline): change SEP construct to use launch templates instead of launch specifications #513

jericht commented Jul 27, 2021 •

edited

Loading

gdday left a comment

jericht commented Aug 9, 2021

gdday commented Aug 9, 2021

gdday left a comment

gdday Sep 1, 2021

jericht Sep 7, 2021

jusiskin left a comment •

edited

Loading

jericht commented Sep 9, 2021

jusiskin left a comment

jericht commented Nov 10, 2021 •

edited

Loading

horsmand commented Nov 15, 2021

jericht commented Nov 15, 2021

jusiskin left a comment

		@@ -522,51 +521,52 @@ export class ConfigureSpotEventPlugin extends Construct {
		* Each congiguration is a mapping between one Deadline Group and one Spot Fleet Request Configuration.

feat(deadline): change SEP construct to use launch templates instead of launch specifications #513

feat(deadline): change SEP construct to use launch templates instead of launch specifications #513

Conversation

jericht commented Jul 27, 2021 • edited Loading

Notes

Testing

gdday left a comment

Choose a reason for hiding this comment

jericht commented Aug 9, 2021

gdday commented Aug 9, 2021

gdday left a comment

Choose a reason for hiding this comment

gdday Sep 1, 2021

Choose a reason for hiding this comment

jericht Sep 7, 2021

Choose a reason for hiding this comment

jusiskin left a comment • edited Loading

Choose a reason for hiding this comment

jericht commented Sep 9, 2021

jusiskin left a comment

Choose a reason for hiding this comment

jericht commented Nov 10, 2021 • edited Loading

horsmand commented Nov 15, 2021

jericht commented Nov 15, 2021

jusiskin left a comment

Choose a reason for hiding this comment

jericht commented Jul 27, 2021 •

edited

Loading

jusiskin left a comment •

edited

Loading

jericht commented Nov 10, 2021 •

edited

Loading