Page MenuHomePhabricator

[CI] Update `eslint_flow_jest` pipeline to enable autoscaling
ClosedPublic

Authored by atul on Jun 29 2022, 11:48 AM.
Tags
None
Referenced Files
F3506676: D4402.id13974.diff
Fri, Dec 20, 5:51 PM
F3505805: D4402.diff
Fri, Dec 20, 2:34 PM
Unknown Object (File)
Tue, Dec 17, 6:59 AM
Unknown Object (File)
Mon, Dec 16, 11:56 PM
Unknown Object (File)
Mon, Dec 16, 10:04 AM
Unknown Object (File)
Mon, Dec 16, 10:03 AM
Unknown Object (File)
Sun, Dec 15, 7:20 PM
Unknown Object (File)
Thu, Dec 5, 4:44 PM
Subscribers

Details

Summary

Run this workflow within node:16.13-bullseye container to enable autoscaling.

Test Plan

Buildkite CI

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.Jun 29 2022, 12:01 PM
.buildkite/eslint_flow_jest.yml
8 ↗(On Diff #13974)

Curious, why do we need Docker here?

.buildkite/eslint_flow_jest.yml
8 ↗(On Diff #13974)

Because we want the autoscaling instances to be as minimal as possible... basically want them to just have Docker.

And then we depend on the container (in this case node:16.13-bullseye) to have any necessary dependencies (in this case node, yarn, etc)

.buildkite/eslint_flow_jest.yml
8 ↗(On Diff #13974)

Basically what's happening here is the docker plugin lets us run the command(s) "within" a container created with the specified image on an autoscaling instance.

The plugin (first-party from buildkite) handles a lot of the complexity for us. We just specify what container the workflow should run "within," and it handles the rest.

.buildkite/eslint_flow_jest.yml
8 ↗(On Diff #13974)

Hmm, I'm still confused. Why are these lines here but not in D4405? Why does specifying a Docker plugin here result in the image "just" having Docker? What's the downside to using a default image that has other things in it?

8 ↗(On Diff #13974)

Commented before I saw your second comment. Looks like you answered question 2, but not question 1 or 3 in my most recent comment

.buildkite/eslint_flow_jest.yml
8 ↗(On Diff #13974)

Why are these lines here but not in D4405?

In D4405, we're running docker-compose build --no-cache tunnelbroker-server directly on an autoscaling instance that has docker already installed. Because there are no other dependencies, we can run it directly.

In this diff, we're running a series of yarn blah commands that depend on node/yarn which aren't on the autoscaling instance. So, we create an environment where those dependencies exist (node:16.13-bullseye with docker plugin) and then run the commands within that environment.

What's the downside to using a default image that has other things in it?

Basically comes down to being a cleaner abstraction and having better separation of concerns. We want the autoscaling instance to be as general purpose as possible and not have to worry what dependencies should be included/what versions of dependencies should be included/etc. Instead, we can push that down to be handled by docker/workflow and keep the autoscaling instances as workflow agnostic as possible.

Let's say that one day someone wants to introduce a service written in golang. We wouldn't want to then have to modify the default image to ensure that it has the golang toolchain (go, gofmt, etc) and then specifically the version of the golang toolchain that we need, etc. We also wouldn't want to have to modify the default image every time dependencies need updating/etc.

By keeping the default image as "dumb" as possible, we save ourselves from having another "thing" that needs to be actively tended to and maintained. The way it's set up now we can pretty much set it once and never have to think about it again.

GitHub Actions takes the opposite approach where they put everything under the sun in the "runner" image (https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-Readme.md)... which makes sense if you're running a CI service and are going to actively maintain things, but doesn't seem like the type of extra work we want to sign up for.

As a side benefit, keeping the instances as lightweight as possible makes it faster for them to spin up and start processing the workflows.

Thanks for taking the time to explain! It all makes sense to me now

This revision is now accepted and ready to land.Jun 30 2022, 6:58 AM

rebase after cherrypicking and before landin

This revision was landed with ongoing or failed builds.Jun 30 2022, 1:44 PM
This revision was automatically updated to reflect the committed changes.