Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimise Dockerfile #390

Open
thewilkybarkid opened this issue Jul 22, 2021 · 0 comments
Open

Optimise Dockerfile #390

thewilkybarkid opened this issue Jul 22, 2021 · 0 comments
Assignees
@thewilkybarkid

Comments

@thewilkybarkid
Copy link
Member

The Docker setup is very slow locally, does too much, and is untested.
thewilkybarkid added a commit that referenced this issue Jul 22, 2021
@thewilkybarkid
build(docker): improve Docker start-up time locally
3948007 
Adds a .dockerignore file to avoid including (almost 1GB worth) of superfluous files in the Docker context. These files
are mostly the result of running the app outside of Docker (primarily the NPM dependencies and Parcel cache). On macOS
at least, this sees a dramatic speed increase.

Refs #390
@thewilkybarkid thewilkybarkid self-assigned this Jul 22, 2021
thewilkybarkid added a commit that referenced this issue Jul 22, 2021
@thewilkybarkid
ci(docker): use Docker BuildKit
d9c33c6 
Turns on Docker BuildKit in CI, which adds new features to the build environment. The most crucial part is that it's
faster, and allows for further performance optimisations.

Refs #390
thewilkybarkid added a commit that referenced this issue Jul 22, 2021
@thewilkybarkid
ci(docker): separate and name steps
4d3c3b2 
This will produce a clearer output in GitHub Actions.

Refs #390
thewilkybarkid added a commit that referenced this issue Jul 22, 2021
@thewilkybarkid
ci(docker): use Docker Compose to build image
6cf476f 
Standardises the method used to build the image, avoiding future naming conflicts and unnecessary rebuilds.

Refs #390
thewilkybarkid added a commit that referenced this issue Jul 22, 2021
@thewilkybarkid
ci(docker): smoke test the built image
c76d5fd 
After building the image, this adds a test that it actually runs and reports itself as healthy. This covers that the
HTTP server runs, not that the content inside (specifically the React application) is working.

Refs #390
thewilkybarkid added a commit that referenced this issue Jul 22, 2021
@thewilkybarkid
build(docker): allow the app to start without a ORCID client ID
b1ebcb2 
The smoke test broke in CI as the app won't start without the ORCID client ID set. This sets a dummy value so that it
can, thought the functionality won't work.

This wasn't caught locally as I have a local .env file, which sees different environment variables passed to the
application. There's no option for not reading from a .env file (docker/compose#6741), so
running the smoke test locally could see different behaviour.

Refs #390
thewilkybarkid added a commit that referenced this issue Jul 23, 2021
@thewilkybarkid
ci(docker): use existing application setting
1d511b0 
The application already has a PREREVIEW_ORCID_SANDBOX setting; the value in the YAML file was incorrect as it needs to
be a string rather than a boolean, so this removes it.

I'm not sure if this is the cause of the dev environment failing; I'd be surprised, but without feedback it's impossible
to tell.

Refs #390
thewilkybarkid added a commit that referenced this issue Jul 26, 2021
@thewilkybarkid
ci(docker): remove database volume
3de57cd 
Finally found in Azure the message "Custom storage volume(s) failed to initialize"; this removes the database volume
to see if it resolves the problem.

This does mean there's no mechanism to persist data between deploys, but given it's a development environment that's the
behaviour I'd expect.

Refs #390
thewilkybarkid added a commit that referenced this issue Jul 27, 2021
@thewilkybarkid
build(docker): add a dev target
27a01d9 
Creates a Docker target for development, which can be used to run both the site and tools.

The code is not yet mounted, so the watchers will not recognise changes.

Refs #390
thewilkybarkid added a commit that referenced this issue Jul 27, 2021
@thewilkybarkid
ci(docker): separate testing and linting from the build
7cca910 
Creates new CI steps to run the lint and test commands, rather than having them run inside the Docker build. This means
that the image can be built even if there are linting issues (which will be useful for local development), and that the
tests will be able to make use of other processes (and given the app has a reliance on React and Postgres, this is
vital).

Refs #388, #390
thewilkybarkid added a commit that referenced this issue Jul 27, 2021
@thewilkybarkid
build(docker): only copy required files
1bd9c73 
'COPY .' causes cache invalidation when any file within the repo is changed, whether they affect the build steps or not.
This means that, locally, layers constantly have to be rebuilt. Instead, this copies the files required at that step.

Refs #390
thewilkybarkid added a commit that referenced this issue Jul 27, 2021
@thewilkybarkid
build(docker): only copy the built files into the production image
86fd1e5 
Rather than copying all the files, this only copies the result of the build process (the 'dist' folder). This means that
the production dependencies have to be installed as a separate step.

This also optimises the use of npm by removing packages and caches not needed in subsequent steps.

Refs #390
thewilkybarkid added a commit that referenced this issue Aug 6, 2021
@thewilkybarkid
build(docker): allow the app to authenticate with the ORCID sandbox
79b8db0 
This sets up the environment variables required to log in with ORCID when running the app locally.

It relies on having a valid PREREVIEW_ORCID_CLIENT_ID and PREREVIEW_ORCID_CLIENT_SECRET in the
local environment.

Refs #388, #390
thewilkybarkid added a commit that referenced this issue Aug 9, 2021
@thewilkybarkid
build(docker): allow the dev environment to rebuild on code changes
29c4582 
This mounts the src directory as a volume, which allows the running watcher to recognise changes
and trigger rebuilds.

There's probably more that could be mounted as they are also watched (such as configuration files).

Refs #390
thewilkybarkid added a commit that referenced this issue Aug 10, 2021
@thewilkybarkid
build(make): encapsulate tasks in Make
7021c95 
Adds a Makefile for simplifying the use of potentially complicated commands in both CI and local development.

Refs #390, #395
thewilkybarkid added a commit that referenced this issue Aug 10, 2021
@thewilkybarkid
build(make): allow existing IMAGE_TAG to be used
1dbf8e6 
The IMAGE_TAG variable was accidentally overwritten in 7021c95, meaning that the first command that tried to use the real variable broke.

Refs #390, #395
thewilkybarkid added a commit that referenced this issue Aug 16, 2021
@thewilkybarkid
build(docker): stop the build step from knowing about code formatting
1884707 
Prettier is run on generated code, which means the build step has to be aware of the Prettier configuration. This tells Prettier to ignore the file since there's no need to format it.

Refs #390
thewilkybarkid added a commit that referenced this issue Aug 17, 2021
@thewilkybarkid
build(docker): require commands in the entry point script to pass
b835e5f 
By default, a shell script carries on even if a step fails. If, for example, a migration fails, the app starts up anyway with an unknown and possibly dangerous result.

This change causes a failure in any command to fail the whole script, except in cases where it's acceptable (as we test the exit code).

Refs #390, #395
thewilkybarkid added a commit that referenced this issue Aug 17, 2021
@thewilkybarkid
build(docker): split script, frontend and backend builds
17973a0 
A change to the frontend code doesn't require the scripts and backend to be built. This change splits the builds into separate Docker steps, which also allows them to be run in parallel.

Refs #390
thewilkybarkid added a commit that referenced this issue Aug 17, 2021
@thewilkybarkid
build(docker): remove debug step
791a2a5 
Accidentally included in 17973a0.

Refs #390
thewilkybarkid added a commit that referenced this issue Sep 10, 2021
@thewilkybarkid
build(docker): allow setting an administrator locally
17cce4e 
This change sets up the environment variable required to mark a user as an administrator.

It relies on having PREREVIEW_ADMIN_USERS in the local environment.

Refs #390
thewilkybarkid added a commit that referenced this issue Oct 1, 2021
@thewilkybarkid
build(docker): allow the app to use the Zenodo sandbox
60f53ae 
This change sets up the environment variables required to use the Zenodo sandbox so that full reviews can be published when running the app locally.

It relies on having a valid PREREVIEW_ZENODO_TOKEN in the local environment.

Refs #388, #390
thewilkybarkid added a commit that referenced this issue Nov 29, 2021
@thewilkybarkid
build: build all the scripts
8ba0510 
This change builds all the scripts in one go, which seems to be faster but also reduces the complexity of running them (since TypeScript is no longer needed).

Refs #390, #399
thewilkybarkid added a commit that referenced this issue Nov 29, 2021
@thewilkybarkid
test(integration): use prod-like image
72f1048 
As the startup scripts no longer depend on TypeScript, this change bases the integration-test image on the production one. As a result, there is less chance of mistakes happening, as it's as close as it can be.

Refs #390, 8ba0510
thewilkybarkid added a commit that referenced this issue Nov 29, 2021
@thewilkybarkid
build: remove the TypeScript dependency when creating migrations
b140f66 
Refs #390
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thewilkybarkid thewilkybarkid

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@thewilkybarkid