Add RESET option to patchdeploy.sh + switch to service names #11178
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the patch deployment script by adding container reset functionality and switching from container-specific names to service-based deployment. The changes improve flexibility for testing monitoring changes and allow patching specific commits within PRs.
- Add
RESET=trueenvironment variable to recreate containers before applying patches - Replace container names (
CONTAINER) with service names (SERVICE) for better Docker Compose integration - Support direct diff/patch URLs in addition to PR numbers
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| echo " Default: container unless patching www nginx changes" | ||
| echo " CONTAINER=container_name Default: openlibrary-web-1 for ol-web hosts," | ||
| echo " openlibrary-web_nginx-1 for ol-www0" | ||
| echo " SERVICE=container_name Default: web for ol-web hosts, web_nginx for ol-www0" |
There was a problem hiding this comment.
The help text description 'container_name' is misleading since this parameter now expects service names, not container names. Consider changing to 'SERVICE=service_name' for clarity.
| echo " SERVICE=container_name Default: web for ol-web hosts, web_nginx for ol-www0" | |
| echo " SERVICE=service_name Default: web for ol-web hosts, web_nginx for ol-www0" |
| echo -n " recreating ... " | ||
| ssh ${host}.us.archive.org " | ||
| export COMPOSE_FILE='/opt/openlibrary/compose.yaml:/opt/openlibrary/compose.production.yaml' | ||
| export HOSTNAME=\$HOSTNAME |
There was a problem hiding this comment.
The same environment variable exports are duplicated in three locations. Consider extracting these to a variable or function to reduce code duplication and improve maintainability.
| export HOSTNAME=\$HOSTNAME | |
| ${COMMON_EXPORTS} |
| ssh ${host}.us.archive.org " | ||
| docker exec ${CONTAINER} bash -c ' | ||
| export COMPOSE_FILE='/opt/openlibrary/compose.yaml:/opt/openlibrary/compose.production.yaml' | ||
| export HOSTNAME=\$HOSTNAME |
There was a problem hiding this comment.
The same environment variable exports are duplicated in three locations. Consider extracting these to a variable or function to reduce code duplication and improve maintainability.
| echo -n '✓ restarting ... ' | ||
| ssh ${host}.us.archive.org " | ||
| export COMPOSE_FILE='/opt/openlibrary/compose.yaml:/opt/openlibrary/compose.production.yaml' | ||
| export HOSTNAME=\$HOSTNAME |
There was a problem hiding this comment.
The same environment variable exports are duplicated in three locations. Consider extracting these to a variable or function to reduce code duplication and improve maintainability.
| export HOSTNAME=\$HOSTNAME | |
| ${COMMON_EXPORTS}; |
Small improvements from #11156 while testing monitoring changes.
New features:
RESET=trueenvironment variable option. When specified, resets the container before applying the patchCONTAINER=openlibrary-web_nginx-1toSERVICE=web_nginx. This was necessary for theRESEToptionTechnical
Testing
Screenshot
Stakeholders