Repair wp/5.5 branch Actions: Replace wp-env's database check with a MariaDB health check

packages/env/CHANGELOG.md

@@ -6,6 +6,10 @@
 
 -   Add a `WP_ENV_MARIADB_VERSION` environment variable to choose the MariaDB version used by the database container ([#83751](https://github.com/WordPress/gutenberg/pull/83751)).
 
+### Bug Fixes
+
+-   Add MySQL healthcheck to prevent race condition where WordPress containers start before MySQL is fully initialized. Uses MariaDB's official `healthcheck.sh` script with `MARIADB_AUTO_UPGRADE` to support both new and existing installations when the image has the script and the healthcheck user, and pings the server over TCP with `mariadb-admin` or `mysqladmin` otherwise, so images for older MariaDB versions also become healthy ([#75046](https://github.com/WordPress/gutenberg/pull/75046), [#83751](https://github.com/WordPress/gutenberg/pull/83751)).
+
 ## 1.6.0-rc.0 (2020-06-24)
 
 ### Bug Fixes

packages/env/README.md

@@ -298,7 +298,7 @@ You can customize the WordPress installation, plugins and themes that the develo
 
 _Note: the port number environment variables (`WP_ENV_PORT` and `WP_ENV_TESTS_PORT`) take precedent over the .wp-env.json values._
 
-_Note: the MariaDB version can be chosen with the `WP_ENV_MARIADB_VERSION` environment variable, such as `WP_ENV_MARIADB_VERSION=10.5`. It defaults to `latest`. MariaDB cannot start on a database written by a newer version. When switching to a lower version, run `wp-env destroy` first. It removes the environment's Docker containers, volumes, and networks, and its local files, not only the database, so everything is recreated on the next start. With a version older than 11.4, `wp db` commands in the `cli` and `tests-cli` containers fail, and `wp-env clean` leaves the database as it is: the MariaDB client in the `wordpress:cli` image requires TLS, and MariaDB servers before 11.4 do not offer it. WordPress itself is not affected. To start over with an empty database, run `wp-env destroy`._
+_Note: the MariaDB version can be chosen with the `WP_ENV_MARIADB_VERSION` environment variable, such as `WP_ENV_MARIADB_VERSION=10.5`. It defaults to `latest`. MariaDB cannot start on a database written by a newer version. When switching to a lower version, run `wp-env destroy` first. It removes the environment's Docker containers, volumes, and networks, and its local files, not only the database, so everything is recreated on the next start. With a version older than 11.4, `wp db` commands in the `cli` and `tests-cli` containers fail, and `wp-env clean` leaves the database as it is: the MariaDB client in the `wordpress:cli` image requires TLS, and MariaDB servers before 11.4 do not offer it. `wp-env start` and WordPress itself are not affected. To start over with an empty database, run `wp-env destroy`._
 
 Several types of strings can be passed into the `core`, `plugins`, `themes`, and `mappings` fields.
 

packages/env/lib/build-docker-compose-config.js

@@ -9,6 +9,41 @@ const path = require( 'path' );
  * @typedef {import('./config').WPConfig} WPConfig
  */
 
+/**
+ * The database healthcheck.
+ *
+ * MariaDB's `healthcheck.sh` runs when the image can use it: --connect
+ * verifies a TCP connection and that the entrypoint has finished, and
+ * --innodb_initialized ensures InnoDB is fully initialized. It connects as a
+ * healthcheck user whose credentials the entrypoint writes to
+ * `.my-healthcheck.cnf` in the data directory, and the MARIADB_AUTO_UPGRADE env
+ * var ensures that user exists for existing installations.
+ *
+ * Images for older MariaDB versions may predate that user or the script (the
+ * script was added to the images in 2022 and the user in 2023, and older tags
+ * were never rebuilt). The file is in the data volume and the script is in the
+ * image, so an older image can find a file a newer one left behind. The script
+ * is used only when both exist, and the server is pinged over TCP otherwise,
+ * with `mariadb-admin` (11.0+ only ships this name) or `mysqladmin` (versions
+ * before 10.4 only ship this name). The image is checked at runtime rather
+ * than by its tag, so every version, including `lts` and `latest`, uses the
+ * same check. Using 127.0.0.1 rather than localhost avoids the Unix socket,
+ * which the temporary server used to initialize a new volume answers before
+ * the real server is listening.
+ *
+ * Timing is generous to support slow CI environments.
+ */
+const MARIADB_HEALTHCHECK = {
+	test: [
+		'CMD-SHELL',
+		'if [ -f /var/lib/mysql/.my-healthcheck.cnf ] && command -v healthcheck.sh > /dev/null; then healthcheck.sh --connect --innodb_initialized; else "$$(command -v mariadb-admin || echo mysqladmin)" ping -h 127.0.0.1 --protocol=tcp -uroot -p"$$MYSQL_ROOT_PASSWORD"; fi',
+	],
+	interval: '5s',
+	timeout: '10s',
+	retries: 12,
+	start_period: '60s',
+};
+
 /**
  * Creates a docker-compose config object which, when serialized into a
  * docker-compose.yml file, tells docker-compose how to run the environment.
@@ -107,20 +142,30 @@ module.exports = function buildDockerComposeConfig( config ) {
 				environment: {
 					MYSQL_ALLOW_EMPTY_PASSWORD: 'yes',
 					MYSQL_DATABASE: 'wordpress',
+					// Ensures healthcheck user is created for existing installations.
+					MARIADB_AUTO_UPGRADE: '1',
 				},
 				volumes: [ 'mysql:/var/lib/mysql' ],
+				healthcheck: MARIADB_HEALTHCHECK,
 			},
 			'tests-mysql': {
 				image: 'mariadb:${WP_ENV_MARIADB_VERSION:-latest}',
 				ports: [ '3306' ],
 				environment: {
 					MYSQL_ALLOW_EMPTY_PASSWORD: 'yes',
 					MYSQL_DATABASE: 'tests-wordpress',
+					// Ensures healthcheck user is created for existing installations.
+					MARIADB_AUTO_UPGRADE: '1',
 				},
 				volumes: [ 'mysql-test:/var/lib/mysql' ],
+				healthcheck: MARIADB_HEALTHCHECK,
 			},
 			wordpress: {
-				depends_on: [ 'mysql' ],
+				depends_on: {
+					mysql: {
+						condition: 'service_healthy',
+					},
+				},
 				image: 'wordpress',
 				ports: [ developmentPorts ],
 				environment: {
@@ -131,7 +176,11 @@ module.exports = function buildDockerComposeConfig( config ) {
 				volumes: developmentMounts,
 			},
 			'tests-wordpress': {
-				depends_on: [ 'tests-mysql' ],
+				depends_on: {
+					'tests-mysql': {
+						condition: 'service_healthy',
+					},
+				},
 				image: 'wordpress',
 				ports: [ testsPorts ],
 				environment: {

packages/env/lib/commands/start.js

@@ -10,7 +10,6 @@ const inquirer = require( 'inquirer' );
 /**
  * Promisified dependencies
  */
-const sleep = util.promisify( setTimeout );
 const rimraf = util.promisify( require( 'rimraf' ) );
 
 /**
@@ -21,7 +20,6 @@ const stop = require( './stop' );
 const initConfig = require( '../init-config' );
 const downloadSource = require( '../download-source' );
 const {
-	checkDatabaseConnection,
 	makeContentDirectoriesWritable,
 	configureWordPress,
 	copyCoreFiles,
@@ -129,19 +127,6 @@ module.exports = async function start( { spinner, debug } ) {
 		] );
 	}
 
-	try {
-		await checkDatabaseConnection( config );
-	} catch ( error ) {
-		// Wait 30 seconds for MySQL to accept connections.
-		await retry( () => checkDatabaseConnection( config ), {
-			times: 30,
-			delay: 1000,
-		} );
-
-		// It takes 3-4 seconds for MySQL to be ready after it starts accepting connections.
-		await sleep( 4000 );
-	}
-
 	// Retry WordPress installation in case MySQL *still* wasn't ready.
 	await Promise.all( [
 		retry( () => configureWordPress( 'development', config ), {

packages/env/lib/wordpress.js

@@ -44,20 +44,6 @@ async function makeContentDirectoriesWritable(
 	);
 }
 
-/**
- * Checks a WordPress database connection. An error is thrown if the test is
- * unsuccessful.
- *
- * @param {WPConfig} config The wp-env config object.
- */
-async function checkDatabaseConnection( { dockerComposeConfigPath, debug } ) {
-	await dockerCompose.run( 'cli', 'wp db check', {
-		config: dockerComposeConfigPath,
-		commandOptions: [ '--rm' ],
-		log: debug,
-	} );
-}
-
 /**
  * Configures WordPress for the given environment by installing WordPress,
  * activating all plugins, and activating the first theme. These steps are
@@ -209,7 +195,6 @@ async function copyCoreFiles( fromPath, toPath ) {
 
 module.exports = {
 	makeContentDirectoriesWritable,
-	checkDatabaseConnection,
 	configureWordPress,
 	resetDatabase,
 	copyCoreFiles,

packages/env/test/build-docker-compose-config.js

@@ -1,3 +1,11 @@
+/**
+ * External dependencies
+ */
+const { execFileSync } = require( 'child_process' );
+const fs = require( 'fs' );
+const os = require( 'os' );
+const path = require( 'path' );
+
 /**
  * Internal dependencies
  */
@@ -101,4 +109,171 @@ describe( 'buildDockerComposeConfig', () => {
 			);
 		}
 	} );
+
+	it( 'should add healthcheck to the mysql service', () => {
+		const config = buildDockerComposeConfig( CONFIG );
+
+		for ( const service of [ 'mysql', 'tests-mysql' ] ) {
+			expect( config.services[ service ].healthcheck ).toEqual( {
+				test: [ 'CMD-SHELL', expect.any( String ) ],
+				interval: '5s',
+				timeout: '10s',
+				retries: 12,
+				start_period: '60s',
+			} );
+
+			// Verify MARIADB_AUTO_UPGRADE is set for existing installations
+			expect(
+				config.services[ service ].environment.MARIADB_AUTO_UPGRADE
+			).toBe( '1' );
+		}
+	} );
+
+	/*
+	 * Runs the health check command in a shell, with stub scripts in place of
+	 * the MariaDB tools. Each stub logs how it was called and exits with the
+	 * code the test gives it. Skipped on Windows, which has no `/bin/sh` to run
+	 * the command or the stubs; the command itself only ever runs in Linux
+	 * containers.
+	 */
+	( process.platform === 'win32' ? describe.skip : describe )(
+		'MariaDB health check command',
+		() => {
+			const STUB =
+				'#!/bin/sh\necho "${0##*/} $*" >> "$STUB_LOG"\nexit "$EXIT_CODE"\n';
+			let directory;
+			let binDirectory;
+			let logFile;
+			let configFile;
+
+			beforeEach( () => {
+				directory = fs.mkdtempSync(
+					path.join( os.tmpdir(), 'wp-env-healthcheck-' )
+				);
+				binDirectory = path.join( directory, 'bin' );
+				logFile = path.join( directory, 'calls.log' );
+				configFile = path.join( directory, '.my-healthcheck.cnf' );
+				fs.mkdirSync( binDirectory );
+			} );
+
+			afterEach( () => {
+				fs.rmSync( directory, { recursive: true, force: true } );
+			} );
+
+			function addStub( name, exitCode ) {
+				const stubPath = path.join( binDirectory, name );
+				fs.writeFileSync(
+					stubPath,
+					STUB.replace( '"$EXIT_CODE"', String( exitCode ) )
+				);
+				fs.chmodSync( stubPath, 0o755 );
+			}
+
+			/*
+			 * Runs the command the way Docker does after Compose replaces each `$$`
+			 * with `$`, with the data directory's config file moved into the temporary
+			 * directory, and only the stubs on the PATH.
+			 */
+			function runHealthcheck() {
+				const [ , command ] = buildDockerComposeConfig(
+					CONFIG
+				).services.mysql.healthcheck.test;
+				const script = command
+					.replace( /\$\$/g, '$' )
+					.replace(
+						'/var/lib/mysql/.my-healthcheck.cnf',
+						configFile
+					);
+
+				let exitCode = 0;
+				try {
+					execFileSync( '/bin/sh', [ '-c', script ], {
+						env: {
+							PATH: binDirectory,
+							STUB_LOG: logFile,
+							MYSQL_ROOT_PASSWORD: 'password',
+						},
+						stdio: 'ignore',
+					} );
+				} catch ( error ) {
+					exitCode = error.status;
+				}
+
+				const calls = fs.existsSync( logFile )
+					? fs.readFileSync( logFile, 'utf8' ).trim().split( '\n' )
+					: [];
+
+				return { exitCode, calls };
+			}
+
+			it.each( [ 0, 1 ] )(
+				'runs healthcheck.sh and returns its exit code %j when the healthcheck user exists',
+				( exitCode ) => {
+					fs.writeFileSync( configFile, '' );
+					addStub( 'healthcheck.sh', exitCode );
+					addStub( 'mariadb-admin', 0 );
+
+					expect( runHealthcheck() ).toEqual( {
+						exitCode,
+						calls: [
+							'healthcheck.sh --connect --innodb_initialized',
+						],
+					} );
+				}
+			);
+
+			it( 'pings instead when the healthcheck user exists but the image has no healthcheck.sh', () => {
+				fs.writeFileSync( configFile, '' );
+				addStub( 'mariadb-admin', 0 );
+
+				expect( runHealthcheck() ).toEqual( {
+					exitCode: 0,
+					calls: [
+						'mariadb-admin ping -h 127.0.0.1 --protocol=tcp -uroot -ppassword',
+					],
+				} );
+			} );
+
+			it.each( [ 0, 1 ] )(
+				'pings with mariadb-admin and returns its exit code %j without the healthcheck user',
+				( exitCode ) => {
+					addStub( 'healthcheck.sh', 0 );
+					addStub( 'mariadb-admin', exitCode );
+					addStub( 'mysqladmin', 0 );
+
+					expect( runHealthcheck() ).toEqual( {
+						exitCode,
+						calls: [
+							'mariadb-admin ping -h 127.0.0.1 --protocol=tcp -uroot -ppassword',
+						],
+					} );
+				}
+			);
+
+			it.each( [ 0, 1 ] )(
+				'pings with mysqladmin and returns its exit code %j when mariadb-admin is missing',
+				( exitCode ) => {
+					addStub( 'mysqladmin', exitCode );
+
+					expect( runHealthcheck() ).toEqual( {
+						exitCode,
+						calls: [
+							'mysqladmin ping -h 127.0.0.1 --protocol=tcp -uroot -ppassword',
+						],
+					} );
+				}
+			);
+		}
+	);
+
+	it( 'should use service_healthy condition for WordPress depends_on', () => {
+		const config = buildDockerComposeConfig( CONFIG );
+
+		expect( config.services.wordpress.depends_on ).toEqual( {
+			mysql: { condition: 'service_healthy' },
+		} );
+		expect( config.services[ 'tests-wordpress' ].depends_on ).toEqual( {
+			'tests-mysql': { condition: 'service_healthy' },
+		} );
+	} );
 } );