otp-runner deployment #316

evansiroky · 2020-05-24T07:08:23Z

Checklist

Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented
The description lists all applicable issues this PR seeks to resolve (fixes Refactor ec2 startup script (potentially into a new project into a scripting language) #289)
The description lists any configuration setting(s) that differ from the default settings
All tests and CI builds passing

Description

This PR changes ec2 deployments to use otp-runner instead of completely bash-inlined in java for ec2 startup scripts. Also, the logic to create bundles and upload them has been moved to otp-runner. This has a number of advantages:

Since the bundles are now created on a separate machine, datatools-server won't crash if multiple large deployments are initiated at the same time.
The asynchronous execution of commands from otp-runner should result in a shorter time before graph building starts
otp-runner is able to easily write a full JSON status file and keep track of many other items that would be much more difficult to do with bash
otp-runner is thoroughly tested, whereas the bash-in-java had zero testing.

In order to run this, the ec2 images need node.js and yarn installed.

codecov-commenter · 2020-05-24T07:12:55Z

Codecov Report

Merging #316 into dev will decrease coverage by 0.10%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##                dev     #316      +/-   ##
============================================
- Coverage     19.91%   19.80%   -0.11%     
- Complexity      410      412       +2     
============================================
  Files           151      153       +2     
  Lines          8331     8417      +86     
  Branches       1157     1165       +8     
============================================
+ Hits           1659     1667       +8     
- Misses         6523     6601      +78     
  Partials        149      149

Flag	Coverage Δ	Complexity Δ
#unit_tests	`19.80% <0.00%> (-0.11%)`	`412.00 <0.00> (+2.00)`	⬇️

Impacted Files	Coverage Δ	Complexity Δ
...com/conveyal/datatools/manager/jobs/DeployJob.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...datatools/manager/jobs/MonitorServerStatusJob.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...eyal/datatools/manager/jobs/OtpRunnerManifest.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...nveyal/datatools/manager/jobs/OtpRunnerStatus.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
.../conveyal/datatools/manager/models/Deployment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...veyal/datatools/manager/persistence/FeedStore.java	`27.20% <0.00%> (-0.22%)`	`13.00 <0.00> (ø)`
...com/conveyal/datatools/manager/models/Project.java	`23.33% <0.00%> (-6.67%)`	`3.00% <0.00%> (+1.00%)`	⬇️
...ols/manager/controllers/api/ProjectController.java	`11.44% <0.00%> (-0.97%)`	`3.00% <0.00%> (+1.00%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ac5c3d...9a2ef56. Read the comment docs.

landonreed

Again, really good stuff. I'm really excited to move this code into the dev environment and give it a whirl. Just a few minor changes/comments.

src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java

src/main/java/com/conveyal/datatools/manager/jobs/MonitorServerStatusJob.java

landonreed · 2020-06-01T14:06:22Z

src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java

+        // NOTE: user data output is logged to `/var/log/cloud-init-output.log` automatically with ec2 instances
+        // Add some items to the $PATH as the $PATH with user-data scripts differs from the ssh $PATH.
+        lines.add("export PATH=\"$PATH:/home/ubuntu/.yarn/bin\"");
+        lines.add("export PATH=\"$PATH:/home/ubuntu/.nvm/versions/node/v12.16.3/bin\"");


Should the node version be hard-coded like this?

When adding it to the PATH variable, yes.

@evansiroky, I'm more so asking should we bump the node version out into its own variable (and then string formatting the export PATH line) just for visibility and easy access.

src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java

src/main/java/com/conveyal/datatools/manager/jobs/OtpRunnerManifest.java

src/main/java/com/conveyal/datatools/manager/jobs/OtpRunnerStatus.java

src/main/java/com/conveyal/datatools/manager/models/Deployment.java

See #316

landonreed

Thanks for the changes, @evansiroky. Just a couple of minor follow ups, but I think this is pretty much ready.

evansiroky · 2020-06-13T07:22:40Z

@landonreed see latest commit. I hope it resolves your concerns.

binh-dam-ibigroup

I am leaning towards extracting the manifest generation into its own method and rename the big
constructUserData method as a result.

src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java

evansiroky · 2020-07-01T18:12:00Z

Let me know how strongly you feel about your recommendations to change things, @binh-dam-ibigroup. I think it might be fine to leave things as they are.

landonreed · 2020-07-06T18:36:30Z

🎉 This PR is included in version 3.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

evansiroky added 4 commits May 9, 2020 01:04

refactor(deployment): WIP on deploying to ec2 using otp-runner

0dc0d3c

refactor(deployment): WIP on deploying to ec2 using otp-runner

b19e011

Merge branch 'dev' into otp-runner-deployment

7d4a53f

refactor(deployment): otp-runner working as of this commit!

60eb77e

evansiroky assigned landonreed and binh-dam-ibigroup May 24, 2020

landonreed suggested changes Jun 1, 2020

View reviewed changes

landonreed assigned evansiroky and unassigned landonreed Jun 1, 2020

refactor(deployment): refactor a few things for PR review

4631a27

See #316

evansiroky assigned landonreed and unassigned evansiroky Jun 12, 2020

landonreed approved these changes Jun 12, 2020

View reviewed changes

landonreed assigned evansiroky and unassigned landonreed Jun 12, 2020

refactor(deploy): improve comment and node version setting

9a2ef56

evansiroky removed their assignment Jun 13, 2020

binh-dam-ibigroup requested changes Jun 16, 2020

View reviewed changes

src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java Show resolved Hide resolved

src/main/java/com/conveyal/datatools/manager/jobs/DeployJob.java Show resolved Hide resolved

binh-dam-ibigroup assigned evansiroky and unassigned binh-dam-ibigroup Jun 16, 2020

evansiroky assigned binh-dam-ibigroup and unassigned evansiroky Jul 1, 2020

binh-dam-ibigroup approved these changes Jul 1, 2020

View reviewed changes

evansiroky merged commit b771ff9 into dev Jul 5, 2020

evansiroky deleted the otp-runner-deployment branch July 5, 2020 17:43

landonreed added the released label Jul 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

otp-runner deployment #316

otp-runner deployment #316

evansiroky commented May 24, 2020

codecov-commenter commented May 24, 2020 •

edited

Loading

landonreed left a comment

landonreed Jun 1, 2020

evansiroky Jun 12, 2020

landonreed Jun 12, 2020

landonreed left a comment

evansiroky commented Jun 13, 2020

binh-dam-ibigroup left a comment

evansiroky commented Jul 1, 2020

landonreed commented Jul 6, 2020

otp-runner deployment #316

otp-runner deployment #316

Conversation

evansiroky commented May 24, 2020

Checklist

Description

codecov-commenter commented May 24, 2020 • edited Loading

Codecov Report

landonreed left a comment

Choose a reason for hiding this comment

landonreed Jun 1, 2020

Choose a reason for hiding this comment

evansiroky Jun 12, 2020

Choose a reason for hiding this comment

landonreed Jun 12, 2020

Choose a reason for hiding this comment

landonreed left a comment

Choose a reason for hiding this comment

evansiroky commented Jun 13, 2020

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

evansiroky commented Jul 1, 2020

landonreed commented Jul 6, 2020

codecov-commenter commented May 24, 2020 •

edited

Loading