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"
-
\$\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– 200_success2019年07月09日 03:02:09 +00:00Commented Jul 9, 2019 at 3:02
1 Answer 1
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 thatsed
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
export
ed variables. - Single use variables can often be inlined without loss of clarity.
printf
is meant to be used with format strings for readability. Soprintf "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 thatlocal foo=[variable or command substitution]
only gets the exit code oflocal
, not of the variable or command substitution. So if you start usingnounset
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.
-
\$\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– Firat Oezcan2019年07月08日 11:04:50 +00:00Commented Jul 8, 2019 at 11:04 -
1\$\begingroup\$ I've added an example to clarify how to use
local
. \$\endgroup\$l0b0– l0b02019年07月08日 19:52:52 +00:00Commented Jul 8, 2019 at 19:52 -
\$\begingroup\$ Ahhh, wow that's really good to know. Thanks for the inconvenience in helping me out :) \$\endgroup\$Firat Oezcan– Firat Oezcan2019年07月09日 11:54:32 +00:00Commented 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\$l0b0– l0b02019年07月09日 19:26:45 +00:00Commented Jul 9, 2019 at 19:26