From 1894192b0f68641a24835cb3d23a01d7bcb98cc7 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 26 May 2023 10:22:42 -0700 Subject: [PATCH] Run hooks at startup This ensures we do not miss events. E.g. before: t0: hash changes to X t1: send webhook(X), waiting for response t2: hash changes to Y t3: queue next webhook(Y) but can't send because previous is not done t4: crash t5: restart t6: find repo at hash Y no webhook(Y) was sent. after: t0: hash changes to X t1: send webhook(X), waiting for response t2: hash changes to Y t3: queue next webhook(Y) but can't send because previous is not done t4: crash t5: restart t6: find repo at hash Y t7: send webhook(Y), waiting for response --- README.md | 62 +++++++++++++------ _test_tools/exechook_command.sh | 1 + main.go | 40 +++++++----- test_e2e.sh | 105 ++++++++++++++++++++------------ v3-to-v4.md | 6 ++ 5 files changed, 140 insertions(+), 74 deletions(-) diff --git a/README.md b/README.md index 76ab501..83606b7 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,7 @@ More documentation on specific topics can be [found here](./docs). ## Manual ``` + GIT-SYNC NAME @@ -179,12 +180,13 @@ OPTIONS --exechook-command , $GITSYNC_EXECHOOK_COMMAND An optional command to be executed after syncing a new hash of the remote repository. This command does not take any arguments and - executes with the synced repo as its working directory. The following - environment variables $GITSYNC_HASH will be set to the git hash that - was synced. The execution is subject to the overall --sync-timeout - flag and will extend the effective period between sync attempts. - This flag obsoletes --sync-hook-command, but if sync-hook-command - is specified, it will take precedence. + executes with the synced repo as its working directory. The + $GITSYNC_HASH environment variable will be set to the git hash that + was synced. If, at startup, git-sync finds that the --root already + has the correct hash, this hook will still be invoked. This means + that hooks can be invoked more than one time per hash, so they + must be idempotent. This flag obsoletes --sync-hook-command, but + if sync-hook-command is specified, it will take precedence. --exechook-timeout , $GITSYNC_EXECHOOK_TIMEOUT The timeout for the --exechook-command. If not specifid, this @@ -244,12 +246,12 @@ OPTIONS "127.0.0.1:1234": listen on localhost, port 1234 --http-metrics, $GITSYNC_HTTP_METRICS - Enable metrics on git-sync's HTTP endpoint. Requires --http-bind - to be specified. + Enable metrics on git-sync's HTTP endpoint at /metrics. Requires + --http-bind to be specified. --http-pprof, $GITSYNC_HTTP_PPROF - Enable the pprof debug endpoints on git-sync's HTTP endpoint. - Requires --http-bind to be specified. + Enable the pprof debug endpoints on git-sync's HTTP endpoint at + /debug/pprof. Requires --http-bind to be specified. --link , $GITSYNC_LINK The path to at which to create a symlink which points to the @@ -324,6 +326,13 @@ OPTIONS The known_hosts file to use when --ssh-known-hosts is specified. If not specified, this defaults to "/etc/git-secret/known_hosts". + --stale-worktree-timeout , $GITSYNC_STALE_WORKTREE_TIMEOUT + The length of time to retain stale (not the current link target) + worktrees before being removed. Once this duration has elapsed, + a stale worktree will be removed during the next sync attempt + (as determined by --sync-timeout). If not specified, this defaults + to 0, meaning that stale worktrees will be removed immediately. + --submodules , $GITSYNC_SUBMODULES The git submodule behavior: one of "recursive", "shallow", or "off". If not specified, this defaults to "recursive". @@ -352,7 +361,16 @@ OPTIONS -v, --verbose Set the log verbosity level. Logs at this level and lower will be - printed. + printed. Logs follow these guidelines: + + - 0: Minimal, just log updates + - 1: More details about updates + - 2: Log the sync loop + - 3: More details about the sync loop + - 4: More details + - 5: Log all executed commands + - 6: Log stdout/stderr of all executed commands + - 9: Tracing and debug messages --version Print the version and exit. @@ -376,6 +394,10 @@ OPTIONS --webhook-url , $GITSYNC_WEBHOOK_URL A URL for optional webhook notifications when syncs complete. The header 'Gitsync-Hash' will be set to the git hash that was synced. + If, at startup, git-sync finds that the --root already has the + correct hash, this hook will still be invoked. This means that + hooks can be invoked more than one time per hash, so they must be + idempotent. EXAMPLE USAGE @@ -417,14 +439,16 @@ AUTHENTICATION HOOKS Webhooks and exechooks are executed asynchronously from the main git-sync - process. If a --webhook-url or --exechook-command is configured, whenever - a new hash is synced the hook(s) will be invoked. For exechook, that means - the command is exec()'ed, and for webhooks that means an HTTP request is - sent using the method defined in --webhook-method. Git-sync will retry - both forms of hooks until they succeed (exit code 0 for exechooks, or - --webhook-success-status for webhooks). If unsuccessful, git-sync will - wait --exechook-backoff or --webhook-backoff (as appropriate) before - re-trying the hook. + process. If a --webhook-url or --exechook-command is configured, they will + be invoked whenever a new hash is synced, including when git-sync starts up + and find that the --root directory already has the correct hash. For + exechook, that means the command is exec()'ed, and for webhooks that means + an HTTP request is sent using the method defined in --webhook-method. + Git-sync will retry both forms of hooks until they succeed (exit code 0 for + exechooks, or --webhook-success-status for webhooks). If unsuccessful, + git-sync will wait --exechook-backoff or --webhook-backoff (as appropriate) + before re-trying the hook. Git-sync does not ensure that hooks are invoked + exactly once, so hooks must be idempotent. Hooks are not guaranteed to succeed on every single hash change. For example, if a hook fails and a new hash is synced during the backoff period, the diff --git a/_test_tools/exechook_command.sh b/_test_tools/exechook_command.sh index f069334..b900150 100755 --- a/_test_tools/exechook_command.sh +++ b/_test_tools/exechook_command.sh @@ -23,3 +23,4 @@ if [ -z "${GITSYNC_HASH}" ]; then fi cat file > exechook echo "ENVKEY=$ENVKEY" > exechook-env +date >> /var/log/runs diff --git a/main.go b/main.go index 36162bc..3a562bc 100644 --- a/main.go +++ b/main.go @@ -399,7 +399,7 @@ func main() { flExechookCommand := pflag.String("exechook-command", envString("", "GITSYNC_EXECHOOK_COMMAND", "GIT_SYNC_EXECHOOK_COMMAND"), - "an optional command to be run when syncs complete") + "an optional command to be run when syncs complete (must be idempotent)") flExechookTimeout := pflag.Duration("exechook-timeout", envDuration(30*time.Second, "GITSYNC_EXECHOOK_TIMEOUT", "GIT_SYNC_EXECHOOK_TIMEOUT"), "the timeout for the exechook") @@ -409,7 +409,7 @@ func main() { flWebhookURL := pflag.String("webhook-url", envString("", "GITSYNC_WEBHOOK_URL", "GIT_SYNC_WEBHOOK_URL"), - "a URL for optional webhook notifications when syncs complete") + "a URL for optional webhook notifications when syncs complete (must be idempotent)") flWebhookMethod := pflag.String("webhook-method", envString("POST", "GITSYNC_WEBHOOK_METHOD", "GIT_SYNC_WEBHOOK_METHOD"), "the HTTP method for the webhook") @@ -956,6 +956,7 @@ func main() { } failCount := 0 + firstLoop := true for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) @@ -972,7 +973,8 @@ func main() { } else { // this might have been called before, but also might not have setRepoReady() - if changed { + // We treat the first loop as a sync, including sending hooks. + if changed || firstLoop { if absTouchFile != "" { if err := touch(absTouchFile); err != nil { log.Error(err, "failed to touch touch-file", "path", absTouchFile) @@ -990,6 +992,7 @@ func main() { } else { updateSyncMetrics(metricKeyNoOp, start) } + firstLoop = false // Clean up old worktree(s) and run GC. if err := git.cleanup(ctx); err != nil { @@ -2288,10 +2291,11 @@ OPTIONS remote repository. This command does not take any arguments and executes with the synced repo as its working directory. The $GITSYNC_HASH environment variable will be set to the git hash that - was synced. The execution is subject to the overall --sync-timeout - flag and will extend the effective period between sync attempts. - This flag obsoletes --sync-hook-command, but if sync-hook-command - is specified, it will take precedence. + was synced. If, at startup, git-sync finds that the --root already + has the correct hash, this hook will still be invoked. This means + that hooks can be invoked more than one time per hash, so they + must be idempotent. This flag obsoletes --sync-hook-command, but + if sync-hook-command is specified, it will take precedence. --exechook-timeout , $GITSYNC_EXECHOOK_TIMEOUT The timeout for the --exechook-command. If not specifid, this @@ -2499,6 +2503,10 @@ OPTIONS --webhook-url , $GITSYNC_WEBHOOK_URL A URL for optional webhook notifications when syncs complete. The header 'Gitsync-Hash' will be set to the git hash that was synced. + If, at startup, git-sync finds that the --root already has the + correct hash, this hook will still be invoked. This means that + hooks can be invoked more than one time per hash, so they must be + idempotent. EXAMPLE USAGE @@ -2540,14 +2548,16 @@ AUTHENTICATION HOOKS Webhooks and exechooks are executed asynchronously from the main git-sync - process. If a --webhook-url or --exechook-command is configured, whenever - a new hash is synced the hook(s) will be invoked. For exechook, that means - the command is exec()'ed, and for webhooks that means an HTTP request is - sent using the method defined in --webhook-method. Git-sync will retry - both forms of hooks until they succeed (exit code 0 for exechooks, or - --webhook-success-status for webhooks). If unsuccessful, git-sync will - wait --exechook-backoff or --webhook-backoff (as appropriate) before - re-trying the hook. + process. If a --webhook-url or --exechook-command is configured, they will + be invoked whenever a new hash is synced, including when git-sync starts up + and find that the --root directory already has the correct hash. For + exechook, that means the command is exec()'ed, and for webhooks that means + an HTTP request is sent using the method defined in --webhook-method. + Git-sync will retry both forms of hooks until they succeed (exit code 0 for + exechooks, or --webhook-success-status for webhooks). If unsuccessful, + git-sync will wait --exechook-backoff or --webhook-backoff (as appropriate) + before re-trying the hook. Git-sync does not ensure that hooks are invoked + exactly once, so hooks must be idempotent. Hooks are not guaranteed to succeed on every single hash change. For example, if a hook fails and a new hash is synced during the backoff period, the diff --git a/test_e2e.sh b/test_e2e.sh index 1c13f78..94340a0 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -101,6 +101,20 @@ function assert_file_contains() { fail "$1 does not contain '$2': $(cat $1)" } +function assert_file_lines_eq() { + N=$(cat "$1" | wc -l) + if (( "$N" != "$2" )); then + fail "$1 is not $2 lines: $N" + fi +} + +function assert_file_lines_ge() { + N=$(cat "$1" | wc -l) + if (( "$N" < "$2" )); then + fail "$1 is not at least $2 lines: $N" + fi +} + function assert_metric_eq() { local val val="$(curl --silent "http://localhost:$HTTP_PORT/metrics" \ @@ -973,7 +987,7 @@ function e2e::sync_branch_switch() { GIT_SYNC \ --one-time \ --repo="file://$REPO" \ - --ref=$OTHER_BRANCH \ + --ref="$OTHER_BRANCH" \ --root="$ROOT" \ --link="link" assert_link_exists "$ROOT/link" @@ -1807,6 +1821,8 @@ function e2e::auth_askpass_url_flaky() { # Test exechook-success ############################################## function e2e::exechook_success() { + cat /dev/null > "$RUNLOG" + # First sync echo "$FUNCNAME 1" > "$REPO/file" git -C "$REPO" commit -qam "$FUNCNAME 1" @@ -1825,6 +1841,7 @@ function e2e::exechook_success() { assert_file_eq "$ROOT/link/file" "$FUNCNAME 1" assert_file_eq "$ROOT/link/exechook" "$FUNCNAME 1" assert_file_eq "$ROOT/link/exechook-env" "$EXECHOOK_ENVKEY=$EXECHOOK_ENVVAL" + assert_file_lines_eq "$RUNLOG" 1 # Move forward echo "$FUNCNAME 2" > "$REPO/file" @@ -1836,6 +1853,7 @@ function e2e::exechook_success() { assert_file_eq "$ROOT/link/file" "$FUNCNAME 2" assert_file_eq "$ROOT/link/exechook" "$FUNCNAME 2" assert_file_eq "$ROOT/link/exechook-env" "$EXECHOOK_ENVKEY=$EXECHOOK_ENVVAL" + assert_file_lines_eq "$RUNLOG" 2 } ############################################## @@ -1859,10 +1877,7 @@ function e2e::exechook_fail_retry() { sleep 3 # give it time to retry # Check that exechook was called - RUNS=$(cat "$RUNLOG" | wc -l) - if (( "$RUNS" < 2 )); then - fail "exechook called $RUNS times, it should be at least 2" - fi + assert_file_lines_ge "$RUNLOG" 2 } ############################################## @@ -1914,14 +1929,47 @@ function e2e::exechook_fail_once() { fi ) - # Check that exechook was called - RUNS=$(cat "$RUNLOG" | wc -l) - if [[ "$RUNS" != 1 ]]; then - fail "exechook called $RUNS times, it should be at exactly 1" - fi assert_link_exists "$ROOT/link" assert_file_exists "$ROOT/link/file" assert_file_eq "$ROOT/link/file" "$FUNCNAME 1" + assert_file_lines_eq "$RUNLOG" 1 +} + +############################################## +# Test exechook at startup with correct SHA +############################################## +function e2e::exechook_startup_after_crash() { + # First sync + echo "$FUNCNAME" > "$REPO/file" + git -C "$REPO" commit -qam "$FUNCNAME" + + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --ref="$MAIN_BRANCH" \ + --root="$ROOT" \ + --link="link" + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_eq "$ROOT/link/file" "$FUNCNAME" + + # No changes to repo + + cat /dev/null > "$RUNLOG" + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --ref="$MAIN_BRANCH" \ + --root="$ROOT" \ + --link="link" \ + --exechook-command="/$EXECHOOK_COMMAND" + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_exists "$ROOT/link/exechook" + assert_file_eq "$ROOT/link/file" "$FUNCNAME" + assert_file_eq "$ROOT/link/exechook" "$FUNCNAME" + assert_file_eq "$ROOT/link/exechook-env" "$EXECHOOK_ENVKEY=$EXECHOOK_ENVVAL" + assert_file_lines_eq "$RUNLOG" 1 } ############################################## @@ -1955,23 +2003,16 @@ function e2e::webhook_success() { # check that basic call works wait_for_sync "${MAXWAIT}" sleep 1 # webhooks are async - HITS=$(cat "$HITLOG" | wc -l) - if (( "$HITS" < 1 )); then - fail "webhook 1 called $HITS times" - fi + assert_file_lines_eq "$HITLOG" 1 # Move forward - cat /dev/null > "$HITLOG" echo "$FUNCNAME 2" > "$REPO/file" git -C "$REPO" commit -qam "$FUNCNAME 2" # check that another call works wait_for_sync "${MAXWAIT}" sleep 1 # webhooks are async - HITS=$(cat "$HITLOG" | wc -l) - if (( "$HITS" < 1 )); then - fail "webhook 2 called $HITS times" - fi + assert_file_lines_eq "$HITLOG" 2 } ############################################## @@ -2005,10 +2046,7 @@ function e2e::webhook_fail_retry() { # Check that webhook was called wait_for_sync "${MAXWAIT}" sleep 1 # webhooks are async - HITS=$(cat "$HITLOG" | wc -l) - if (( "$HITS" < 1 )); then - fail "webhook 1 called $HITS times" - fi + assert_file_lines_ge "$HITLOG" 1 # Now return 200, ensure that it gets called docker_kill "$CTR" @@ -2022,10 +2060,7 @@ function e2e::webhook_fail_retry() { echo ') sleep 2 # webhooks are async - HITS=$(cat "$HITLOG" | wc -l) - if (( "$HITS" < 1 )); then - fail "webhook 2 called $HITS times" - fi + assert_file_lines_eq "$HITLOG" 1 } ############################################## @@ -2057,10 +2092,7 @@ function e2e::webhook_success_once() { --link="link" # check that basic call works - HITS=$(cat "$HITLOG" | wc -l) - if [[ "$HITS" != 1 ]]; then - fail "webhook called $HITS times" - fi + assert_file_lines_eq "$HITLOG" 1 } ############################################## @@ -2098,14 +2130,10 @@ function e2e::webhook_fail_retry_once() { fi ) - # Check that webhook was called - HITS=$(cat "$HITLOG" | wc -l) - if [[ "$HITS" != 1 ]]; then - fail "webhook called $HITS times" - fi assert_link_exists "$ROOT/link" assert_file_exists "$ROOT/link/file" assert_file_eq "$ROOT/link/file" "$FUNCNAME 1" + assert_file_lines_eq "$HITLOG" 1 } ############################################## @@ -2140,10 +2168,7 @@ function e2e::webhook_fire_and_forget() { # check that basic call works wait_for_sync "${MAXWAIT}" sleep 1 # webhooks are async - HITS=$(cat "$HITLOG" | wc -l) - if (( "$HITS" < 1 )); then - fail "webhook called $HITS times" - fi + assert_file_lines_eq "$HITLOG" 1 } ############################################## diff --git a/v3-to-v4.md b/v3-to-v4.md index 94924e6..12d0087 100644 --- a/v3-to-v4.md +++ b/v3-to-v4.md @@ -133,6 +133,12 @@ git-sync v3 container images defaulted `--root` to "/tmp/git". In v4, that has moved to "/git". Users who mount a volume and expect to use the default `--root` must mount it on "/git". +## Hooks + +git-sync v3 could "lose" exechook and webhook calls in the face of the app +restarting. In v4, app startup is treated as a sync, even if the correct hash +was already present, which means that hooks are always called. + ## Other changes git-sync v3 would allow invalidly formatted env vars (e.g. a value that was