sdk_lib,run_sdk_container: Modernize a bit

krnowak · krnowak · commit 6b326744d868 · 2023-10-25T14:49:35.000+02:00
- Make cosmetic fixes in help output.

- There is usually no need for putting variables inside quotes in
  assignments.

- Use [[ ]] to avoid putting everything into strings.

- Use arrays instead of relying on strings to be split on whitespace
  as it was the case for invoking docker and getting GPG volume flags
  for docker.

- Make sure that some cleanup and trap strings quote variables
  properly.

- Add a "call_docker" function to avoid dealing with "docker" and a
  new "docker_a" variables when willing to invoke docker. The "docker"
  variable rather shouldn't be used, but it is still there in case
  some other scripts were using it.
diff --git a/run_sdk_container b/run_sdk_container
@@ -12,38 +12,38 @@ source sdk_lib/sdk_container_common.sh
 arch="all"
 name=""
 
-os_version="$(get_git_version)"
-sdk_version="$(get_sdk_version_from_versionfile)"
+os_version=$(get_git_version)
+sdk_version=$(get_sdk_version_from_versionfile)
 custom_image=""
-tty=""
+tty=()
 remove=""
 cleanup=""
 
 usage() {
     echo "  Usage:"
-    echo "  $0 [-t] [-v <version>] [-V sdk version] [-a arch] [-n <name> ] [-x <script>] [-C custom-container] [--rm] [container-command]"
+    echo "  $0 [-t] [-v <version>] [-V <SDK version>] [-a <amd64|arm64|all>] [-n <name> ] [-x <script>] [-C <custom-container>] [--rm] [-U] [<container-command>]"
     echo "       Start an SDK container of a given SDK release version."
     echo "       This will create the container if it does not exist, otherwise start the existing container."
     echo "       If the container is already running then it will exec into the container."
     echo
-    echo "      container-command - command to be run in the container instead of"
+    echo "      <container-command> - Command to be run in the container instead of"
     echo "                     an interactive shell."
-    echo "      -t             Attach docker to a TTY (docker -t)"
+    echo "      -t             Attach docker to a TTY (docker -t)."
     echo "      -v <version> - Sourcetree (OS image) version to use."
     echo "                     Defaults to '$os_version' (current git commit)."
     echo "                     FLATCAR_VERSION[_ID] in '$sdk_container_common_versionfile'"
     echo "                     will be updated accordingly."
-    echo "      -V <SDK ver> - SDK version to use. Defaults to '${sdk_version}'"
+    echo "      -V <SDK version> - SDK version to use. Defaults to '${sdk_version}'"
     echo "                     (FLATCAR_SDK_VERSION from '$sdk_container_common_versionfile')."
     echo "      -a <amd64|arm64|all> - Target architecture (board support) of the SDK."
     echo "                     'all' (the default) contains support for both amd64 and arm64."
     echo "      -n <name>    - Custom name to use for the container."
-    echo "      --rm           Remove container afterwards"
+    echo "      --rm           Remove container afterwards."
     echo "      -x <script>  - For each resource generated during build (container etc.)"
     echo "                     add a cleanup line to <script> which, when run, will free"
     echo "                     the resource. Useful for CI."
-    echo "      -C           - Use an entirely custom container image instead of the SDK's"
-    echo "                     $sdk_container_common_registry/flatcar-sdk-[ARCH]:[SDK VERSION]"
+    echo "      -C <custom-container> - Use an entirely custom container image instead of the SDK's"
+    echo "                     $sdk_container_common_registry/flatcar-sdk-[ARCH]:[SDK VERSION]."
     echo "                     Useful for CI."
     echo "      -U             Do not update the versionfile. Instead, use the version from the versionfile as-is."
     echo "      -h             Print this help."
@@ -52,49 +52,49 @@ usage() {
 # --
 
 update_versionfile=x
-while [ 0 -lt $# ] ; do
+while [[ $# -gt 0 ]] ; do
     case "$1" in
     -h) usage; exit 0;;
     --help) usage; exit 0;;
-    -t) tty="-t";                  shift;;
-    -v) os_version="$2";    shift; shift;;
-    -V) sdk_version="$2";   shift; shift;;
-    -a) arch="$2";          shift; shift;;
-    -n) name="$2";          shift; shift;;
-    --rm) remove=true;             shift;;
-    -x) cleanup="$2";       shift; shift;;
-    -C) custom_image="$2";  shift; shift;;
-    -U) sdk_version="$(get_sdk_version_from_versionfile)"
-        os_version="$(get_version_from_versionfile)"
+    -t) tty=( -t );                shift;;
+    -v) os_version=$2;      shift; shift;;
+    -V) sdk_version=$2;     shift; shift;;
+    -a) arch=$2;            shift; shift;;
+    -n) name=$2;            shift; shift;;
+    --rm) remove=x;                shift;;
+    -x) cleanup=$2;         shift; shift;;
+    -C) custom_image=$2;    shift; shift;;
+    -U) sdk_version=$(get_sdk_version_from_versionfile)
+        os_version=$(get_version_from_versionfile)
         update_versionfile=
         shift;;
     *) break;;
     esac
 done
 
-if [ -n "$custom_image" ] ; then
-    container_image_name="${custom_image}"
+if [[ -n ${custom_image} ]] ; then
+    container_image_name=${custom_image}
 else
-    docker_sdk_vernum="$(vernum_to_docker_image_version "${sdk_version}")"
-    container_image_name="$sdk_container_common_registry/flatcar-sdk-${arch}:${docker_sdk_vernum}"
+    docker_sdk_vernum=$(vernum_to_docker_image_version "${sdk_version}")
+    container_image_name="${sdk_container_common_registry}/flatcar-sdk-${arch}:${docker_sdk_vernum}"
 fi
 
 if [[ -n ${update_versionfile} ]] ; then
     create_versionfile "$sdk_version" "$os_version"
 fi
 
-if [ -z "$name" ] ; then
-    docker_sdk_vernum="$(vernum_to_docker_image_version "${sdk_version}")"
-    docker_os_vernum="$(vernum_to_docker_image_version "${os_version}")"
+if [[ -z ${name} ]] ; then
+    docker_sdk_vernum=$(vernum_to_docker_image_version "${sdk_version}")
+    docker_os_vernum=$(vernum_to_docker_image_version "${os_version}")
     name="flatcar-sdk-${arch}-${docker_sdk_vernum}_os-${docker_os_vernum}"
 fi
 
 filter="^/"
 if "${is_podman}"; then
-  filter=""
+    filter=""
 fi
-stat="$($docker ps --all --no-trunc --filter name="${filter}$name\$" --format '{{.Status}}'\
-        | cut -f1 -d' ')"
+stat=$(call_docker ps --all --no-trunc --filter name="${filter}${name}\$" --format '{{.Status}}' \
+           | cut -f1 -d' ')
 
 # pass SDK related environment variables and gcloud auth
 #  into container
@@ -104,59 +104,66 @@ setup_gsutil
 mkdir -p "__build__/images"
 mkdir -p "sdk_container/.cache/sdks"
 
-hostname="${name:0:63}"
-hostname="${hostname//./_}"
+hostname=${name:0:63}
+hostname=${hostname//./_}
 
-if [ -n "$cleanup" ] ; then
-    echo "$docker container rm -f '${name}'" >> "$cleanup"
+if [[ -n ${cleanup} ]] ; then
+    echo "${docker_a[@]@Q} container rm -f ${name@Q}" >>"${cleanup}"
 fi
 
-if [ -z "$stat" ] ; then
+if [[ -z ${stat} ]] ; then
     yell "Creating a new container '$name'"
 
-    gpg_volumes=$(gnupg_ssh_gcloud_mount_opts)
+    gpg_volumes=()
+    gnupg_ssh_gcloud_mount_opts gpg_volumes
 
-    if [ -z "$custom_image" ]; then
-	(
+    if [[ -z ${custom_image} ]]; then
+        (
             source ci-automation/ci_automation_common.sh
             docker_image_from_registry_or_buildcache "flatcar-sdk-${arch}" "${docker_sdk_vernum}"
-	)
+        )
     else
         # We could split the container_image_name in parts to call docker_image_from_registry_or_buildcache
         # bur for now just try to ensure that we use the latest image if using a container registry,
         # for the tar-ball-imported images we rely on the ci-automation scripts to call
         # docker_image_from_registry_or_buildcache explicitly.
-        $docker pull "${container_image_name}" || true
+        call_docker pull "${container_image_name}" || true
     fi
 
-    $docker create $tty -i \
-       -v /dev:/dev \
-       -v "$(pwd)/sdk_container:/mnt/host/source/" \
-       -v "$(pwd)/__build__/images:/mnt/host/source/src/build" \
-       -v "$(pwd):/mnt/host/source/src/scripts" \
-       $gpg_volumes \
-       --privileged \
-       --network host \
-       -e SDK_USER_ID="$(id -u)" \
-       -e SDK_GROUP_ID="$(id -g)" \
-       --name="$name" \
-       --hostname="$hostname" \
-       --entrypoint /bin/bash \
-       "${container_image_name}" -l
+    docker_flags=(
+        "${tty[@]}"
+        -i
+        -v /dev:/dev
+        -v "${PWD}/sdk_container:/mnt/host/source/"
+        -v "${PWD}/__build__/images:/mnt/host/source/src/build"
+        -v "${PWD}:/mnt/host/source/src/scripts"
+        "${gpg_volumes[@]}"
+        --privileged
+        --network host
+        -e SDK_USER_ID="$(id -u)"
+        -e SDK_GROUP_ID="$(id -g)"
+        --name="${name}"
+        --hostname="${hostname}"
+        --entrypoint /bin/bash
+        "${container_image_name}"
+        -l
+    )
+
+    call_docker create "${docker_flags[@]}"
 fi
 
-if [ "$stat" != "Up" ] ; then
+if [[ ${stat} != "Up" ]] ; then
     yell "Starting stopped container '$name'"
-    if [ "${remove}" = "true" ]; then
-      remove_command="$docker rm -f $name"
+    if [[ -n ${remove} ]]; then
+      remove_command="call_docker rm -f ${name@Q}"
     else
       remove_command=":"
     fi
-    trap "$docker stop -t 0 $name ; ${remove_command}" EXIT
-    $docker start "$name"
+    trap "call_docker stop -t 0 ${name@Q} ; ${remove_command}" EXIT
+    call_docker start "${name}"
 fi
 
 # Workaround: The SDK expects to be able to write to /etc/hosts
-$docker exec "$name" sh -c 'cp /etc/hosts /etc/hosts2; umount /etc/hosts ; mv /etc/hosts2 /etc/hosts'
+call_docker exec "${name}" sh -c 'cp /etc/hosts /etc/hosts2; umount /etc/hosts ; mv /etc/hosts2 /etc/hosts'
 
-$docker exec $tty -i "$name" /mnt/host/source/src/scripts/sdk_lib/sdk_entry.sh "$@"
+call_docker exec "${tty[@]}" -i "${name}" /mnt/host/source/src/scripts/sdk_lib/sdk_entry.sh "$@"
diff --git a/sdk_lib/sdk_container_common.sh b/sdk_lib/sdk_container_common.sh
@@ -32,10 +32,16 @@ if command -v podman >/dev/null; then
     fi
 fi
 
-docker="docker"
+docker_a=( docker )
 if "${is_podman}"; then
-  docker="sudo podman"
+  docker_a=( sudo podman )
 fi
+docker=${docker_a[*]}
+
+function call_docker() {
+    "${docker_a[@]}" "${@}"
+}
+# --
 
 # Common "echo" function
 
@@ -263,34 +269,39 @@ EOF
     export GOOGLE_APPLICATION_CREDENTIALS
 }
 
-   
 # --
 
 # Generate volume mount command line options for docker
 #  to pass gpg, ssh, and gcloud auth host directories
 #  into the SDK container.
 
 function gnupg_ssh_gcloud_mount_opts() {
+    local -n args_ref="${1}"; shift
+
     local sdk_gnupg_home="/home/sdk/.gnupg"
     local gpgagent_dir="/run/user/$(id -u)/gnupg"
 
+    args_ref=()
     # pass host GPG home and Agent directories to container
-    if [ -d "$GNUPGHOME" ] ; then
-        echo "-v $GNUPGHOME:$sdk_gnupg_home"
+    if [[ -d ${GNUPGHOME} ]] ; then
+        args_ref+=( -v "$GNUPGHOME:$sdk_gnupg_home" )
     fi
-    if [ -d "$gpgagent_dir" ] ; then
-        echo "-v $gpgagent_dir:$gpgagent_dir"
+    if [[ -d ${gpgagent_dir} ]] ; then
+        args_ref+=( -v "${gpgagent_dir}:${gpgagent_dir}" )
     fi
 
-    if [ -e "${SSH_AUTH_SOCK:-}" ] ; then
-        local sshsockdir="$(dirname "$SSH_AUTH_SOCK")"
-        echo "-v $sshsockdir:/run/sdk/ssh"
+    local sshsockdir
+    if [[ -e ${SSH_AUTH_SOCK:-} ]] ; then
+        sshsockdir=$(dirname "$SSH_AUTH_SOCK")
+        args_ref+=( -v "${sshsockdir}:/run/sdk/ssh" )
     fi
 
-    if [ -e "${GOOGLE_APPLICATION_CREDENTIALS:-}" ] ; then
-        local creds_dir="$(dirname "${GOOGLE_APPLICATION_CREDENTIALS}")"
-        if [ -d "$creds_dir" ] ; then
+    local creds_dir
+    if [[ -e ${GOOGLE_APPLICATION_CREDENTIALS:-} ]] ; then
+        creds_dir=$(dirname "${GOOGLE_APPLICATION_CREDENTIALS}")
+        if [[ -d ${creds_dir} ]] ; then
             echo "-v $creds_dir:$creds_dir"
+            args_ref+=( -v "${creds_dir}:${creds_dir}" )
         fi
     fi
 }
