Repository navigation
fix(migration): make container upgrades recoverable and stop needless pulls and compose downloads - #1953
Open
mrrobot47 wants to merge 19 commits into
Open
fix(migration): make container upgrades recoverable and stop needless pulls and compose downloads#1953mrrobot47 wants to merge 19 commits into
mrrobot47 wants to merge 19 commits into
Conversation
…or() A helper that calls EE::error() inside a migration step exited the process, so none of the completed steps were reverted, and a PHP Error was fatal for the same reason. DB and container migration failures also called exit(1) directly, bypassing the step processor and its error message. RevertableStepProcessor can now capture exits: with it on, EE::error()/EE::halt() inside an up or down step throw EE\ExitException, which fails that step and runs the rollback. The migration pipeline and the image migration turn it on. Both processors now catch Throwable, the DB and container migration executors rethrow instead of exiting, and EE::runcommand() restores the previous capture setting instead of clearing it.
…ness gate The image migration stopped and removed every updated global service in one step, regenerated the compose file and only then brought them back, so all sites were down for the whole window, and an exit in between left the proxy and the DB removed. Nothing checked that a recreated service came back: a failed `up -d` or a crash-looping container counted as success, the new tags were saved and the old images pruned. Now the new compose file is written while the old containers keep running, and each updated service is recreated on its own with `up -d <service>` in the order cron, newrelic-daemon, redis, db, nginx-proxy. The proxy goes last so a failing backend is rolled back before every site loses its proxy; the DB goes just before it so its datadir is only reverted onto the old image when the proxy fails. Each recreate waits until the container runs without restarting, is healthy if the image has a healthcheck, passes its probe (nginx-proxy: `nginx -t` and an HTTP response on the published port 80; db: `mariadb-admin`/`mysqladmin ping` over TCP, which the entrypoint's temporary server doesn't answer; redis: `redis-cli ping`), and still does so 2 s later without a restart. cron and newrelic-daemon have no readiness signal and only need to have started: cron restart-loops while it has no jobs, which must not roll back every upgrade, so that only warns. The timeout is 60 s (300 s for the DB), overridable with `global-service-ready-timeout` in config.yml. A failure throws, so the existing revert restores the old compose file and recreates the services from it; that revert now checks their readiness too and warns.
The image migration pulled every image in img-versions.json that no running container used and the DB didn't track, on every run, without ever recording it. That is easyengine/mailhog and the amd64-only easyengine/php (stable), which no site type uses: the pull costs time and Docker Hub quota everywhere, fails without registry access, and aborts the whole migration on arm64 hosts. Such images have nothing to upgrade, so they are no longer pulled; Compose pulls them when a container first needs them. easyengine/php also joins the images that are left to Compose when they do change.
…and verify it CheckAndUpdateDockerOne never called the parent constructor, so its fresh-install skip was dead code: every first run replaced the host's docker-compose, whatever its version, with a downloaded 1.29.2 (a URL that 404s on aarch64), and the image migration then replaced that with 2.27.0. A host with compose v5 ended up on 2.27.0. The download had no --fail and no checksum and ignored its exit codes, so an HTTP error page could become the binary, and the revert never restored anything because it copied without overwriting. update_docker_compose() now installs 2.27.0 only when docker-compose is missing or older than 2.26.1, the minimum EasyEngine checks for, so it never downgrades. It resolves the release asset from the OS and architecture (x86_64, aarch64/arm64, armv6/7, ppc64le, s390x, riscv64), downloads with --fail, checks the published sha256 and that the new binary runs and reports at least 2.26.1, and only then moves it into place. A failure warns and keeps the installed binary. The revert restores the backup only if this run replaced the binary, never a stale one from an earlier run. The migration now calls the parent constructor, so it is skipped on fresh installs, and on upgrades it uses the same helper instead of forcing 1.29.2.
EE_DB decided whether the schema existed by whether the file existed, so a 0-byte ee.sqlite (left by running the sqlite3 CLI on the not-yet-created DB, or by a first run that died after PDO created the file) made every command that reaches the migration check die with an uncaught 'no such table: options' until the file was deleted. A missing, empty or table-less database now gets the base tables, created with IF NOT EXISTS in one transaction. A database that has tables but lacks migrations or options, or isn't a SQLite file, is left untouched and ee stops with a message naming the file and how to recover, instead of an uncaught exception.
… run When no docker-compose was installed, update_docker_compose() made no backup but still marked the binary as replaced, so a failed image migration restored whatever docker-compose.backup an earlier run had left over the new install. The old backup is now removed before a new one is taken.
…outs in the readiness gate redis-cli ping answers NOAUTH when redis.conf sets requirepass, so a working redis failed the gate and every upgrade rolled back; NOAUTH now counts as alive, as 'Access denied' already does for the db. A fractional global-service-ready-timeout (e.g. 0.5) was truncated to 0 and failed the first probe at once; it is now rounded up. The comment on the upgrade order no longer claims the db is only reverted when the proxy fails: a failing site step after the globals reverts all of them, as before.
…t-found error Both executors threw `new Exception()` inside the EE\Migration namespace, which is an Error for the missing class EE\Migration\Exception. That used to end in exit(1); now that the executors rethrow, the step processor would report 'Class "EE\Migration\Exception" not found' as the reason. They throw \Exception with the missing path instead.
…ge migration's own step The replaced flag was process-wide, so when the CheckAndUpdateDockerOne DB migration (which has no undo) installed 2.27.0 and the image migration's update-compose step then had nothing to do, a later failure in the image migration still restored the old binary, possibly compose v1, before the site and global reverts ran against containers the v2 binary had created. Each update_docker_compose() call now starts with the flag cleared, so the step's revert undoes only its own replacement.
With exit_error on (the default), runcommand() ended the process with a raw exit() when a launched command failed, and an in-process command's EE::error() was swallowed into a return code whenever the caller was already capturing exits. Inside a migration step (e.g. auth-command's global auth update reached from a site's admin-tools enable) the first skipped the rollback and the second hid the failure. A failed launched command now goes through EE::halt(), which exits as before unless exits are captured, and an in-process exit with a non-zero code is rethrown to a capturing caller.
Recreating a service in place with `up -d <service>` fails with docker-compose v1 on Docker 25 and newer: v1 stops and renames the old container, then crashes with KeyError: 'ContainerConfig', and a retry or the rollback's `up -d` fails the same way, so the service stays down. The old flow stopped and removed the services first, which v1 still handles, and only updated docker-compose at the end, so a v1 host would lose its first recreated global and never reach the update. The update-compose step now runs first in the image migration (and so is reverted last), and when docker-compose is still older than 2.26.1 (the download failed) the global recreates and the rollback stop and remove the service before `up -d`.
…se download, bypass http_proxy in the probe A vhost.d/conf.d/htpasswd file broken after the running proxy loaded it made the recreated proxy fail, and the rollback's proxy on the old image failed the same way, taking every site down; the proxy step now runs nginx -t in the running container first and fails before touching it. The docker-compose download and its .sha256 had only a connect timeout, so a stalled transfer hung the upgrade; they now have --max-time. The proxy probe's curl followed an exported http_proxy, so a corporate proxy could time the probe out or answer for it; it now uses --noproxy '*'.
…ice is recreated The nginx -t check of the running proxy ran in the proxy's own upgrade step, which comes last, so a config already broken on disk only stopped the upgrade after cron, redis and the database were recreated on their new images, and the rollback then started the old database image on a datadir the new one may have upgraded. The check now runs as its own step before the global compose file is backed up and regenerated, so a broken proxy config stops the upgrade before any service is touched.
…-compose v1 must recreate Every recreate and the rollback stopped and removed the listed services before up -d whenever docker-compose was older than 2.26.1. Only v1 needs that (it fails with KeyError: 'ContainerConfig' when it recreates a container on Docker 25+); v2 builds below 2.26.1 recreate in place. And because the rollback listed every updated service, a failure early in the upgrade also stopped and removed the database and the proxy that were still running untouched on their old images, taking every site down while the rollback ran. Now only v1 takes the stop and remove path, and only for the services this run has actually recreated; up -d leaves the others alone on both versions. The rollback also derives the service names with the same helper as the upgrade steps.
curl retries a transfer that hit --max-time, so with --retry 3 a stalled download could hold the upgrade for about 20 minutes, and the upgrade tries the install up to twice per run when it fails. --retry-max-time stops new attempts after two minutes (one for the checksum).
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Interrupted upgrades still have recovery gaps, and a blocked Redis probe can prevent timeout-driven rollback.
3 open issues
What changed in this PR
This PR improves EasyEngine’s container upgrade pipeline with staged service recreation, readiness checks, and rollback handling, while avoiding unnecessary image pulls and Compose downloads.
Changes:
- Recreate global services in order, check readiness, and capture migration exits for rollback.
- Install verified Compose binaries only when needed and skip unused image pulls.
- Initialize empty SQLite databases and clarify missing-migration errors.
| File | Description |
|---|---|
php/EE/Runner.php |
Enable exit capture for migration steps. |
php/EE/RevertableStepProcessor.php |
Capture exits and handle throwable failures during execution and rollback. |
php/EE/Migration/GlobalContainers.php |
Add ordered recreation, readiness probes, and rollback checks. |
php/EE/Migration/Executor.php |
Propagate migration failures and identify missing files. |
php/EE/Migration/CustomContainerMigrations.php |
Propagate container migration failures and identify missing files. |
php/EE/Migration/Containers.php |
Coordinate staged upgrades, selective pulls, and verified Compose installation. |
php/class-ee.php |
Preserve exit-capture state and propagate captured command failures. |
php/class-ee-db.php |
Initialize empty databases and validate the base schema. |
migrations/db/20240502102550_easyengine_check_and_update_docker_one.php |
Restore fresh-install skipping and reuse the Compose updater. |
Review effort: Balanced
…id not complete An upgrade killed after it recreated some global services leaves their new tags in the global compose file. The next attempt copied that half-upgraded file over the snapshot, so if it then failed, its rollback restored the new tags and recreated the services it had not touched yet, the database and the proxy included, on their new images instead of putting them back. The snapshot is now marked pending until the upgrade completes or is rolled back, and a run that finds a pending snapshot keeps it instead of taking a new one.
…already upgraded A service that a killed upgrade had already recreated runs its new image, so the next attempt doesn't count it as updated. When that attempt failed, the rollback restored the old compose file but left the service on the new image, out of step with its compose configuration and recorded tag. The rollback now also recreates every global service whose running image differs from the one in the restored compose file, and checks its readiness like the others.
… phar ships The phar leaves out symfony/yaml, so the rollback stopped with a class-not-found error before it recreated anything.
A killed upgrade leaves the services it recreated on their new tags without recording them, and running containers take precedence over the recorded tags. The next run therefore skipped them: one stuck in a restart loop on a broken new image was never gated, and with nothing else to update the run reported success and left the pending snapshot behind. While a pending snapshot exists, the global services now count by their recorded tags, so the next run recreates (a no-op when nothing changed) and gates them, and can roll them back to the snapshot. A restart-looping global service counts as present, both when choosing the services to upgrade and when a rollback looks for services off the snapshot.
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.
The container upgrade that
ee cli update(and the first command after it) runs can leave a host with every site down, can report success while the database crash-loops, and fails outright on hosts without Docker Hub access or on arm64. This PR makes that upgrade recoverable and stops it from pulling or downloading things it doesn't need.Problems and fixes
1. All global services are stopped before any is recreated. The image migration first stops and removes every global service whose image changed (proxy, database, redis, cron), and only then starts them again. If the update is interrupted or killed in between, the proxy and database stay removed and every site is down. Running
eeagain doesn't help: it only recreates global services that are running, so they stay down until someone recreates them by hand.Fix: there is no stop-everything step any more. Each service keeps running until it is recreated, one at a time, with the proxy last (cron, newrelic-daemon, redis, database, proxy), so a failing backend is rolled back before the sites ever lose their proxy. The snapshot of the global compose file that a rollback restores stays marked pending until the upgrade completes or is rolled back, so a retry after an interrupted run keeps the snapshot from before it instead of the half-upgraded file, and while such a snapshot is pending the services an interrupted run already recreated are counted by their recorded tags, so the retry gates them again and its rollback puts them back too.
2. A crash-looping database counts as a successful upgrade. Nothing checked that a recreated service actually works, so a database that restarts in a loop (or an unhealthy redis) was accepted, its tag saved and the old image pruned.
Fix: each recreated service has to pass a readiness gate before the next one is touched: running, not restarting, healthy if the image has a healthcheck, its probe passes (database:
mariadb-admin/mysqladmin pingover TCP, where "access denied" still means alive; redis:redis-cli pinganswering PONG, or NOAUTH when it is password-protected; proxy:nginx -tand any HTTP answer on the configured port, with--noproxyso an exportedhttp_proxycan't answer for it), and it is still so 2 s later. cron and newrelic-daemon have no readiness signal and only need to have started. The timeout is 60 s (300 s for the database, which may run crash recovery or a datadir upgrade), overridable withglobal-service-ready-timeoutin config.yml. A failure rolls back: the old compose file is restored and the services this run recreated are recreated from it, and their readiness is checked again (as warnings).3. A broken proxy config only shows up after the database was recreated. The new check that the running proxy's config passes
nginx -tnow runs before any global service is touched, so a config that is already broken on disk (and would stop both the new and the restored proxy) stops the upgrade with a clear message instead of rolling back a database that may already have been opened by a newer image.4.
EE::error()inside a migration step exits without rolling back. Many migration helpers end inEE::error()(directly or throughEE::runcommand()), which exited the process in the middle of the pipeline, so nothing was reverted and the host was left half upgraded.Fix: the migration pipeline's step processor now turns those exits into failures of the step, so its rollback runs.
EE::runcommand()useshalt()instead ofexit()and hands a captured exit back to a caller that captures exits itself, and it restores the previous capture setting instead of clearing it. The step processor only does this when asked (the default is unchanged for other users).5. docker-compose is replaced on every fresh install, unverified. A migration meant to be skipped on fresh installs never called its parent constructor, so every first run replaced the installed docker-compose, whatever its version, with 1.29.2 (a URL that 404s on aarch64), and the image migration later replaced that with 2.27.0. The download had no
--failand no checksum, so an HTTP error page could become the binary; the revert never restored anything (it didn't overwrite) or could restore a stale backup from an earlier run; and compose was only updated after the global services were recreated, although compose v1 can't recreate containers on Docker 25+.Fix: docker-compose 2.27.0 is installed only when it is missing or older than 2.26.1 (the minimum EasyEngine checks for), so a newer one is never downgraded. The release asset is resolved from the OS and architecture, downloaded with
--fail, bounded retries and timeouts, checked against the published sha256, and must run and report at least 2.26.1 before it is moved into place. A failure warns and keeps the installed binary. The revert restores only what this run replaced. Compose is updated before any global service is recreated, and only compose v1 still gets the stop-and-remove-first path, for the services this run actually recreated.6. Images nothing uses are pulled on every run. The image migration pulled every image in
img-versions.jsonthat no container ran and the database didn't track, on every run, without recording it:easyengine/mailhogand the amd64-onlyeasyengine/php. That costs time and Docker Hub quota everywhere, fails without registry access, and aborts the whole upgrade on arm64.Fix: only images that a container runs or the database tracks, and whose tag changed, are pulled; the others are pulled by Compose when something first uses them.
7. An empty
ee.sqlitebreaks every command. An empty database file (left by the sqlite3 CLI or an interrupted first run) skipped the schema creation and failed with an uncaughtno such table: options. It now gets the schema; a file that can't be opened, or has tables but not EasyEngine's base tables, gets an error that says which file and how to recover.8. A missing migration file is reported as a class-not-found error. It now says which file is missing.
Testing
All on fork CI (throwaway runners), against this branch's code:
PHPUnit (core unit tests for these fixes, 437 tests) on PHP 7.4, 8.0, 8.1, 8.2, 8.3, 8.4 and 8.5: run 39977512970, and 39992260760 after the last commit.
Upgrade harnesses, run 39977513181: the readiness gate against real containers (13 cases: database ready after the entrypoint's temporary server, crash loop, stopped, missing; redis ready, unhealthy, password-protected; proxy ready, broken config, unpublished port; cron restart loop accepted, exited cron rejected); the docker-compose install with real downloads (1.29.2 replaced by a verified 2.27.0 and reverted, 2.26.1 and 5.5.1 left alone, github.com unreachable, a download that isn't docker-compose); and a real
eeboot of this branch and of develop with Docker stubbed (emptyee.sqlite, fresh install with compose 5.5.1, a DB migration that callsEE::error()and the next run).Upgrade end to end, run 39977513056: stable 4.13.3 installed fresh with an HTML, a WordPress (with cache) and a PHP site, then upgraded to a phar of this branch (or of develop for comparison) built as 4.13.4, with every global service and the site nginx on a new tag, a 0.2 s probe on every site and one injected failure per job (the last two rows and a rerun of every scenario on this branch after the last commit: run 39992260551):
easyengine/mailhogandeasyengine/phpeekilled (SIGKILL) right after the first global service changed, then run againeasyengine/mailhogpullEE::error()while upgrading a siteeekilled after redis was recreated, and the retry fails on a crash-looping databaseeekilled while the new, crash-looping database is being waited for, then run againIn every job on this branch the sites answer 200 afterwards, the database, redis and proxy answer their probes, the version and image tags are either all new or all old, the migration rows are unchanged, no image a container uses was removed,
easyengine/phpwas not pulled and docker-compose 1.29.2 was not downloaded.Plain upgrade, same run: stable 4.13.3 with a WordPress and a PHP site upgraded to a phar of this branch with the real image pins: exit 0, version and migrations recorded, all containers up, database, redis and proxy answering, both sites 200, a new site works, no image removed, nothing pulled or downloaded that isn't needed.
Fresh first run, same run: with docker-compose 5.5.1 installed, develop downloads 1.29.2 and then 2.27.0 over it and pulls
easyengine/mailhogandeasyengine/php; this branch leaves 5.5.1 alone and pulls neither. On arm64 (ubuntu-24.04-arm) the first run of both still stops at the first image pull, because the current images are amd64-only;update_docker_compose()itself installs a verified aarch64 2.27.0 there.This repository's own CI workflow (core and package Behat suites, PHP 7.4 to 8.5): run 39977512918, and 39992260611 after the last commit.
The core Behat suite from test(easyengine): add a Behat suite and CI workflow #1950 (main, migration and uninstall lanes, PHP 7.4 and 8.5): run 39974359620. Its scenario "A failed image migration reverts the container migrations of the same run" needs one change to its "the next image migration pulls at least one image" step, because untracked images are no longer pulled: with that change it passes with this branch (run 39974359620) and on develop (run 39974363775).
Compatibility
PHP 7.4 syntax only; the changed files lint clean on PHP 7.4 and 8.5. No new dependencies.
Upgrade-path notes
global-service-ready-timeoutchanges the wait, for the database too.Containers::DOCKER_COMPOSE_MIN_VERSIONand inRunner::check_requirements(); they should be unified in a follow-up.