Repository navigation
Conversation
… on enable `ee service enable` started an existing stopped container with `docker start`, so it came back on whatever image and config it was created with. An EasyEngine upgrade leaves a stopped global service as it is (on its old image, with the old tag recorded) and only regenerates the global compose file with the new tag, so enabling it kept the old image; and where an upgrade's network cleanup had removed its network, `docker start` failed while the command still reported success. `ee service enable <service>` now runs `docker-compose up -d <service>` for a container that isn't running: an unchanged container is just started, one whose image or config no longer matches the compose file is recreated (Compose pulls a missing image), and a missing network is created. A failure is reported as an error instead of "Service … enabled." nginx-proxy already goes through `up -d`. Other callers of `init_global_container()` (site create, cron) keep `docker start`, so they never recreate a database or redis as a side effect. This also repairs hosts where an earlier upgrade left a stopped service on its old image.
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, preserves existing callers’ behavior, and has no identified blocking issues.
No open issues 🎉
What changed in this PR
Updates ee service enable to bring stopped global services up from the current Compose configuration, applying image and configuration changes left pending by an upgrade.
Changes:
- Opts service enable into
docker-compose up -dfor services that are not running. - Reports Compose startup failures instead of printing success.
- Preserves existing startup behavior for other callers, including site creation and cron.
| File | Description |
|---|---|
src/Service_Command.php |
Enables Compose-based startup for global services. |
src/helper/service-utils.php |
Adds an optional recreation mode with startup error handling. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ee service enable <service>started an existing stopped container withdocker start, so the service came back on the image and config the container was created with, whatever the global compose file says now.An EasyEngine upgrade only recreates the global services that are running. It leaves a stopped one as it is and writes the new tag to the global compose file (EasyEngine/easyengine#1954, which this complements). With
docker start, enabling that service kept the old image. On hosts where an earlier upgrade's network cleanup had removed its network,docker startfailed outright, while the command still printed "Service … enabled."ee service enablenow runsdocker-compose up -d <service>for a container that isn't running:If that fails, the command reports an error instead of success. nginx-proxy already went through
up -d. The other callers ofinit_global_container()(site create, cron) keepdocker start, so they never recreate a database or redis as a side effect.This also repairs hosts that an earlier upgrade left with a stopped service on its old image.
Testing
Fork CI upgrade end to end (run 40019379300): stable 4.13.3, with the global database killed and redis disabled, upgraded to a 4.13.4 phar with new image tags, built with this change:
ee service enable dbandee service enable redisbring both up on their new images.ee service enablealso brings both up on their new images. Without this change, the database stays down.