Merge pull request #15479 from ClickHouse/aku/docker-shellcheck

Check docker scripts with shellcheck

docker/builder/build.sh

 #!/usr/bin/env bash
+set -e
 
 #ccache -s # uncomment to display CCache statistics
 mkdir -p /server/build_docker
 cd /server/build_docker
-cmake -G Ninja /server -DCMAKE_C_COMPILER=`which gcc-9` -DCMAKE_CXX_COMPILER=`which g++-9`
+cmake -G Ninja /server "-DCMAKE_C_COMPILER=$(command -v gcc-9)" "-DCMAKE_CXX_COMPILER=$(command -v g++-9)"
 
 # Set the number of build jobs to the half of number of virtual CPU cores (rounded up).
 # By default, ninja use all virtual CPU cores, that leads to very high memory consumption without much improvement in build time.

docker/packager/binary/build.sh

@@ -17,7 +17,8 @@ ccache --show-stats ||:
 ccache --zero-stats ||:
 ln -s /usr/lib/x86_64-linux-gnu/libOpenCL.so.1.0.0 /usr/lib/libOpenCL.so ||:
 rm -f CMakeCache.txt
-cmake --debug-trycompile --verbose=1 -DCMAKE_VERBOSE_MAKEFILE=1 -LA -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DSANITIZE=$SANITIZER -DENABLE_CHECK_HEAVY_BUILDS=1 $CMAKE_FLAGS ..
+cmake --debug-trycompile --verbose=1 -DCMAKE_VERBOSE_MAKEFILE=1 -LA "-DCMAKE_BUILD_TYPE=$BUILD_TYPE" "-DSANITIZE=$SANITIZER" -DENABLE_CHECK_HEAVY_BUILDS=1 "$CMAKE_FLAGS" ..
+# shellcheck disable=SC2086 # No quotes because I want it to expand to nothing if empty.
 ninja $NINJA_FLAGS clickhouse-bundle
 mv ./programs/clickhouse* /output
 mv ./src/unit_tests_dbms /output ||: # may not exist for some binary builds

docker/packager/deb/build.sh

@@ -4,16 +4,16 @@ set -x -e
 
 ccache --show-stats ||:
 ccache --zero-stats ||:
-build/release --no-pbuilder $ALIEN_PKGS | ts '%Y-%m-%d %H:%M:%S'
+build/release --no-pbuilder "$ALIEN_PKGS" | ts '%Y-%m-%d %H:%M:%S'
 mv /*.deb /output
-mv *.changes /output
-mv *.buildinfo /output
+mv -- *.changes /output
+mv -- *.buildinfo /output
 mv /*.rpm /output ||: # if exists
 mv /*.tgz /output ||: # if exists
 
 if [ -n "$BINARY_OUTPUT" ] && { [ "$BINARY_OUTPUT" = "programs" ] || [ "$BINARY_OUTPUT" = "tests" ] ;}
 then
-  echo Place $BINARY_OUTPUT to output
+  echo "Place $BINARY_OUTPUT to output"
   mkdir /output/binary ||: # if exists
   mv /build/obj-*/programs/clickhouse* /output/binary
   if [ "$BINARY_OUTPUT" = "tests" ]

docker/packager/unbundled/build.sh

@@ -4,10 +4,10 @@ set -x -e
 
 ccache --show-stats ||:
 ccache --zero-stats ||:
-build/release --no-pbuilder $ALIEN_PKGS | ts '%Y-%m-%d %H:%M:%S'
+build/release --no-pbuilder "$ALIEN_PKGS" | ts '%Y-%m-%d %H:%M:%S'
 mv /*.deb /output
-mv *.changes /output
-mv *.buildinfo /output
+mv -- *.changes /output
+mv -- *.buildinfo /output
 mv /*.rpm /output ||: # if exists
 mv /*.tgz /output ||: # if exists
 

docker/test/fuzzer/run-fuzzer.sh

@@ -58,7 +58,7 @@ function watchdog
 
     echo "Fuzzing run has timed out"
     killall clickhouse-client ||:
-    for x in {1..10}
+    for _ in {1..10}
     do
         if ! pgrep -f clickhouse-client
         then
@@ -81,6 +81,9 @@ function fuzz
     echo Server started
 
     fuzzer_exit_code=0
+    # SC2012: Use find instead of ls to better handle non-alphanumeric filenames.
+    # They are all alphanumeric.
+    # shellcheck disable=SC2012
     ./clickhouse-client --query-fuzzer-runs=1000 \
         < <(for f in $(ls ch/tests/queries/0_stateless/*.sql | sort -R); do cat "$f"; echo ';'; done) \
         > >(tail -10000 > fuzzer.log) \

docker/test/integration/runner/dockerd-entrypoint.sh

@@ -7,7 +7,7 @@ set +e
 reties=0
 while true; do
     docker info &>/dev/null && break
-    reties=$[$reties+1]
+    reties=$((reties+1))
     if [[ $reties -ge 100 ]]; then # 10 sec max
         echo "Can't start docker daemon, timeout exceeded." >&2
         exit 1;

docker/test/performance-comparison/compare-releases.sh

@@ -9,7 +9,7 @@ right_version=${2}
 
 if [ "$left_version" == "" ] || [ "$right_version" == "" ]
 then
-    >&2 echo Usage: $(basename "$0") left_version right_version
+    >&2 echo "Usage: $(basename "$0") left_version right_version"
     exit 1
 fi
 

docker/test/performance-comparison/download.sh

@@ -10,7 +10,7 @@ mkdir left ||:
 left_pr=$1
 left_sha=$2
 
-right_pr=$3
+# right_pr=$3 not used for now
 right_sha=$4
 
 datasets=${CHPC_DATASETS:-"hits1 hits10 hits100 values"}

docker/test/stateful/run.sh

@@ -26,11 +26,12 @@ function start()
         fi
         timeout 120 service clickhouse-server start
         sleep 0.5
-        counter=$(($counter + 1))
+        counter=$((counter + 1))
     done
 }
 
 start
+# shellcheck disable=SC2086 # No quotes because I want to split it into words.
 /s3downloader --dataset-names $DATASETS
 chmod 777 -R /var/lib/clickhouse
 clickhouse-client --query "SHOW DATABASES"
@@ -43,8 +44,8 @@ clickhouse-client --query "RENAME TABLE datasets.hits_v1 TO test.hits"
 clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits"
 clickhouse-client --query "SHOW TABLES FROM test"
 
-if cat /usr/bin/clickhouse-test | grep -q -- "--use-skip-list"; then
+if grep -q -- "--use-skip-list" /usr/bin/clickhouse-test ; then
     SKIP_LIST_OPT="--use-skip-list"
 fi
 
-clickhouse-test --testname --shard --zookeeper --no-stateless "$SKIP_LIST_OPT" $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt
+clickhouse-test --testname --shard --zookeeper --no-stateless "$SKIP_LIST_OPT" "$ADDITIONAL_OPTIONS" "$SKIP_TESTS_OPTION" 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt

docker/test/stateful_with_coverage/run.sh

 #!/bin/bash
 
 kill_clickhouse () {
-    kill `pgrep -u clickhouse` 2>/dev/null
+    kill "$(pgrep -u clickhouse)" 2>/dev/null
 
-    for i in {1..10}
+    for _ in {1..10}
     do
-        if ! kill -0 `pgrep -u clickhouse`; then
+        if ! kill -0 "$(pgrep -u clickhouse)"; then
             echo "No clickhouse process"
             break
         else
-            echo "Process" `pgrep -u clickhouse` "still alive"
+            echo "Process $(pgrep -u clickhouse) still alive"
             sleep 10
         fi
     done
@@ -20,19 +20,19 @@ start_clickhouse () {
 }
 
 wait_llvm_profdata () {
-    while kill -0 `pgrep llvm-profdata-10`;
+    while kill -0 "$(pgrep llvm-profdata-10)"
     do
-        echo "Waiting for profdata" `pgrep llvm-profdata-10` "still alive"
+        echo "Waiting for profdata $(pgrep llvm-profdata-10) still alive"
         sleep 3
     done
 }
 
 merge_client_files_in_background () {
-    client_files=`ls /client_*profraw 2>/dev/null`
-    if [ ! -z "$client_files" ]
+    client_files=$(ls /client_*profraw 2>/dev/null)
+    if [ -n "$client_files" ]
     then
-        llvm-profdata-10 merge -sparse $client_files -o merged_client_`date +%s`.profraw
-        rm $client_files
+        llvm-profdata-10 merge -sparse "$client_files" -o "merged_client_$(date +%s).profraw"
+        rm "$client_files"
     fi
 }
 
@@ -66,12 +66,13 @@ function start()
         fi
         timeout 120 service clickhouse-server start
         sleep 0.5
-        counter=$(($counter + 1))
+        counter=$((counter + 1))
     done
 }
 
 start
 
+# shellcheck disable=SC2086 # No quotes because I want to split it into words.
 if ! /s3downloader --dataset-names $DATASETS; then
     echo "Cannot download datatsets"
     exit 1
@@ -100,11 +101,11 @@ LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "RENAME TA
 LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits"
 LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "SHOW TABLES FROM test"
 
-if cat /usr/bin/clickhouse-test | grep -q -- "--use-skip-list"; then
+if grep -q -- "--use-skip-list" /usr/bin/clickhouse-test; then
     SKIP_LIST_OPT="--use-skip-list"
 fi
 
-LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-test --testname --shard --zookeeper --no-stateless "$SKIP_LIST_OPT" $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt
+LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-test --testname --shard --zookeeper --no-stateless "$SKIP_LIST_OPT" "$ADDITIONAL_OPTIONS" "$SKIP_TESTS_OPTION" 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt
 
 kill_clickhouse
 

docker/test/stateless/run.sh

@@ -13,8 +13,8 @@ dpkg -i package_folder/clickhouse-test_*.deb
 
 service clickhouse-server start && sleep 5
 
-if cat /usr/bin/clickhouse-test | grep -q -- "--use-skip-list"; then
+if grep -q -- "--use-skip-list" /usr/bin/clickhouse-test; then
     SKIP_LIST_OPT="--use-skip-list"
 fi
 
-clickhouse-test --testname --shard --zookeeper "$SKIP_LIST_OPT" $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt
+clickhouse-test --testname --shard --zookeeper "$SKIP_LIST_OPT" "$ADDITIONAL_OPTIONS" "$SKIP_TESTS_OPTION" 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt

docker/test/stateless_unbundled/run.sh

@@ -13,8 +13,8 @@ dpkg -i package_folder/clickhouse-test_*.deb
 
 service clickhouse-server start && sleep 5
 
-if cat /usr/bin/clickhouse-test | grep -q -- "--use-skip-list"; then
+if grep -q -- "--use-skip-list" /usr/bin/clickhouse-test; then
     SKIP_LIST_OPT="--use-skip-list"
 fi
 
-clickhouse-test --testname --shard --zookeeper "$SKIP_LIST_OPT" $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt
+clickhouse-test --testname --shard --zookeeper "$SKIP_LIST_OPT" "$ADDITIONAL_OPTIONS" "$SKIP_TESTS_OPTION" 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt

docker/test/stateless_with_coverage/run.sh

 #!/bin/bash
 
 kill_clickhouse () {
-    echo "clickhouse pids" `ps aux | grep clickhouse` | ts '%Y-%m-%d %H:%M:%S'
-    kill `pgrep -u clickhouse` 2>/dev/null
+    echo "clickhouse pids $(pgrep -u clickhouse)" | ts '%Y-%m-%d %H:%M:%S'
+    kill "$(pgrep -u clickhouse)" 2>/dev/null
 
-    for i in {1..10}
+    for _ in {1..10}
     do
-        if ! kill -0 `pgrep -u clickhouse`; then
+        if ! kill -0 "$(pgrep -u clickhouse)"; then
             echo "No clickhouse process" | ts '%Y-%m-%d %H:%M:%S'
             break
         else
-            echo "Process" `pgrep -u clickhouse` "still alive" | ts '%Y-%m-%d %H:%M:%S'
+            echo "Process $(pgrep -u clickhouse) still alive" | ts '%Y-%m-%d %H:%M:%S'
             sleep 10
         fi
     done
 
     echo "Will try to send second kill signal for sure"
-    kill `pgrep -u clickhouse` 2>/dev/null
+    kill "$(pgrep -u clickhouse)" 2>/dev/null
     sleep 5
-    echo "clickhouse pids" `ps aux | grep clickhouse` | ts '%Y-%m-%d %H:%M:%S'
+    echo "clickhouse pids $(pgrep -u clickhouse)" | ts '%Y-%m-%d %H:%M:%S'
 }
 
 start_clickhouse () {
@@ -47,11 +47,11 @@ start_clickhouse
 sleep 10
 
 
-if cat /usr/bin/clickhouse-test | grep -q -- "--use-skip-list"; then
+if grep -q -- "--use-skip-list" /usr/bin/clickhouse-test; then
     SKIP_LIST_OPT="--use-skip-list"
 fi
 
-LLVM_PROFILE_FILE='client_coverage.profraw' clickhouse-test --testname --shard --zookeeper "$SKIP_LIST_OPT" $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt
+LLVM_PROFILE_FILE='client_coverage.profraw' clickhouse-test --testname --shard --zookeeper "$SKIP_LIST_OPT" "$ADDITIONAL_OPTIONS" "$SKIP_TESTS_OPTION" 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt
 
 kill_clickhouse
 

docker/test/stress/run.sh

@@ -13,7 +13,7 @@ function stop()
     timeout 120 service clickhouse-server stop
 
     # Wait for process to disappear from processlist and also try to kill zombies.
-    while kill -9 $(pidof clickhouse-server)
+    while kill -9 "$(pidof clickhouse-server)"
     do
         echo "Killed clickhouse-server"
         sleep 0.5
@@ -35,7 +35,7 @@ function start()
         fi
         timeout 120 service clickhouse-server start
         sleep 0.5
-        counter=$(($counter + 1))
+        counter=$((counter + 1))
     done
 }
 
@@ -49,6 +49,7 @@ export ASAN_OPTIONS='malloc_context_size=10 verbosity=1 allocator_release_to_os_
 
 start
 
+# shellcheck disable=SC2086 # No quotes because I want to split it into words.
 /s3downloader --dataset-names $DATASETS
 chmod 777 -R /var/lib/clickhouse
 clickhouse-client --query "ATTACH DATABASE IF NOT EXISTS datasets ENGINE = Ordinary"

docker/test/test_runner.sh

@@ -2,17 +2,19 @@
 
 set -e -x
 
+# Not sure why shellcheck complains that rc is not assigned before it is referenced.
+# shellcheck disable=SC2154
 trap 'rc=$?; echo EXITED WITH: $rc; exit $rc' EXIT
 
 # CLI option to prevent rebuilding images, just re-run tests with images leftover from previuos time
 readonly NO_REBUILD_FLAG="--no-rebuild"
 
-readonly CLICKHOUSE_DOCKER_DIR="$(realpath ${1})"
+readonly CLICKHOUSE_DOCKER_DIR="$(realpath "${1}")"
 readonly CLICKHOUSE_PACKAGES_ARG="${2}"
 CLICKHOUSE_SERVER_IMAGE="${3}"
 
-if [ ${CLICKHOUSE_PACKAGES_ARG} != ${NO_REBUILD_FLAG} ]; then
-    readonly CLICKHOUSE_PACKAGES_DIR="$(realpath ${2})" # or --no-rebuild
+if [ "${CLICKHOUSE_PACKAGES_ARG}" != "${NO_REBUILD_FLAG}" ]; then
+    readonly CLICKHOUSE_PACKAGES_DIR="$(realpath "${2}")" # or --no-rebuild
 fi
 
 
@@ -25,7 +27,7 @@ fi
 
 # TODO: optionally mount most recent clickhouse-test and queries directory from local machine
 
-if [ ${CLICKHOUSE_PACKAGES_ARG} != ${NO_REBUILD_FLAG} ]; then
+if [ "${CLICKHOUSE_PACKAGES_ARG}" != "${NO_REBUILD_FLAG}" ]; then
     docker build --network=host \
         -f "${CLICKHOUSE_DOCKER_DIR}/test/stateless/clickhouse-statelest-test-runner.Dockerfile" \
         --target clickhouse-test-runner-base \
@@ -49,7 +51,7 @@ fi
 if [ -z "${CLICKHOUSE_SERVER_IMAGE}" ]; then
     CLICKHOUSE_SERVER_IMAGE="yandex/clickhouse-server:local"
 
-    if [ ${CLICKHOUSE_PACKAGES_ARG} != ${NO_REBUILD_FLAG} ]; then
+    if [ "${CLICKHOUSE_PACKAGES_ARG}" != "${NO_REBUILD_FLAG}" ]; then
         docker build --network=host \
             -f "${CLICKHOUSE_DOCKER_DIR}/server/local.Dockerfile" \
             --target clickhouse-server-base \

docker/test/testflows/runner/dockerd-entrypoint.sh

@@ -7,7 +7,7 @@ set +e
 reties=0
 while true; do
     docker info &>/dev/null && break
-    reties=$[$reties+1]
+    reties=$((reties+1))
     if [[ $reties -ge 100 ]]; then # 10 sec max
         echo "Can't start docker daemon, timeout exceeded." >&2
         exit 1;

utils/check-style/check-style

@@ -76,6 +76,9 @@ CURDIR=$(dirname "${BASH_SOURCE[0]}")
 # Check sh tests with Shellcheck
 (cd $ROOT_PATH/tests/queries/0_stateless/ && shellcheck --check-sourced --external-sources --severity info --exclude SC1071,SC2086 *.sh ../1_stateful/*.sh)
 
+# Check docker scripts with shellcheck
+find "$ROOT_PATH/docker" -executable -type f -exec file -F'	' --mime-type {} \; | awk -F'	' '$2==" text/x-shellscript" {print $1}' | xargs shellcheck
+
 # There shouldn't be any docker containers outside docker directory
 find $ROOT_PATH -not -path $ROOT_PATH'/docker*' -not -path $ROOT_PATH'/contrib*' -name Dockerfile -type f 2>/dev/null | xargs --no-run-if-empty -n1 echo "Please move Dockerfile to docker directory:"