chore(staging): isolate apps VM stack for whatsapp feature work - #42
chore(staging): isolate apps VM stack for whatsapp feature work#42mikezio wants to merge 1 commit into
Conversation
Summary of ChangesHello @mikezio, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes a robust and isolated staging environment designed specifically for feature development, such as the upcoming WhatsApp functionality. By introducing a dedicated Docker Compose stack and helper scripts, developers can now work on new features without impacting existing environments, ensuring a cleaner and more controlled development workflow. The changes also include comprehensive documentation to guide users through the setup and management of this new isolated staging setup. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively isolates a staging environment by adding a new Docker Compose stack, helper scripts, and documentation. This is a great initiative for safer feature development. My review focuses on improving the maintainability and portability of the new configuration. The main suggestions are to avoid hardcoded paths and IP addresses in the compose file and scripts by using environment variables or relative paths, and to reduce configuration duplication between the production and staging compose files. These changes will make the setup more robust and easier to manage.
| context: /srv/apps/instalab-staging | ||
| container_name: instalab-api-staging | ||
| user: "1001:1003" | ||
| command: ddtrace-run python server.py | ||
| env_file: | ||
| - /srv/secrets/instalab-staging.env | ||
| environment: | ||
| INSTALAB_ENV: /srv/secrets/instalab-staging.env | ||
| INSTALAB_DB_HOST: apps-postgres | ||
| HOME: /data/instalab/home | ||
| DD_SERVICE: instalab-api-staging | ||
| DD_ENV: homelab-staging | ||
| DD_VERSION: "1" | ||
| DD_TRACE_ENABLED: "false" | ||
| DD_DYNAMIC_INSTRUMENTATION_ENABLED: "false" | ||
| DD_TRACE_EXCLUDED_URLS: "/api/health,/api/health/detail" | ||
| DD_EXCEPTION_REPLAY_ENABLED: "false" | ||
| DD_LOGS_INJECTION: "false" | ||
| DD_RUNTIME_METRICS_ENABLED: "false" | ||
| volumes: | ||
| - /srv/apps/instalab-staging/app:/app | ||
| - /srv/secrets/instalab-staging.env:/srv/secrets/instalab-staging.env:ro | ||
| - /srv/secrets/instalab-staging-logins.json:/srv/secrets/instalab-logins.json | ||
| - /srv/secrets/instalab-staging-cookies:/data/instalab/cookies | ||
| - /srv/secrets/instalab-staging-private:/data/instalab/private | ||
| - /srv/data/instalab-staging:/data/instalab | ||
| ports: | ||
| - "192.168.4.30:5100:5000" |
There was a problem hiding this comment.
This Docker Compose file contains hardcoded absolute paths (e.g., /srv/apps/instalab-staging, /srv/secrets/..., /srv/data/...) and a hardcoded IP address (192.168.4.30). This makes the configuration brittle and not portable, as it's tied to a specific host machine setup.
Consider using environment variables with default values to make these paths and the IP configurable. This would improve maintainability and reusability.
For example:
services:
instalab-api-staging:
build:
context: ${APP_ROOT:-/srv/apps/instalab-staging}
...
ports:
- "${HOST_IP:-192.168.4.30}:5100:5000"
volumes:
- ${APP_ROOT:-/srv/apps/instalab-staging}/app:/app
- ${SECRETS_ROOT:-/srv/secrets}/instalab-staging.env:/srv/secrets/instalab-staging.env:ro
# ... and so on for other pathsThese variables could then be defined in an .env file.
| services: | ||
| instalab-api-staging: | ||
| build: | ||
| context: /srv/apps/instalab-staging | ||
| container_name: instalab-api-staging | ||
| user: "1001:1003" | ||
| command: ddtrace-run python server.py | ||
| env_file: | ||
| - /srv/secrets/instalab-staging.env | ||
| environment: | ||
| INSTALAB_ENV: /srv/secrets/instalab-staging.env | ||
| INSTALAB_DB_HOST: apps-postgres | ||
| HOME: /data/instalab/home | ||
| DD_SERVICE: instalab-api-staging | ||
| DD_ENV: homelab-staging | ||
| DD_VERSION: "1" | ||
| DD_TRACE_ENABLED: "false" | ||
| DD_DYNAMIC_INSTRUMENTATION_ENABLED: "false" | ||
| DD_TRACE_EXCLUDED_URLS: "/api/health,/api/health/detail" | ||
| DD_EXCEPTION_REPLAY_ENABLED: "false" | ||
| DD_LOGS_INJECTION: "false" | ||
| DD_RUNTIME_METRICS_ENABLED: "false" | ||
| volumes: | ||
| - /srv/apps/instalab-staging/app:/app | ||
| - /srv/secrets/instalab-staging.env:/srv/secrets/instalab-staging.env:ro | ||
| - /srv/secrets/instalab-staging-logins.json:/srv/secrets/instalab-logins.json | ||
| - /srv/secrets/instalab-staging-cookies:/data/instalab/cookies | ||
| - /srv/secrets/instalab-staging-private:/data/instalab/private | ||
| - /srv/data/instalab-staging:/data/instalab | ||
| ports: | ||
| - "192.168.4.30:5100:5000" | ||
| restart: unless-stopped | ||
| healthcheck: | ||
| test: | ||
| [ | ||
| "CMD", | ||
| "python", | ||
| "-c", | ||
| "import urllib.request; urllib.request.urlopen('http://127.0.0.1:5000/api/health', timeout=5).read()", | ||
| ] | ||
| interval: 30s | ||
| timeout: 5s | ||
| retries: 3 | ||
| start_period: 20s | ||
| networks: | ||
| - apps-db | ||
|
|
||
| instalab-ui-staging: | ||
| build: | ||
| context: /srv/apps/instalab-staging | ||
| container_name: instalab-ui-staging | ||
| user: "1001:1003" | ||
| command: /app/scripts/start_ui.sh | ||
| env_file: | ||
| - /srv/secrets/instalab-staging.env | ||
| environment: | ||
| INSTALAB_ENV: /srv/secrets/instalab-staging.env | ||
| INSTALAB_API_BASE: http://instalab-api-staging:5000/ | ||
| INSTALAB_DB_HOST: apps-postgres | ||
| DD_SERVICE: instalab-ui-staging | ||
| DD_ENV: homelab-staging | ||
| DD_VERSION: "1" | ||
| DD_TRACE_ENABLED: "false" | ||
| DD_DYNAMIC_INSTRUMENTATION_ENABLED: "false" | ||
| DD_TRACE_EXCLUDED_URLS: "/healthz/" | ||
| DD_EXCEPTION_REPLAY_ENABLED: "false" | ||
| DD_LOGS_INJECTION: "false" | ||
| DD_RUNTIME_METRICS_ENABLED: "false" | ||
| volumes: | ||
| - /srv/apps/instalab-staging/app:/app | ||
| - /srv/secrets/instalab-staging.env:/srv/secrets/instalab-staging.env:ro | ||
| ports: | ||
| - "127.0.0.1:8102:8000" | ||
| depends_on: | ||
| - instalab-api-staging | ||
| restart: unless-stopped | ||
| healthcheck: | ||
| test: | ||
| [ | ||
| "CMD", | ||
| "python", | ||
| "-c", | ||
| "import urllib.request; urllib.request.urlopen('http://127.0.0.1:8000/healthz/', timeout=5).read()", | ||
| ] | ||
| interval: 30s | ||
| timeout: 5s | ||
| retries: 3 | ||
| start_period: 20s | ||
| networks: | ||
| - apps-db | ||
|
|
||
| networks: | ||
| apps-db: | ||
| external: true | ||
| name: apps-db |
There was a problem hiding this comment.
This file is almost a complete copy of docker-compose.yml, with only names, ports, and a few environment variables changed. This duplication can lead to maintenance issues, as changes to one service definition might need to be manually synchronized with the other.
To reduce duplication and improve maintainability, consider using Docker Compose's extends feature or YAML anchors and aliases. You could define a base service in a common file and then extend it in docker-compose.yml and docker-compose.staging.yml to apply environment-specific configurations.
| @@ -0,0 +1,4 @@ | |||
| #!/usr/bin/env bash | |||
| set -euo pipefail | |||
| cd /srv/apps/instalab-staging | |||
There was a problem hiding this comment.
The script hardcodes the path /srv/apps/instalab-staging. This makes the script less portable and harder to use in different environments (e.g., local development). It's better to determine the project root relative to the script's location to make it runnable from anywhere.
| cd /srv/apps/instalab-staging | |
| cd "$(dirname "${BASH_SOURCE[0]}")/.." |
| @@ -0,0 +1,4 @@ | |||
| #!/usr/bin/env bash | |||
| set -euo pipefail | |||
| cd /srv/apps/instalab-staging | |||
There was a problem hiding this comment.
The script hardcodes the path /srv/apps/instalab-staging. This makes the script less portable and harder to use in different environments (e.g., local development). It's better to determine the project root relative to the script's location to make it runnable from anywhere.
| cd /srv/apps/instalab-staging | |
| cd "$(dirname "${BASH_SOURCE[0]}")/.." |
| @@ -0,0 +1,4 @@ | |||
| #!/usr/bin/env bash | |||
| set -euo pipefail | |||
| cd /srv/apps/instalab-staging | |||
There was a problem hiding this comment.
The script hardcodes the path /srv/apps/instalab-staging. This makes the script less portable and harder to use in different environments (e.g., local development). It's better to determine the project root relative to the script's location to make it runnable from anywhere.
| cd /srv/apps/instalab-staging | |
| cd "$(dirname "${BASH_SOURCE[0]}")/.." |
Summary
docker-compose.staging.yml) for feature workscripts/staging-up.sh,staging-status.sh,staging-down.sh).env.staging.examplewith non-secret template valuesREADME.mdIsolation guarantees
instalab-api-staging,instalab-ui-staging5100, UI8102/srv/secrets/instalab-staging.*/srv/data/instalab-staginginstalab_stagingValidation on apps VM
instalab-api-stagingon192.168.4.30:5100instalab-ui-stagingon127.0.0.1:81025000/8002runs=0,schedules=0)