2
\$\begingroup\$

Recently I had the requirement to allow multiple containers of the same application to be run on a single development server. This requirement drastically changed how we retrieve logs over SSH from the servers, and because I am not extremely well versed in bash, I wanted to get this script reviewed to see if there is an easier/cleaner way to approach this problem.

The goal of the script is to produce a single log file on the local system for each container on that is running on the remote server. It is currently working as expected for 1 and N containers running on a single server.

The script I wrote does the following:

  1. Verify that the server being requested can provide the logs
  2. SSH into the remote server and save the logs for each container to a file on the local system
  3. Post-Process the logs into individual log files for each container that is present

I am fairly certain there is a much easier way to accomplish this task, hence the post here to see if anyone can point me in the right direction (I have a feeling, as I write this, that I can just scp the docker log files for each container to the local system instead of all of this.... But I also want to practice some bash, so I would still like this reviewed)

Script:

#!/bin/bash

# Add a help parameter to the script
if [ "$1" == "-h" ] || [ "$1" == "-help" ]; then
    echo "Usage: $(basename "$0")"
    echo " Logs will be placed in /c/temp/logs/<server>.log"
    echo " Prompts:"
    echo "  - Username: AD Username for Logins"
    echo "  - Server: This is a dev server, valid choices are:"
    echo "      (<SERVER_NAME>1 <SERVER_NAME>2 <SERVER_NAME>3 <SERVER_NAME>4)"
    exit 0
fi

# Create an array of valid servers to check for logs
servers=(<SERVER_NAME>1 <SERVER_NAME>2 <SERVER_NAME>3 <SERVER_NAME>4)

# Ask the user where they want to get the logs from
read -rp "Username: " user
read -rp "Server: " server

# Validate the server can retrieve logs
foundServer=0
for i in "${servers[@]}"; do
    if [ "$i" == "$server" ]; then
        foundServer=1
        break
    fi
done

if [ $foundServer == 0 ]; then
    echo "Please select a dev server"
    echo "Use -h or -help for help with $(basename "$0")"
    exit 1
fi

# Set the working directory
(cd /c/temp/logs/temp/ &&

    # Get the logs. Servers with multiple containers will have the following structure:
    #   HeaderA
    #   A
    #   A
    #   A
    #   HeaderB
    #   B
    #   B
    #   B
    now=$(date '+%Y%m%d_%H%M')
    ssh -tt "$user"@"$server" "sudo setfacl --modify user:${user}:rw /var/run/docker.sock && docker ps --format '{{.Names}}' | xargs -L 1 -t docker logs" > ./"${server}_${now}.log"

    # Process the temp file into individual files for each container ....
    # Get the headers for each Log File
    grep -n "docker logs" ./"${server}_${now}.log" >> ./docker_logs.txt
    total_matches=$(wc -l ./docker_logs.txt | cut -d' ' -f1)

    # From Line 1 -> First Match == Log A, From First Match -> Next Match == Log B, etc
    start_line=1

    # Get the file name for the first file
    file_name_match=$(head -n 1 ./docker_logs.txt)
    current_name=$(echo "$file_name_match" | cut -f2 -d: | sed "s/^docker logs //")

    # Get the end line index for the first file
    end_line_text=$(awk "NR==2" ./docker_logs.txt | cut -f1 -d:)
    if [[ $end_line_text ]]; then
        end_line=$(("$end_line_text"-1))
    else
        end_line="$"
    fi

    # Skipping the first file (starting from index 2), loop through any log files that are present
    for (( n=2; n<="$total_matches"+1; n++ )) do

        # Copy the current file from start -> end to the log file for the current name, if end_line was EOF
        # then break from the loop
        sed -n "${start_line},${end_line}p" ./"${server}_${now}.log" > ../"${server}_${now}_${current_name}.log"
        if [[ $end_line == "$" ]]; then
            break
        fi

        # Get the start line of the next file
        start_line=$(("$end_line"+1))

        # Get the file name of the next log file
        file_name_match=$(awk "NR==$n" ./docker_logs.txt)
        current_name=$(echo "$file_name_match" | cut -f2 -d: | sed "s/^docker logs //")

        # Get the ending index of the next log file, if there is nothing returned, set end_line to
        # be the EOF
        end_index=$(("$n"+1))
        end_line_text=$(awk "NR==$end_index" ./docker_logs.txt | cut -f1 -d:)
        if [[ $end_line_text ]]; then
            end_line=$(("$end_line_text"-1))
        else
            end_line="$"
        fi
    done

    # Remove the temp files
    rm -f /c/temp/logs/temp/*
)

# Open the logs directory for the user
start /c/temp/logs/

This produces N number of logfiles on the local system, of the format ${server}_${now}_${container_name}.log.

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Error messages should be redirected to stream 2 (standard error):

if [ "$foundServer" == 0 ]
then
    echo >&2 "Please select a dev server"
    echo >&2 "Use -h or -help for help with $(basename "$0")"
    exit 1
fi

Or,

if [ "$foundServer" == 0 ]
then
    exec >&2
    echo "Please select a dev server"
    echo "Use -h or -help for help with $(basename "$0")"
    exit 1
fi
\$\endgroup\$
1
\$\begingroup\$

good

  • Good shbang line. (Consider env for PATH portability.)
  • Good variable names.
  • Good indentation.
  • Good comments

suggestions

  • I'm assuming you're replacing <SERVER_NAME> with some value in practice. If that is true, you might consider making that a variable so you can use this for other kinds of servers.
  • Using the subshell to let you change directories and get back to where you started is interesting, but unless you're getting some other non-obvious benefit from the subshell I'd use pushd /c/temp/logs/temp/ to get into the directory and popd to get out of it.
  • Double square brackets for shell conditionals have fewer surprises.
  • When doing shell math such as $(("$n"+1)) you don't need the quotes or the dollar sign. The double parens tell it to evaluate mathematically and that gets you variables too. So $((n+1)) will work just as well and is easier to read.
\$\endgroup\$
2
  • \$\begingroup\$ I disagree on pushd and popd. They are much better suited for interactive use, and a nuisance in scripts. They also produce output that would need to be redirected away. \$\endgroup\$ Commented Feb 23, 2022 at 10:47
  • \$\begingroup\$ They are also good for interactive use. Using a subshell still seems like the wrong choice. You could also store $PWD in another variable like $OLDPWD and cd back to that. \$\endgroup\$
    – chicks
    Commented Feb 23, 2022 at 14:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.