From d4dd0ad53c66f7bdaaf352a6020e97c719bff14a Mon Sep 17 00:00:00 2001 From: Joakim Roubert Date: Mon, 14 Sep 2020 10:48:32 +0200 Subject: [PATCH] log-dump.sh: Fix shellcheck issues Mitigate the shellcheck warning for the assignment of local variable. Add rest of shellcheck fixes from #88349 Signed-off-by: Joakim Roubert --- cluster/log-dump/log-dump.sh | 141 ++++++++++++++++++++--------------- hack/.shellcheck_failures | 1 - 2 files changed, 79 insertions(+), 63 deletions(-) diff --git a/cluster/log-dump/log-dump.sh b/cluster/log-dump/log-dump.sh index a9fbde5f4dd..e87018f5397 100755 --- a/cluster/log-dump/log-dump.sh +++ b/cluster/log-dump/log-dump.sh @@ -30,7 +30,7 @@ readonly logexporter_namespace="${3:-logexporter}" # check for a function named log_dump_custom_get_instances. If it's # defined, we assume the function can me called with one argument, the # role, which is either "master" or "node". -echo "Checking for custom logdump instances, if any" +echo 'Checking for custom logdump instances, if any' if [[ $(type -t log_dump_custom_get_instances) == "function" ]]; then readonly use_custom_instance_list=yes else @@ -67,10 +67,10 @@ readonly max_dump_processes=25 function setup() { KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/../.. if [[ -z "${use_custom_instance_list}" ]]; then - : ${KUBE_CONFIG_FILE:="config-test.sh"} - echo "Sourcing kube-util.sh" + : "${KUBE_CONFIG_FILE:='config-test.sh'}" + echo 'Sourcing kube-util.sh' source "${KUBE_ROOT}/cluster/kube-util.sh" - echo "Detecting project" + echo 'Detecting project' detect-project 2>&1 elif [[ "${KUBERNETES_PROVIDER}" == "gke" ]]; then echo "Using 'use_custom_instance_list' with gke, skipping check for LOG_DUMP_SSH_KEY and LOG_DUMP_SSH_USER" @@ -80,17 +80,17 @@ function setup() { source "${KUBE_ROOT}/cluster/gce/util.sh" ZONE="${gke_zone}" elif [[ -z "${LOG_DUMP_SSH_KEY:-}" ]]; then - echo "LOG_DUMP_SSH_KEY not set, but required when using log_dump_custom_get_instances" + echo 'LOG_DUMP_SSH_KEY not set, but required when using log_dump_custom_get_instances' exit 1 elif [[ -z "${LOG_DUMP_SSH_USER:-}" ]]; then - echo "LOG_DUMP_SSH_USER not set, but required when using log_dump_custom_get_instances" + echo 'LOG_DUMP_SSH_USER not set, but required when using log_dump_custom_get_instances' exit 1 fi source "${KUBE_ROOT}/hack/lib/util.sh" } function log-dump-ssh() { - if [[ "${gcloud_supported_providers}" =~ "${KUBERNETES_PROVIDER}" ]]; then + if [[ "${gcloud_supported_providers}" =~ ${KUBERNETES_PROVIDER} ]]; then ssh-to-node "$@" return fi @@ -102,12 +102,14 @@ function log-dump-ssh() { } # Copy all files /var/log/{$3}.log on node $1 into local dir $2. -# $3 should be a space-separated string of files. +# $3 should be a string array of file names. # This function shouldn't ever trigger errexit, but doesn't block stderr. function copy-logs-from-node() { local -r node="${1}" local -r dir="${2}" - local files=( ${3} ) + shift + shift + local files=("$@") # Append "*" # The * at the end is needed to also copy rotated logs (which happens # in large clusters and long runs). @@ -117,12 +119,13 @@ function copy-logs-from-node() { # Comma delimit (even the singleton, or scp does the wrong thing), surround by braces. local -r scp_files="{$(printf "%s," "${files[@]}")}" - if [[ "${gcloud_supported_providers}" =~ "${KUBERNETES_PROVIDER}" ]]; then + if [[ "${gcloud_supported_providers}" =~ ${KUBERNETES_PROVIDER} ]]; then # get-serial-port-output lets you ask for ports 1-4, but currently (11/21/2016) only port 1 contains useful information gcloud compute instances get-serial-port-output --project "${PROJECT}" --zone "${ZONE}" --port 1 "${node}" > "${dir}/serial-1.log" || true gcloud compute scp --recurse --project "${PROJECT}" --zone "${ZONE}" "${node}:${scp_files}" "${dir}" > /dev/null || true elif [[ "${KUBERNETES_PROVIDER}" == "aws" ]]; then - local ip=$(get_ssh_hostname "${node}") + local ip + ip=$(get_ssh_hostname "${node}") scp -oLogLevel=quiet -oConnectTimeout=30 -oStrictHostKeyChecking=no -i "${AWS_SSH_KEY}" "${SSH_USER}@${ip}:${scp_files}" "${dir}" > /dev/null || true elif [[ -n "${use_custom_instance_list}" ]]; then scp -oLogLevel=quiet -oConnectTimeout=30 -oStrictHostKeyChecking=no -i "${LOG_DUMP_SSH_KEY}" "${LOG_DUMP_SSH_USER}@${node}:${scp_files}" "${dir}" > /dev/null || true @@ -139,26 +142,34 @@ function copy-logs-from-node() { function save-logs() { local -r node_name="${1}" local -r dir="${2}" - local files="${3}" + local files=() + IFS=' ' read -r -a files <<< "$3" local opt_systemd_services="${4:-""}" local on_master="${5:-"false"}" - files="${files} ${extra_log_files}" + local extra=() + IFS=' ' read -r -a extra <<< "$extra_log_files" + files+=("${extra[@]}") if [[ -n "${use_custom_instance_list}" ]]; then if [[ -n "${LOG_DUMP_SAVE_LOGS:-}" ]]; then - files="${files} ${LOG_DUMP_SAVE_LOGS:-}" + local dump=() + IFS=' ' read -r -a dump <<< "${LOG_DUMP_SAVE_LOGS:-}" + files+=("${dump[@]}") fi else + local providerlogs=() case "${KUBERNETES_PROVIDER}" in gce|gke) - files="${files} ${gce_logfiles}" + IFS=' ' read -r -a providerlogs <<< "${gce_logfiles}" ;; aws) - files="${files} ${aws_logfiles}" + IFS=' ' read -r -a providerlogs <<< "${aws_logfiles}" ;; esac + files+=("${providerlogs[@]}") fi - local -r services=( ${systemd_services} ${opt_systemd_services} ${extra_systemd_services} ) + local services + read -r -a services <<< "${systemd_services} ${opt_systemd_services} ${extra_systemd_services}" if log-dump-ssh "${node_name}" "command -v journalctl" &> /dev/null; then if [[ "${on_master}" == "true" ]]; then @@ -178,7 +189,11 @@ function save-logs() { log-dump-ssh "${node_name}" "sudo journalctl --output=short-precise" > "${dir}/systemd.log" || true fi else - files="${kern_logfile} ${files} ${initd_logfiles} ${supervisord_logfiles}" + local tmpfiles=() + for f in "${kern_logfile}" "${initd_logfiles}" "${supervisord_logfiles}"; do + IFS=' ' read -r -a tmpfiles <<< "$f" + files+=("${tmpfiles[@]}") + done fi # Try dumping coverage profiles, if it looks like coverage is enabled in the first place. @@ -192,15 +207,15 @@ function save-logs() { run-in-docker-container "${node_name}" "kube-proxy" "cat /tmp/k8s-kube-proxy.cov" > "${dir}/kube-proxy.cov" || true fi else - echo "Coverage profiles seem to exist, but cannot be retrieved from inside containers." + echo 'Coverage profiles seem to exist, but cannot be retrieved from inside containers.' fi fi - echo "Changing logfiles to be world-readable for download" + echo 'Changing logfiles to be world-readable for download' log-dump-ssh "${node_name}" "sudo chmod -R a+r /var/log" || true - echo "Copying '${files}' from ${node_name}" - copy-logs-from-node "${node_name}" "${dir}" "${files}" + echo "Copying '${files[*]}' from ${node_name}" + copy-logs-from-node "${node_name}" "${dir}" "${files[@]}" } # Saves a copy of the Windows Docker event log to ${WINDOWS_LOGS_DIR}\docker.log @@ -246,8 +261,9 @@ function save-windows-logs-via-diagnostics-tool() { local node="${1}" local dest_dir="${2}" - gcloud compute instances add-metadata ${node} --metadata enable-diagnostics=true --project=${PROJECT} --zone=${ZONE} - local logs_archive_in_gcs=$(gcloud alpha compute diagnose export-logs ${node} --zone=${ZONE} --project=${PROJECT} | tail -n 1) + gcloud compute instances add-metadata "${node}" --metadata enable-diagnostics=true --project="${PROJECT}" --zone="${ZONE}" + local logs_archive_in_gcs + logs_archive_in_gcs=$(gcloud alpha compute diagnose export-logs "${node}" "--zone=${ZONE}" "--project=${PROJECT}" | tail -n 1) local temp_local_path="${node}.zip" for retry in {1..20}; do if gsutil mv "${logs_archive_in_gcs}" "${temp_local_path}" > /dev/null 2>&1; then @@ -259,8 +275,8 @@ function save-windows-logs-via-diagnostics-tool() { done if [[ -f "${temp_local_path}" ]]; then - unzip ${temp_local_path} -d "${dest_dir}" > /dev/null - rm -f ${temp_local_path} + unzip "${temp_local_path}" -d "${dest_dir}" > /dev/null + rm -f "${temp_local_path}" fi } @@ -273,14 +289,14 @@ function save-windows-logs-via-ssh() { export-windows-docker-images-list "${node}" local remote_files=() - for file in ${windows_node_logfiles[@]}; do + for file in "${windows_node_logfiles[@]}"; do remote_files+=( "${WINDOWS_LOGS_DIR}\\${file}" ) done remote_files+=( "${windows_node_otherfiles[@]}" ) # TODO(pjh, yujuhong): handle rotated logs and copying multiple files at the # same time. - for remote_file in ${remote_files[@]}; do + for remote_file in "${remote_files[@]}"; do # Retry up to 3 times to allow ssh keys to be properly propagated and # stored. for retry in {1..3}; do @@ -302,7 +318,7 @@ function save-logs-windows() { local -r node="${1}" local -r dest_dir="${2}" - if [[ ! "${gcloud_supported_providers}" =~ "${KUBERNETES_PROVIDER}" ]]; then + if [[ ! "${gcloud_supported_providers}" =~ ${KUBERNETES_PROVIDER} ]]; then echo "Not saving logs for ${node}, Windows log dumping requires gcloud support" return fi @@ -324,28 +340,28 @@ function run-in-docker-container() { local node_name="$1" local container="$2" shift 2 - log-dump-ssh "${node_name}" "docker exec \"\$(docker ps -f label=io.kubernetes.container.name=${container} --format \"{{.ID}}\")\" $@" + log-dump-ssh "${node_name}" "docker exec \"\$(docker ps -f label=io.kubernetes.container.name=${container} --format \"{{.ID}}\")\" $*" } function dump_masters() { local master_names if [[ -n "${use_custom_instance_list}" ]]; then - master_names=( $(log_dump_custom_get_instances master) ) - elif [[ ! "${master_ssh_supported_providers}" =~ "${KUBERNETES_PROVIDER}" ]]; then + while IFS='' read -r line; do master_names+=("$line"); done < <(log_dump_custom_get_instances master) + elif [[ ! "${master_ssh_supported_providers}" =~ ${KUBERNETES_PROVIDER} ]]; then echo "Master SSH not supported for ${KUBERNETES_PROVIDER}" return elif [[ -n "${KUBEMARK_MASTER_NAME:-}" ]]; then master_names=( "${KUBEMARK_MASTER_NAME}" ) else if ! (detect-master); then - echo "Master not detected. Is the cluster up?" + echo 'Master not detected. Is the cluster up?' return fi master_names=( "${MASTER_NAME}" ) fi if [[ "${#master_names[@]}" == 0 ]]; then - echo "No masters found?" + echo 'No masters found?' return fi @@ -378,16 +394,16 @@ function dump_nodes() { local node_names=() local windows_node_names=() if [[ -n "${1:-}" ]]; then - echo "Dumping logs for nodes provided as args to dump_nodes() function" + echo 'Dumping logs for nodes provided as args to dump_nodes() function' node_names=( "$@" ) elif [[ -n "${use_custom_instance_list}" ]]; then - echo "Dumping logs for nodes provided by log_dump_custom_get_instances() function" - node_names=( $(log_dump_custom_get_instances node) ) - elif [[ ! "${node_ssh_supported_providers}" =~ "${KUBERNETES_PROVIDER}" ]]; then + echo 'Dumping logs for nodes provided by log_dump_custom_get_instances() function' + while IFS='' read -r line; do node_names+=("$line"); done < <(log_dump_custom_get_instances node) + elif [[ ! "${node_ssh_supported_providers}" =~ ${KUBERNETES_PROVIDER} ]]; then echo "Node SSH not supported for ${KUBERNETES_PROVIDER}" return else - echo "Detecting nodes in the cluster" + echo 'Detecting nodes in the cluster' detect-node-names &> /dev/null if [[ -n "${NODE_NAMES:-}" ]]; then node_names=( "${NODE_NAMES[@]}" ) @@ -398,7 +414,7 @@ function dump_nodes() { fi if [[ "${#node_names[@]}" == 0 && "${#windows_node_names[@]}" == 0 ]]; then - echo "No nodes found!" + echo 'No nodes found!' return fi @@ -410,7 +426,7 @@ function dump_nodes() { linux_nodes_selected_for_logs=() if [[ -n "${LOGDUMP_ONLY_N_RANDOM_NODES:-}" ]]; then # We randomly choose 'LOGDUMP_ONLY_N_RANDOM_NODES' many nodes for fetching logs. - for index in `shuf -i 0-$(( ${#node_names[*]} - 1 )) -n ${LOGDUMP_ONLY_N_RANDOM_NODES}` + for index in $(shuf -i 0-$(( ${#node_names[*]} - 1 )) -n "${LOGDUMP_ONLY_N_RANDOM_NODES}") do linux_nodes_selected_for_logs+=("${node_names[$index]}") done @@ -473,10 +489,10 @@ function find_non_logexported_nodes() { local file="${gcs_artifacts_dir}/logexported-nodes-registry" echo "Listing marker files ($file) for successful nodes..." succeeded_nodes=$(gsutil ls "${file}") || return 1 - echo "Successfully listed marker files for successful nodes" + echo 'Successfully listed marker files for successful nodes' NON_LOGEXPORTED_NODES=() for node in "${NODE_NAMES[@]}"; do - if [[ ! "${succeeded_nodes}" =~ "${node}" ]]; then + if [[ ! "${succeeded_nodes}" =~ ${node} ]]; then NON_LOGEXPORTED_NODES+=("${node}") fi done @@ -486,20 +502,21 @@ function find_non_logexported_nodes() { # does not run on Windows nodes. function dump_nodes_with_logexporter() { if [[ -n "${use_custom_instance_list}" ]]; then - echo "Dumping logs for nodes provided by log_dump_custom_get_instances() function" - NODE_NAMES=( $(log_dump_custom_get_instances node) ) + echo 'Dumping logs for nodes provided by log_dump_custom_get_instances() function' + NODE_NAMES=() + while IFS='' read -r line; do NODE_NAMES+=("$line"); done < <(log_dump_custom_get_instances node) else - echo "Detecting nodes in the cluster" + echo 'Detecting nodes in the cluster' detect-node-names &> /dev/null fi if [[ -z "${NODE_NAMES:-}" ]]; then - echo "No nodes found!" + echo 'No nodes found!' return fi # Obtain parameters required by logexporter. - local -r service_account_credentials="$(cat ${GOOGLE_APPLICATION_CREDENTIALS} | base64 | tr -d '\n')" + local -r service_account_credentials="$(base64 "${GOOGLE_APPLICATION_CREDENTIALS}" | tr -d '\n')" local -r cloud_provider="${KUBERNETES_PROVIDER}" local -r enable_hollow_node_logs="${ENABLE_HOLLOW_NODE_LOGS:-false}" local -r logexport_sleep_seconds="$(( 90 + NUM_NODES / 3 ))" @@ -526,7 +543,7 @@ function dump_nodes_with_logexporter() { # Create the logexporter namespace, service-account secret and the logexporter daemonset within that namespace. KUBECTL="${KUBE_ROOT}/cluster/kubectl.sh" if ! "${KUBECTL}" create -f "${manifest_yaml}"; then - echo "Failed to create logexporter daemonset.. falling back to logdump through SSH" + echo 'Failed to create logexporter daemonset.. falling back to logdump through SSH' "${KUBECTL}" delete namespace "${logexporter_namespace}" || true dump_nodes "${NODE_NAMES[@]}" return @@ -538,7 +555,7 @@ function dump_nodes_with_logexporter() { while true; do now="$(date +%s)" if [[ $((now - start)) -gt ${logexport_sleep_seconds} ]]; then - echo "Waiting for all nodes to be logexported timed out." + echo 'Waiting for all nodes to be logexported timed out.' break fi if find_non_logexported_nodes; then @@ -569,14 +586,13 @@ function dump_nodes_with_logexporter() { done; wait) # List registry of marker files (of nodes whose logexporter succeeded) from GCS. - local nodes_succeeded for retry in {1..10}; do if find_non_logexported_nodes; then break else echo "Attempt ${retry} failed to list marker files for successful nodes" if [[ "${retry}" == 10 ]]; then - echo "Final attempt to list marker files failed.. falling back to logdump through SSH" + echo 'Final attempt to list marker files failed.. falling back to logdump through SSH' "${KUBECTL}" delete namespace "${logexporter_namespace}" || true dump_nodes "${NODE_NAMES[@]}" return @@ -599,32 +615,33 @@ function dump_nodes_with_logexporter() { "${KUBECTL}" get pods --namespace "${logexporter_namespace}" || true "${KUBECTL}" delete namespace "${logexporter_namespace}" || true if [[ "${#failed_nodes[@]}" != 0 ]]; then - echo -e "Dumping logs through SSH for the following nodes:\n${failed_nodes[@]}" + echo -e "Dumping logs through SSH for the following nodes:\n${failed_nodes[*]}" dump_nodes "${failed_nodes[@]}" fi } function detect_node_failures() { - if ! [[ "${gcloud_supported_providers}" =~ "${KUBERNETES_PROVIDER}" ]]; then + if ! [[ "${gcloud_supported_providers}" =~ ${KUBERNETES_PROVIDER} ]]; then return fi detect-node-names if [[ "${KUBERNETES_PROVIDER}" == "gce" ]]; then - local all_instance_groups=(${INSTANCE_GROUPS[@]} ${WINDOWS_INSTANCE_GROUPS[@]}) + local all_instance_groups=("${INSTANCE_GROUPS[@]}" "${WINDOWS_INSTANCE_GROUPS[@]}") else - local all_instance_groups=(${INSTANCE_GROUPS[@]}) + local all_instance_groups=("${INSTANCE_GROUPS[@]}") fi if [ -z "${all_instance_groups:-}" ]; then return fi for group in "${all_instance_groups[@]}"; do - local creation_timestamp=$(gcloud compute instance-groups managed describe \ - "${group}" \ - --project "${PROJECT}" \ - --zone "${ZONE}" \ - --format='value(creationTimestamp)') + local creation_timestamp + creation_timestamp=$(gcloud compute instance-groups managed describe \ + "${group}" \ + --project "${PROJECT}" \ + --zone "${ZONE}" \ + --format='value(creationTimestamp)') echo "Failures for ${group} (if any):" gcloud logging read --order=asc \ --format='table(timestamp,jsonPayload.resource.name,jsonPayload.event_subtype)' \ @@ -644,7 +661,7 @@ function main() { echo "Dumping logs from master locally to '${report_dir}'" dump_masters if [[ "${DUMP_ONLY_MASTER_LOGS:-}" == "true" ]]; then - echo "Skipping dumping of node logs" + echo 'Skipping dumping of node logs' return fi diff --git a/hack/.shellcheck_failures b/hack/.shellcheck_failures index 328563b2507..92255aa036c 100644 --- a/hack/.shellcheck_failures +++ b/hack/.shellcheck_failures @@ -1,4 +1,3 @@ ./cluster/gce/gci/configure.sh ./cluster/gce/gci/master-helper.sh ./cluster/gce/util.sh -./cluster/log-dump/log-dump.sh