diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 37a0630..8cace0b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -130,6 +130,7 @@ jobs: bash tests/linux/test_minion_conf_parsing.sh bash tests/linux/test_script_opts.sh bash tests/linux/test_script_opts_main.sh + bash tests/linux/test_onedir_detection.sh - name: Set Exit Status if: always() diff --git a/.github/workflows/templates/ci.yml b/.github/workflows/templates/ci.yml index 7caeb36..68660c6 100644 --- a/.github/workflows/templates/ci.yml +++ b/.github/workflows/templates/ci.yml @@ -130,6 +130,7 @@ jobs: bash tests/linux/test_minion_conf_parsing.sh bash tests/linux/test_script_opts.sh bash tests/linux/test_script_opts_main.sh + bash tests/linux/test_onedir_detection.sh - name: Set Exit Status if: always() diff --git a/linux/svtminion.sh b/linux/svtminion.sh index f47b06e..156e2b2 100755 --- a/linux/svtminion.sh +++ b/linux/svtminion.sh @@ -84,10 +84,6 @@ reconfig default " -readonly salt_wrapper_file_list="minion -call -" - readonly salt_minion_service_wrapper=\ "# Copyright (c) 2021-2026 Broadcom Inc. All Rights Reserved. # SPDX-License-Identifier: Apache-2 @@ -113,6 +109,8 @@ WantedBy=multi-user.target # Onedir detection locations readonly onedir_post_3005_location="${salt_dir}/salt-minion" +# pre-3006 layout, can no longer be installed by this script, but is still +# detected so an existing installation can be reported on and removed readonly onedir_pre_3006_location="${salt_dir}/run/run" declare -a list_of_onedir_locations_check @@ -1729,7 +1727,9 @@ _check_classic_minion_install() { # _check_onedir_minion_install # # Checks whether a 'pre_3006' or 'post_3005' onedir salt-minion package is -# installed on the OS (e.g., via an RPM or DEB package). +# installed on the OS (e.g., via an RPM or DEB package). A 'pre_3006' +# installation can no longer be installed by this script, it is detected so +# it can still be reported on and removed. # # Results: # Outputs one of the following status codes to stdout: @@ -1778,12 +1778,9 @@ _check_onedir_minion_install() { _find_salt_pid() { # find the pid for salt-minion if active local salt_pid=0 - if [[ ${POST_3005_FLAG} -eq 1 ]]; then - salt_pid=$(pgrep -f "\/usr\/bin\/salt-minion" | head -n 1) - else - salt_pid=$(pgrep -f "${salt_name}\/run\/run minion" | head -n 1 | - awk -F " " '{print $1}') - fi + # 3006 and later runs /usr/bin/salt-minion, pre-3006 onedir runs run/run + salt_pid=$(pgrep -f "\/usr\/bin\/salt-minion|${salt_name}\/run\/run minion" | + head -n 1) _debug_log "$0:${FUNCNAME[0]} checking for salt-minion process id, "\ "found '${salt_pid}'" echo "${salt_pid}" @@ -1842,63 +1839,6 @@ _ensure_id_or_fqdn () { } -# -# _create_pre_3006_helper_scripts -# -# Creates wrapper helper scripts for 'salt-call' and 'salt-minion' in -# pre-3006 onedir installations. -# -# Example: -# _create_pre_3006_helper_scripts -# -# Side Effects: -# Writes helper scripts for 'salt-call' and 'salt-minion' to the filesystem. -# -# Results: -# Returns 0 on success, or exits with an error code on failure. -# - -_create_pre_3006_helper_scripts() { - - for idx in ${salt_wrapper_file_list} - do - local abs_filepath="" - abs_filepath="/usr/bin/salt-${idx}" - - _debug_log "$0:${FUNCNAME[0]} creating helper file 'salt-${idx}' "\ - "in directory /usr/bin" - - echo "#!/usr/bin/env bash - -# Copyright (c) 2021-2026 Broadcom Inc. All Rights Reserved. -" > "${abs_filepath}" || { - _error_log "$0:${FUNCNAME[0]} failed to create helper file "\ - "'salt-${idx}' in directory /usr/bin, retcode '$?'"; - } - { - echo -n "exec /opt/saltstack/salt/run/run ${idx}"; - echo -n "\"$"; - echo -n "{"; - echo -n "@"; - echo -n ":"; - echo -n "1}"; - echo -n "\""; - } >> "${abs_filepath}" || { - _error_log "$0:${FUNCNAME[0]} failed to finish creating helper "\ - "file 'salt-${idx}' in directory /usr/bin, retcode '$?'"; - } - echo "" >> "${abs_filepath}" - - # ensure executable - chmod 755 "${abs_filepath}" || { - _error_log "$0:${FUNCNAME[0]} failed to make helper file "\ - "'salt-${idx}' executable in directory /usr/bin, retcode '$?'"; - } - done - -} - - # # _status_fn # @@ -1942,10 +1882,6 @@ _status_fn() { _debug_log "$0:${FUNCNAME[0]} no standardized classic install found" install_onedir_chk=$(_check_onedir_minion_install) - if [[ ${install_onedir_chk} -eq 2 ]]; then - POST_3005_FLAG=1 # ensure note 3006 and above - fi - svpid=$(_find_salt_pid) if [[ ${install_onedir_chk} -eq 0 && -z ${svpid} ]]; then # not installed and no process id @@ -2025,10 +1961,6 @@ _stop_fn() { _debug_log "$0:${FUNCNAME[0]} no standardized classic install found" install_onedir_chk=$(_check_onedir_minion_install) - if [[ ${install_onedir_chk} -eq 2 ]]; then - POST_3005_FLAG=1 # ensure note 3006 and above - fi - if [[ ${install_onedir_chk} -eq 0 ]]; then # not installed CURRENT_STATUS=${STATUS_CODES_ARY[notInstalled]} @@ -2116,10 +2048,6 @@ _restart_fn() { _debug_log "$0:${FUNCNAME[0]} no standardized classic install found" install_onedir_chk=$(_check_onedir_minion_install) - if [[ ${install_onedir_chk} -eq 2 ]]; then - POST_3005_FLAG=1 # ensure note 3006 and above - fi - if [[ ${install_onedir_chk} -eq 0 ]]; then # not installed CURRENT_STATUS=${STATUS_CODES_ARY[notInstalled]} @@ -2374,17 +2302,7 @@ _install_fn () { "read configuration for salt-minion"; fi - if [[ ${_retn} -eq 0 && -f "${onedir_pre_3006_location}" ]]; then - # create helper scripts for /usr/bin to ensure they are present - # before attempting to use them in _ensure_id_or_fqdn - # this is for earlier than 3006 versions of Salt onedir - _debug_log "$0:${FUNCNAME[0]} creating helper files salt-call "\ - "and salt-minion in directory /usr/bin" - _create_pre_3006_helper_scripts || { - _error_log "$0:${FUNCNAME[0]} failed to create helper files "\ - "salt-call or salt-minion in directory /usr/bin, retcode '$?'"; - } - elif [[ ${_retn} -eq 0 && -f "${onedir_post_3005_location}" ]]; then + if [[ ${_retn} -eq 0 && -f "${onedir_post_3005_location}" ]]; then # create symbolic links for /usr/bin to ensure they are present _debug_log "$0:${FUNCNAME[0]} creating symbolic links for salt-call "\ "and salt-minion in directory /usr/bin" diff --git a/tests/linux/test_onedir_detection.sh b/tests/linux/test_onedir_detection.sh new file mode 100644 index 0000000..bbd23d2 --- /dev/null +++ b/tests/linux/test_onedir_detection.sh @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +# +# Lightweight, fixture-free regression test for linux/svtminion.sh's detection +# of an installed onedir salt-minion (_check_onedir_minion_install) and its +# lookup of the running salt-minion process (_find_salt_pid). +# +# The functions under test are extracted verbatim from the real script. The +# onedir location is redirected to a temporary directory and pgrep is mocked, +# so this does not need root, a Salt installation or a running minion. +# +# Run directly: bash tests/linux/test_onedir_detection.sh + +set -o pipefail + +_test_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +_repo_root="$(cd "${_test_dir}/../.." && pwd)" +_script="${_repo_root}/linux/svtminion.sh" + +_work_dir="$(mktemp -d)" +_extracted="${_work_dir}/extracted.sh" +trap 'rm -rf "${_work_dir}"' EXIT + +_error_log() { echo "ERROR: $*" 1>&2; } +_info_log() { :; } +_debug_log() { :; } + +# Extract what is under test verbatim from the real script +for _fn in _check_onedir_minion_install _find_salt_pid; do + if ! sed -n "/^${_fn} *() {/,/^}/p" "${_script}" >> "${_extracted}" \ + || ! grep -q "^${_fn} *() {" "${_extracted}"; then + echo "FAILED: unable to extract ${_fn} from ${_script}" + exit 1 + fi +done +# shellcheck disable=SC1090 +source "${_extracted}" + +# Constants from the real script, with the install location redirected to a +# temporary directory +_fake_base="${_work_dir}/opt/saltstack" +eval "$(grep -E '^(readonly (salt_name|base_salt_location|salt_dir|onedir_[a-z0-9_]+_location)=|declare -a list_of_onedir_locations_check|list_of_onedir_locations_check\[[0-9]+\]=)' \ + "${_script}" | sed "s|\"/opt/saltstack\"|\"${_fake_base}\"|; s|^readonly ||")" + +_failed=0 + +_assert_eq() { + local name="$1" expected="$2" actual="$3" + if [[ "${actual}" == "${expected}" ]]; then + echo "PASS: ${name}" + else + echo "FAIL: ${name}: expected '${expected}', got '${actual}'" + _failed=1 + fi +} + +_reset_install() { rm -rf "${_fake_base}"; mkdir -p "${salt_dir}"; } + +# --- _check_onedir_minion_install --- + +_reset_install +_assert_eq "no onedir installed returns 0" "0" \ + "$(_check_onedir_minion_install)" + +_reset_install +touch "${salt_dir}/salt-minion" +_assert_eq "salt/salt-minion (3006 and later layout) returns 2" "2" \ + "$(_check_onedir_minion_install)" + +_reset_install +mkdir -p "${salt_dir}/run" +touch "${salt_dir}/run/run" +_assert_eq "salt/run/run (pre-3006 layout) is still detected, returns 1" "1" \ + "$(_check_onedir_minion_install)" + +_reset_install +mkdir -p "${salt_dir}/run" +touch "${salt_dir}/run/run" "${salt_dir}/salt-minion" +_assert_eq "both layouts present, run/run takes precedence" "1" \ + "$(_check_onedir_minion_install)" + +# --- _find_salt_pid --- + +# mock pgrep: record the pattern searched for, and report a pid +pgrep() { + echo "${2}" > "${_work_dir}/pgrep_pattern" + echo "4242" +} + +_assert_eq "minion pid found" "4242" "$(_find_salt_pid)" +_assert_eq "pid lookup matches /usr/bin/salt-minion" "1" \ + "$(grep -c -F '\/usr\/bin\/salt-minion' "${_work_dir}/pgrep_pattern")" +_assert_eq "pid lookup matches salt/run/run minion (pre-3006)" "1" \ + "$(grep -c -F 'salt\/run\/run minion' "${_work_dir}/pgrep_pattern")" + +# no process running +pgrep() { return 1; } +_assert_eq "no minion process returns empty" "" "$(_find_salt_pid)" + +if [[ ${_failed} -ne 0 ]]; then + echo "FAILED" + exit 1 +fi +echo "ALL PASSED"