3
\$\begingroup\$

Here's a little bash script I wrote, which basically checks something on an Amazon EC2 instance and reports data to CloudWatch every 60 seconds. I'll run this inside a container. Everything works just fine, but I think it's clunky, doesn't write anything to stdout/stderr so no logs, and doesn't handle any errors. I am open to refactoring this or even writing it in Python, to make my docker image more efficient. Any suggestions, criticism is most welcome.

Here's the full script less some sensitive data that I've xxx'd:

#!/bin/bash

INSTANCEID=$(curl --silent http://169.254.169.254/latest/meta-data/instance-id/)
AZ=$(curl --silent http://169.254.169.254/latest/meta-data/placement/availability-zone)
REGION= $(echo $AZ | sed -e 's:\([0-9][0-9]*\)[a-z]*\$:\\1:')

URL="xxx"

putdata() {
    aws cloudwatch put-metric-data "some other stuff" $1 "xxx"      
    sleep 60
}
while true; do
    HTTP_RESPONSE=$(curl --write-out "%{http_code}" --silent --output /dev/null "$URL")
    if [ "$HTTP_RESPONSE" = "200" ]; then
        putdata 0

    else
        putdata 1
    fi
done

EDIT: just realized that the sleep command runs as a separate PID inside the container, which is why the container takes quite a while to stop. Running with --init flag improves the exit workflow as it better reaps the processes upon exit but that's another unwanted PID.

\$\endgroup\$
3
  • \$\begingroup\$ Hi, removing the tag makes sense. I am not asking anyone to rewrite my code, just suggestions how this can be made better. \$\endgroup\$
    – Ali
    May 21 '20 at 18:49
  • \$\begingroup\$ FWIW "I am open to refactoring this or even writing it in Python" with the python can be misconstrued as that :) All the best. \$\endgroup\$
    – Peilonrayz
    May 21 '20 at 18:51
  • \$\begingroup\$ ah ok.. gotcha, thx! \$\endgroup\$
    – Ali
    May 21 '20 at 19:00
2
\$\begingroup\$

Good

  • good variable names
  • nice indentation
  • good use of $()
  • double quoting $ variable interpolation

Challenges

I'm guessing some part of what you removed and xxxd has caused the putdata function to not do anything with the argument you are passing in. If I guessed wrong, then you should remove the arguments so it doesn't cause someone else confusion.

Do you want this to be totally silent? I'm not sure if the aws command produces any output here. Adding a date command in the while loop would help debugging if you come back to the terminal later to figure out what happened. I'd run this in a terminal under tmux for a few weeks before I started trying it in containers. This sort of thing can have fun edge cases when some API falls over. It can take a while to catch all of those.

It sounds like you'd like some logging. That can be done in bash with redirection such as >> logfile. Of course, there are more advanced choices.

I'd put a blank link before the while loop and get rid of the blank line inside of it.

You mention rewriting in python. I wouldn't worry about it for anything this small. But generally for AWS programming python is the winning choice since AWS provides their API that way. Functionality may be available through the API before it is available in console. Everything in the aws CLI command is built with this. You can use jq to process JSON that comes back from running the aws command to do the same stuff in bash, but it is more work than python where you get a dictionary back and not have to parse any JSON.

Finally, using shellcheck is a good habit to get into when shell coding.

\$\endgroup\$
2
  • \$\begingroup\$ Hey there, thanks for the response. Great suggestions, especially date and time, and yes, the put metric data command doesn't produce any output, that's the crappy part: docs.aws.amazon.com/cli/latest/reference/cloudwatch/… which is making logging hard. Maybe I should output the exit code of that aws command with the time for the logs. And btw I work for AWS, more of a deployment guy, am new to scripting/coding. \$\endgroup\$
    – Ali
    May 21 '20 at 21:04
  • 1
    \$\begingroup\$ Cool that you're part of the cloud! :). Logging the exit code and the date/time would have been my first suggestion. And if something breaks it might produce more output, so try to log that. \$\endgroup\$
    – chicks
    May 21 '20 at 21:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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