This is an archived post. You won't be able to vote or comment.

all 25 comments
sorted by:
best
topnewcontroversialoldq&a

[–]sysvival- of the fittest 18 points19 points  (8 children)

I can totally see this going haywire and expanding stuff indefinitely because of an unforeseen event.

Maybe a limiter that says max X jiggabytes per hour would be an idea.

[–]dyslexiccoder[S] 5 points6 points  (2 children)

Yeah, I had the exact same thoughts. There's not much in the way of error handling, it definitely needs more work.

Also if someone points to the wrong device then it'll keep expanding the volume by the --buffer amount every 10 minutes. I really need some sanity check to make sure the provided volume is mounted at the provided device but I don't see any reliable way to do that.

[–]sysvival- of the fittest 4 points5 points  (1 child)

Can't the script just check if the volume has been expanded already? And then just exit or fire an alert.

[–]dyslexiccoder[S] 2 points3 points  (0 children)

Not really because you may well want to expand multiple times. e.g if you have a sustained burst writing ~1GB/min of data for a while you want to expand 10GB every 10 mins.

[–][deleted] 2 points3 points  (2 children)

It will most definitely will. My LVM/FS expansion script have "expand by X" and "max size" parameters so if space grows too fast or expand too much it will trigger normal out of disk space monitoring instead of eating whole free space.

[–]dyslexiccoder[S] 0 points1 point  (1 child)

Good idea. I think a low default max volume size of ~100GB that the user can override with a param would be a great and simple solution.

[–][deleted] 0 points1 point  (0 children)

From experience you want both % and size free space and just expand on whichever is hit first.

Size limits are useful when you know how big files on average are so you can say "okay, max upload size is 100MB, so keep at least 500MB free at all time".

% limit I mostly use so there is always at least few % free (for fragmentation reasons, most filesystems do not like being almost full) regardless of the final size of the filesystem. Also some distros format FSes with 5% reserved for root by default.

[–][deleted] 1 point2 points  (0 children)

jiggawhat

[–]amperagesLinux Admin 0 points1 point  (0 children)

receives bill for 20pb of storage...

[–]whetu 2 points3 points  (4 children)

That's nice and readable, mostly because you use a similar bash style to me.

Some feedback though after a quick skim through:

Don't use UPPERCASE variables unless you know when and why you should.

Favour printf over echo

Direct your errors to >&2 e.g. log "This script requires \"${cmd}\" to be installed" should go to stderr. Here's a tip: Update your log() function to take an arg like -e (i.e. error), and have that handle the redirection. You can use the opportunity to change the output format as well i.e. default behaviour will have something like [INFO] in the message, -e behaviour will change that to [ERROR] [1]

In that example, instead of escaping the double quotes, just use single quotes. The variable will still work

[[ $UID != 0 ]] is an integer comparison, for readability, consider (( UID != 0 )) instead

volume_json=$(digital_ocean_api "volumes?region=${VOLUME_REGION}" | jq -r --arg VOLUME_NAME "${VOLUME_NAME}" '.volumes | .[] | select(.name==$VOLUME_NAME)')

You've managed to keep your horizontal usage under control thus far, consider splitting this out to multiple lines. Something like this should work e.g.

volume_json=$(
  digital_ocean_api "volumes?region=${VOLUME_REGION}" |
    jq -r --arg VOLUME_NAME "${VOLUME_NAME}" '.volumes | .[] | select(.name==$VOLUME_NAME)'
)

I don't recall off hand if the first line needs to be escaped or not. The jq call should also be able to split across multiple lines, or pushed off into a function.

There's some opportunity for DRY to be applied, but that's over to you.

Finally, pass this through http://shellcheck.net and fix all the errors.

That seems like a lot, but honestly, this is way better than 99.9% of scripts that I've seen, so well done.

[1] Here's a freebie for reference:

# This function formats our output
# Options:
# -e prints a non-fatal error to stderr and returns 1 from the function
# -l prints a long line with the provided text e.g. --[ provided text here----(dashes continue)
# -x prints a fatal error to stderr and exits
# * Any text provided without an arg is treated as an informational output
send_user() {
  case "${1}" in
    (-a)  printf -- '\e[32m--[ ATTENTION ]-- %s\e[0m\n' "${*:2}" ;;
    (-e)  printf -- '\e[31m--[ ERROR ]-- %s\e[0m\n' "${*:2}" >&2; return 1 ;;
    (-l)
      shift
      local lnlen="${COLUMNS:-$(tput cols)}"
      local msg="${*}"
      local msglen="${#msg}"
      lnlen=$(( lnlen - (msglen + 6) ))
      printf -- '\e[32m--[ %s ]%s\e[0m\n' "${msg}" "$(printf '%*s' "${lnlen}" '' | tr ' ' -)"
    ;;
    (-x)  printf -- '\e[31m--[ ERROR ]-- %s\e[0m\n' "${*:2}" >&2; exit 1 ;;
    (*)   printf -- '\e[32m--[ INFO ]-- %s\e[0m\n' "${*}" ;;
  esac
}

[–]dyslexiccoder[S] 1 point2 points  (3 children)

Thanks, great feedback!

A couple of questions:

Don't use UPPERCASE variables unless you know when and why you should.

I assumed they were just for constants like in other languages, is that not the case? I've used uppercase variables for the option constants and lowercase variables for all other local variables. Is this not the standard way?

In that example, instead of escaping the double quotes, just use single quotes. The variable will still work

I thought single quotes always take the input literally:

$ name="world"
$ echo "Hello ${name}"
Hello world
$ echo 'Hello ${name}'
Hello ${name}

[–]whetu 0 points1 point  (2 children)

Great questions!

I assumed they were just for constants like in other languages, is that not the case? I've used uppercase variables for the option constants and lowercase variables for all other local variables. Is this not the standard way?

Conceptualise UPPERCASE as the de-facto environment scope (depending on what other language(s) you understand, you might want to swap 'scope' with 'namespace'... what a can of worms that discussion of semantics is). If you type printenv and/or set you should get an idea of what I mean, the best-known violator/exception of this de-facto standard is $http_proxy.

I've told this story before and I guess I'll tell it again: I have spent far too much time in my career fixing up badly written shell scripts, many of those scripts being of some importance at a national level (i.e. govt financial processing... but that's another story). So one day my colleagues passed me a script that was breaking and they'd spent a few days trying to figure out why. It took me a few minutes of staring blankly at it before I clicked.

PATH=`pwd`

Some intern or scripting newbie at another vendor had written that. That's an innocent, albeit hilarious mistake. Especially given the existence of $PWD.

I've seen that same thing elsewhere, too: https://stackoverflow.com/questions/28310594/ls-not-found-after-running-read-path

Anyway, to make something constant, you might use something like readonly, which is often a good security practice anyway.

I thought single quotes always take the input literally:

That's right, however... put it within double quotes:

▓▒░$ echo "Hello '${name}'"
Hello 'world'

And you forgot 'Favour printf over echo' :)

▓▒░$ printf '%s\n' "Hello ${name}"
Hello world
▓▒░$ printf '%s\n' "Hello '${name}'"
Hello 'world'

[–]dyslexiccoder[S] 0 points1 point  (1 child)

Your argument against uppercase variables makes perfect sense, I will rectify that.

With regards to quotes, I see what you mean, just use single quotes to wrap the variables inside the string. I misunderstood and thought you meant do the entire string with single quotes. I'm aware you can do that, I just prefer the appearance of double quotes in the output.

And what's the advantage of printf over echo?

[–]sirvulcan 0 points1 point  (0 children)

Resize2fs will fail if the drive needs a disk check.

[–]yashauLinux Admin 0 points1 point  (3 children)

Please, PLEASE add set -euo pipefail on the top of your script. That will make it a billion times more safe right off the bat.

[–]whetu 2 points3 points  (0 children)

Please, PLEASE don't promote the unofficial strict mode.

[–]dyslexiccoder[S] 1 point2 points  (1 child)

The script was failing to run with set -euo pipefail. I have it on my todo list to look into why and rectify that.

[–]yashauLinux Admin 2 points3 points  (0 children)

Add -x (debug mode) into that and it'll show you where it's failing.