Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the MINT deployment from the legacy mint-v1 setup to the new mint-v2 architecture, introducing a modern containerized deployment with simplified proxy configuration.
- Replaces the old mint/mintr services with new mint-frontend and mintr containers from GitHub Container Registry
- Switches from nginx proxy to Caddy for simpler SSL and reverse proxy configuration
- Updates deployment scripts to remove git repository management and simplify the deployment process
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| mint-deploy | Refactored deployment script to remove git operations and simplify service management |
| docker-compose.yml | Removed legacy compose file for mint-v1 services |
| conf/Caddyfile | Added new Caddy configuration for reverse proxy with SSL termination |
| compose.yml | New compose file defining mint-v2 services including Redis, frontend, and Caddy proxy |
| README.md | Updated documentation to reflect new environment variables and deployment process |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
…ploy into mrc-6649-mint-v2
absternator
requested review from
Copilot and
plietar
and removed request for
plietar
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
plietar
left a comment
plietar
left a comment
There was a problem hiding this comment.
This all seems reasonable to me
README.md
Outdated
|
|
||
| 1. Log into Vault | ||
| 1. `echo SERVER=mint >.env` (or `mint-dev` for staging server - this matches path in Vault and filter in Kibana) | ||
| 1. `echo DOMAIN=mint-dev.dide.ic.ac.uk >.env` (use `mint-dev.dide.ic.ac.uk` for staging, or `mint.dide.ic.ac.uk` for production; this matches path in Vault and filter in Kibana) |
There was a problem hiding this comment.
| 1. `echo DOMAIN=mint-dev.dide.ic.ac.uk >.env` (use `mint-dev.dide.ic.ac.uk` for staging, or `mint.dide.ic.ac.uk` for production; this matches path in Vault and filter in Kibana) | |
| 1. `echo DOMAIN=mint-dev.dide.ic.ac.uk >.env` (use `mint-dev.dide.ic.ac.uk` for staging, or `mint.dide.ic.ac.uk` for production) |
compose.yml
Outdated
| env_file: | ||
| - .env |
There was a problem hiding this comment.
Is this needed? I thought docker-compose already loads .env by default?
I guess the .env is only available automatically in the YAML to the container, and docker-compose still needs to forward the env vars to the containers. Maybe it is clearer to use environment: [ DOMAIN ] instead
compose.yml
Outdated
| env_file: | ||
| - .env |
There was a problem hiding this comment.
Same comment as the proxy container
compose.yml
Outdated
| env_file: | ||
| - .env |
| FRONTEND_REF=mrc-2186 | ||
| API_REF=mrc-2186 | ||
| EOF | ||
| ``` |
There was a problem hiding this comment.
| FRONTEND_REF=mrc-2186 | |
| API_REF=mrc-2186 | |
| EOF | |
| ``` | |
| FRONTEND_REF=mrc-2186 | |
| API_REF=mrc-2186 |
That command is a bit misleading because it will overwrite the DOMAIN that is set. I would remove the actual command part of it and just show the contents that need to be added to the file. It is straightforward to figure out that the file needs to be edited.
plietar
left a comment
plietar
left a comment
There was a problem hiding this comment.
Could you add a note in the README about using ACME_BUDDY_STAGING=1 when running locally? This makes sure we get test certificates that aren't accepted by browsers. It also avoids running into rate limits with Let's Encrypt.
See for example https://github.com/jameel-institute/daedalus-deploy and https://github.com/mrc-ide/wodin-epimodels?tab=readme-ov-file#deploying-for-the-first-time
Sorry I thought I had included this in my original review but it seems I forgot.
compose.yml
Outdated
| - 80:80 | ||
| - 443:443 | ||
| environment: | ||
| - DOMAIN=${DOMAIN} |
There was a problem hiding this comment.
| - DOMAIN=${DOMAIN} | |
| - DOMAIN |
This does the same thing right?
…DOMAIN environment variable in compose.yml
Set up new mint-deploy for new mint-v2. The Docker Compose is similar to the one found in mint-v2.
Testing:
The app is deployed on mint-dev
UPDATE: using nginx (sadge) 😢 😭 👎 📛 😨 😿