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:
evalis evil, and I'm pretty sure you can completely avoid it in your script. Other than a massive gain in sanity, all thatsedstuff 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 pipefailwould 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.
printfis 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}"localis 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 usingnounsetand the like you should declare variables local before assigning them separately:deploy() { local deploy_name tag deploy_name="1ドル" tag="2ドル" ... }shellcheckcan 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
localpart. All the other best practices make sense, especially shellcheck is really helpful but as far as I understood,localis 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