Page MenuHomePhabricator

[services] [electron update server] Add a Dockerfile
ClosedPublic

Authored by michal on Jan 23 2023, 11:29 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 12, 9:23 PM
Unknown Object (File)
Fri, Nov 1, 1:53 PM
Unknown Object (File)
Fri, Nov 1, 1:53 PM
Unknown Object (File)
Fri, Nov 1, 1:53 PM
Unknown Object (File)
Fri, Nov 1, 1:52 PM
Unknown Object (File)
Fri, Nov 1, 1:50 PM
Unknown Object (File)
Fri, Nov 1, 1:49 PM
Unknown Object (File)
Fri, Nov 1, 1:29 PM
Subscribers

Details

Summary

For easy distribution we need to package the update server into a container like the rest of the services.

Test Plan

Check if the update server correctly runs inside docker

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

michal retitled this revision from [electron update server] Add a Dockerfile to [services] [electron update server] Add a Dockerfile.Jan 23 2023, 11:30 AM
services/electron-update-server/Dockerfile
11 ↗(On Diff #21225)

I think it would be better for this command to copy "everything else" so that if we introduce a new file, we don't need to update the Dockerfile to mention it

services/electron-update-server/Dockerfile
11 ↗(On Diff #21225)

Agreed, and use a .dockerignore to filter out what we don't want being copied.

Now we copy all files from the folder. Additionally, in the previous diff I changed the directory to be a yarn workspace member so now this dockerfile has to be build in the context of the root folder. I've added docker-compose.yml to make that explicit. Note that I don't copy package.json's from all of the workspace members to make the build faster. yarn seems to handle it fine and just ignore the non-existant workspace members. But we can do it more like the keyserver dockerfile and build the whole monorepo if that's prefered.

I'm surprised this works to be honest. If you're confident about it, I won't block you from landing. Would be good to understand how this is different from keyserver/Dockerfile, though

services/electron-update-server/Dockerfile
11 ↗(On Diff #21360)

Does this not crash because some of the workspaces are missing? keyserver/Dockerfile ends up needing changes any time a new workspace is added, which makes me wonder about this. Maybe --production or --frozen-lockfile make it work fine?

This revision is now accepted and ready to land.Jan 26 2023, 7:28 AM

Make sure to investigate CI failures before landing as well

I think I've figured out why it's working. The values in the workspaces in the root package.json can be glob patterns. So you can do something like this:

"workspaces": [
  "packages/*"
]

So if we just have for example "keyserver" in there it's treated as a glob pattern and it just doesn't match anything.


The build issues are from D6391, didn't have time to investigate them yet.

services/electron-update-server/Dockerfile
11 ↗(On Diff #21360)

yarn cleaninstall would probably crash because it calls the workspaces explicitly but yarn install works fine.

services/electron-update-server/Dockerfile
11 ↗(On Diff #21360)

Thanks for explaining, everything makes sense now

One thing to consider when running with --production is that we might some calls to devDependencies in postinstall hooks

services/electron-update-server/docker-compose.yml
7 ↗(On Diff #21360)

Nit, we should probably add newlines at the end of these files

Add newlines, remove --production.

Add native to the dockerfile, because the postinstall.sh requires native to work.