Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: check_or_install talosctl #15

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Feature: check_or_install talosctl #15

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
809caaa
chore: add .gitignore
TrooperT Aug 8, 2024
397799d
feat: check_or_install talosctl@latest
TrooperT Aug 8, 2024
40861ba
Update talos-bootstrap
TrooperT Aug 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
**/scratch
55 changes: 53 additions & 2 deletions talos-bootstrap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,53 @@
# This code should (try to) follow Google's Shell Style Guide
# - https://google.github.io/styleguide/shell.xml
#

node=
OP=

# Prepend $HOME/.tbs/bin to $PATH so we pickup on any
# downloaded/preferred binary versions
export PATH=$HOME/.tbs/bin:$PATH

# Test for a binary, and prompt to install to /usr/local/bin,
# or download temp copy using J.Pillora's hosted copy of installer:
# https://github.com/jpillora/installer
function check_or_install {
PROG=$1
REPO=$2
# TODO: Versioned install/download
VERSION=${4:-"latest"}
METHOD=${3:-}
NEEDS_INSTALLED=
BIN_DIR=$HOME/.tbs/bin
# test if $PROG already exists: if yes NEEDS_INSTALLED remains unset,
# therefore we skip over the
which $PROG > /dev/null || NEEDS_INSTALLED=true
# No install method specified, prompt
if [[ $NEEDS_INSTALLED && -z "$METHOD" ]]; then
install_option=$(dialog --keep-tite --title "Missing: $PROG..." \
--menu "Required program missing: $PROG\nSelect an option" 0 0 0 \
1 "Install $PROG to /usr/local/bin" \
2 "Download temporary copy of $PROG" \
3 "Do nothing and exit" 3>&1 1>&2 2>&3) || exit 0
case $install_option in
1) METHOD="install" ;;
2) METHOD="download" ;;
*) exit 0 ;;
esac
fi
if [[ $NEEDS_INSTALLED && $METHOD == "install" ]]; then
curl -s "https://i.jpillora.com/${REPO}\!?as=${PROG}&type=script" | bash
elif [[ $NEEDS_INSTALLED && $METHOD == "download" ]]; then
# store PWD
pushd $PWD > /dev/null
mkdir -p $HOME/.tbs/bin && cd $HOME/.tbs/bin
# url without ! will attempt to download to $PWD
curl -s "https://i.jpillora.com/${REPO}?as=${PROG}&type=script" | bash
Copy link
Member

@kvaps kvaps Aug 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This adds additional dependency - bash and all dependencies used by i.jpillora.com, which was not required to use this script before.

  2. I have a fear using external services without informing user about this. curl | bash is very bad pattern to include into script. But it can be described in a README.md

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about parsing architecture and platform using uname and preform url from this:

https://github.com/siderolabs/talos/releases/latest/download/talosctl-<platform>-<arch>

Or if version specified:

https://github.com/siderolabs/talos/releases/download/<version>/talosctl-<platform>-<arch>

See approach from this file:

https://github.com/deckhouse/deckhouse/blob/ab6faa3ad220f203117626a15052333bf84154dc/Makefile#L11-L46

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are great points.

  • I agree the curl | bash is a less-than-ideal pattern, as well as an additional dependency along with all the binaries that the generated install script from J.Pillora's installer checks for [although I believe all the checks are for binaries expected on a POSIX-compliant system]. The design was a "quick-fix" I chose w/o considering the extra deps it added

I'll see what I can do to address your concerns by specializing the check_or_install function to our specific use case for talosctl

FWIW, I initially chose to design in this way anticipation of another feature contribution which will allow the user to select the to-be-installed talosctl version from a list as it would need to ensure yq was available for easily parsing Github API results, BUT I can still accomplish that and narrow dependencies to just the actual binaries that we need: talosctl and yq.

Ill hammer away and get back with you :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @TrooperT ❤️

I mean, that I don't against using curl | bash but it should be described in README.md insead of placing it into script.
So user will invoke it at their own risk, or they still have the opportunity to install it from their distribution.

From the other side script can check if required binary exists and can be running.

# return to original directory
popd > /dev/null
fi
}

while [ $# -gt 0 ]; do
key="$1"
case $key in
Expand Down Expand Up @@ -56,6 +100,13 @@ case "$OP" in
;;
esac

# TODO: install and utilize YQ
# check_or_install yq mikefarah/yq download
check_or_install talosctl siderolabs/talos # download
# yq --help > /dev/null && echo "Utilizing yq $(yq --version | cut -d' ' -f 4) [$(which yq)]" || exit 1
talosctl --help > /dev/null && echo "Utilizing talosctl $(talosctl version --client --short | tail -1| cut -d' ' -f2) [$(which talosctl)]" || exit 1
echo "Current working directory: $PWD"

# Load cluster configuration
if [ -f cluster.conf ]; then
for key in BOOTSTRAP_ETCD CLUSTER_NAME KUBERNETES_API_ENDPOINT VIP_ADDRESS; do
Expand Down Expand Up @@ -252,7 +303,7 @@ if [ "$interface_kind" = bond ]; then
# Screen: Select slave interfaces for bonding
interface_index=$(talosctl -e "${node}" -n "${node}" get link "${interface}" ${OPTS} -o jsonpath='{.spec.index}') || exit $?
default_slave_interfaces=$(talosctl -e "${node}" -n "${node}" get link ${OPTS} -o jsonpath='{.metadata.id}{.spec}' | \
awk '{sub("}", "\n"); print}' ORS=""| awk -F '{' '/masterIndex/ {sub("{.*masterIndex", ""); print}' | awk '{sub(",.*", ""); sub("\":", ""); if ($2 == '"${interface_index}"') print $1}')
awk '{sub("}", "\n"); print}' ORS=""| awk -F '{' '/masterIndex/ {sub("{.*masterIndex", ""); print}' | awk '{sub(",.*", ""); sub("\":", ""); if ($2 == '"${interface_index}"') print $1}')
TrooperT marked this conversation as resolved.
Show resolved Hide resolved
slave_interfaces_list=$(echo "$interface_list" | awk '$1 !~ /^bond/' | while read link details; do
echo "$link" "$details" "$(echo "$default_slave_interfaces" | awk "\$1 == \"$link\" {found=1; exit} END {print found ? \"on\" : \"off\"}")"
done)
Expand Down