From b707890f1048a188c0182642a5dc01d02661fc54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Tue, 27 May 2025 20:09:59 +0200 Subject: [PATCH 1/7] Use PID1 to determine which command to use when toggeling services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- automated install/basic-install.sh | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh index d84c8750..f4b51c6d 100755 --- a/automated install/basic-install.sh +++ b/automated install/basic-install.sh @@ -228,6 +228,13 @@ is_command() { command -v "${check_command}" >/dev/null 2>&1 } +is_pid1() { + # Checks to see if the given command runs as PID 1 + local is_pid1="$1" + + ps -p 1 -o comm= | grep -q "${is_pid1}" +} + # Compatibility package_manager_detect() { @@ -1152,7 +1159,7 @@ installConfigs() { fi # Install pihole-FTL systemd or init.d service, based on whether systemd is the init system or not - if ps -p 1 -o comm= | grep -q systemd; then + if is_pid1 systemd; then install -T -m 0644 "${PI_HOLE_LOCAL_REPO}/advanced/Templates/pihole-FTL.systemd" '/etc/systemd/system/pihole-FTL.service' # Remove init.d service if present @@ -1220,9 +1227,12 @@ stop_service() { # Can softfail, as process may not be installed when this is called local str="Stopping ${1} service" printf " %b %s..." "${INFO}" "${str}" - if is_command systemctl; then + # If systemd is PID 1, + if is_pid1 systemd; then + # use that to restart the service systemctl -q stop "${1}" || true else + # Otherwise, fall back to the service command service "${1}" stop >/dev/null || true fi printf "%b %b %s...\\n" "${OVER}" "${TICK}" "${str}" @@ -1233,8 +1243,8 @@ restart_service() { # Local, named variables local str="Restarting ${1} service" printf " %b %s..." "${INFO}" "${str}" - # If systemctl exists, - if is_command systemctl; then + # If systemd is PID 1, + if is_pid1 systemd; then # use that to restart the service systemctl -q restart "${1}" else @@ -1249,8 +1259,8 @@ enable_service() { # Local, named variables local str="Enabling ${1} service to start on reboot" printf " %b %s..." "${INFO}" "${str}" - # If systemctl exists, - if is_command systemctl; then + # If systemd is PID1, + if is_pid1 systemd; then # use that to enable the service systemctl -q enable "${1}" else @@ -1265,8 +1275,8 @@ disable_service() { # Local, named variables local str="Disabling ${1} service" printf " %b %s..." "${INFO}" "${str}" - # If systemctl exists, - if is_command systemctl; then + # If systemd is PID1, + if is_pid1 systemd; then # use that to disable the service systemctl -q disable "${1}" else @@ -1277,8 +1287,8 @@ disable_service() { } check_service_active() { - # If systemctl exists, - if is_command systemctl; then + # If systemd is PID1, + if is_pid1 systemd; then # use that to check the status of the service systemctl -q is-enabled "${1}" 2>/dev/null else From 137338e6a8445347b97cc04693bdb2801e60fa3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Tue, 27 May 2025 21:23:56 +0200 Subject: [PATCH 2/7] Use service wrappers in all scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- advanced/Scripts/piholeARPTable.sh | 11 +++++++++-- advanced/Scripts/piholeLogFlush.sh | 11 +++++++++-- automated install/uninstall.sh | 11 ++++++----- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/advanced/Scripts/piholeARPTable.sh b/advanced/Scripts/piholeARPTable.sh index c62acdbc..120df5b8 100755 --- a/advanced/Scripts/piholeARPTable.sh +++ b/advanced/Scripts/piholeARPTable.sh @@ -20,6 +20,13 @@ utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh" # shellcheck source=./advanced/Scripts/utils.sh source "${utilsfile}" +readonly PI_HOLE_FILES_DIR="/etc/.pihole" +SKIP_INSTALL="true" +# shellcheck source="./automated install/basic-install.sh" +source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh" +# stop_service() is defined in basic-install.sh +# restart_service() is defined in basic-install.sh + # Determine database location DBFILE=$(getFTLConfigValue "files.database") if [ -z "$DBFILE" ]; then @@ -33,7 +40,7 @@ flushARP(){ fi # Stop FTL to prevent database access - if ! output=$(service pihole-FTL stop 2>&1); then + if ! output=$(stop_service pihole-FTL 2>&1); then echo -e "${OVER} ${CROSS} Failed to stop FTL" echo " Output: ${output}" return 1 @@ -65,7 +72,7 @@ flushARP(){ fi # Start FTL again - if ! output=$(service pihole-FTL restart 2>&1); then + if ! output=$(restart_service pihole-FTL 2>&1); then echo -e "${OVER} ${CROSS} Failed to restart FTL" echo " Output: ${output}" return 1 diff --git a/advanced/Scripts/piholeLogFlush.sh b/advanced/Scripts/piholeLogFlush.sh index ca70f31b..ac0c196f 100755 --- a/advanced/Scripts/piholeLogFlush.sh +++ b/advanced/Scripts/piholeLogFlush.sh @@ -17,6 +17,12 @@ utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh" # shellcheck source="./advanced/Scripts/utils.sh" source "${utilsfile}" +SKIP_INSTALL="true" +# shellcheck source="./automated install/basic-install.sh" +source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh" +# stop_service() is defined in basic-install.sh +# restart_service() is defined in basic-install.sh + # In case we're running at the same time as a system logrotate, use a # separate logrotate state file to prevent stepping on each other's # toes. @@ -104,13 +110,14 @@ else fi # Stop FTL to make sure it doesn't write to the database while we're deleting data - service pihole-FTL stop + stop_service pihole-FTL >/dev/null + # Delete most recent 24 hours from FTL's database, leave even older data intact (don't wipe out all history) deleted=$(pihole-FTL sqlite3 -ni "${DBFILE}" "DELETE FROM query_storage WHERE timestamp >= strftime('%s','now')-86400; select changes() from query_storage limit 1") # Restart FTL - service pihole-FTL restart + restart_service pihole-FTL >/dev/null if [[ "$*" != *"quiet"* ]]; then echo -e "${OVER} ${TICK} Deleted ${deleted} queries from long-term query database" fi diff --git a/automated install/uninstall.sh b/automated install/uninstall.sh index a158e595..eb1e9e29 100755 --- a/automated install/uninstall.sh +++ b/automated install/uninstall.sh @@ -13,6 +13,11 @@ source "/opt/pihole/COL_TABLE" # shellcheck source="./advanced/Scripts/utils.sh" source "/opt/pihole/utils.sh" +SKIP_INSTALL="true" +# shellcheck source="./automated install/basic-install.sh" +source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh" +# stop_service() is defined in basic-install.sh + ADMIN_INTERFACE_DIR=$(getFTLConfigValue "webserver.paths.webroot")$(getFTLConfigValue "webserver.paths.webhome") readonly ADMIN_INTERFACE_DIR @@ -102,11 +107,7 @@ removePiholeFiles() { # Remove FTL if command -v pihole-FTL &> /dev/null; then echo -ne " ${INFO} Removing pihole-FTL..." - if [[ -x "$(command -v systemctl)" ]]; then - systemctl stop pihole-FTL - else - service pihole-FTL stop - fi + stop_service pihole-FTL ${SUDO} rm -f /etc/systemd/system/pihole-FTL.service if [[ -d '/etc/systemd/system/pihole-FTL.service.d' ]]; then read -rp " ${QST} FTL service override directory /etc/systemd/system/pihole-FTL.service.d detected. Do you wish to remove this from your system? [y/N] " answer From f3166d7a785b43efd97d96a15977105359da33c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Tue, 27 May 2025 23:51:04 +0200 Subject: [PATCH 3/7] Adjust test to mock PID1 to be systemd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- test/_centos_10.Dockerfile | 2 +- test/_centos_9.Dockerfile | 2 +- test/_fedora_40.Dockerfile | 2 +- test/_fedora_41.Dockerfile | 2 +- test/_fedora_42.Dockerfile | 2 +- test/test_any_automated_install.py | 16 +++++++++------- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/test/_centos_10.Dockerfile b/test/_centos_10.Dockerfile index 78a89789..c6b2ca75 100644 --- a/test/_centos_10.Dockerfile +++ b/test/_centos_10.Dockerfile @@ -1,7 +1,7 @@ FROM quay.io/centos/centos:stream10 # Disable SELinux RUN echo "SELINUX=disabled" > /etc/selinux/config -RUN yum install -y --allowerasing curl git initscripts +RUN yum install -y --allowerasing curl git ENV GITDIR=/etc/.pihole ENV SCRIPTDIR=/opt/pihole diff --git a/test/_centos_9.Dockerfile b/test/_centos_9.Dockerfile index 73f53fa5..0e12edab 100644 --- a/test/_centos_9.Dockerfile +++ b/test/_centos_9.Dockerfile @@ -1,7 +1,7 @@ FROM quay.io/centos/centos:stream9 # Disable SELinux RUN echo "SELINUX=disabled" > /etc/selinux/config -RUN yum install -y --allowerasing curl git initscripts +RUN yum install -y --allowerasing curl git ENV GITDIR=/etc/.pihole ENV SCRIPTDIR=/opt/pihole diff --git a/test/_fedora_40.Dockerfile b/test/_fedora_40.Dockerfile index 43913895..56be9d84 100644 --- a/test/_fedora_40.Dockerfile +++ b/test/_fedora_40.Dockerfile @@ -1,5 +1,5 @@ FROM fedora:40 -RUN dnf install -y git initscripts +RUN dnf install -y git ENV GITDIR=/etc/.pihole ENV SCRIPTDIR=/opt/pihole diff --git a/test/_fedora_41.Dockerfile b/test/_fedora_41.Dockerfile index c03371a5..2a9ecf70 100644 --- a/test/_fedora_41.Dockerfile +++ b/test/_fedora_41.Dockerfile @@ -1,5 +1,5 @@ FROM fedora:41 -RUN dnf install -y git initscripts +RUN dnf install -y git ENV GITDIR=/etc/.pihole ENV SCRIPTDIR=/opt/pihole diff --git a/test/_fedora_42.Dockerfile b/test/_fedora_42.Dockerfile index 90b17c0b..34c7ef5d 100644 --- a/test/_fedora_42.Dockerfile +++ b/test/_fedora_42.Dockerfile @@ -1,5 +1,5 @@ FROM fedora:42 -RUN dnf install -y git initscripts +RUN dnf install -y git ENV GITDIR=/etc/.pihole ENV SCRIPTDIR=/opt/pihole diff --git a/test/test_any_automated_install.py b/test/test_any_automated_install.py index 0fa0453a..f10d2576 100644 --- a/test/test_any_automated_install.py +++ b/test/test_any_automated_install.py @@ -66,6 +66,14 @@ def test_installPihole_fresh_install_readableFiles(host): mock_command("dialog", {"*": ("", "0")}, host) # mock git pull mock_command_passthrough("git", {"pull": ("", "0")}, host) + # mock PID 1 to pretend to be systemd + mock_command_2( + "ps", + { + "-p 1": ("systemd", "0"), + }, + host, + ) # mock systemctl to not start FTL mock_command_2( "systemctl", @@ -73,6 +81,7 @@ def test_installPihole_fresh_install_readableFiles(host): "enable pihole-FTL": ("", "0"), "restart pihole-FTL": ("", "0"), "start pihole-FTL": ("", "0"), + "stop pihole-FTL": ("", "0"), "*": ('echo "systemctl call with $@"', "0"), }, host, @@ -131,13 +140,6 @@ def test_installPihole_fresh_install_readableFiles(host): check_macvendor = test_cmd.format("r", "/etc/pihole/macvendor.db", piholeuser) actual_rc = host.run(check_macvendor).rc assert exit_status_success == actual_rc - # check readable and executable /etc/init.d/pihole-FTL - check_init = test_cmd.format("x", "/etc/init.d/pihole-FTL", piholeuser) - actual_rc = host.run(check_init).rc - assert exit_status_success == actual_rc - check_init = test_cmd.format("r", "/etc/init.d/pihole-FTL", piholeuser) - actual_rc = host.run(check_init).rc - assert exit_status_success == actual_rc # check readable and executable manpages if maninstalled is True: check_man = test_cmd.format("x", "/usr/local/share/man", piholeuser) From 69473a7b54869233ea40a8d16bf17030ac835656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Wed, 28 May 2025 19:25:35 +0200 Subject: [PATCH 4/7] Add awk to meta package dependencie (is missing on Fedora 42 by default) and order dependencies alphabetically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- automated install/basic-install.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh index f4b51c6d..94ef8002 100755 --- a/automated install/basic-install.sh +++ b/automated install/basic-install.sh @@ -116,11 +116,11 @@ c=70 PIHOLE_META_PACKAGE_CONTROL_APT=$( cat < Architecture: all Description: Pi-hole dependency meta package -Depends: grep,dnsutils,binutils,git,iproute2,dialog,ca-certificates,cron | cron-daemon,curl,iputils-ping,psmisc,sudo,unzip,libcap2-bin,dns-root-data,libcap2,netcat-openbsd,procps,jq,lshw,bash-completion +Depends: awk,bash-completion,binutils,ca-certificates,cron|cron-daemon,curl,dialog,dnsutils,dns-root-data,git,grep,iproute2,iputils-ping,jq,libcap2,libcap2-bin,lshw,netcat-openbsd,procps,psmisc,sudo,unzip Section: contrib/metapackages Priority: optional EOM @@ -130,12 +130,12 @@ EOM PIHOLE_META_PACKAGE_CONTROL_RPM=$( cat < Date: Wed, 28 May 2025 20:47:55 +0200 Subject: [PATCH 5/7] Add gwak to Fedorea 42 test image as other tests also rely on awk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- test/_fedora_42.Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/_fedora_42.Dockerfile b/test/_fedora_42.Dockerfile index 34c7ef5d..0d235e2d 100644 --- a/test/_fedora_42.Dockerfile +++ b/test/_fedora_42.Dockerfile @@ -1,5 +1,5 @@ FROM fedora:42 -RUN dnf install -y git +RUN dnf install -y git gawk ENV GITDIR=/etc/.pihole ENV SCRIPTDIR=/opt/pihole From d177c4c776be2f9e7adf044660827f3405ec1905 Mon Sep 17 00:00:00 2001 From: yubiuser Date: Fri, 30 May 2025 19:03:12 +0200 Subject: [PATCH 6/7] Add useful comment Co-authored-by: Dan Schaper Signed-off-by: yubiuser --- automated install/basic-install.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh index 94ef8002..6cc69008 100755 --- a/automated install/basic-install.sh +++ b/automated install/basic-install.sh @@ -235,7 +235,9 @@ is_pid1() { # Checks to see if the given command runs as PID 1 local is_pid1="$1" - ps -p 1 -o comm= | grep -q "${is_pid1}" + # select PID 1, format output to show only CMD column without header + # quietly grep for a match on the function passed parameter + ps --pid 1 --format comm= | grep -q "${is_pid1}" } # Compatibility From fd40fa6f396273c8d6b95d123ecbc8590b47bf25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Fri, 30 May 2025 20:52:37 +0200 Subject: [PATCH 7/7] Test need adjustment to long arument syntax MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- test/test_any_automated_install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_any_automated_install.py b/test/test_any_automated_install.py index f10d2576..64d8c28a 100644 --- a/test/test_any_automated_install.py +++ b/test/test_any_automated_install.py @@ -70,7 +70,7 @@ def test_installPihole_fresh_install_readableFiles(host): mock_command_2( "ps", { - "-p 1": ("systemd", "0"), + "--pid 1": ("systemd", "0"), }, host, )