Merge pull request #1899 from pi-hole/improve/check_shutdown

Improve/check shutdown

Author: PromoFaux

src/bash_functions.sh

@@ -104,11 +104,9 @@ migrate_gravity() {
     echo "  [i] Gravity migration checks"
     gravityDBfile=$(getFTLConfigValue files.gravity)
 
-    if [[ -z "${PYTEST}" ]]; then
-        if [[ ! -f /etc/pihole/adlists.list ]]; then
-            echo "  [i] No adlist file found, creating one with a default blocklist"
-            echo "https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts" >/etc/pihole/adlists.list
-        fi
+    if [[ ! -f /etc/pihole/adlists.list ]]; then
+        echo "  [i] No adlist file found, creating one with a default blocklist"
+        echo "https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts" >/etc/pihole/adlists.list
     fi
 
     if [ ! -f "${gravityDBfile}" ]; then

src/start.sh

@@ -77,7 +77,7 @@ start() {
 
     # Wait until the FTL log contains the "FTL started" message before continuing, timeout after 10 seconds
     # exit if we do not find it
-    pihole-FTL wait-for '########## FTL started' /var/log/pihole/FTL.log 10 "0 > /dev/null"
+    pihole-FTL wait-for '########## FTL started' /var/log/pihole/FTL.log 10 0 > /dev/null
     if [ $? -ne 0 ]; then
         echo "  [✗] FTL did not start - stopping container"
         exit 1
@@ -146,12 +146,6 @@ stop() {
     echo "      https://docs.docker.com/engine/containers/start-containers-automatically/#use-a-restart-policy"
     echo ""
 
-    # If we are running pytest, keep the container alive for a little longer
-    # to allow the tests to complete
-    if [[ ${PYTEST} ]]; then
-        sleep 10
-    fi
-
     exit "${FTL_EXIT_CODE}"
 
 }

test/tests/conftest.py

@@ -45,10 +45,6 @@ def docker(request):
     for env_var in env_vars:
         cmd.extend(["-e", env_var])
 
-    # ensure PYTEST=1 is set
-    if not any("PYTEST=1" in arg for arg in cmd):
-        cmd.extend(["-e", "PYTEST=1"])
-
     # add default TZ if not already set
     if not any("TZ=" in arg for arg in cmd):
         cmd.extend(["-e", 'TZ="Europe/London"'])

test/tests/test_general.py

@@ -49,19 +49,44 @@ def test_pihole_ftl_architecture(docker):
     assert platform in func.stdout
 
 
-# Wait 5 seconds for startup, then kill the start.sh script
-# Finally, check the FTL log to see if it has been shut down cleanly
-def test_pihole_ftl_clean_shutdown(docker):
-    func = docker.run(
-        """
-        sleep 5
-        killall --signal 15 start.sh
-        sleep 5
-        grep 'terminated' /var/log/pihole/FTL.log
-    """
+# Wait for FTL to start up, then stop the container gracefully
+# Finally, check the container logs to see if FTL was shut down cleanly
+def test_pihole_ftl_starts_and_shuts_down_cleanly(docker):
+    import subprocess
+    import time
+
+    # Get the container ID from the docker fixture
+    container_id = docker.backend.name
+
+    # Wait for FTL to fully start up by checking logs
+    max_wait_time = 60  # Maximum wait time in seconds
+    start_time = time.time()
+    ftl_started = False
+
+    while time.time() - start_time < max_wait_time:
+        result = subprocess.run(
+            ["docker", "logs", container_id], capture_output=True, text=True
+        )
+
+        if "########## FTL started" in result.stdout:
+            ftl_started = True
+            break
+
+        time.sleep(1)  # Check every second
+
+    assert ftl_started, f"FTL did not start within {max_wait_time} seconds"
+
+    # Stop the container gracefully (sends SIGTERM)
+    subprocess.run(["docker", "stop", container_id], check=True)
+
+    # Get the container logs
+    result = subprocess.run(
+        ["docker", "logs", container_id], capture_output=True, text=True
     )
-    assert "INFO: ########## FTL terminated after" in func.stdout
-    assert "(code 0)" in func.stdout
+
+    # Check for clean shutdown messages in the logs
+    assert "INFO: ########## FTL terminated after" in result.stdout
+    assert "(code 0)" in result.stdout
 
 
 def test_cronfile_valid(docker):