refactor: docker setup to be worktree-friendly#11291
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +4.53 kB (0%) Total Size: 954 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the Docker development setup to support running multiple WooPayments branches simultaneously using git worktrees. The key architectural change is splitting the infrastructure (database and phpMyAdmin) from the WordPress containers, allowing each worktree to have its own isolated WordPress instance while sharing a common database server.
Changes:
- Split docker-compose.yml into shared infrastructure (docker-compose.infra.yml) and per-worktree WordPress containers
- Implemented automatic port allocation and worktree identification via .env.local files
- Added per-worktree test database isolation to prevent conflicts when running tests in parallel
- Updated all scripts and npm commands to support the new worktree-friendly architecture
Reviewed changes
Copilot reviewed 8 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated npm scripts to use .env.local for env-specific configuration and added new commands for infrastructure management and worktree cleanup |
| docker-compose.yml | Refactored to define only the WordPress container, using external network and environment variables for worktree isolation |
| docker-compose.infra.yml | New file defining shared infrastructure (MariaDB and phpMyAdmin) used across all worktrees |
| default.env | Cleaned up to remove database-specific variables now defined in docker-compose.infra.yml |
| bin/docker-port-setup.sh | New script that auto-allocates available ports and generates unique worktree identifiers |
| bin/docker-setup.sh | Updated to detect and handle shared database scenarios, avoiding duplicate WordPress installs across worktrees |
| bin/docker-worktree-cleanup.sh | New script to clean up Docker resources before removing a worktree |
| bin/run-tests.sh | Updated to use worktree-specific test databases for parallel test execution |
| bin/check-test-coverage.sh | Updated to support .env.local configuration |
| bin/jurassic-tube-setup.sh | Updated to detect WordPress port from .env.local |
| bin/cli.sh | Updated to support .env.local configuration |
| .husky/post-checkout | Updated to auto-configure Docker settings when checking out code |
| docker/README.md | Comprehensive rewrite documenting the new worktree-friendly setup with architecture diagrams and workflows |
| .gitignore | Added .env.local to ignored files |
| .env.local.example | New example file showing configuration options |
| changelog/refactor-docker-worktree-friendly-setup | Added changelog entry for this dev-focused change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| networks: | ||
| wcpay-network: | ||
| name: wcpay-network | ||
|
|
||
| services: | ||
| db: | ||
| container_name: wcpay_db | ||
| image: mariadb:10.5.8 | ||
| restart: unless-stopped | ||
| ports: | ||
| - "5678:3306" | ||
| environment: | ||
| MYSQL_ROOT_PASSWORD: wordpress | ||
| MYSQL_DATABASE: wordpress | ||
| MYSQL_USER: wordpress | ||
| MYSQL_PASSWORD: wordpress | ||
| volumes: | ||
| - ./docker/data:/var/lib/mysql | ||
| networks: | ||
| - wcpay-network | ||
|
|
||
| phpmyadmin: | ||
| container_name: wcpay_phpmyadmin | ||
| image: phpmyadmin:latest | ||
| restart: unless-stopped | ||
| ports: | ||
| - "8083:80" | ||
| environment: | ||
| PMA_HOST: db | ||
| PMA_USER: root | ||
| PMA_PASSWORD: wordpress | ||
| depends_on: | ||
| - db | ||
| networks: | ||
| - wcpay-network |
There was a problem hiding this comment.
This is a breaking change for existing users. The container names have changed from woocommerce_payments_wordpress, woocommerce_payments_mysql, and woocommerce_payments_phpmyadmin to wcpay_wp_${WORKTREE_ID}, wcpay_db, and wcpay_phpmyadmin respectively.
When users run npm run infra:up, they'll create new database containers with empty databases, losing their existing development data. Consider adding migration documentation or a migration script that:
- Backs up data from the old woocommerce_payments_mysql container
- Restores it to the new wcpay_db container
- Cleans up the old containers
Alternatively, document this breaking change clearly in the README so users know to export their data before upgrading.
There was a problem hiding this comment.
So I tried this out, and the DB contents do persist after running npm run infra:up. It would be nice to get someone else to confirm.
There was a problem hiding this comment.
Yeah, I think Copilot missed the mounted volume with the actual database files. The comment stands true if someone creates a container with database storage inside the container filesystem and runs it forever, but even a mere system cleanup will destroy the data in such a scenario.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 23 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
.husky/post-merge:44
- The variable DEV_TOOLS_PLUGIN_PATH is used without quotes in several places (lines 33, 37, 40, 44). If the path contains spaces or special characters, this could cause the script to fail or behave unexpectedly. While the path is typically sanitized, it's best practice to always quote variable expansions in shell scripts to handle edge cases. Change instances like 'cd $DEV_TOOLS_PLUGIN_PATH' to 'cd "$DEV_TOOLS_PLUGIN_PATH"'.
if [ "$(cd $DEV_TOOLS_PLUGIN_PATH && git rev-parse --show-toplevel 2>/dev/null)" = "$(cd $DEV_TOOLS_PLUGIN_PATH && pwd)" ]; then
echo
echo "\033[32mDetermining if there is an update for the WCPay Dev Tools plugin...\033[0m"
DEV_TOOLS_BRANCH=$(cd $DEV_TOOLS_PLUGIN_PATH && git branch --show-current)
if [ "$DEV_TOOLS_BRANCH" = "trunk" ]; then
echo " \033[32mThe current branch is trunk. Check if we are safe to pull from origin/trunk...\033[0m"
if [ "$(cd $DEV_TOOLS_PLUGIN_PATH && git status --porcelain)" ]; then
echo "\033[33m There are uncommitted local changes on the WCPay Dev Tools repo. Skipping any attempt to update it.\033[0m"
else
echo " \033[32mPulling the latest changes from origin/trunk, if any...\033[0m"
cd $DEV_TOOLS_PLUGIN_PATH && git pull
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
package.json
Outdated
| "start": "npm run watch", | ||
| "dev": "npm run up && npm run watch", | ||
| "up:recreate": "bash bin/docker-preflight.sh && docker compose up --build --force-recreate -d && ./bin/docker-setup.sh && bash -c 'source .env 2>/dev/null; echo \"Site available at http://localhost:${WORDPRESS_PORT:-8082}\"'", | ||
| "up": "bash bin/docker-preflight.sh && docker compose up --build -d && bash bin/docker-seed-volumes.sh && bash -c 'source .env 2>/dev/null; echo \"Site available at http://localhost:${WORDPRESS_PORT:-8082}\"'", |
There was a problem hiding this comment.
The 'up' script on line 61 calls docker-seed-volumes.sh but doesn't call docker-setup.sh. This means that when users run 'npm run up' (for subsequent startups), the WordPress setup won't be triggered. While this might be intentional for performance (assuming setup only needs to run once), it could lead to confusion if containers are stopped and restarted but the setup state is lost (e.g., if WordPress files are corrupted or the container is recreated externally). Consider documenting this behavior clearly or adding a check in the 'up' command to detect if setup is needed.
| "up": "bash bin/docker-preflight.sh && docker compose up --build -d && bash bin/docker-seed-volumes.sh && bash -c 'source .env 2>/dev/null; echo \"Site available at http://localhost:${WORDPRESS_PORT:-8082}\"'", | |
| "up": "bash bin/docker-preflight.sh && docker compose up --build -d && bash bin/docker-seed-volumes.sh && ./bin/docker-setup.sh && bash -c 'source .env 2>/dev/null; echo \"Site available at http://localhost:${WORDPRESS_PORT:-8082}\"'", |
RadoslavGeorgiev
left a comment
RadoslavGeorgiev
left a comment
There was a problem hiding this comment.
Lol, I didn't realize how much code would be involved in this setup :D
It's amazing that you have considered all existing scripts, but it will take me a while to review everything. I'll continue next week, but it looks pretty promising at first glance! I like that you decided to use individual test DBs instead of whole separate instances for them.
My main concern so far is maintaining the wp-content directories easily accessible. I still have to check how that works with local mappings, as my current setup includes:
services:
wordpress:
volumes:
- /Users/rado/Projects/woocommerce/plugins/woocommerce:/var/www/html/wp-content/plugins/woocommerce
- /Users/rado/Projects/product-feed:/var/www/html/wp-content/plugins/open-ai-feedIt would be cool to be able to maintain those external directories.
Leaving my notes for now, and I'll continue the review/testing next week.
| echo "Updating the WordPress database..." | ||
| cli wp core update-db --quiet | ||
|
|
||
| echo "Configuring WordPress to work with ngrok (in order to allow creating a Jetpack-WPCOM connection)"; |
There was a problem hiding this comment.
I moved these lines up in this file, to ensure they exist in the wp-config.php
frosso
requested review from
a team and
mgascam
and removed request for
a team
mgascam
left a comment
mgascam
left a comment
There was a problem hiding this comment.
Thanks for working on this @frosso.
I tried the worktree setup while working on a couple of tasks and it worked well for me. Definitely much better than having two separate checkouts. My setup is pretty much "vanilla" (Docker-based with no customizations) but the whole flow — worktree:setup, up:recreate, running both sites in parallel — was smooth and easy to follow.
I'm happy to approve as I think it's a great improvement.
Just one idea for the future: it would be nice to have a utility to list all worktrees and their associated ports. Feels like the building blocks are already in place.
Co-authored-by: Miguel Gasca <miguel.gasca@automattic.com>
Co-authored-by: Miguel Gasca <miguel.gasca@automattic.com>
|
Thank you @mgascam ! That's a good idea - @dmallory42 had a similar command in mind, and I think it would make sense in this setup because it could just be using I'll wait for @RadoslavGeorgiev 's review as well, before merging. |
RadoslavGeorgiev
left a comment
RadoslavGeorgiev
left a comment
There was a problem hiding this comment.
For such a significant change, all of this looks soo good! Thank you for also implementing my idea regarding the Docker volumes and not having to manually copy files!
I have a few comments, but they are all minor. So far I've tested with an old machine, and everything seems to work as advertised. I intentionally went vanilla and did not use the worktree-cli package, I'll leave that for next time.
Just curious, is there a specific migration path you have in mind for other devs? It doesn't really seem like one is needed, as npm run up without the infra running will already tell you what to do, but I wanted to check.
| docker exec $WP_CONTAINER bash -c "grep -q '\\\$_SERVER\[.HTTP_HOST.\] = DOCKER_HOST' /var/www/html/wp-config.php || sed -i \"/define.*'DOCKER_HOST'/a \\\\\\\$_SERVER['HTTP_HOST'] = DOCKER_HOST;\" /var/www/html/wp-config.php" | ||
| cli wp config set DOCKER_REQUEST_URL "( ! empty( \$_SERVER['HTTPS'] ) ? 'https://' : 'http://' ) . DOCKER_HOST" --raw | ||
| cli wp config set WP_SITEURL DOCKER_REQUEST_URL --raw | ||
| cli wp config set WP_HOME DOCKER_REQUEST_URL --raw | ||
|
|
There was a problem hiding this comment.
This is a cool set of defaults. In the future, we might want to try to read these from a file.
|
I tested it without adding a worktree and there is a permamanent warning about orphan containers. E.g. But it seems to be a false alert, as the container is used for the default repository. |
|
Thanks @RadoslavGeorgiev and @dmvrtx for the most recent reviews!
Yeah, like you said, I don't think we need any specific migration path. My intention was to have the whole thing as painless as possible if somebody doesn't want to use worktrees or if they aren't using a specific WT utility. |
RadoslavGeorgiev
left a comment
RadoslavGeorgiev
left a comment
There was a problem hiding this comment.
Thanks! Looks good from my side :)
dmvrtx
left a comment
dmvrtx
left a comment
There was a problem hiding this comment.
Thanks for quickly addressing the pseudo-orphans! Looks good!
|
@RadoslavGeorgiev and @frosso it broke my local with the next error: what should I do to fix it? I have followed the PR instructions with npm run infra:up and npm run up:recreate. |
@oaratovskyi Most probably it is the new Dev Tools that broke it. Go to |
|
That was it @dmvrtx. Thanks! |
Fixes WOOPMNT-5683
Changes proposed in this Pull Request
Making the docker setup friendlier to worktrees.
When running multiple WooPayments branches simultaneously using
git worktrees, there are issues withport 8082 already in use, database tables colliding, yada yada.The current docker setup assumes one setup, one developer, one branch at a time.
I reworked the docker configuration to be slightly more friendly towards having multiple feature branches running in parallel, whether it's work that you're reviewing or work that you have agents working on.
Each worktree has its own isolated WordPress instance, but they're all sharing a common database infrastructure and wp-content directories (plugins, themes, uploads).
Testing instructions
npm run downto stop your current containersnpm run infra:up(optional)npm run up:recreateto start wordpress (this also seeds existing plugins/themes from your local directories into the shared volumes)To test the worktree flow:
Tip
I used this utility to create worktrees and set them up: https://github.com/johnlindquist/worktree-cli
I use it in combination with this
worktree.jsonfile:`worktree.json` on my local setup
{ "setup-worktree": [ "[ -s \"$NVM_DIR/nvm.sh\" ] && . \"$NVM_DIR/nvm.sh\" && nvm use || true", "npx concurrently -n npm,composer,worktree -c blue,green,yellow 'npm ci' 'composer install --prefer-dist --ignore-platform-req=php' 'npm run worktree:setup'", "npx concurrently -n build,docker -c cyan,magenta 'npm run build:client' 'npm run up:recreate'", "echo 'Setup complete, you can run `npm start` to have webpack watch your files.'" ] }git worktree add ../my-feature-branch developto create a new worktreecd ../my-feature-branchto navigate to itnpm ci && composer install --prefer-distto install the dependenciesnpm run worktree:setupto configure port and worktree IDnpm run up:recreateto start wordpress.env)To test cleanup:
npm run test:php -- --filter=some_quick_testto create a test databasenpm run worktree:cleanupnpm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge