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.

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

feat: Add parallelism to major chunk of hooks. Check Parallelism section in README #620

Merged
merged 55 commits into from
Feb 17, 2024
Merged
Changes from 1 commit
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
6f44b5d
Init paralelizm
MaxymVlasov Feb 8, 2024
4011cfc
Remove not more needed.
MaxymVlasov Feb 8, 2024
007b752
[WIP] Add debug logs for testing
MaxymVlasov Feb 8, 2024
819d913
If plugin_cache_dir specified - lock it to avoid bug
MaxymVlasov Feb 8, 2024
30b3069
fixup flock
MaxymVlasov Feb 8, 2024
b6017f1
Test flock on directory rather than stdin
MaxymVlasov Feb 8, 2024
357b72e
Revert "Test flock on directory rather than stdin"
MaxymVlasov Feb 8, 2024
2b04472
Try --no-fork option and locking plugin_cache_dir, which more logical
MaxymVlasov Feb 9, 2024
92cc899
Try without --no-fork
MaxymVlasov Feb 9, 2024
2c52da1
Add fallback for macOS&Co w/o flock and hook-config=--parallelism_limit
MaxymVlasov Feb 9, 2024
d1a3a70
TEST that tha non-flock works
MaxymVlasov Feb 9, 2024
fd8becf
Add lockdir cleanup after unexcpected end
MaxymVlasov Feb 9, 2024
e430641
return flock, testing parallelism
MaxymVlasov Feb 9, 2024
467959c
dbg
MaxymVlasov Feb 9, 2024
91ffb99
Fix issue when all hook-config ignored except first
MaxymVlasov Feb 9, 2024
6051543
Cleanup debug log
MaxymVlasov Feb 9, 2024
f8d26f8
Add docs and rename CPU_NUM to CPU
MaxymVlasov Feb 9, 2024
8edae71
docs
MaxymVlasov Feb 9, 2024
6475c4b
Add flock to bug report templates
MaxymVlasov Feb 9, 2024
3d60692
Apply suggestions from code review
MaxymVlasov Feb 9, 2024
4f7d28c
Apply suggestions from code review
MaxymVlasov Feb 9, 2024
7932ee1
Merge branch 'master' into feat/parallelizm
MaxymVlasov Feb 9, 2024
a5d0980
Apply review suggestions
MaxymVlasov Feb 9, 2024
7c07f9e
Update hooks/_common.sh - config fix
MaxymVlasov Feb 9, 2024
778350a
Apply suggestions from code review
MaxymVlasov Feb 10, 2024
f034438
Apply review suggestions
MaxymVlasov Feb 10, 2024
114a653
Set to rmdir for safety reasons
MaxymVlasov Feb 10, 2024
9cc5e0e
Apply suggestions from code review
MaxymVlasov Feb 12, 2024
a1cac64
DRY tf-init command
MaxymVlasov Feb 12, 2024
4f76448
Revert "DRY tf-init command"
MaxymVlasov Feb 12, 2024
e0dda37
Apply suggestions from code review
MaxymVlasov Feb 12, 2024
66a8f38
Apply suggestions from code review
MaxymVlasov Feb 12, 2024
2cf3ee7
fixup
MaxymVlasov Feb 12, 2024
d9ec149
Add self-healing mechanizm to lock implementation
MaxymVlasov Feb 12, 2024
f640b7c
Apply suggestions from code review
MaxymVlasov Feb 12, 2024
48d1521
Revie suggestions
MaxymVlasov Feb 12, 2024
7f755b2
Remove duplicated tool names
MaxymVlasov Feb 12, 2024
c9f71b8
Cleanup
MaxymVlasov Feb 12, 2024
fe57983
Apply suggestions from code review
MaxymVlasov Feb 12, 2024
cd2fddc
Fix stat and add comments as requested
MaxymVlasov Feb 12, 2024
92ee184
Suppres errors during serach for CPU cores
MaxymVlasov Feb 12, 2024
35c060d
Fix OSX stat
MaxymVlasov Feb 12, 2024
c17035b
Locks not working, try to live without them
MaxymVlasov Feb 13, 2024
a427a7c
Update test results to new implementation
MaxymVlasov Feb 13, 2024
ac4d89f
Apply suggestions from code review
MaxymVlasov Feb 13, 2024
7022a29
Apply suggestions from code review
MaxymVlasov Feb 13, 2024
598d16b
f
MaxymVlasov Feb 13, 2024
ec22d70
feat: Investigate and fix issue with wrong CPU count for containers (…
MaxymVlasov Feb 15, 2024
01d3236
Fix issue with "no logs shown" when something goes wrong (#624)
MaxymVlasov Feb 15, 2024
2bf56bc
Count --parallelism-ci-cpu-cores only in edge-cases
MaxymVlasov Feb 15, 2024
60a106d
Update README.md
MaxymVlasov Feb 15, 2024
89ccdcc
Update hooks/_common.sh
MaxymVlasov Feb 15, 2024
4cf480f
Add more warnings and deal with possible bugs
MaxymVlasov Feb 16, 2024
60bfeea
Improve comment
MaxymVlasov Feb 16, 2024
d84ce5d
Merge branch 'master' into feat/parallelizm
antonbabenko Feb 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
Prev Previous commit
Next Next commit
Add fallback for macOS&Co w/o flock and hook-config=--parallelism_limit
MaxymVlasov committed Feb 9, 2024
commit 2c52da1ae6050c9758a3b634ba7caf791cee5330
85 changes: 59 additions & 26 deletions hooks/_common.sh
Original file line number Diff line number Diff line change
@@ -219,6 +219,13 @@ function common::per_dir_hook {

# Lookup hook-config for modifiers that impact common behavior
local change_dir_in_unique_part=false
# Limit the number of parallel processes to the number of CPU cores -1
# `nproc` - linux, `sysctl -n hw.ncpu` - macOS, `echo 1` - fallback
local CPU_NUM
CPU_NUM=$(nproc || sysctl -n hw.ncpu || echo 1)
local parallelism_limit
local parallelism_disabled=false

IFS=";" read -r -a configs <<< "${HOOK_CONFIG[*]}"
for c in "${configs[@]}"; do
IFS="=" read -r -a config <<< "$c"
@@ -233,20 +240,28 @@ function common::per_dir_hook {
change_dir_in_unique_part="delegate_chdir"
fi
;;
--parallelism_limit)
# this flag will limit the number of parallel processes
# to the number of CPU cores -1
if [[ $value ]]; then
parallelism_limit=$((value))
fi
;;
esac
done

if [[ ! $parallelism_limit ]]; then
parallelism_limit=$((CPU_NUM - 1))
elif [[ $parallelism_limit == 1 ]]; then
parallelism_disabled=true
fi

# preserve errexit status
shopt -qo errexit

local final_exit_code=0
local pids=()

# Limit the number of parallel processes to the number of CPU cores -1
# `nproc` - linux, `sysctl -n hw.ncpu` - macOS, `echo 1` - fallback
local cpu_num
cpu_num=$(nproc || sysctl -n hw.ncpu || echo 1)
local parallelism_limit=$((cpu_num - 1))
echo "$(date "+%s %N") DBG parallelism_limit $parallelism_limit"
echo "$(date "+%s %N") DBG TF_PLUGIN_CACHE_DIR: $TF_PLUGIN_CACHE_DIR"

@@ -261,13 +276,15 @@ function common::per_dir_hook {
pushd "$dir_path" > /dev/null
fi

per_dir_hook_unique_part "$dir_path" "$change_dir_in_unique_part" "${args[@]}"
per_dir_hook_unique_part "$dir_path" "$change_dir_in_unique_part" "$parallelism_disabled" "${args[@]}"
} &
pid=$!
pids+=("$pid")

echo "$(date "+%s %N") DBG $dir_path $pid: right after send to background"
if [ "$i" != 0 ] && [ $((i % parallelism_limit)) == 0 ] || [ "$i" == $last_index ]; then
if [ $parallelism_disabled ] ||
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
[ "$i" != 0 ] && [ $((i % parallelism_limit)) == 0 ] || # don't stop on first iteration when parallelism_limit>1
[ "$i" == $last_index ]; then
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved

for pid in "${pids[@]}"; do
echo "$(date "+%s %N") DBG $pid: wait result"
@@ -327,15 +344,19 @@ function common::colorify {
# command_name (string) command that will tun after successful init
# dir_path (string) PATH to dir relative to git repo root.
# Can be used in error logging
# parallelism_disabled (bool) if true - skip lock mechanism
# Globals (init and populate):
# TF_INIT_ARGS (array) arguments for `terraform init` command
# TF_PLUGIN_CACHE_DIR (string) user defined env var to directory
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
# which can't be R/W concurrently
# Outputs:
# If failed - print out terraform init output
#######################################################################
# TODO: v2.0: Move it inside terraform_validate.sh
function common::terraform_init {
local -r command_name=$1
local -r dir_path=$2
local -r parallelism_disabled=$3
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved

local exit_code=0
local init_output
@@ -346,28 +367,40 @@ function common::terraform_init {
fi

if [ ! -d .terraform/modules ] || [ ! -d .terraform/providers ]; then
# # https://github.com/hashicorp/terraform/issues/31964
# if [ -n "$TF_PLUGIN_CACHE_DIR" ]; then
# echo "$(date "+%s %N") DBG $dir_path: 1. flock --exclusive"
# flock --exclusive 0 #! NOT EXIST IN MAC - https://stackoverflow.com/questions/10526651/mac-os-x-equivalent-of-linux-flock1-command
# fi

echo "$(date "+%s %N") DBG $dir_path: 2. before tf init"

if [ -n "$TF_PLUGIN_CACHE_DIR" ]; then
echo "$(date "+%s %N") DBG $dir_path: 2.1. Cache-dir lock"
# https://github.com/hashicorp/terraform/issues/31964
init_output=$(flock --exclusive "$TF_PLUGIN_CACHE_DIR" terraform init -backend=false "${TF_INIT_ARGS[@]}" 2>&1)
exit_code=$?
else
echo "$(date "+%s %N") DBG $dir_path: 2.1. No lock"

echo "$(date "+%s %N") DBG $dir_path: 1. before tf init"

# Plugin cache dir can't be write concurrently or read during writing
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
# https://github.com/hashicorp/terraform/issues/31964
if [ -z "$TF_PLUGIN_CACHE_DIR" ] || $parallelism_disabled; then
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
echo "$(date "+%s %N") DBG $dir_path: 2. No lock"
init_output=$(terraform init -backend=false "${TF_INIT_ARGS[@]}" 2>&1)
exit_code=$?
else

if command -v flock &> /dev/null; then
echo "$(date "+%s %N") DBG $dir_path: 2. Cache-dir lock"

init_output=$(flock --exclusive "$TF_PLUGIN_CACHE_DIR" terraform init -backend=false "${TF_INIT_ARGS[@]}" 2>&1)
exit_code=$?
# Fall back "simple-lock" mechanizm if `flock` is not available
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
else
lockdir="/tmp/TF_PLUGIN_CACHE_DIR_lock"
while true; do
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
if mkdir "$lockdir" 2> /dev/null; then
init_output=$(terraform init -backend=false "${TF_INIT_ARGS[@]}" 2>&1)
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
exit_code=$?
rmdir "$lockdir"
break
fi
sleep 1
done

common::colorify "green" "For better parallelism performance and stability install 'flock' - https://www.howtodojo.com/flock-command-not-found/"
common::colorify "green" "Or disable parallelism by setting '--hook-config=--parallelism_limit=1'"
fi
fi
# if [ -n "$TF_PLUGIN_CACHE_DIR" ]; then
# flock --unlock 0 #! NOT EXIST IN MAC - https://stackoverflow.com/questions/10526651/mac-os-x-equivalent-of-linux-flock1-command
# echo "$(date "+%s %N") DBG $dir_path: 3. after tf init. flock --unlock"
# fi
echo "$(date "+%s %N") DBG $dir_path: 3. after tf init"

if [ $exit_code -ne 0 ]; then
common::colorify "red" "'terraform init' failed, '$command_name' skipped: $dir_path"
5 changes: 4 additions & 1 deletion hooks/terraform_checkov.sh
Original file line number Diff line number Diff line change
@@ -34,6 +34,7 @@ function main {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -43,7 +44,9 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
# shellcheck disable=SC2034 # Unused var.
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

checkov -d . "${args[@]}"
5 changes: 4 additions & 1 deletion hooks/terraform_fmt.sh
Original file line number Diff line number Diff line change
@@ -31,6 +31,7 @@ function main {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -40,7 +41,9 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
# shellcheck disable=SC2034 # Unused var.
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

# pass the arguments to hook
6 changes: 4 additions & 2 deletions hooks/terraform_providers_lock.sh
Original file line number Diff line number Diff line change
@@ -85,6 +85,7 @@ function lockfile_contains_all_needed_sha {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -93,7 +94,8 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

local platforms_count=0
@@ -136,7 +138,7 @@ function per_dir_hook_unique_part {
common::colorify "yellow" "DEPRECATION NOTICE: We introduced '--mode' flag for this hook.
Check migration instructions at https://github.com/antonbabenko/pre-commit-terraform#terraform_providers_lock
"
common::terraform_init 'terraform providers lock' "$dir_path" || {
common::terraform_init 'terraform providers lock' "$dir_path" "$parallelism_disabled" || {
exit_code=$?
return $exit_code
}
5 changes: 4 additions & 1 deletion hooks/terraform_tflint.sh
Original file line number Diff line number Diff line change
@@ -44,14 +44,17 @@ function main {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
#######################################################################
function per_dir_hook_unique_part {
local -r dir_path="$1"
local -r change_dir_in_unique_part="$2"
shift 2
# shellcheck disable=SC2034 # Unused var.
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

if [ "$change_dir_in_unique_part" == "delegate_chdir" ]; then
5 changes: 4 additions & 1 deletion hooks/terraform_tfsec.sh
Original file line number Diff line number Diff line change
@@ -37,6 +37,7 @@ function main {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -46,7 +47,9 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
# shellcheck disable=SC2034 # Unused var.
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

# pass the arguments to hook
5 changes: 4 additions & 1 deletion hooks/terraform_trivy.sh
Original file line number Diff line number Diff line change
@@ -29,6 +29,7 @@ function main {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -38,7 +39,9 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
# shellcheck disable=SC2034 # Unused var.
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

# pass the arguments to hook
8 changes: 5 additions & 3 deletions hooks/terraform_validate.sh
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -75,6 +75,7 @@ function match_validate_errors {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -83,7 +84,8 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

local exit_code
@@ -121,7 +123,7 @@ function per_dir_hook_unique_part {

# In case `terraform validate` failed to execute
# - check is simple `terraform init` will help
common::terraform_init 'terraform validate' "$dir_path" || {
common::terraform_init 'terraform validate' "$dir_path" "$parallelism_disabled" || {
exit_code=$?
return $exit_code
}
@@ -150,7 +152,7 @@ function per_dir_hook_unique_part {

common::colorify "yellow" "Re-validating: $dir_path"

common::terraform_init 'terraform validate' "$dir_path" || {
common::terraform_init 'terraform validate' "$dir_path" "$parallelism_disabled" || {
exit_code=$?
return $exit_code
}
5 changes: 4 additions & 1 deletion hooks/terragrunt_fmt.sh
Original file line number Diff line number Diff line change
@@ -27,6 +27,7 @@ function main {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -36,7 +37,9 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
# shellcheck disable=SC2034 # Unused var.
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

# pass the arguments to hook
5 changes: 4 additions & 1 deletion hooks/terragrunt_validate.sh
Original file line number Diff line number Diff line change
@@ -27,6 +27,7 @@ function main {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -36,7 +37,9 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
# shellcheck disable=SC2034 # Unused var.
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

# pass the arguments to hook
5 changes: 4 additions & 1 deletion hooks/terrascan.sh
Original file line number Diff line number Diff line change
@@ -27,6 +27,7 @@ function main {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -36,7 +37,9 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
# shellcheck disable=SC2034 # Unused var.
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

# pass the arguments to hook
5 changes: 4 additions & 1 deletion hooks/tfupdate.sh
Original file line number Diff line number Diff line change
@@ -37,6 +37,7 @@ function main {
# change_dir_in_unique_part (string/false) Modifier which creates
# possibilities to use non-common chdir strategies.
# Availability depends on hook.
# parallelism_disabled (bool) if true - skip lock mechanism
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
@@ -46,7 +47,9 @@ function per_dir_hook_unique_part {
local -r dir_path="$1"
# shellcheck disable=SC2034 # Unused var.
local -r change_dir_in_unique_part="$2"
shift 2
# shellcheck disable=SC2034 # Unused var.
local -r parallelism_disabled="$3"
shift 3
local -a -r args=("$@")

# pass the arguments to hook