Page MenuHomePhabricator

[services] Remove dependency sources from Docker image
ClosedPublic

Authored by jim on Mar 10 2022, 1:25 PM.
Tags
None
Referenced Files
F3377468: D3403.id10504.diff
Wed, Nov 27, 5:51 AM
Unknown Object (File)
Tue, Nov 19, 5:52 AM
Unknown Object (File)
Tue, Nov 19, 4:41 AM
Unknown Object (File)
Thu, Nov 14, 8:52 AM
Unknown Object (File)
Fri, Nov 8, 10:39 PM
Unknown Object (File)
Fri, Nov 8, 8:08 PM
Unknown Object (File)
Sat, Nov 2, 9:53 AM
Unknown Object (File)
Thu, Oct 31, 9:29 PM

Details

Summary

The dependency libraries are built and installed to system directories (eg. /usr/local/lib). The source code is not needed and just bloats the Docker image.

This change reduces the size of the tunnelbroker-server image from 5.37GB to 3.52GB.

Test Plan

npm run build-all

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat added inline comments.
services/backup/docker-base/contents/install_aws_sdk.sh
5–9 ↗(On Diff #10280)

Assume these conditions are safe to remove because proper build caching will prevent these build scripts from being re-run?

This revision is now accepted and ready to land.Mar 10 2022, 8:08 PM
services/backup/docker-base/contents/install_aws_sdk.sh
5–9 ↗(On Diff #10280)

Not even a caching thing, it's just that Docker images are built by running each step once, in order. If the package is already there for some reason, the whole install step should be removed from the Dockerfile. Basically this is a "runtime" check happening at build time.

services/backup/docker-base/contents/install_aws_sdk.sh
5–9 ↗(On Diff #10280)

I'm a bit confused, any resources you can link with more context?

What I'm confused about:

If the package is already there for some reason, the whole install step should be removed from the Dockerfile

The package already being there depends on install_aws_sdk.sh being run, right? But if the install step is removed from the Dockerfile, isn't it the case that install_aws_sdk.sh will never be run?

services/backup/docker-base/contents/install_aws_sdk.sh
5–9 ↗(On Diff #10280)

Yeah, I'm just saying I think it's fine to assume it isn't being run twice because they would be fairly obvious from looking at the Dockerfiles. But if we were concerned, the right way to do it now would be to use pkg-config to find the library and abort with an error if it exists so the dev can fix in the build.

services/backup/docker-base/contents/install_aws_sdk.sh
5–9 ↗(On Diff #10280)

I think my confusion here is because there's two possible ways to interpret what this condition was for:

  1. Preventing the build from compiling the library twice if install_aws_sdk.sh somehow got run twice for a single build
  2. Preventing the build from recompiling the library if some build cache was invalidated but this library didn't change

I guess you're saying this wouldn't work for 2, so it's really only about 1. And 1 is probably not an issue because it'd be "fairly obvious", but if it was an issue then the right solution would be pkg-config, not keeping the source files lying around. Right?

services/backup/docker-base/contents/install_aws_sdk.sh
5–9 ↗(On Diff #10280)

Right

karol added inline comments.
services/backup/docker-base/contents/install_aws_sdk.sh
8–9 ↗(On Diff #10280)

why the _ underscore is still here?

services/tunnelbroker/docker/install_aws_sdk.sh
8–9 ↗(On Diff #10280)

_?

jim added inline comments.
services/backup/docker-base/contents/install_aws_sdk.sh
8–9 ↗(On Diff #10280)

No good reason, I'll change it to build.

  • Rename build dir from _build to build
This revision is now accepted and ready to land.Mar 15 2022, 10:35 AM