feat(deadline): add custom user data commands to Worker instance startup #239

kozlove-aws · 2020-11-17T23:59:38Z

Problem

Users do not have possibility to run user data scripts before worker configured and run by RFDK configuration script.
Was created issue with request to add this functional: Worker nodes and UserData

Solution

Was chosen solution that allows user to create class with methods that add user data scripts in all stages of worker configuration.
Was added interface IInstanceUserDataProvider and class InstanceUserDataProvider with three methods.
Every method can be overridden by user and has possibility to modify worker user data.
Were added examples for this functionality for Python and Typescript.

Testing

Was added integration tests that cover 100% of new functionality.
Were deployed Python and Typescript examples and verified that all expected code was added to userdata and example script was copied to worker instance and run.

Was removed validation S3Bucket in version query test because these values are changed periodically.

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

ddneilson · 2020-11-18T15:10:32Z

examples/deadline/All-In-AWS-Infrastructure-Basic/scripts/configure_worker.sh

@@ -0,0 +1 @@
+mkdir test_dir


nit: Missing newline at end of file.

ddneilson · 2020-11-18T15:12:08Z

yarn.lock

@@ -10460,6 +10460,17 @@ ts-node@^8.0.2:
    source-map-support "^0.5.17"
    yn "3.1.1"

+ts-node@^9.0.0:


I suspect this wasn't an intended change.

packages/aws-rfdk/lib/deadline/test/version-query.test.ts

ddneilson · 2020-11-19T16:05:41Z

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

+  postWorkerLaunch(host: IHost): void;
+}
+
+export class InstanceUserDataProvider implements IInstanceUserDataProvider{


This should have a docstring explaining what it's for.

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

jusiskin

Generally, this looks good. The main request is to add an additional callback and make the base class a construct.

examples/deadline/All-In-AWS-Infrastructure-Basic/python/package/lib/compute_tier.py

examples/deadline/All-In-AWS-Infrastructure-Basic/scripts/configure_worker.sh

jusiskin · 2020-11-20T17:45:52Z

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

+  // Method that is invoked before configuration Cloud Watch Agent.
+  preCloudWatchAgent(host: IHost): void;


these should be made into proper TSDoc strings so that they are visible in our API docs and discoverable from IDEs.

This was missed.

Unless I'm wrong here, the // syntax is just a comment and not a docstring. i.e. it doesn't end up in the API documentation.

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

jusiskin · 2020-11-20T17:54:01Z

packages/aws-rfdk/lib/deadline/test/worker-fleet.test.ts

+    expect(userData).toContain('echo preCloudWatchAgent');
+    expect(userData).toContain('echo preDeadlineConfiguration');
+    expect(userData).toContain('echo postWorkerLaunch');


These make no guarantees that the user data are injected in the correct places. Any ideas on how we can test that here?

packages/aws-rfdk/lib/deadline/test/worker-fleet.test.ts

jusiskin · 2020-11-20T21:30:54Z

We haven't been great about doing this lately, but do you mind also adding documentation for this in packages/aws-rfdk/lib/deadline/README.md?

ddneilson · 2020-11-25T23:32:06Z

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

+  // Method that is invoked before configuration Cloud Watch Agent.
+  preCloudWatchAgent(host: IHost): void;


This was missed.

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

ddneilson · 2020-11-26T17:03:01Z

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

@@ -29,6 +29,39 @@ import {
  Version,
 } from './version';

+/**
+ * Provider for adding user data scripts
+ * Methods of this class will be invoked in WorkerInstanceConfiguration


nit: "Methods of this class"

This isn't a class... it's an interface

ddneilson · 2020-11-26T17:03:16Z

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

+  postWorkerLaunch(host: IHost): void;
+}
+
+export class InstanceUserDataProvider extends Construct  implements IInstanceUserDataProvider{


Missing a docstring on this class.

I think this needs more documentation. This class is the API that RFDK users should use in order to achieve custom user data. In here, we should explain that users can sub-class this and override the desired methods to add custom user data commands for WorkerInstanceFleet or WorkerInstanceConfiguration.

ddneilson · 2020-11-26T17:03:55Z

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

 }

 /**
 * This construct can be used to configure Deadline Workers on an instance to connect to a RenderQueue, stream their
 * log files to CloudWatch, and configure various settings of the Deadline Worker.
 *
 * The configuration happens on instance start-up using user data scripting.
+ * 
+ * The configuration has next steps:


Maybe "This configuration performs the following steps in order:"
?

ddneilson · 2020-11-26T17:05:14Z

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

+  // Method that is invoked before configuration Cloud Watch Agent.
+  preCloudWatchAgent(host: IHost): void;


Unless I'm wrong here, the // syntax is just a comment and not a docstring. i.e. it doesn't end up in the API documentation.

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

jusiskin

Looking better, but there are a few more things to address here - mostly around the docstrings.

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

examples/deadline/All-In-AWS-Infrastructure-Basic/python/package/lib/compute_tier.py

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

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

jusiskin · 2020-11-26T18:19:46Z

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

+  postWorkerLaunch(host: IHost): void;
+}
+
+export class InstanceUserDataProvider extends Construct  implements IInstanceUserDataProvider{


I think this needs more documentation. This class is the API that RFDK users should use in order to achieve custom user data. In here, we should explain that users can sub-class this and override the desired methods to add custom user data commands for WorkerInstanceFleet or WorkerInstanceConfiguration.

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts

jusiskin

I notice an issue in the python example that needs to be updated after some PR changes.

jusiskin · 2020-11-26T22:07:56Z

examples/deadline/All-In-AWS-Infrastructure-Basic/python/package/lib/compute_tier.py

+    def pre_render_queue_configuration(self, host) -> None:
+        host.user_data.add_commands("echo preRenderQueueConfiguration")
+
+    def pre_deadline_configuration(self, host) -> None:


I believe this needs to be updated with the new hook name.

kozlove-aws force-pushed the add_pre_configure_userdata branch from 311b5d5 to 0f526b5 Compare November 18, 2020 00:05

ddneilson reviewed Nov 18, 2020

View reviewed changes

kozlove-aws force-pushed the add_pre_configure_userdata branch 3 times, most recently from 3e1d81e to daf8bd6 Compare November 18, 2020 18:21

ddneilson reviewed Nov 18, 2020

View reviewed changes

packages/aws-rfdk/lib/deadline/test/version-query.test.ts Show resolved Hide resolved

ddneilson approved these changes Nov 19, 2020

View reviewed changes

ddneilson requested changes Nov 19, 2020

View reviewed changes

kozlove-aws force-pushed the add_pre_configure_userdata branch from daf8bd6 to 2e4a186 Compare November 19, 2020 16:23

jusiskin requested changes Nov 20, 2020

View reviewed changes

kozlove-aws force-pushed the add_pre_configure_userdata branch from 2e4a186 to a96ea05 Compare November 25, 2020 22:40

ddneilson requested review from jusiskin and ddneilson November 25, 2020 23:30

ddneilson requested changes Nov 25, 2020

View reviewed changes

ddneilson reviewed Nov 26, 2020

View reviewed changes

packages/aws-rfdk/lib/deadline/lib/worker-configuration.ts Show resolved Hide resolved

jusiskin added the contribution/core This is a PR that came from AWS. label Nov 26, 2020

kozlove-aws force-pushed the add_pre_configure_userdata branch from a96ea05 to 91ac916 Compare November 26, 2020 16:58

ddneilson requested changes Nov 26, 2020

View reviewed changes

kozlove-aws force-pushed the add_pre_configure_userdata branch 4 times, most recently from fefa0f4 to c5aab19 Compare November 26, 2020 18:16

jusiskin requested changes Nov 26, 2020

View reviewed changes

kozlove-aws force-pushed the add_pre_configure_userdata branch 3 times, most recently from 95c7f0f to 882d388 Compare November 26, 2020 19:58

ddneilson previously approved these changes Nov 26, 2020

View reviewed changes

jusiskin requested changes Nov 26, 2020

View reviewed changes

fix(deadline): User Data execution on Worker Nodes

64960a1

kozlove-aws dismissed ddneilson’s stale review via 64960a1 November 26, 2020 22:19

kozlove-aws force-pushed the add_pre_configure_userdata branch from 882d388 to 64960a1 Compare November 26, 2020 22:19

ddneilson approved these changes Nov 26, 2020

View reviewed changes

ddneilson requested a review from jusiskin November 26, 2020 22:36

jusiskin approved these changes Nov 26, 2020

View reviewed changes

jusiskin changed the title ~~fix(deadline): User Data execution on Worker Nodes~~ feature(deadline): add custom user data commands to Worker instance startup Nov 26, 2020

jusiskin changed the title ~~feature(deadline): add custom user data commands to Worker instance startup~~ feat(deadline): add custom user data commands to Worker instance startup Nov 26, 2020

jusiskin merged commit bdef391 into mainline Nov 26, 2020

jusiskin deleted the add_pre_configure_userdata branch November 26, 2020 22:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(deadline): add custom user data commands to Worker instance startup #239

feat(deadline): add custom user data commands to Worker instance startup #239

kozlove-aws commented Nov 17, 2020 •

edited

Loading

ddneilson Nov 18, 2020

ddneilson Nov 18, 2020

ddneilson Nov 19, 2020

jusiskin left a comment

jusiskin Nov 20, 2020

ddneilson Nov 25, 2020

ddneilson Nov 26, 2020

jusiskin Nov 20, 2020

jusiskin commented Nov 20, 2020

ddneilson Nov 25, 2020

ddneilson Nov 26, 2020

ddneilson Nov 26, 2020

jusiskin Nov 26, 2020

ddneilson Nov 26, 2020

ddneilson Nov 26, 2020

jusiskin left a comment

jusiskin Nov 26, 2020

jusiskin left a comment

jusiskin Nov 26, 2020

		// Method that is invoked before configuration Cloud Watch Agent.
		preCloudWatchAgent(host: IHost): void;

feat(deadline): add custom user data commands to Worker instance startup #239

feat(deadline): add custom user data commands to Worker instance startup #239

Conversation

kozlove-aws commented Nov 17, 2020 • edited Loading

Problem

Solution

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jusiskin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jusiskin commented Nov 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jusiskin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jusiskin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kozlove-aws commented Nov 17, 2020 •

edited

Loading