Update CI checks for script QA (#306)

* Fix shellcheck issue * Add check for formatting scripts with shfmt * Format scripts using shfmt
apache · Dec 13, 2024 · bd5e18d · bd5e18d
1 parent 7763f60
commit bd5e18d
Show file tree

Hide file tree

Showing 16 changed files with 228 additions and 126 deletions.
diff --git a/.github/workflows/shellcheck.yaml b/.github/workflows/shellcheck.yaml
@@ -7,7 +7,7 @@
 # "License"); you may not use this file except in compliance
 # with the License.  You may obtain a copy of the License at
 #
-#   http://www.apache.org/licenses/LICENSE-2.0
+#   https://www.apache.org/licenses/LICENSE-2.0
 #
 # Unless required by applicable law or agreed to in writing,
 # software distributed under the License is distributed on an
@@ -25,13 +25,29 @@ on:
   pull_request:
     branches: [ '*' ]
 
+permissions:
+  contents: read
+
 jobs:
+  shfmt:
+    name: shfmt
+    timeout-minutes: 3
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - name: Show the first log message
+      run: git log -n1
+    - name: Install shfmt
+      run: contrib/ci/install-shfmt.sh
+    - name: Checking formatting of all scripts
+      run: contrib/ci/run-shfmt.sh
+
   shellcheck:
     name: ShellCheck
     timeout-minutes: 3
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
       - name: Running shellcheck on bin/** and conf/uno.conf
-        run: contrib/run-shellcheck
+        run: contrib/ci/run-shellcheck.sh
 
diff --git a/bin/impl/commands.sh b/bin/impl/commands.sh
@@ -22,7 +22,7 @@ source "$bin"/impl/util.sh
 
 function uno_install_main() {
   case "$1" in
-    accumulo|hadoop|fluo|fluo-yarn|zookeeper)
+    accumulo | hadoop | fluo | fluo-yarn | zookeeper)
       if install_component "$@"; then
         echo "Installation of $1 complete."
       else
@@ -42,7 +42,7 @@ function uno_run_main() {
   echo "Running $1 (detailed logs in $LOGS_DIR/setup)..."
   save_console_fd
   case "$1" in
-    accumulo|hadoop|fluo|fluo-yarn|zookeeper)
+    accumulo | hadoop | fluo | fluo-yarn | zookeeper)
       if run_component "$@"; then
         echo "Running $1 complete."
       else
@@ -62,7 +62,7 @@ function uno_setup_main() {
   echo "Setting up $1 (detailed logs in $LOGS_DIR/setup)..."
   save_console_fd
   case "$1" in
-    accumulo|hadoop|fluo|fluo-yarn|zookeeper)
+    accumulo | hadoop | fluo | fluo-yarn | zookeeper)
       if setup_component "$@"; then
         echo "Setup of $1 complete."
       else
@@ -108,11 +108,11 @@ function uno_env_main() {
   fi
   if [[ -z $1 || $1 == '--paths' ]]; then
     echo -n "export PATH=\"\$PATH:$UNO_HOME/bin:$HADOOP_HOME/bin:$ZOOKEEPER_HOME/bin:$ACCUMULO_HOME/bin"
-    [[ -d "$SPARK_HOME" ]]     && echo -n ":$SPARK_HOME/bin"
-    [[ -d "$FLUO_HOME" ]]      && echo -n ":$FLUO_HOME/bin"
-    [[ -d "$FLUO_YARN_HOME" ]] && echo -n ":$FLUO_YARN_HOME/bin"
-    [[ -d "$INFLUXDB_HOME" ]]  && echo -n ":$INFLUXDB_HOME/bin"
-    [[ -d "$GRAFANA_HOME" ]]   && echo -n ":$GRAFANA_HOME/bin"
+    [[ -d $SPARK_HOME ]] && echo -n ":$SPARK_HOME/bin"
+    [[ -d $FLUO_HOME ]] && echo -n ":$FLUO_HOME/bin"
+    [[ -d $FLUO_YARN_HOME ]] && echo -n ":$FLUO_YARN_HOME/bin"
+    [[ -d $INFLUXDB_HOME ]] && echo -n ":$INFLUXDB_HOME/bin"
+    [[ -d $GRAFANA_HOME ]] && echo -n ":$GRAFANA_HOME/bin"
     echo '"'
   fi
 }
@@ -145,19 +145,22 @@ function uno_start_main() {
         tmp="$(pgrep -f QuorumPeerMain | tr '\n' ' ')"
         if [[ -z $tmp ]]; then
           "$ZOOKEEPER_HOME"/bin/zkServer.sh start
-        else echo "ZooKeeper   already running at: $tmp"
+        else
+          echo "ZooKeeper   already running at: $tmp"
         fi
 
         tmp="$(pgrep -f hadoop\\.hdfs | tr '\n' ' ')"
         if [[ -z $tmp ]]; then
           "$HADOOP_HOME"/sbin/start-dfs.sh
-        else echo "Hadoop DFS  already running at: $tmp"
+        else
+          echo "Hadoop DFS  already running at: $tmp"
         fi
 
         tmp="$(pgrep -f hadoop\\.yarn | tr '\n' ' ')"
         if [[ -z $tmp ]]; then
           "$HADOOP_HOME"/sbin/start-yarn.sh
-        else echo "Hadoop Yarn already running at: $tmp"
+        else
+          echo "Hadoop Yarn already running at: $tmp"
         fi
       fi
 
@@ -168,7 +171,8 @@ function uno_start_main() {
         else
           "$ACCUMULO_HOME"/bin/accumulo-cluster start
         fi
-      else echo "Accumulo    already running at: $tmp"
+      else
+        echo "Accumulo    already running at: $tmp"
       fi
       ;;
     hadoop)
@@ -177,13 +181,15 @@ function uno_start_main() {
       tmp="$(pgrep -f hadoop\\.hdfs | tr '\n' ' ')"
       if [[ -z $tmp ]]; then
         "$HADOOP_HOME"/sbin/start-dfs.sh
-      else echo "Hadoop DFS  already running at: $tmp"
+      else
+        echo "Hadoop DFS  already running at: $tmp"
       fi
 
       tmp="$(pgrep -f hadoop\\.yarn | tr '\n' ' ')"
       if [[ -z $tmp ]]; then
         "$HADOOP_HOME"/sbin/start-yarn.sh
-      else echo "Hadoop Yarn already running at: $tmp"
+      else
+        echo "Hadoop Yarn already running at: $tmp"
       fi
       ;;
     zookeeper)
@@ -192,7 +198,8 @@ function uno_start_main() {
       tmp="$(pgrep -f QuorumPeerMain | tr '\n' ' ')"
       if [[ -z $tmp ]]; then
         "$ZOOKEEPER_HOME"/bin/zkServer.sh start
-      else echo "ZooKeeper   already running at: $tmp"
+      else
+        echo "ZooKeeper   already running at: $tmp"
       fi
       ;;
     *)
@@ -285,14 +292,20 @@ function uno_jshell_main() {
 }
 
 function uno_zk_main() {
-  check_dirs ZOOKEEPER_HOME  || return 1
+  check_dirs ZOOKEEPER_HOME || return 1
   "$ZOOKEEPER_HOME"/bin/zkCli.sh "$@"
 }
 
 function uno_fetch_main() {
-  hash mvn 2>/dev/null || { echo >&2 "Maven must be installed & on PATH. Aborting."; return 1; }
-  hash wget 2>/dev/null || { echo >&2 "wget must be installed & on PATH. Aborting."; return 1; }
-  if [[ "$1" == "all" ]]; then
+  hash mvn 2>/dev/null || {
+    echo >&2 "Maven must be installed & on PATH. Aborting."
+    return 1
+  }
+  hash wget 2>/dev/null || {
+    echo >&2 "wget must be installed & on PATH. Aborting."
+    return 1
+  }
+  if [[ $1 == "all" ]]; then
     "$bin"/impl/fetch.sh fluo
   else
     "$bin"/impl/fetch.sh "$1" "$2"
@@ -304,7 +317,7 @@ function uno_wipe_main() {
   uno_kill_main
   read -r -p "Are you sure you want to wipe '$INSTALL'? " yn
   case "$yn" in
-    [yY]|[yY][eE][sS])
+    [yY] | [yY][eE][sS])
       if [[ -d $INSTALL && $INSTALL != '/' ]]; then
         echo "removing $INSTALL"
         rm -rf "${INSTALL:?}"

diff --git a/bin/impl/fetch.sh b/bin/impl/fetch.sh
@@ -29,7 +29,7 @@ function fetch_zookeeper() {
 function fetch_accumulo() {
   [[ $1 != '--no-deps' ]] && fetch_hadoop && fetch_zookeeper
 
-  if [[ -n "$ACCUMULO_REPO" ]]; then
+  if [[ -n $ACCUMULO_REPO ]]; then
     declare -a maven_args=(-DskipTests -DskipFormat)
     if [[ $HADOOP_VERSION =~ 3\..* ]]; then
       maven_args=("${maven_args[@]}" '-Dhadoop.profile=3')
@@ -42,7 +42,7 @@ function fetch_accumulo() {
     rm -f "${DOWNLOADS:?}/${ACCUMULO_TARBALL:?}"
     (cd "$ACCUMULO_REPO" && mvn -V -e clean package "${maven_args[@]}")
     accumulo_built_tarball=$ACCUMULO_REPO/assemble/target/$ACCUMULO_TARBALL
-    if [[ ! -f "$accumulo_built_tarball" ]]; then
+    if [[ ! -f $accumulo_built_tarball ]]; then
       cat <<EOF
 
 The following file does not exist :
@@ -63,11 +63,11 @@ EOF
 
 function fetch_fluo() {
   [[ $1 != '--no-deps' ]] && fetch_accumulo
-  if [[ -n "$FLUO_REPO" ]]; then
+  if [[ -n $FLUO_REPO ]]; then
     rm -f "${DOWNLOADS:?}/${FLUO_TARBALL:?}"
     (cd "$FLUO_REPO" && mvn -V -e clean package -DskipTests -Dformatter.skip)
     fluo_built_tarball=$FLUO_REPO/modules/distribution/target/$FLUO_TARBALL
-    if [[ ! -f "$fluo_built_tarball" ]]; then
+    if [[ ! -f $fluo_built_tarball ]]; then
       echo "The tarball $fluo_built_tarball does not exist after building from the FLUO_REPO=$FLUO_REPO"
       echo "Does your repo contain code matching the FLUO_VERSION=$FLUO_VERSION set in uno.conf?"
       exit 1
@@ -88,36 +88,36 @@ if [[ -z $apache_mirror ]]; then
 fi
 
 case "$1" in
-accumulo)
-  fetch_accumulo "$2"
-  ;;
-fluo)
-  fetch_fluo "$2"
-  ;;
-fluo-yarn)
-  [[ $2 != '--no-deps' ]] && fetch_fluo
-  if [[ -n $FLUO_YARN_REPO ]]; then
-    rm -f "${DOWNLOADS:?}/${FLUO_YARN_TARBALL:?}"
-    (cd "$FLUO_YARN_REPO" && mvn -V -e clean package -DskipTests -Dformatter.skip)
-    built_tarball=$FLUO_YARN_REPO/target/$FLUO_YARN_TARBALL
-    if [[ ! -f "$built_tarball" ]]; then
-      echo "The tarball $built_tarball does not exist after building from the FLUO_YARN_REPO=$FLUO_YARN_REPO"
-      echo "Does your repo contain code matching the FLUO_YARN_VERSION=$FLUO_YARN_VERSION set in uno.conf?"
-      exit 1
+  accumulo)
+    fetch_accumulo "$2"
+    ;;
+  fluo)
+    fetch_fluo "$2"
+    ;;
+  fluo-yarn)
+    [[ $2 != '--no-deps' ]] && fetch_fluo
+    if [[ -n $FLUO_YARN_REPO ]]; then
+      rm -f "${DOWNLOADS:?}/${FLUO_YARN_TARBALL:?}"
+      (cd "$FLUO_YARN_REPO" && mvn -V -e clean package -DskipTests -Dformatter.skip)
+      built_tarball=$FLUO_YARN_REPO/target/$FLUO_YARN_TARBALL
+      if [[ ! -f $built_tarball ]]; then
+        echo "The tarball $built_tarball does not exist after building from the FLUO_YARN_REPO=$FLUO_YARN_REPO"
+        echo "Does your repo contain code matching the FLUO_YARN_VERSION=$FLUO_YARN_VERSION set in uno.conf?"
+        exit 1
+      fi
+      cp "$built_tarball" "$DOWNLOADS"/
+    else
+      download_apache "fluo/fluo-yarn/$FLUO_YARN_VERSION" "$FLUO_YARN_TARBALL" "$FLUO_YARN_HASH"
     fi
-    cp "$built_tarball" "$DOWNLOADS"/
-  else
-    download_apache "fluo/fluo-yarn/$FLUO_YARN_VERSION" "$FLUO_YARN_TARBALL" "$FLUO_YARN_HASH"
-  fi
-  ;;
-hadoop)
-  fetch_hadoop
-  ;;
-zookeeper)
-  fetch_zookeeper
-  ;;
-*)
-  cat <<EOF
+    ;;
+  hadoop)
+    fetch_hadoop
+    ;;
+  zookeeper)
+    fetch_zookeeper
+    ;;
+  *)
+    cat <<EOF
 Usage: uno fetch <component>
 
 Possible components:
@@ -131,7 +131,8 @@ Options:
     --no-deps  Dependencies will be fetched unless this option is specified. Only works for fluo & accumulo components.
     --test     Copy the test jar built in accumulo to the downloads directory
 EOF
-  exit 1
+    exit 1
+    ;;
 esac
 
 # fetch.sh
diff --git a/bin/impl/install/accumulo.sh b/bin/impl/install/accumulo.sh
@@ -53,8 +53,8 @@ sed -i'' -e 's!paste -sd:)!paste -sd: -)!' "$conf/accumulo-env.sh"
 
 cp "$UNO_HOME"/conf/accumulo/common/* "$conf"
 if [[ $ACCUMULO_VERSION =~ ^1\..*$ ]]; then
-    print_to_console "Accumulo 1 is not supported; use an earlier uno or a newer accumulo"
-    exit 1
+  print_to_console "Accumulo 1 is not supported; use an earlier uno or a newer accumulo"
+  exit 1
 else
   accumulo_conf=$conf/accumulo.properties
   cp "$UNO_HOME"/conf/accumulo/2/* "$conf"

diff --git a/bin/impl/install/hadoop.sh b/bin/impl/install/hadoop.sh
@@ -60,18 +60,18 @@ $SED "s#YARN_NM_CPU_VCORES#$YARN_NM_CPU_VCORES#g" "$hadoop_conf/yarn-site.xml"
   echo "export JAVA_HOME=\"$JAVA_HOME\""
   echo "export HADOOP_LOG_DIR=\"$HADOOP_LOG_DIR\""
   echo "export HADOOP_MAPRED_HOME=\"$HADOOP_HOME\""
-} >> "$hadoop_conf/hadoop-env.sh"
-[[ $HADOOP_VERSION =~ ^2\..*$ ]] && echo "export YARN_LOG_DIR=$HADOOP_LOG_DIR" >> "$hadoop_conf/yarn-env.sh"
+} >>"$hadoop_conf/hadoop-env.sh"
+[[ $HADOOP_VERSION =~ ^2\..*$ ]] && echo "export YARN_LOG_DIR=$HADOOP_LOG_DIR" >>"$hadoop_conf/yarn-env.sh"
 
 # Yarn requires extra JVM args to start with Java 17+
 jver=$("$JAVA_HOME"/bin/java -version 2>&1 | grep version | cut -f2 -d'"' | cut -f1 -d.)
 
 if [[ $jver -gt 11 ]]; then
   echo "Setting yarn JVM args for java $jver"
   {
-    echo "export YARN_RESOURCEMANAGER_OPTS=\"--add-opens java.base/java.lang=ALL-UNNAMED\""
-    echo "export YARN_NODEMANAGER_OPTS=\"--add-opens java.base/java.lang=ALL-UNNAMED\""
-  } >> "$hadoop_conf/yarn-env.sh"
+    echo 'export YARN_RESOURCEMANAGER_OPTS="--add-opens java.base/java.lang=ALL-UNNAMED"'
+    echo 'export YARN_NODEMANAGER_OPTS="--add-opens java.base/java.lang=ALL-UNNAMED"'
+  } >>"$hadoop_conf/yarn-env.sh"
 fi
 
 true

diff --git a/bin/impl/load-env.sh b/bin/impl/load-env.sh
@@ -18,7 +18,7 @@
 : "${bin:?"'\$bin' should be set by 'uno' script"}"
 
 # Determine UNO_HOME - Use env variable set by user. If none set, calculate using bin dir
-UNO_HOME="${UNO_HOME:-$( cd -P "${bin}"/.. && pwd )}"
+UNO_HOME="${UNO_HOME:-$(cd -P "${bin}"/.. && pwd)}"
 export UNO_HOME
 if [[ -z $UNO_HOME || ! -d $UNO_HOME ]]; then
   echo "UNO_HOME=$UNO_HOME is not a valid directory. Please make sure it exists"
@@ -52,34 +52,34 @@ function env_error() {
 
 # Confirm that hadoop, accumulo, and zookeeper env variables are not set
 if [[ ! "version env" =~ $1 ]]; then
-  [[ -n "$HH" && "$HH" != "$HADOOP_HOME" ]] && env_error 'HADOOP_HOME' "$HH" "$HADOOP_HOME"
-  [[ -n "$HC" && "$HC" != "$HADOOP_CONF_DIR" ]] && env_error 'HADOOP_CONF_DIR' "$HC" "$HADOOP_CONF_DIR"
-  [[ -n "$ZH" && "$ZH" != "$ZOOKEEPER_HOME" ]] && env_error 'ZOOKEEPER_HOME' "$ZH" "$ZOOKEEPER_HOME"
-  [[ -n "$SH" && "$SH" != "$SPARK_HOME" ]] && env_error 'SPARK_HOME' "$SH" "$SPARK_HOME"
-  [[ -n "$AH" && "$AH" != "$ACCUMULO_HOME" ]] && env_error 'ACCUMULO_HOME' "$AH" "$ACCUMULO_HOME"
-  [[ -n "$FH" && "$FH" != "$FLUO_HOME" ]] && env_error 'FLUO_HOME' "$FH" "$FLUO_HOME"
+  [[ -n $HH && $HH != "$HADOOP_HOME" ]] && env_error 'HADOOP_HOME' "$HH" "$HADOOP_HOME"
+  [[ -n $HC && $HC != "$HADOOP_CONF_DIR" ]] && env_error 'HADOOP_CONF_DIR' "$HC" "$HADOOP_CONF_DIR"
+  [[ -n $ZH && $ZH != "$ZOOKEEPER_HOME" ]] && env_error 'ZOOKEEPER_HOME' "$ZH" "$ZOOKEEPER_HOME"
+  [[ -n $SH && $SH != "$SPARK_HOME" ]] && env_error 'SPARK_HOME' "$SH" "$SPARK_HOME"
+  [[ -n $AH && $AH != "$ACCUMULO_HOME" ]] && env_error 'ACCUMULO_HOME' "$AH" "$ACCUMULO_HOME"
+  [[ -n $FH && $FH != "$FLUO_HOME" ]] && env_error 'FLUO_HOME' "$FH" "$FLUO_HOME"
 fi
 
 # Confirm that env variables were set correctly
-if [[ -n "$FLUO_REPO" && ! -d "$FLUO_REPO" ]]; then
+if [[ -n $FLUO_REPO && ! -d $FLUO_REPO ]]; then
   echo "FLUO_REPO=$FLUO_REPO is not a valid directory. Please make sure it exists"
   exit 1
 fi
-if [[ -n "$ACCUMULO_REPO" && ! -d "$ACCUMULO_REPO" ]]; then
+if [[ -n $ACCUMULO_REPO && ! -d $ACCUMULO_REPO ]]; then
   echo "ACCUMULO_REPO=$ACCUMULO_REPO is not a valid directory. Please make sure it exists"
   exit 1
 fi
 
-if [[ -z "$INSTALL" ]]; then
+if [[ -z $INSTALL ]]; then
   echo "INSTALL=$INSTALL needs to be set in uno.conf"
   exit 1
 fi
 
-if [[ ! -d "$INSTALL" ]]; then
+if [[ ! -d $INSTALL ]]; then
   mkdir -p "$INSTALL"
 fi
 
-if [[ -z "$JAVA_HOME" || ! -d "$JAVA_HOME" ]]; then
+if [[ -z $JAVA_HOME || ! -d $JAVA_HOME ]]; then
   echo "JAVA_HOME must be set in your shell to a valid directory. Currently, JAVA_HOME=$JAVA_HOME"
   exit 1
 fi
@@ -119,8 +119,14 @@ if [[ -z $ACCUMULO_HASH && ! $ACCUMULO_VERSION =~ SNAPSHOT ]]; then
   exit 1
 fi
 
-hash shasum 2>/dev/null || { echo >&2 "shasum must be installed & on PATH. Aborting."; exit 1; }
-hash sed 2>/dev/null || { echo >&2 "sed must be installed & on PATH. Aborting."; exit 1; }
+hash shasum 2>/dev/null || {
+  echo >&2 "shasum must be installed & on PATH. Aborting."
+  exit 1
+}
+hash sed 2>/dev/null || {
+  echo >&2 "sed must be installed & on PATH. Aborting."
+  exit 1
+}
 
 if sed --version >/dev/null 2>&1; then
   # GNU sed supports --version and -i without a backup suffix parameter