3
\$\begingroup\$

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.

team="webdev"
project="sick-project"
registry="myfancyregistry"
tagPrefix="$registry/$team/$project"
namespace="webdev-dev"
kubectl="kubectl -n=$namespace"
declare -A NAME_TO_TAGS
declare COMMANDBUILDER
declare -A REPLACEMENTS
# Colors :)
RED='033円[0;31m'
GREEN='033円[0;32m'
BLUE='033円[0;34m'
CYAN='033円[0;36m'
YELLOW='033円[1;33m'
NC='033円[0m' # No Color
function setReplacements() {
 while IFS= read -r line
 do
 [ -z "$line" ] && continue
 key=${line%%=*}
 value=${line#*=}
 REPLACEMENTS[$key]=$value
 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*")
 NAME_TO_TAGS["$name"]="$tag"
 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"
 return
 fi
 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"
 return
 }
 local calls=(
 "fn_magic ReactNginx frontend ./frontend"
 "fn_magic DockerfileNode backend ./backend"
 )
 for call in "${calls[@]}"; do 
 eval $call
 done
}
function deploy() {
 local deployName="1ドル"
 local tag="2ドル"
 local filter=""
 REPLACEMENTS[TAG]="$tag"
 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\""
 done
 COMMANDBUILDER="$COMMANDBUILDER && cat "${BASH_SOURCE%/*}/$deployName.yaml" $filter | $kubectl apply -f -"
}
setReplacements
buildAndPush
for name in "${!NAME_TO_TAGS[@]}"; do 
 deploy $name ${NAME_TO_TAGS[$name]}
done
COMMANDBUILDER="echo $COMMANDBUILDER"
eval "$COMMANDBUILDER"
dfhwze
14.1k3 gold badges40 silver badges101 bronze badges
asked Jul 8, 2019 at 9:47
\$\endgroup\$
1
  • \$\begingroup\$ So that we can clearly understand what this code is intended to accomplish, could you include some example inputs and results? \$\endgroup\$ Commented Jul 9, 2019 at 3:02

1 Answer 1

2
\$\begingroup\$

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
     deploy_name="1ドル"
     tag="2ドル"
     ...
    }
    
  • shellcheck can give you more tips.
answered Jul 8, 2019 at 10:15
\$\endgroup\$
4
  • \$\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\$ Commented Jul 8, 2019 at 11:04
  • 1
    \$\begingroup\$ I've added an example to clarify how to use local. \$\endgroup\$ Commented Jul 8, 2019 at 19:52
  • \$\begingroup\$ Ahhh, wow that's really good to know. Thanks for the inconvenience in helping me out :) \$\endgroup\$ Commented Jul 9, 2019 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\$ Commented Jul 9, 2019 at 19:26

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.