feat(deadline): add ConfigureSpotEventPlugin and SpotEventPluginFleet constructs

[ryyakobe]

Fixes #131

Adds a new ConfigureSpotEventPlugin construct that can be used to create and save Spot Event Plugin Configurations.

---

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

[ddneilson]

It would be really great to have an abstraction layer overtop of the specific GET/POST requests.

ex: Have a EventPluginRequests class. Constructor takes a DeadlineClient, and the class itself has methods that make it easier to construct the specific requests for, say, saving ServerData and Configuring the SpotEventPlugin.

[ddneilson]

Should put the return-type declaration on these -- Promise<Response>

Or, alternatively, declare them as public async GetRequest(...): Response

[ryyakobe]

Fixed

[jusiskin]

This is a partial review - I am still going through code, but I thought it would help to post some of my feedback so far.

[horsmand]

Need to get through the examples still, but this is a long one so I thought I'd give some feedback now.

[jusiskin]

Second batch of code review feedback.

One other thing I wanted to request is that we add documentation for this new construct in packages/aws-rfdk/lib/deadline/README.md. This is a good place to provide more detailed documentation for constructs that we don't want to necessarily want to cram into the TSDoc string.

[jusiskin]

Few things:

1. This link is pretty useful for the overall configuration of SEP. Can we also add it to the TSDoc string for the SEPConfigurationProperties interface as a general reference?
2. The URL contains ?highlight=... which makes those terms appear highlighted by our docs. Can we remove that from the link?
3. Does it make sense to also link to the Worker extra info docs?

[ryyakobe]

1. I though the same earlier and added it before.
2. Good catch, thanks.
3. Thanks! Good idea!

[jusiskin]

Unless I missed it, I believe 2 still needs to be addressed.

[jusiskin]

We need to be careful here when specifying what CDK refers to as a physical name:

Assigning physical names to resources has some disadvantages in AWS CloudFormation. Most importantly, any changes to deployed resources that require a resource replacement, such as changes to a resource's properties that are immutable after creation, will fail if a resource has a physical name assigned. If you end up in a state like that, the only solution is to delete the AWS CloudFormation stack, then deploy the AWS CDK app again. See the AWS CloudFormation documentation for details.

Is there any reason why we can't just not specify roleName and have CloudFormation generate a physical name for us?

[ddneilson]

Requirement of the Spot Event Plugin. It requires that the role name start with "DeadlineSpot"

[ryyakobe]

I wonder if we have this requirement only because in AWSThinkboxDeadlineSpotEventPluginAdminPolicy
we have:
"Resource": [ "arn:aws:iam::*:role/aws-ec2-spot-fleet-tagging-role", "arn:aws:iam::*:role/DeadlineSpot*" ]
We might then use the approach we used with the fleetRole: just create a new role and add policy to the render queue, but then we would have a few unused statements in the RQ policy, which is not great. I don't think it's worth it.

[ddneilson]

There's no harm in those statements being unused in the managed policy...

[jusiskin]

👍 I agree - unused statements are unfortunate but acceptable here. Adding a broadening policy to the RQ seems like a good way to avoid specifying a physical name.

[ryyakobe]

Seems to be working after allowing PassRole. In AWSThinkboxDeadlineSpotEventPluginAdminPolicy we also allow GetRole for both fleet role and fleet instance role, but it seems that everything is working if I only allow PassRole, so I will keep the permissions strict for now.
We will be able to allow GetRole n the future, if something won't work.

[jusiskin]

Great work Roman! This will add huge value for RFDK users.