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.

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

Fix docker build #801

Merged
merged 4 commits into from
Aug 23, 2024
Merged

Fix docker build #801

merged 4 commits into from
Aug 23, 2024

Conversation

snim2
Copy link
Collaborator

@snim2 snim2 commented Aug 22, 2024

FIX the broken build that can be seen on #800 and https://github.com/nhsx/nhsx-website/actions/runs/10491953953/job/29062294843?pr=800

  • Use docker compose where docker-compose is not available
  • Dockerfile uses sudo package to install envkey. The version of Alpine used as the base image here is pinned and the envkey install script has not changed in three years, so it is difficult to see how this broke. However, without installing the sudo package the sudo command in install.sh cannot run and the Dockerfile cannot be built. To ensure that the final image does not change we also remove the sudo package after running the install script.
  • Install poetry more defensively Avoid errors when building the Dockerfile by:
    • Creating directories and files that 'poetry install' expects
    • Adding poetry to the current $PATH
  • Remove deprecated 'version' from docker-compose

Testing

  1. Run ./script/test and check it runs green.
  2. Run ./script/server and check you can see a running site at http://localhost:5000

The version of Alpine used as the base image here is
pinned and the envkey install script has not changed
in three years, so it is difficult to see how this
broke. However, without installing the sudo package
the sudo command in install.sh cannot run and the
Dockerfile cannot be built.

To ensure that the final image does not change we also
remove the sudo package after running the install script.
Avoid errors when building the Dockerfile by:

* Creating directories and files that 'poetry install' expects
* Adding poetry to the current $PATH
@snim2 snim2 changed the title Fix/docker build Fix docker build Aug 22, 2024
@snim2 snim2 marked this pull request as ready for review August 22, 2024 13:27
@@ -71,7 +71,9 @@ RUN pip install --no-cache-dir \
FROM base AS app

# Install envkey-source
RUN curl -s https://raw.githubusercontent.com/envkey/envkey-source/master/install.sh | bash
RUN set -ex && apk add --no-cache sudo \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you set the docker user to root rather than using sudo?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah... it probably explicitly invokes sudo... sure.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't call USER at all in this file, so I'd assumed the user is already root (but I didn't check!)

@snim2 snim2 merged commit 64145c4 into dev Aug 23, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants