Skip to content

fix(migration): upgrade or keep stopped global services and disabled sites instead of only recording their new tags - #1954

Open
mrrobot47 wants to merge 26 commits into
EasyEngine:developfrom
mrrobot47:fix/migrate-stopped-and-disabled
Open

mrrobot47 wants to merge 26 commits into
EasyEngine:developfrom
mrrobot47:fix/migrate-stopped-and-disabled

Conversation

@mrrobot47

@mrrobot47 mrrobot47 commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

Stacked on #1953. This branch starts from #1953's head (ef097e19), and only the commits after it are new. Please merge it after #1953, or retarget it to develop once #1953 is merged.

The container upgrade saved the new tag of every changed image, even when nothing had moved to it. Stopped global services and disabled sites were skipped, then recorded as upgraded, and the cleanup that followed removed things they still needed.

What users hit

1. A stopped global service stays on its old image, and the database says it was upgraded. Only running global services are recreated. If redis was stopped (ee service disable redis) or the database had exited when ee cli update ran, its container stays on the old image but the new tag is recorded, so no later upgrade ever touches it. ee service enable then docker starts the old container. If nothing else was using its network, the cleanup has removed that network, so docker start fails while the command still prints "Service … enabled."

2. A disabled site comes back on old images. The sites to upgrade are picked from docker-compose images, which lists nothing for a disabled site, because ee site disable removes its containers. Its docker-compose.yml keeps the old tags while the new ones are recorded and the old images are pruned. ee site enable (without --refresh) then pulls the old images again and runs them next to config that a container migration may already have moved to the new format. If only stopped containers used the global backend network, the cleanup has removed it, and ee site enable of a disabled WordPress site fails.

3. A service with no container is later created from the old tag. When no running global service changed, the global compose file wasn't regenerated. A service created later from it (e.g. global-redis at the first --cache site) used the old tag.

Fixes

  • Updated global services are split by the state of their container:

    • Running (or restart-looping): recreated and gated as before; their new tags are recorded.
    • Stopped: left alone, and the old tag stays recorded. The regenerated compose file carries the new tag, so ee service enable can bring the service up on it (fix(service): recreate a stopped global service from the compose file on enable service-command#84 makes enable recreate instead of docker start). If its container already runs the new tag, as after an interrupted upgrade, that tag is recorded.
    • Missing: the regenerated compose file is what it is created from later; the new tag is recorded.

    The global compose file is regenerated, and restored on a rollback, whenever any global image changed.

  • Why stopped services aren't recreated stopped (up --no-start):

    • Docker brings a manually stopped restart: always container back when the daemon starts, but never one that was only created. After a reboot, a stopped proxy or database would then behave differently than before the upgrade.
    • The old image would also be pruned. That leaves no way back if the new image (e.g. a MariaDB major version) can't open the datadir.

    Leaving the container as it is keeps its reboot behaviour and its image, can't fail or roll back the upgrade, and keeps every recreate behind fix(migration): make container upgrades recoverable and stop needless pulls and compose downloads #1953's readiness gate: the next upgrade that finds the service running moves it.

  • An image that only stopped services use, and that nothing running uses, is not pulled. Compose pulls it when something first needs it. When nothing is left to do but those stopped services, and their compose file already has the new tags, the migration stops early. Otherwise every later run would pull the image again and prune it again. On nightly that would happen on every command, and every command would fail while Docker Hub was unreachable.

  • A fresh rollback snapshot of the global compose file sets each service to the image its container actually runs. A rollback therefore never brings back a tag that an earlier upgrade only deferred for a stopped service.

  • The cleanup no longer runs docker network prune. It removes only EasyEngine networks that no container, running or stopped, is attached to, and never the two global networks.

  • Disabled sites are picked from their docker-compose.yml when it sets an EasyEngine image to a tag other than this release's. They are checked on every run, even when no image changed, so a site an interrupted or earlier upgrade left behind is caught too. Their file is backed up and regenerated the way ee site enable --refresh does it, and nothing is started, so a plain ee site enable starts them on the current images.

    • Only the main docker-compose.yml is regenerated; docker-compose-custom.yml and user-docker-compose/ are left as they are.
    • The step only warns if it fails, including when the regenerated file has an image without a tag, and keeps the old file. A site that serves nothing must not block or roll back the upgrade of live ones.
    • A rollback restores only a backup taken in the same run.

Testing

All on fork CI (throwaway runners):

  • PHPUnit (core unit tests, 454 including the new ones) on PHP 7.4 to 8.5: run 40019379268.

  • Upgrade end to end, run 40019379300. Each job installs stable 4.13.3 and upgrades to a phar of this branch built as 4.13.4, with every global service and the site nginx retagged so they count as changed. In the new jobs, before the upgrade an HTML and a PHP site are enabled, the global database is killed, redis is disabled with ee service disable redis, and a WordPress site (with cache) and a PHP site are disabled:

    Job Result
    fix(migration): make container upgrades recoverable and stop needless pulls and compose downloads #1953's head Bug reproduced: the new database and redis tags are recorded while both containers stay on the old images. The disabled sites keep the old nginx tag. ee service enable db leaves the old container. ee site enable of the disabled WordPress site pulls the pruned old nginx again, or fails outright because the cleanup removed the backend network.
    This branch Exit 0. Proxy, cron and site nginx are upgraded and recorded. The stopped database and redis keep their containers, old images and old tags, and their images aren't pulled. The global backend network is kept. Both disabled sites' compose files carry the new nginx tag. ee service enable db/redis starts them, and ee site enable (no --refresh) starts both disabled sites on the new nginx without pulling an old image. All four sites answer 200.
    This branch plus the service-command change Same, and ee service enable brings the database and redis up on their new images.
    This branch, failure injected while upgrading the last enabled site (after the disabled sites were updated) Rolled back, exit 1. Version and tags old, the disabled sites' compose files and the global compose file restored, stopped services untouched. Afterwards everything enables and serves on the old images.
    fix(migration): make container upgrades recoverable and stop needless pulls and compose downloads #1953's head plus the service-command change ee service enable repairs a host the old upgrade left behind: the database and redis come up on their new images.

    fix(migration): make container upgrades recoverable and stop needless pulls and compose downloads #1953's existing scenarios (baseline, kill on start, crash loop, unhealthy redis, EE::error(), three interrupted-run variants, busy port, offline, old docker-compose), the plain upgrade and the fresh installs pass as before.

  • Upgrade harnesses: run 40019379541. This repository's CI workflow (core and package Behat suites, PHP 7.4 to 8.5): run 40019379338.

  • The core Behat suite from test(easyengine): add a Behat suite and CI workflow #1950, with the step fix that fix(migration): make container upgrades recoverable and stop needless pulls and compose downloads #1953 needs: run 40019382049.

Related

  • fix(service): recreate a stopped global service from the compose file on enable service-command#84: ee service enable recreates a stopped service from the compose file. Without it, a stopped service keeps its old image after enable, with its old tag recorded, until a later upgrade finds it running.
  • Not covered here:
    • The migration's cron config step docker restarts the cron container, so it also starts a stopped cron (unchanged behaviour).
    • Images that have neither a running container nor a recorded tag, such as php7.4–8.1, are still not checked for disabled sites.

…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).
…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.
…nce it is upgraded

The image migration recreated only the global services whose container was running, but saved the new tag of every changed image. A stopped or exited service (e.g. after `ee service disable redis`) kept its old container while the database said it was upgraded, so nothing would ever move it, and `ee service enable` started it on the old image. When no running global service changed, the global compose file wasn't regenerated either, so a service without a container (e.g. global-redis before the first cache site) was later created from the old tag.

Now the updated global services are split by the state of their container:

- running (or restart-looping): recreated and gated as before, and their new tags recorded;
- stopped (exited, created, paused, dead): left alone, and their old tags stay recorded, so the database again says what each container runs;
- missing: nothing to recreate; their new tags are recorded and the regenerated compose file is what they are created from later.

The global compose file is now regenerated (and restored on a rollback) whenever any global image changed, so it carries the new tags of the stopped and missing services too: `ee service enable` brings a stopped service up from it on the new image once it recreates rather than restarts the container (a separate service-command change).

Why not recreate a stopped service on its new image without starting it (`up --no-start`)? Docker brings a manually stopped `restart: always` container back when the daemon starts, but never a created one, so the upgrade would silently change what a stopped proxy or database does after a reboot. Its old image would also be pruned, leaving no way back if the new one (e.g. a MariaDB major version) can't open the datadir, and the datadir upgrade would happen later with no readiness gate. Leaving the stopped container as it is keeps its reboot behaviour and its image, cannot fail or roll back the upgrade, and keeps every recreate behind the readiness gate: the next upgrade that finds it running moves it, gated.
…l networks in the cleanup

After the image migration, `docker network prune` removed every EasyEngine network with no running container. That includes the networks of stopped containers, which `docker start` (e.g. `ee service enable` of a stopped global service) then fails on with "network not found", and the global networks that disabled sites join as external networks, so `ee site enable` of a disabled WordPress site failed when only stopped services used ee-global-backend-network.

The cleanup now removes only EasyEngine networks that no container, running or stopped, is attached to (`docker ps -a --filter network=…`), and never the two global networks.
The image migration picked the sites to upgrade from `docker-compose images`, which lists nothing for a disabled site (`ee site disable` removes its containers). Disabled sites were therefore skipped while their images were recorded as upgraded and the old ones pruned, so a later `ee site enable` (without `--refresh`) re-pulled the old tags and ran them next to config a container migration may already have moved to the new format.

A disabled site is now selected from its own docker-compose.yml, when it sets one of the changed images to a tag other than this release's (the same rule as for enabled sites, which are selected from their containers). Its file is backed up and regenerated the way `ee site enable --refresh` does it (`dump_docker_compose_yml()`), and nothing is started, so `ee site enable` then starts the site on the current images and site-command needs no change. Only the main docker-compose.yml is regenerated, as for enabled sites; `docker-compose-custom.yml` and `user-docker-compose/` stay as they are.

The backup and the regeneration are one step that only warns when it fails (with the `ee site enable --refresh` hint) and puts the old file back: a site that serves nothing must not block or roll back the upgrade of the live ones. A regenerated file with an image that has no tag (what a missing `img-versions.json` entry produces, with only a PHP warning) counts as a failure. A rollback of the run restores the file only if this run backed it up, never a backup left by an earlier run.

The new tags of a disabled site's images may still be pruned when no container uses them; `ee site enable` pulls them like any site create, and no longer needs the old ones.
…pped global services use

A stopped global service keeps its old tag recorded, so its image counts as changed on every later image migration. Each run pulled it, and the cleanup then pruned the pulled image again because no container uses it: one wasted pull per upgrade on stable, and on nightly (where the migration runs on every command) a full pull on every command, which failed every command while Docker Hub was unreachable or rate-limited.

Now:

- an image only stopped global services use, and nothing running, is not pulled; Compose pulls it when something first uses it (`ee service enable`), as for untracked images. A stopped service already on its new tag has it locally;
- when the only changed images are those, the global compose file already has their new tags and no disabled site needs updating, the migration ends there instead of regenerating files and inspecting every site again;
- a stopped service whose container already runs the new tag (recreated by an interrupted run) has that tag recorded, so the database says what it runs and the cleanup may prune the old image.

Images of missing services are still pulled and then recorded, as before, so they don't come back on the next run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Interrupted-run discovery and rollback snapshot handling can leave sites on old images or prevent recovery to a working service image.

3 open issues
What changed in this PR

This PR builds on #1953’s recoverable migration work to handle stopped global services and disabled sites without recording image upgrades that never occurred.

Changes:

  • Classifies global services by container state, preserving deferred image tags and avoiding unnecessary pulls.
  • Refreshes disabled-site compose files without starting containers and preserves networks needed for later enablement.
  • Includes the base migration’s readiness checks, rollback handling, safer Compose installation, and database initialization fixes.
File Description
php/​EE/​Runner.php Enables exit capture for migration rollback.
php/​EE/​RevertableStepProcessor.php Captures step exits and handles throwable failures.
php/​EE/​Migration/​SiteContainers.php Reads compose image tags and refreshes disabled-site files with backups.
php/​EE/​Migration/​GlobalContainers.php Adds state-aware upgrades, readiness checks, and rollback snapshots.
php/​EE/​Migration/​Executor.php Propagates migration failures and clarifies missing-file errors.
php/​EE/​Migration/​CustomContainerMigrations.php Propagates custom migration failures for rollback.
php/​EE/​Migration/​Containers.php Coordinates deferred upgrades, disabled-site updates, cleanup, and Compose installation.
php/​class-ee.php Preserves exit-capture settings and propagates captured command failures.
php/​class-ee-db.php Initializes empty databases and validates existing base tables.
migrations/​db/​20240502102550_easyengine_check_and_update_docker_one.php Fixes fresh-install detection and reuses the Compose updater.

Review effort: Balanced

Comment thread php/EE/Migration/Containers.php Outdated
Comment thread php/EE/Migration/GlobalContainers.php
Comment thread php/EE/Migration/Containers.php
…lback snapshot to running images, clear a finished snapshot

Three gaps in the handling of stopped services and disabled sites:

- A disabled site was only checked for the images that changed in this run. An interrupted run that had already recreated the enabled sites' nginx, or an earlier upgrade that recorded nginx without updating the disabled sites, left nginx out of that list, so those sites kept the old tag (and with no other change the migration stopped before looking at sites at all). Disabled sites are now compared with this release's tags, and checked even when no image changed; nothing else runs then (no pull, no recreate, no cleanup).
- A stopped service an earlier upgrade deferred has its new tag in the global compose file while its container keeps the old image. Once Docker restarted it (e.g. on boot), the next upgrade's snapshot took that untested tag, so a failed readiness gate rolled the service back onto it instead of the image that worked. A fresh snapshot now sets each global service to the image its container was created from; a pending snapshot from an interrupted run is kept as it is.
- When only deferred stopped services were left, the early end didn't clear a pending snapshot left by an interrupted run, so a later upgrade would have rolled back to that outdated snapshot. Nothing running needs a gate in that case, so the snapshot is now marked done.
@rtBot
rtBot requested a balanced review from Copilot October 23, 2026 19:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Persistent-data rollback and companion service-command integration warrant human review, and the pending-snapshot lifecycle still has an unresolved issue.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Clear stale pending marker after successful empty update

php/​EE/​Migration/​Containers.php:85

If an upgrade is interrupted after saving all new image tags but before clearing .pending, the retry reaches this empty-update return and leaves the marker intact. A later release then reuses the stale snapshot, so a failed upgrade can restore images and configuration from before an already completed release. Clear the pending marker on this successful path after any disabled-site updates finish, as the stopped-services early return already does.

3 resolved since last review

Review effort: Balanced

A run interrupted after it saved the new tags but before it marked the global compose snapshot done left the snapshot pending. The retry found no changed image and returned early, so the marker stayed, and the next release's upgrade reused that outdated snapshot: a failed upgrade there would have restored images and configuration from before an already completed release. With every tag recorded and nothing changed there is nothing left to roll back, so the early return now marks the snapshot done.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

State-dependent database rollback and interrupted-upgrade recovery require human validation, and the Redis readiness probe remains unbounded.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Bound Redis readiness probe to prevent migration hangs

php/​EE/​Migration/​GlobalContainers.php:335

If Redis accepts a connection but stops processing requests, redis-cli ping can wait indefinitely because EE::launch() does not impose a subprocess timeout. The readiness loop then cannot reach its elapsed-time check, so the migration hangs instead of failing and rolling back. Bound the probe by the remaining readiness deadline and treat expiry as a failed probe.

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants