I've written a small script that automates the tasks I previously did manually for deploying onto our kubernetes clusters and I am quite happy with the results I have, but I do think that what my approach was (building a command and executing it at the end) could probably be improved.

Especially the fn_magic part is something I dont know could be done better.

kubectl="kubectl -n=$namespace"

declare -A NAME_TO_TAGS

# Colors :)
NC='\033[0m' # No Color

function setReplacements() {
  while IFS= read -r line
    [ -z "$line" ] && continue
  done < "${BASH_SOURCE%/*}/secrets"

function buildAndPush() {
  function fn_magic() {
    local dockerfile=$1
    local name=$2
    local context=$3
    local tag=$(tar cf - --exclude='node_modules' $context | sha1sum | grep -o "\w*")
    local output=$(docker images $tagPrefix/$project-$name:$tag | awk '{print $2}' | sed 1,1d)
    printf "SHA1 for ${CYAN}$name${NC} is ${YELLOW}$tag${NC}.\n"
    if [[ "$output" ]]; then
      printf "Skipping building ${CYAN}$name${NC} because image already exists locally.\n"
      COMMANDBUILDER="$COMMANDBUILDER && printf \"Pushing ${CYAN}$name${NC}.\n\" && docker push $tagPrefix/$project-$name:$tag"
    COMMANDBUILDER="$COMMANDBUILDER && printf \"Building ${CYAN}$name${NC}.\n\" && docker build -f dockerfiles/$dockerfile.dockerfile -t $tagPrefix/$project-$name:$tag $context"
    COMMANDBUILDER="$COMMANDBUILDER && printf \"Pushing ${CYAN}$name${NC}.\n\" && docker push $tagPrefix/$project-$name:$tag"
  local calls=(
    "fn_magic ReactNginx frontend ./frontend"
    "fn_magic DockerfileNode backend ./backend"

  for call in "${calls[@]}"; do 
    eval $call

function deploy() {
  local deployName="$1"
  local tag="$2"

  local filter=""
  for key in "${!REPLACEMENTS[@]}"; do
    local REPLACE=$(echo ${REPLACEMENTS[$key]} | sed -e 's/\\/\\\\/g; s/\//\\\//g; s/&/\\\&/g')
    filter="$filter | sed \"s/{{$key}}/$REPLACE/g\""

  COMMANDBUILDER="$COMMANDBUILDER && cat "${BASH_SOURCE%/*}/$deployName.yaml" $filter | $kubectl apply -f -"

for name in "${!NAME_TO_TAGS[@]}"; do 
  deploy $name ${NAME_TO_TAGS[$name]}

  • \$\begingroup\$ So that we can clearly understand what this code is intended to accomplish, could you include some example inputs and results? \$\endgroup\$ – 200_success Jul 9 '19 at 3:02

Some suggestions in rough order of decreasing importance:

  • eval is evil, and I'm pretty sure you can completely avoid it in your script. Other than a massive gain in sanity, all that sed stuff to escape strings is not going to be necessary if you instead just use plain shell.
  • Naming is really important, and comments are a good way to detect things which should be renamed.
  • set -o errexit -o nounset -o pipefail would be very handy in this script. Just make sure you're aware of the caveats (that page is needlessly snarky but has some useful bits, and that site is fantastic).
  • A simple array is the best way to build a command. I would thoroughly recommend not building complex commands using a shell script. Instead, run each command as soon as you have all the pieces. Bundling up commands to run isn't going to make things faster; in most cases it'll make things significantly slower by making >50% of the code about building the command rather than just running it.
  • Nested functions aren't. There are no closures in Bash, so defining a function within another function just makes it harder to read.
  • In Bash the convention is that uppercase names are reserved for exported variables.
  • Single use variables can often be inlined without loss of clarity.
  • printf is meant to be used with format strings for readability. So

    printf "SHA1 for ${cyan}$name${end_format} is ${yellow}$tag${end_format}.\n"

    would normally be written as

    printf "SHA1 for %s is %s.\n" "${cyan}${name}${end_format}" "${yellow}${tag}${end_format}"
  • local is considered a command for exit code purposes, which means that local foo=[variable or command substitution] only gets the exit code of local, not of the variable or command substitution. So if you start using nounset and the like you should declare variables local before assigning them separately:

    deploy() {
        local deploy_name tag
  • shellcheck can give you more tips.
  • \$\begingroup\$ Thanks for these suggestions. An array definitely makes more sense to build a command. However I dont quite understand the local part. All the other best practices make sense, especially shellcheck is really helpful but as far as I understood, local is used to have a variable not in the global scope or did I misunderstand that? \$\endgroup\$ – Firat Oezcan Jul 8 '19 at 11:04
  • 1
    \$\begingroup\$ I've added an example to clarify how to use local. \$\endgroup\$ – l0b0 Jul 8 '19 at 19:52
  • \$\begingroup\$ Ahhh, wow that's really good to know. Thanks for the inconvenience in helping me out :) \$\endgroup\$ – Firat Oezcan Jul 9 '19 at 11:54
  • \$\begingroup\$ No worries, I wouldn't be doing this unless I got something out of it as well - code reading and review training, writing training, and the pleasure of helping. And it often forces me to rethink what I consider best practices and articulate why I think they are preferable to the alternatives. \$\endgroup\$ – l0b0 Jul 9 '19 at 19:26

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.