services: Remove gRPC source after installing
ClosedPublic
Actions

Authored by • jim on Mar 10 2022, 11:36 AM.

Details

Reviewers

• karol
ashoat
• max

Commits

rCOMM7f17a64e3155: services: Remove gRPC source after installing

Summary

This reduces size of the Docker base image from 3.03GB to 1.29GB.

Also:

services: Pass gRPC build flags at build time
services: Better Docker layering in base image
services: Remove unused gRPC build variable

Test Plan

Run npm run build-all and run-all

Diff Detail

Repository

rCOMM Comm

Branch

grpc-shrink

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jim created this revision.Mar 10 2022, 11:36 AM

Herald added subscribers: • benschac, atul, • adrian, tomek. · View Herald TranscriptMar 10 2022, 11:36 AM

Harbormaster completed remote builds in B7286: Diff 10274.Mar 10 2022, 12:17 PM

• jim requested review of this revision.Mar 10 2022, 12:17 PM

ashoat accepted this revision.Mar 10 2022, 8:05 PM

ashoat added inline comments.

services/base-image/contents/install_grpc.sh
30–31	Thanks for adding this comment

This revision is now accepted and ready to land.Mar 10 2022, 8:05 PM

What is the reason behind having grpc flags passed to the cmake in the bash script instead of having them directly in the cmake file?

services/backup/docker-server/contents/build_server.sh
5	What are these changes for? I think it's safer with `/`s

In D3399#92777, @karol-bisztyga wrote:

What is the reason behind having grpc flags passed to the cmake in the bash script instead of having them directly in the cmake file?

Because that is when gRPC is built. In the CMake file, it's just using the pre-installed gRPC library.

See comments here https://phabricator.ashoat.com/D3198?id=10038#inline-19455 and here https://linear.app/comm/issue/ENG-824#comment-6555f66b.

services/backup/docker-server/contents/build_server.sh
5	The changes are needed because I changed the WORKDIR in the base image from `/`. And I agree that it's preferable to change to an absolute path in this script OR in the backup service Dockerfile do `RUN cd /transferred/server && ../scripts/build-server.sh`. As in, if this script requires itself to be run from a certain place, that place should be the target directory, not the root directory.

Ah yes, sorry I get it now, it's alright!

services/backup/docker-server/contents/build_server.sh
5	I just prefer an absolute path, it's a matter of design I believe. Either you make it more error-resistant in terms of where the script is being run from or in terms of where exactly is the target path in the file system.