[services][blob] Use Rust implementation in Dockerfile
ClosedPublic
Actions

Authored by bartek on Nov 25 2022, 1:53 AM.

Details

Reviewers

varun
• jon
ashoat
tomek

Commits

rCOMM71d6e99debd9: [services][blob] Use Rust implementation in Dockerfile

Summary

Resolves ENG-2305

I've decided to use a multi-staged build is described in the above Linear issue. In short, Tonic >= 0.8 requires both rust and protobuf to be installed, and in other configurations the image is either huge or long to build.

Other minor changes:

Added Rust target directory to dockerignore
Modified the docker-compose volumes to load AWS credentials appropriately
Fixed the Localstack URL - it should look like this from the begginning (see other services)
Deleted the links entry in Cargo.toml as we don't link any native library here and release build fails.

Test Plan

Locally, e.g. by running:

cd services
yarn init-local-cloud
yarn run-blob-service-in-sandbox

# in another terminal
yarn run-integration-tests blob

CI builds Blob docker image successfully

Other CI jobs might fail as I haven't updated test workflows (they expect C++ test build) but they don't seem to be triggered (e.g. .buildkite/blob_unittests.yml)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Nov 25 2022, 1:53 AM

bartek held this revision as a draft.

Herald added a subscriber: atul. · View Herald TranscriptNov 25 2022, 1:53 AM

Harbormaster completed remote builds in B13747: Diff 18830.Nov 25 2022, 2:04 AM

bartek edited the test plan for this revision. (Show Details)Nov 25 2022, 2:04 AM

bartek published this revision for review.Nov 25 2022, 2:08 AM

I wonder if we could just use D5727 in place of this? Don't want to hold up landing the goal before the end of the month, though!

bartek edited parent revisions, added: D5728: [services][blob] Upload 2/2 - Handle data chunks; removed: D5703: [services][blob] Upload 1/2 - Introduce Put handlers.Nov 28 2022, 2:25 AM

• jon requested changes to this revision.Nov 28 2022, 8:44 AM

• jon added inline comments.

services/blob/Dockerfile
20 ↗	(On Diff #18830)	subsequent calls to `rm` don't actually reduce image size, since a previous layer will already have added the files. And a docker image is just the culmination of all layers (plus some metadata). If you do want to do size reduction, then you need to need to run all the commands within a single RUN docker command (e.g. `RUN cargo build --release && rm ./target/release/deps/*)
services/blob/src/constants.rs
5 ↗	(On Diff #18830)	Is this intentional? you would have to setup a localstack host entry for this to work, and I think `localhost` is what you would want anyway.

This revision now requires changes to proceed.Nov 28 2022, 8:44 AM

bartek added inline comments.Nov 28 2022, 8:49 AM

services/blob/Dockerfile
20 ↗	(On Diff #18830)	Right, my oversight
services/blob/src/constants.rs
5 ↗	(On Diff #18830)	Is this intentional? Yes, it is. Mentioned it in the description - this is the localstack hostname in docker-compose network. (Related ENG-2310).