[CI] Add `blob_build` workflow to Buildkite/GH Actions
ClosedPublic
Actions

Authored by atul on Jun 28 2022, 11:45 PM.

Details

Reviewers

• jacek
tomek
• abosh
• karol

Commits

rCOMM82575f1efe0a: [CI] Add `blob_build` workflow to Buildkite/GH Actions

Summary

This was previously handled as part of the "Services CI" workflow that will be deprecated later in this stack. This diff allows us to increase concurrency as we move over to the autoscaling EC2 instances.

Differences on the Buildkite side:

We use the actual shell command instead of yarn build-blob-service because the autoscaling instances are intended to be as minimal as possible and don't include node, yarn, etc.

We use the --no-cache flag to force a new complete build on each run. The previous "Services CI" workflow depended heavily on caches, because running 3-4 complete buils would take a long time. Now, we can separate these out and take advantage of the concurrency provided by the autoscaling instances.

Depends on D4389

Test Plan

Buildkite: Copy/pasted from what I have in the Buildkite GUI that has been "active" for the last 2-3 days without issue.

GitHub: Will see what happens when it lands, but fairly confident it should be good to go.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Jun 28 2022, 11:45 PM

Herald added subscribers: • adrian, ashoat. · View Herald TranscriptJun 28 2022, 11:45 PM

Harbormaster completed remote builds in B10070: Diff 13924.Jun 28 2022, 11:58 PM

atul requested review of this revision.Jun 28 2022, 11:58 PM

tomek requested changes to this revision.Jun 29 2022, 8:20 AM

tomek added a reviewer: • karol.

tomek added a subscriber: • karol.

tomek added inline comments.

.buildkite/blob_build.yml
2 ↗	(On Diff #13924)	I think the same D4389 applies here @karol-bisztyga

This revision now requires changes to proceed.Jun 29 2022, 8:20 AM

• abosh added inline comments.Jun 29 2022, 8:23 AM

.buildkite/blob_build.yml
2 ↗	(On Diff #13924)	Yup, it would be `./scripts/run_server_image.sh "blob"`

atul added inline comments.Jun 29 2022, 11:40 AM

.buildkite/blob_build.yml
2 ↗	(On Diff #13924)	In addition to `docker-compose build $SERVICE-server`, doesn't this also invoke `docker-compose up $SERVICE-server`? We were previously running `yarn build-all` for the Services CI which was an "alias" for `docker-compose build`, is there a reason we want to change that now?

atul requested review of this revision.Jun 29 2022, 11:40 AM

atul added a child revision: D4403: [CI] Add `identity_build` workflow to Buildkite/GH Actions.Jun 29 2022, 12:01 PM

• abosh added inline comments.Jun 30 2022, 7:39 AM

.buildkite/blob_build.yml
2 ↗	(On Diff #13924)	You're right, I didn't realize that until just now looking at `run_server_image.sh`. Not only does it also invoke a `docker-compose up` command, it also runs without the `--no-cache` option. I think we can write a new script for this if we're re-running this command in so many other Buildkite workflows, though. Assuming it'll be quite similar to `run_server_image.sh` with the added options of running with flags (like `--no-cache`) and without the `docker-compose up` command. That is, we use this same command (`docker-compose build --no-cache`) in: D4389 D4390 D4403 D4405 We were previously running yarn build-all for the Services CI which was an "alias" for docker-compose build, is there a reason we want to change that now? `yarn build-all` doesn't run with the `--no-cache` flag I think. So this would need its own script, right?

@yayabosh, make sure you hit "Request Changes" when passing back a diff, even if you're not sure it requires changes! Otherwise, the diff won't be on @atul's queue

even if you're not sure it requires changes! Otherwise, the diff won't be on @atul's queue

Ah, that makes sense. "even if you're not sure it requires changes" was the only reason I was hesitant but I didn't realize it wouldn't pop into @atul's queue otherwise.

This revision now requires changes to proceed.Jun 30 2022, 9:01 AM

Not exactly sure what I need to address before this is good to land? Is there something actionable I can do to push this forward?

@yayabosh seems to have asked you a question? Maybe respond to his comment? I haven't read the comment so don't have full context...

I guess you're right about that, since the scope of this diff is only to get the workflow added! The only thing I would consider is @palys-swm and @karol-bisztyga both were wondering if we could use run_server_image.sh for the command. You noted that the run_server_image.sh script has an extra command and I noted that it doesn't run with the --no-cache flag. But their concern is still valid since the command is repeated many times over a lot of the service workflows. I guess for the future, for all the workflow diffs you put up with that command we should write a script, right? But I wasn't sure if that was something we all agreed on, so I wanted your input, hence the request changes so you'd see the inline comment in your queue.

So this would need its own script, right?

I don't think so. I think docker-compose build --no-cache {service} is already clear and concise enough. I don't see any benefit to adding another level of indirection by writing a separate script?

That does make sense. I think the only reason I'm suggesting it is because the old services CI used the run_server_image.sh script, so the new one here can also use its own script. But maybe since this is only one command (versus run_server_image.sh being two) it is not a big deal.

The only thing I would consider is @palys-swm and @karol-bisztyga both were wondering if we could use run_server_image.sh for the command

Not sure I understand that. The point of these workflows is to build each service to ensure that they build. ./scripts/run_server_image.sh just starts the service after it's built and runs indefinitely...?

tomek accepted this revision.Jul 1 2022, 3:20 AM

tomek added inline comments.

.buildkite/blob_build.yml
2 ↗	(On Diff #13924)	It sounds reasonable to not run the container when we only want to build it. @karol-bisztyga can we use `docker-compose` or should we create a separate script for it?