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

bacpop-145 Docker quick run #46

Merged
merged 7 commits into from
Feb 12, 2024
Merged

bacpop-145 Docker quick run #46

merged 7 commits into from
Feb 12, 2024

Conversation

EmmaLRussell
Copy link
Collaborator

@EmmaLRussell EmmaLRussell commented Feb 5, 2024

This branch adds a (hopefully) easy fully dockerised quick start option for local testing.

As described in the README, you should just need to run ./scripts/run_docker_decrypt to get things set up with secrets from the vault (you'll be prompted for your token) and then run the app. On subsequent runs, use ./scripts/run_docker to skip the decrypt step. The app should be available at https://localhost

I've created github and google OAuth apps to work with this set-up. For google, we need to specify test users, so let me know your email address that you use with Google if you find you can't sign in that way.

This branch currently targeting merge to bacpop-25.

@EmmaLRussell EmmaLRussell changed the base branch from main to bacpop-25 February 5, 2024 13:43
@@ -7,4 +7,4 @@ while [ ! -e $PATH_CONFIG ]; do
sleep 1
done

node dist/index.js --config /app/src/resources
ts-node --transpile-only src/index.ts --config src/resources
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Running direct from the js started throwing some fairly fundamental errors (struggling with import types). I think running ts-node should be ok with transpile-only to improve startup time.

Copy link
Contributor

Choose a reason for hiding this comment

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

is this just to be done so we can run it locally? because this works already in prod? sorry or im mistaken?

@@ -8,6 +8,5 @@
"GOOGLE_CLIENT_SECRET": "$GOOGLE_SECRET",
"SESSION_SECRET": "$EXPRESS_SESSION_SECRET",
"GITHUB_CLIENT_ID":"$GITHUB_ID",
"GITHUB_CLIENT_SECRET":"$GITHUB_SECRET",
"skip_auth": false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this flag as we weren't using it anywhere!

@EmmaLRussell EmmaLRussell marked this pull request as ready for review February 6, 2024 17:44
Copy link
Collaborator

@richfitz richfitz left a comment

Choose a reason for hiding this comment

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

I hit issues with the container names, which I've bypassed with this patch:

rfitzjoh@wpia-dide300:~/Documents/Projects/bacpop/beebop$ git diff 
diff --git a/scripts/run_docker b/scripts/run_docker
index de1a1a2..03be7a5 100755
--- a/scripts/run_docker
+++ b/scripts/run_docker
@@ -16,9 +16,9 @@ export HOST
 export GIT_SHA=$(git -C . rev-parse --short=7 HEAD)
 
 docker-compose up -d
-docker cp app/server/src/resources/config.json beebop_beebop-server_1:/app/src/resources/config.json
-docker cp proxy/ssl/dhparam.pem beebop_proxy_1:/run/proxy/
-docker cp proxy/$SSL_PATH/certificate.pem beebop_proxy_1:/run/proxy/
-docker cp proxy/$SSL_PATH/key.pem beebop_proxy_1:/run/proxy/
-docker run --rm -v beebop_beebop-storage:/beebop/storage mrcide/beebop-py:main \
+docker cp app/server/src/resources/config.json beebop-beebop-server-1:/app/src/resources/config.json
+docker cp proxy/ssl/dhparam.pem beebop-proxy-1:/run/proxy/
+docker cp proxy/$SSL_PATH/certificate.pem beebop-proxy-1:/run/proxy/
+docker cp proxy/$SSL_PATH/key.pem beebop-proxy-1:/run/proxy/
+docker run --rm -v beebop-beebop-storage:/beebop/storage mrcide/beebop-py:main \
        ./scripts/download_db --small storage

but then I hit an error

[+] Running 5/0
 ✔ Container beebop-beebop-redis-1      Running                                                                                                                                                                  0.0s 
 ✔ Container beebop-beebop-py-api-1     Running                                                                                                                                                                  0.0s 
 ✔ Container beebop-beebop-server-1     Running                                                                                                                                                                  0.0s 
 ✔ Container beebop-proxy-1             Running                                                                                                                                                                  0.0s 
 ✔ Container beebop-beebop-py-worker-1  Running                                                                                                                                                                  0.0s 
+++ docker cp app/server/src/resources/config.json beebop-beebop-server-1:/app/src/resources/config.json
Successfully copied 2.56kB to beebop-beebop-server-1:/app/src/resources/config.json
+++ docker cp proxy/ssl/dhparam.pem beebop-proxy-1:/run/proxy/
Successfully copied 2.56kB to beebop-proxy-1:/run/proxy/
+++ docker cp proxy/ssl/certificate.pem beebop-proxy-1:/run/proxy/
Successfully copied 3.58kB to beebop-proxy-1:/run/proxy/
+++ docker cp proxy/ssl/key.pem beebop-proxy-1:/run/proxy/
Successfully copied 5.12kB to beebop-proxy-1:/run/proxy/
+++ docker run --rm -v beebop-beebop-storage:/beebop/storage mrcide/beebop-py:main ./scripts/download_db --small storage
Downloading GPS_v4_references.tar.bz2
--2024-02-07 10:41:40--  https://sketchdb.blob.core.windows.net/public-dbs/GPS_v4_references.tar.bz2
Resolving sketchdb.blob.core.windows.net (sketchdb.blob.core.windows.net)... failed: Name or service not known.
wget: unable to resolve host address ‘sketchdb.blob.core.windows.net’

which does seem like a legit failure - any tips?

@@ -11,6 +12,9 @@ else
SSL_PATH=ssl
fi
export HOST

export GIT_SHA=$(git -C . rev-parse --short=7 HEAD)

docker-compose up -d
docker cp app/server/src/resources/config.json beebop_beebop-server_1:/app/src/resources/config.json
docker cp proxy/ssl/dhparam.pem beebop_proxy_1:/run/proxy/
Copy link
Collaborator

Choose a reason for hiding this comment

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

This failed for me; docker-compose has brought things up as beebop-proxy-1, etc.

I have

rfitzjoh@wpia-dide300:~/Documents/Projects/bacpop/beebop$ docker-compose --version
Docker Compose version v2.24.1

It looks like we can pass a flag in here to get this behaviour back: https://docs.docker.com/compose/migrate/#service-container-names

or we should probably update to use a newer docker-compose everwhere

@@ -11,6 +12,9 @@ else
SSL_PATH=ssl
fi
export HOST

export GIT_SHA=$(git -C . rev-parse --short=7 HEAD)
Copy link
Collaborator

Choose a reason for hiding this comment

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

docker pull mrcide/beebop-py:main

Copy link
Contributor

@absternator absternator left a comment

Choose a reason for hiding this comment

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

Just wondering what the purpose of this PR is? is it just to run the app locally in docker? If thats the case when would we want to run it in this fashion?

Also when i logged in i got a 502 bad gateway? maybe i was doing something wrong?

All else the code is great and love the config updates

@@ -7,4 +7,4 @@ while [ ! -e $PATH_CONFIG ]; do
sleep 1
done

node dist/index.js --config /app/src/resources
ts-node --transpile-only src/index.ts --config src/resources
Copy link
Contributor

Choose a reason for hiding this comment

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

is this just to be done so we can run it locally? because this works already in prod? sorry or im mistaken?

"server_url": "https://localhost/api",
"redis_url": "redis://beebop-redis:6379",
"GOOGLE_CLIENT_ID": "1234",
"GOOGLE_CLIENT_SECRET": "1234",
"GOOGLE_CLIENT_ID": "$DOCKER_GOOGLE_ID",
Copy link
Contributor

@absternator absternator Feb 7, 2024

Choose a reason for hiding this comment

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

nice 😄

@@ -11,6 +11,10 @@ FILE_CLEAR_SERVER=app/server/src/resources/config.json

export GITHUB_ID=$(vault read -field=clientid secret/beebop/auth/github)
export GITHUB_SECRET=$(vault read -field=secret secret/beebop/auth/github)
export DOCKER_GITHUB_ID=$(vault read -field=clientid secret/beebop/auth/devdocker/github)
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry just wondering what exactly is the DOCKER_ secrets? is this for local when we want to use docker?

@EmmaLRussell
Copy link
Collaborator Author

I've updated this for the latest docker compose, and also made it pull latest images, and remove volume on teardown.

@EmmaLRussell
Copy link
Collaborator Author

EmmaLRussell commented Feb 7, 2024

Just wondering what the purpose of this PR is? is it just to run the app locally in docker? If thats the case when would we want to run it in this fashion?

Probably not something you'll use when developing, but useful for a reviewer like Rich to spin up on a branch in a single line without having to faff around getting the front end and backend all installed and running separately, along with the deps.

Also when i logged in i got a 502 bad gateway? maybe i was doing something wrong?

I saw that when the server had failed to start one time. Maybe try again with the latest docker compose config (and check you're on a recent version of docker config)?

is this just to be done so we can run it locally? because this works already in prod? sorry or im mistaken?

I don't really know why this had changed, but I was getting runtime errors. We haven't tried to run in the rebuilt docker containers for a while, and as we've seen it runs fine on metal with ts-node so I guess it's some change with latest versions of dependencies, and this just seemed like the easiest way to harmonise with the development approach.

sorry just wondering what exactly is the DOCKER_ secrets? is this for local when we want to use docker?

Yeah - as with packet. we need different versions of the OAuth apps for running in prod and locally (inside and outside docker) because they need to redirect to different base urls when auth is complete (e.g. localhost or localhost:8080 or beebop.dide.ic.ac.uk).

Copy link
Contributor

@absternator absternator left a comment

Choose a reason for hiding this comment

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

Just wondering what the purpose of this PR is? is it just to run the app locally in docker? If thats the case when would we want to run it in this fashion?

Probably not something you'll use when developing, but useful for a reviewer like Rich to spin up on a branch in a single line without having to faff around getting the front end and backend all installed and running separately, along with the deps.

Also when i logged in i got a 502 bad gateway? maybe i was doing something wrong?

I saw that when the server had failed to start one time. Maybe try again with the latest docker compose config (and check you're on a recent version of docker config)?

is this just to be done so we can run it locally? because this works already in prod? sorry or im mistaken?

I don't really know why this had changed, but I was getting runtime errors. We haven't tried to run in the rebuilt docker containers for a while, and as we've seen it runs fine on metal with ts-node so I guess it's some change with latest versions of dependencies, and this just seemed like the easiest way to harmonise with the development approach.

sorry just wondering what exactly is the DOCKER_ secrets? is this for local when we want to use docker?

Yeah - as with packet. we need different versions of the OAuth apps for running in prod and locally (inside and outside docker) because they need to redirect to different base urls when auth is complete (e.g. localhost or localhost:8080 or beebop.dide.ic.ac.uk).

Copy link
Contributor

@absternator absternator left a comment

Choose a reason for hiding this comment

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

THanks for answering my questions makes sense now.. Also worked thanks this time 😄

@richfitz richfitz merged commit 12673ff into bacpop-25 Feb 12, 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.

3 participants