Merge pull request #679 from heroku/boot-scripts-test-option

Add --test option for boot scripts
heroku · Jan 24, 2024 · 39f44df · 39f44df
2 parents 69109c3 + c419588
commit 39f44df
Show file tree

Hide file tree

Showing 4 changed files with 150 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # heroku-buildpack-php CHANGELOG
 
+## v245 (2024-01-??)
+
+### ADD
+
+- Boot scripts now have a `--test`/`-t` option to test PHP-FPM and web server configs and then exit. Can be repeated to dump configs for either or both, see `--help` for details. [David Zuelke]
+
 ## v244 (2024-01-24)
 
 ### ADD

diff --git a/bin/heroku-php-apache2 b/bin/heroku-php-apache2
@@ -22,6 +22,7 @@ fi
 bp_dir=$(cd $(dirname $(realpath $0)); cd ..; pwd)
 
 verbose=
+conftest=
 
 php_passthrough() {
 	local dir=$(dirname "$1")
@@ -143,6 +144,9 @@ print_help() {
 		                          is not given, then the port number to use is read from
 		                          the \$PORT environment variable, or a random port is
 		                          chosen if that variable does not exist.
+		  -t, --test              Test PHP-FPM and Apache2 configuration. When repeated,
+		                          will dump PHP-FPM config (-tt), Apache2 config (-ttt),
+		                          or both (-tttt).
 		  -v, --verbose           Be more verbose during startup.
 		
 		The <BPDIR> placeholder above represents the base directory of this buildpack:
@@ -193,7 +197,7 @@ export PHP_INI_SCAN_DIR="${PHP_INI_SCAN_DIR}${bp_dir}/conf/php/apm-nostart-overr
 # init logs array here as empty before parsing options; -l could append to it, but the default list gets added later since we use $PORT in there and that can be set using -p
 declare -a logs
 
-optstring=":-:C:c:F:f:i:l:p:vh"
+optstring=":-:C:c:F:f:i:l:p:vth"
 
 # process flags first
 while getopts "$optstring" opt; do
@@ -203,6 +207,9 @@ while getopts "$optstring" opt; do
 				verbose)
 					verbose=1
 					;;
+				test)
+					let "conftest++" || echo "$(basename "$0"): Config test mode" >&2 # increment, so it can be repeated (exits 1 for foo=)
+					;;
 				help)
 					print_help 2>&1
 					exit
@@ -216,6 +223,9 @@ while getopts "$optstring" opt; do
 		v)
 			verbose=1
 			;;
+		t)
+			let "conftest++" || echo "$(basename "$0"): Config test mode" >&2 # increment, so it can be repeated (exits 1 for foo=)
+			;;
 		h)
 			print_help 2>&1
 			exit
@@ -382,6 +392,52 @@ else
 	echo '$WEB_CONCURRENCY env var is set, skipping automatic calculation' >&2
 fi
 
+fpm_conftest=
+httpd_conftest=
+fpm_config_tmp=$fpm_config
+case $conftest in
+	[24])
+		# to dump the FPM config, we need to ensure the log level is NOTICE
+		fpm_config_tmp=$(mktemp "$fpm_config.XXXXX")
+		cp "$fpm_config" "$fpm_config_tmp"
+		trap 'trap - EXIT; rm "$fpm_config_tmp"' EXIT
+		echo -e "\n[global]\nlog_level = notice" >> "$fpm_config_tmp"
+		;;& # resume
+	1)
+		fpm_conftest="-t"
+		httpd_conftest="-t"
+		;;
+	2)
+		fpm_conftest="-tt"
+		httpd_conftest="-t"
+		;;
+	3)
+		fpm_conftest="-t"
+		httpd_conftest="-S"
+		;;
+	4)
+		fpm_conftest="-tt"
+		httpd_conftest="-S"
+		;;
+esac
+
+fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
+httpd_pidfile=$(httpd -t -D DUMP_RUN_CFG 2> /dev/null | sed -n -E 's/PidFile: "(.+)"/\1/p') # get PidFile location
+
+# build command string arrays for PHP-FPM and HTTPD
+# we're using an array, because we need to correctly preserve quoting, spaces, etc
+fpm_command=( php-fpm ${fpm_conftest} --pid "$fpm_pidfile" --nodaemonize -y "$fpm_config_tmp" ${php_config:+-c "$php_config"} )
+httpd_command=( httpd ${httpd_conftest} -D NO_DETACH -c "Include $httpd_config" )
+
+if [[ $conftest ]]; then
+	echo -e "\n$(basename "$0"): Config testing php-fpm using ${fpm_command[@]}:" >&2
+	"${fpm_command[@]}"
+	echo -e "\n$(basename "$0"): Config testing httpd using ${httpd_command[@]}:" >&2
+	"${httpd_command[@]}"
+	echo -e "\n$(basename "$0"): All configs okay." >&2
+	exit 0
+fi
+
 # make a shared pipe; we'll write the name of the process that exits to it once that happens, and wait for that event below
 # this particular call works on Linux and Mac OS (will create a literal ".XXXXXX" on Mac, but that doesn't matter).
 wait_pipe=$(mktemp -t "heroku.waitpipe-$PORT.XXXXXX" -u)
@@ -503,7 +559,6 @@ fi
 	wait 2> /dev/null || true # redirect stderr to prevent possible trap race condition warnings
 ) & pids+=($!)
 
-fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
 (
 	trap 'echo "php-fpm" >&3;' EXIT
 	trap '' INT;
@@ -519,7 +574,8 @@ fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
 		export PHP_INI_SCAN_DIR=$_PHP_INI_SCAN_DIR
 	fi
 
-	php-fpm --pid "$fpm_pidfile" --nodaemonize -y "$fpm_config" ${php_config:+-c "$php_config"} & pid=$!
+	# execute the command we built earlier, with the correct quoting etc expanded
+	"${fpm_command[@]}" & pid=$!
 	# wait for the pidfile in the trap; otherwise, a previous subshell failing may result in us getting a SIGTERM and forwarding it to the child process before that child process is ready to process signals
 	trap 'echo "Stopping php-fpm gracefully..." >&2; wait_pid_and_pidfile $pid "$fpm_pidfile"; kill -QUIT $pid 2> /dev/null || true' USR1
 	# we always want to try and stop gracefully, especially since on Heroku we might be getting a process group wide SIGTERM but running a patched PHP-FPM that ignores SIGTERM; so if that env var is set, we ignore SIGTERM in this subshell as well
@@ -535,7 +591,6 @@ fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
 	wait $pid 2> /dev/null || true # redirect stderr to prevent possible trap race condition warnings
 ) & pids+=($!)
 
-httpd_pidfile=$(httpd -t -D DUMP_RUN_CFG 2> /dev/null | sed -n -E 's/PidFile: "(.+)"/\1/p') # get PidFile location
 (
 	trap 'echo "httpd" >&3;' EXIT
 	trap '' INT;
@@ -548,7 +603,8 @@ httpd_pidfile=$(httpd -t -D DUMP_RUN_CFG 2> /dev/null | sed -n -E 's/PidFile: "(
 
 	echo "Starting httpd..." >&2
 
-	httpd -D NO_DETACH -c "Include $httpd_config" & pid=$!
+	# execute the command we built earlier, with the correct quoting etc expanded
+	"${httpd_command[@]}" & pid=$!
 	# wait for the pidfile in the trap; otherwise, a previous subshell failing may result in us getting a SIGTERM and forwarding it to the child process before that child process is ready to process signals
 	trap 'echo "Stopping httpd gracefully..." >&2; wait_pid_and_pidfile $pid "$httpd_pidfile"; kill -WINCH $pid 2> /dev/null || true' USR1
 	# we always want to try and stop gracefully, especially since on Heroku we might be getting a process group wide SIGTERM but running a patched HTTPD that ignores SIGTERM; so if that env var is set, we ignore SIGTERM in this subshell as well

diff --git a/bin/heroku-php-nginx b/bin/heroku-php-nginx
@@ -22,6 +22,7 @@ fi
 bp_dir=$(cd $(dirname $(realpath $0)); cd ..; pwd)
 
 verbose=
+conftest=
 
 php_passthrough() {
 	local dir=$(dirname "$1")
@@ -143,6 +144,9 @@ print_help() {
 		                          is not given, then the port number to use is read from
 		                          the \$PORT environment variable, or a random port is
 		                          chosen if that variable does not exist.
+		  -t, --test              Test PHP-FPM and Nginx configuration. When repeated,
+		                          will dump PHP-FPM config (-tt), Nginx config (-ttt),
+		                          or both (-tttt).
 		  -v, --verbose           Be more verbose during startup.
 		
 		The <BPDIR> placeholder above represents the base directory of this buildpack:
@@ -192,7 +196,7 @@ export PHP_INI_SCAN_DIR="${PHP_INI_SCAN_DIR}${bp_dir}/conf/php/apm-nostart-overr
 # init logs array here as empty before parsing options; -l could append to it, but the default list gets added later since we use $PORT in there and that can be set using -p
 declare -a logs
 
-optstring=":-:C:c:F:f:i:l:p:vh"
+optstring=":-:C:c:F:f:i:l:p:vth"
 
 # process flags first
 while getopts "$optstring" opt; do
@@ -202,6 +206,9 @@ while getopts "$optstring" opt; do
 				verbose)
 					verbose=1
 					;;
+				test)
+					let "conftest++" || echo "$(basename "$0"): Config test mode" >&2 # increment, so it can be repeated (exits 1 for foo=)
+					;;
 				help)
 					print_help 2>&1
 					exit
@@ -215,6 +222,9 @@ while getopts "$optstring" opt; do
 		v)
 			verbose=1
 			;;
+		t)
+			let "conftest++" || echo "$(basename "$0"): Config test mode" >&2 # increment, so it can be repeated (exits 1 for foo=)
+			;;
 		h)
 			print_help 2>&1
 			exit
@@ -382,6 +392,52 @@ else
 	echo '$WEB_CONCURRENCY env var is set, skipping automatic calculation' >&2
 fi
 
+fpm_conftest=
+nginx_conftest=
+fpm_config_tmp=$fpm_config
+case $conftest in
+	[24])
+		# to dump the FPM config, we need to ensure the log level is NOTICE
+		fpm_config_tmp=$(mktemp "$fpm_config.XXXXX")
+		cp "$fpm_config" "$fpm_config_tmp"
+		trap 'trap - EXIT; rm "$fpm_config_tmp"' EXIT
+		echo -e "\n[global]\nlog_level = notice" >> "$fpm_config_tmp"
+		;;& # resume
+	1)
+		fpm_conftest="-t"
+		nginx_conftest="-t"
+		;;
+	2)
+		fpm_conftest="-tt"
+		nginx_conftest="-t"
+		;;
+	3)
+		fpm_conftest="-t"
+		nginx_conftest="-T"
+		;;
+	4)
+		fpm_conftest="-tt"
+		nginx_conftest="-T"
+		;;
+esac
+
+fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
+nginx_pidfile=$(mktemp -t "heroku.nginx.pid-$PORT.XXXXXX" -u)
+
+# build command string arrays for PHP-FPM and Nginx
+# we're using an array, because we need to correctly preserve quoting, spaces, etc
+fpm_command=( php-fpm ${fpm_conftest} --pid "$fpm_pidfile" --nodaemonize -y "$fpm_config_tmp" ${php_config:+-c "$php_config"} )
+nginx_command=( nginx ${nginx_conftest} -c "$nginx_main" -g "pid $nginx_pidfile; include $nginx_config;" )
+
+if [[ $conftest ]]; then
+	echo -e "\n$(basename "$0"): Config testing php-fpm using ${fpm_command[@]}:" >&2
+	"${fpm_command[@]}"
+	echo -e "\n$(basename "$0"): Config testing nginx using ${nginx_command[@]}:" >&2
+	"${nginx_command[@]}"
+	echo -e "\n$(basename "$0"): All configs okay." >&2
+	exit 0
+fi
+
 # make a shared pipe; we'll write the name of the process that exits to it once that happens, and wait for that event below
 # this particular call works on Linux and Mac OS (will create a literal ".XXXXXX" on Mac, but that doesn't matter).
 wait_pipe=$(mktemp -t "heroku.waitpipe-$PORT.XXXXXX" -u)
@@ -503,7 +559,6 @@ fi
 	wait 2> /dev/null || true # redirect stderr to prevent possible trap race condition warnings
 ) & pids+=($!)
 
-fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
 (
 	trap 'echo "php-fpm" >&3;' EXIT
 	trap '' INT;
@@ -519,7 +574,8 @@ fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
 		export PHP_INI_SCAN_DIR=$_PHP_INI_SCAN_DIR
 	fi
 
-	php-fpm --pid "$fpm_pidfile" --nodaemonize -y "$fpm_config" ${php_config:+-c "$php_config"} & pid=$!
+	# execute the command we built earlier, with the correct quoting etc expanded
+	"${fpm_command[@]}" & pid=$!
 	# wait for the pidfile in the trap; otherwise, a previous subshell failing may result in us getting a SIGTERM and forwarding it to the child process before that child process is ready to process signals
 	trap 'echo "Stopping php-fpm gracefully..." >&2; wait_pid_and_pidfile $pid "$fpm_pidfile"; kill -QUIT $pid 2> /dev/null || true' USR1
 	# we always want to try and stop gracefully, especially since on Heroku we might be getting a process group wide SIGTERM but running a patched PHP-FPM that ignores SIGTERM; so if that env var is set, we ignore SIGTERM in this subshell as well
@@ -535,7 +591,6 @@ fpm_pidfile=$(mktemp -t "heroku.php-fpm.pid-$PORT.XXXXXX" -u)
 	wait $pid 2> /dev/null || true # redirect stderr to prevent possible trap race condition warnings
 ) & pids+=($!)
 
-nginx_pidfile=$(mktemp -t "heroku.nginx.pid-$PORT.XXXXXX" -u)
 (
 	trap 'echo "nginx" >&3;' EXIT
 	trap '' INT;
@@ -548,7 +603,8 @@ nginx_pidfile=$(mktemp -t "heroku.nginx.pid-$PORT.XXXXXX" -u)
 
 	echo "Starting nginx..." >&2
 
-	nginx -c "$nginx_main" -g "pid $nginx_pidfile; include $nginx_config;" & pid=$!
+	# execute the command we built earlier, with the correct quoting etc expanded
+	"${nginx_command[@]}" & pid=$!
 	# wait for the pidfile in the trap; otherwise, a previous subshell failing may result in us getting a SIGTERM and forwarding it to the child process before that child process is ready to process signals
 	trap 'echo "Stopping nginx gracefully..." >&2; wait_pid_and_pidfile $pid "$nginx_pidfile"; kill -QUIT $pid 2> /dev/null || true' USR1
 	# we always want to try and stop gracefully, especially since on Heroku we might be getting a process group wide SIGTERM but running a patched Nginx that ignores SIGTERM; so if that env var is set, we ignore SIGTERM in this subshell as well

diff --git a/test/spec/php_shared_concurrency.rb b/test/spec/php_shared_concurrency.rb
@@ -18,90 +18,90 @@
 		context "setting concurrency via .user.ini memory_limit" do
 			it "calculates concurrency correctly" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose docroot/", :return_obj => true) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt docroot/", :return_obj => true) }.output)
 						 .to match("PHP memory_limit is 32M Bytes")
-						.and match("Starting php-fpm with 16 workers...")
+						.and match("pm.max_children = 16")
 				end
 			end
 			it "always launches at least one worker" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose docroot/onegig/", :return_obj => true) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt docroot/onegig/", :return_obj => true) }.output)
 						 .to match("PHP memory_limit is 1024M Bytes")
-						.and match("Starting php-fpm with 1 workers...")
+						.and match("pm.max_children = 1")
 				end
 			end
 			it "is only done for a .user.ini directly in the document root" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose", :return_obj => true) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt", :return_obj => true) }.output)
 						 .to match("PHP memory_limit is 128M Bytes")
-						.and match("Starting php-fpm with 4 workers...")
+						.and match("pm.max_children = 4")
 				end
 			end
 		end
 
 		context "setting concurrency via FPM config memory_limit" do
 			it "calculates concurrency correctly" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose -F conf/fpm.include.conf", :return_obj => true) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt -F conf/fpm.include.conf", :return_obj => true) }.output)
 						 .to match("PHP memory_limit is 32M Bytes")
-						.and match("Starting php-fpm with 16 workers...")
+						.and match("pm.max_children = 16")
 				end
 			end
 			it "always launches at least one worker" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose -F conf/fpm.onegig.conf", :return_obj => true) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt -F conf/fpm.onegig.conf", :return_obj => true) }.output)
 						 .to match("PHP memory_limit is 1024M Bytes")
-						.and match("Starting php-fpm with 1 workers...")
+						.and match("pm.max_children = 1")
 				end
 			end
 			it "takes precedence over a .user.ini memory_limit" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose -F conf/fpm.include.conf docroot/onegig/", :return_obj => true) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt -F conf/fpm.include.conf docroot/onegig/", :return_obj => true) }.output)
 						 .to match("PHP memory_limit is 32M Bytes")
-						.and match("Starting php-fpm with 16 workers...")
+						.and match("pm.max_children = 16")
 				end
 			end
 		end
 
 		context "setting WEB_CONCURRENCY explicitly" do
 			it "uses the explicit value" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose", :return_obj => true, :heroku => {:env => "WEB_CONCURRENCY=22"}) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt", :return_obj => true, :heroku => {:env => "WEB_CONCURRENCY=22"}) }.output)
 						 .to match("\\$WEB_CONCURRENCY env var is set, skipping automatic calculation")
-						.and match("Starting php-fpm with 22 workers...")
+						.and match("pm.max_children = 22")
 				end
 			end
 			it "overrides a .user.ini memory_limit" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose docroot/onegig/", :return_obj => true, :heroku => {:env => "WEB_CONCURRENCY=22"}) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt docroot/onegig/", :return_obj => true, :heroku => {:env => "WEB_CONCURRENCY=22"}) }.output)
 						 .to match("\\$WEB_CONCURRENCY env var is set, skipping automatic calculation")
-						.and match("Starting php-fpm with 22 workers...")
+						.and match("pm.max_children = 22")
 				end
 			end
 			it "overrides an FPM config memory_limit" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose -F conf/fpm.onegig.conf", :return_obj => true, :heroku => {:env => "WEB_CONCURRENCY=22"}) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt -F conf/fpm.onegig.conf", :return_obj => true, :heroku => {:env => "WEB_CONCURRENCY=22"}) }.output)
 						 .to match("\\$WEB_CONCURRENCY env var is set, skipping automatic calculation")
-						.and match("Starting php-fpm with 22 workers...")
+						.and match("pm.max_children = 22")
 				end
 			end
 		end
 
 		context "running on a Performance-L dyno" do
 			it "restricts the app to 6 GB of RAM", :if => series < "7.4" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose", :return_obj => true, :heroku => {:size => "Performance-L"}) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt", :return_obj => true, :heroku => {:size => "Performance-L"}) }.output)
 						 .to match("Detected 15032385536 Bytes of RAM")
 						.and match("Limiting to 6G Bytes of RAM usage")
-						.and match("Starting php-fpm with 48 workers...")
+						.and match("pm.max_children = 48")
 				end
 			end
 
 			it "uses all available RAM for PHP-FPM workers", :unless => series < "7.4" do
 				retry_until retry: 3, sleep: 5 do
-					expect(expect_exit(code: 0) { @app.run("./waitforit.sh 15 'ready for connections' heroku-php-#{server} --verbose", :return_obj => true, :heroku => {:size => "Performance-L"}) }.output)
+					expect(expect_exit(code: 0) { @app.run("heroku-php-#{server} -tt", :return_obj => true, :heroku => {:size => "Performance-L"}) }.output)
 						 .to match("Detected 15032385536 Bytes of RAM")
-						.and match("Starting php-fpm with 112 workers...")
+						.and match("pm.max_children = 112")
 				end
 			end
 		end