Merge pull request #2602 from pi-hole/fix/download_FTL_earlier

Download FTL earlier in the installer

automated install/basic-install.sh

@@ -115,6 +115,9 @@ else
     OVER="\\r\\033[K"
 fi
 
+# Define global binary variable
+binary="tbd"
+
 # A simple function that just echoes out our logo in ASCII format
 # This lets users know that it is a Pi-hole, LLC product
 show_ascii_berry() {
@@ -1434,9 +1437,9 @@ stop_service() {
 }
 
 # Start/Restart service passed in as argument
-start_service() {
+restart_service() {
     # Local, named variables
-    local str="Starting ${1} service"
+    local str="Restarting ${1} service"
     printf "  %b %s..." "${INFO}" "${str}"
     # If systemctl exists,
     if is_command systemctl ; then
@@ -1906,8 +1909,9 @@ installPihole() {
     installCron
     # Install the logrotate file
     installLogrotate
-    # Check if FTL is installed
+    # Check if dnsmasq is present. If so, disable it and back up any possible
-    FTLdetect || printf "  %b FTL Engine not installed\\n" "${CROSS}"
+    # config file
+    disable_dnsmasq
     # Configure the firewall
     if [[ "${useUpdateVars}" == false ]]; then
         configureFirewall
@@ -2130,7 +2134,6 @@ clone_or_update_repos() {
 # Download FTL binary to random temp directory and install FTL binary
 FTLinstall() {
     # Local, named variables
-    local binary="${1}"
     local latesttag
     local str="Downloading and Installing FTL"
     printf "  %b %s..." "${INFO}" "${str}"
@@ -2174,33 +2177,18 @@ FTLinstall() {
         # If we downloaded binary file (as opposed to text),
         if sha1sum --status --quiet -c "${binary}".sha1; then
             printf "transferred... "
-            # Stop FTL
+
+            # Stop pihole-FTL service if available
             stop_service pihole-FTL &> /dev/null
+
             # Install the new version with the correct permissions
             install -T -m 0755 "${binary}" /usr/bin/pihole-FTL
+
             # Move back into the original directory the user was in
             popd > /dev/null || { printf "Unable to return to original directory after FTL binary download.\\n"; return 1; }
-            # Install the FTL service
+
+            # Installed the FTL service
             printf "%b  %b %s\\n" "${OVER}" "${TICK}" "${str}"
-            # dnsmasq can now be stopped and disabled if it exists
-            if which dnsmasq &> /dev/null; then
-                if check_service_active "dnsmasq";then
-                    printf "  %b FTL can now resolve DNS Queries without dnsmasq running separately\\n" "${INFO}"
-                    stop_service dnsmasq
-                    disable_service dnsmasq
-                fi
-            fi
-
-            # Backup existing /etc/dnsmasq.conf if present and ensure that
-            # /etc/dnsmasq.conf contains only "conf-dir=/etc/dnsmasq.d"
-            local conffile="/etc/dnsmasq.conf"
-            if [[ -f "${conffile}" ]]; then
-                printf "  %b Backing up %s to %s.old\\n" "${INFO}" "${conffile}" "${conffile}"
-                mv "${conffile}" "${conffile}.old"
-            fi
-            # Create /etc/dnsmasq.conf
-            echo "conf-dir=/etc/dnsmasq.d" > "${conffile}"
-
             return 0
         # Otherwise,
         else
@@ -2220,6 +2208,27 @@ FTLinstall() {
     fi
 }
 
+disable_dnsmasq() {
+    # dnsmasq can now be stopped and disabled if it exists
+    if which dnsmasq &> /dev/null; then
+        if check_service_active "dnsmasq";then
+            printf "  %b FTL can now resolve DNS Queries without dnsmasq running separately\\n" "${INFO}"
+            stop_service dnsmasq
+            disable_service dnsmasq
+        fi
+    fi
+
+    # Backup existing /etc/dnsmasq.conf if present and ensure that
+    # /etc/dnsmasq.conf contains only "conf-dir=/etc/dnsmasq.d"
+    local conffile="/etc/dnsmasq.conf"
+    if [[ -f "${conffile}" ]]; then
+        printf "  %b Backing up %s to %s.old\\n" "${INFO}" "${conffile}" "${conffile}"
+        mv "${conffile}" "${conffile}.old"
+    fi
+    # Create /etc/dnsmasq.conf
+    echo "conf-dir=/etc/dnsmasq.d" > "${conffile}"
+}
+
 get_binary_name() {
     # This gives the machine architecture which may be different from the OS architecture...
     local machine
@@ -2377,7 +2386,7 @@ FTLdetect() {
     printf "\\n  %b FTL Checks...\\n\\n" "${INFO}"
 
     if FTLcheckUpdate ; then
-        FTLinstall "${binary}" || return 1
+        FTLinstall || return 1
     fi
 }
 
@@ -2537,6 +2546,11 @@ main() {
     else
         LIGHTTPD_ENABLED=false
     fi
+    # Check if FTL is installed - do this early on as FTL is a hard dependency for Pi-hole
+    if ! FTLdetect; then
+        printf "  %b FTL Engine not installed\\n" "${CROSS}"
+        exit 1
+    fi
 
     # Install and log everything to a file
     installPihole | tee -a /proc/$$/fd/3
@@ -2567,7 +2581,7 @@ main() {
     if [[ "${INSTALL_WEB_SERVER}" == true ]]; then
 
         if [[ "${LIGHTTPD_ENABLED}" == true ]]; then
-            start_service lighttpd
+            restart_service lighttpd
             enable_service lighttpd
         else
             printf "  %b Lighttpd is disabled, skipping service restart\\n" "${INFO}"
@@ -2582,7 +2596,7 @@ main() {
     # Fixes a problem reported on Ubuntu 18.04 where trying to start
     # the service before enabling causes installer to exit
     enable_service pihole-FTL
-    start_service pihole-FTL
+    restart_service pihole-FTL
 
     # Download and compile the aggregated block list
     runGravity

test/test_automated_install.py

@@ -481,10 +481,10 @@ def test_FTL_download_aarch64_no_errors(Pihole):
     '''
     confirms only aarch64 package is downloaded for FTL engine
     '''
-    # mock uname to return generic platform
     download_binary = Pihole.run('''
     source /opt/pihole/basic-install.sh
-    FTLinstall pihole-FTL-aarch64-linux-gnu
+    binary="pihole-FTL-aarch64-linux-gnu"
+    FTLinstall
     ''')
     expected_stdout = tick_box + ' Downloading and Installing FTL'
     assert expected_stdout in download_binary.stdout
@@ -495,10 +495,26 @@ def test_FTL_download_unknown_fails_no_errors(Pihole):
     '''
     confirms unknown binary is not downloaded for FTL engine
     '''
-    # mock uname to return generic platform
     download_binary = Pihole.run('''
     source /opt/pihole/basic-install.sh
-    FTLinstall pihole-FTL-mips
+    binary="pihole-FTL-mips"
+    FTLinstall
+    ''')
+    expected_stdout = cross_box + ' Downloading and Installing FTL'
+    assert expected_stdout in download_binary.stdout
+    error1 = 'Error: URL https://github.com/pi-hole/FTL/releases/download/'
+    assert error1 in download_binary.stdout
+    error2 = 'not found'
+    assert error2 in download_binary.stdout
+
+
+def test_FTL_download_binary_unset_no_errors(Pihole):
+    '''
+    confirms unset binary variable does not download FTL engine
+    '''
+    download_binary = Pihole.run('''
+    source /opt/pihole/basic-install.sh
+    FTLinstall
     ''')
     expected_stdout = cross_box + ' Downloading and Installing FTL'
     assert expected_stdout in download_binary.stdout