From 1e02613a512e5d85056cd77a75188b3cc84352f0 Mon Sep 17 00:00:00 2001 From: Tylen Wells Date: Tue, 1 Jul 2025 16:28:52 -0700 Subject: [PATCH 1/3] implement --hooks-async --- main.go | 9 ++++++- pkg/hook/hook.go | 65 ++++++++++++++++++++++++++++++------------------ test_e2e.sh | 28 ++++++++++++++++++--- 3 files changed, 74 insertions(+), 28 deletions(-) diff --git a/main.go b/main.go index 95dbec7..a1f3cc9 100644 --- a/main.go +++ b/main.go @@ -207,7 +207,8 @@ func main() { flGroupWrite := pflag.Bool("group-write", envBool(false, "GITSYNC_GROUP_WRITE", "GIT_SYNC_GROUP_WRITE"), "ensure that all data (repo, worktrees, etc.) is group writable") - flStaleWorktreeTimeout := pflag.Duration("stale-worktree-timeout", envDuration(0, "GITSYNC_STALE_WORKTREE_TIMEOUT"), + flStaleWorktreeTimeout := pflag.Duration("stale-worktree-timeout", + envDuration(0, "GITSYNC_STALE_WORKTREE_TIMEOUT"), "how long to retain non-current worktrees") flExechookCommand := pflag.String("exechook-command", @@ -236,6 +237,10 @@ func main() { envDuration(3*time.Second, "GITSYNC_WEBHOOK_BACKOFF", "GIT_SYNC_WEBHOOK_BACKOFF"), "the time to wait before retrying a failed webhook") + flHooksAsync := pflag.Bool("hooks-async", + envBool(true, "GITSYNC_HOOKS_ASYNC", "GIT_SYNC_HOOKS_ASYNC"), + "run hooks asynchronously") + flUsername := pflag.String("username", envString("", "GITSYNC_USERNAME", "GIT_SYNC_USERNAME"), "the username to use for git auth") @@ -844,6 +849,7 @@ func main() { hook.NewHookData(), log, *flOneTime, + *flHooksAsync, ) go webhookRunner.Run(context.Background()) } @@ -868,6 +874,7 @@ func main() { hook.NewHookData(), log, *flOneTime, + *flHooksAsync, ) go exechookRunner.Run(context.Background()) } diff --git a/pkg/hook/hook.go b/pkg/hook/hook.go index 5ba5a5c..76cb702 100644 --- a/pkg/hook/hook.go +++ b/pkg/hook/hook.go @@ -89,10 +89,10 @@ func (d *hookData) send(newHash string) { } // NewHookRunner returns a new HookRunner. -func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log logintf, oneTime bool) *HookRunner { - hr := &HookRunner{hook: hook, backoff: backoff, data: data, log: log} - if oneTime { - hr.oneTimeResult = make(chan bool, 1) +func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log logintf, oneTime bool, async bool) *HookRunner { + hr := &HookRunner{hook: hook, backoff: backoff, data: data, log: log, oneTime: oneTime, async: async} + if oneTime || !async { + hr.result = make(chan bool, 1) } return hr } @@ -107,9 +107,13 @@ type HookRunner struct { data *hookData // Logger log logintf - // Used to send a status result when running in one-time mode. + // Used to send a status result when running in one-time or non-async mode. // Should be initialised to a buffered channel of size 1. - oneTimeResult chan bool + result chan bool + // Bool for whether this is a one-time hook or not. + oneTime bool + // Bool for whether this is an async hook or not. + async bool } // Just the logr methods we need in this package. @@ -120,8 +124,17 @@ type logintf interface { } // Send sends hash to hookdata. -func (r *HookRunner) Send(hash string) { +func (r *HookRunner) Send(hash string) error { r.data.send(hash) + if !r.async { + r.log.V(1).Info("waiting for completion", "hash", hash, "name", r.hook.Name()) + err := r.WaitForCompletion() + r.log.V(1).Info("hook completed", "hash", hash, "err", err, "name", r.hook.Name()) + if err != nil { + return err + } + } + return nil } // Run waits for trigger events from the channel, and run hook when triggered. @@ -144,45 +157,49 @@ func (r *HookRunner) Run(ctx context.Context) { r.log.Error(err, "hook failed", "hash", hash, "retry", r.backoff) updateHookRunCountMetric(r.hook.Name(), "error") // don't want to sleep unnecessarily terminating anyways - r.sendOneTimeResultAndTerminate(false) + r.sendResult(false) time.Sleep(r.backoff) } else { updateHookRunCountMetric(r.hook.Name(), "success") lastHash = hash - r.sendOneTimeResultAndTerminate(true) + r.sendResult(true) break } } } } -// If oneTimeResult is nil, does nothing. Otherwise, forwards the caller -// provided success status (as a boolean) of HookRunner to receivers of -// oneTimeResult, closes said chanel, and terminates this goroutine. -// Using this function to write to oneTimeResult ensures it is only ever -// written to once. -func (r *HookRunner) sendOneTimeResultAndTerminate(completedSuccessfully bool) { - if r.oneTimeResult != nil { - r.oneTimeResult <- completedSuccessfully - close(r.oneTimeResult) +func (r *HookRunner) sendResult(completedSuccessfully bool) { + // if onetime is true, we send the result then exit + if r.oneTime { + r.result <- completedSuccessfully + close(r.result) runtime.Goexit() + } else if !r.async { + // if onetime is false, and we've set non-async we send but don't exit. + r.result <- completedSuccessfully } + // if neither oneTime nor !async, we do nothing here. } // WaitForCompletion waits for HookRunner to send completion message to // calling thread and returns either true if HookRunner executed successfully // and some error otherwise. -// Assumes that r.oneTimeResult is not nil, but if it is, returns an error. +// Assumes that either r.oneTime or !r.async, otherwise returns an error. func (r *HookRunner) WaitForCompletion() error { // Make sure function should be called - if r.oneTimeResult == nil { + if r.result == nil { return fmt.Errorf("HookRunner.WaitForCompletion called on async runner") } - // Perform wait on HookRunner - hookRunnerFinishedSuccessfully := <-r.oneTimeResult - if !hookRunnerFinishedSuccessfully { - return fmt.Errorf("hook completed with error") + // If oneTimeResult is not nil, we wait for its result. + if r.result != nil { + hookRunnerFinishedSuccessfully := <-r.result + r.log.V(1).Info("hook completed", "success", hookRunnerFinishedSuccessfully, + "oneTime", r.oneTime, "async", r.async, "name", r.hook.Name()) + if !hookRunnerFinishedSuccessfully { + return fmt.Errorf("hook completed with error") + } } return nil diff --git a/test_e2e.sh b/test_e2e.sh index a65f381..6b5b87f 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -2406,7 +2406,7 @@ function e2e::exechook_fail_retry() { --exechook-command="/$EXECHOOK_COMMAND_FAIL" \ --exechook-backoff=1s \ & - sleep 3 # give it time to retry + sleep 4 # give it time to retry # Check that exechook was called assert_file_lines_ge "$RUNLOG" 2 @@ -2486,6 +2486,29 @@ function e2e::exechook_startup_after_crash() { assert_file_lines_eq "$RUNLOG" 1 } +############################################## +# Test exechook-success with --hooks-async=false +############################################## +function e2e::exechook_success_hooks_non_async() { + cat /dev/null > "$RUNLOG" + + GIT_SYNC \ + --hooks-async=false \ + --period=100ms \ + --repo="file://$REPO" \ + --root="$ROOT" \ + --link="link" \ + --exechook-command="/$EXECHOOK_COMMAND_SLEEPY" \ + & + sleep 5 # give it time to run + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_exists "$ROOT/link/exechook" + assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]}" + assert_file_eq "$ROOT/link/exechook" "${FUNCNAME[0]}" + assert_file_eq "$ROOT/link/exechook-env" "$EXECHOOK_ENVKEY=$EXECHOOK_ENVVAL" +} + ############################################## # Test webhook success ############################################## @@ -3797,8 +3820,7 @@ for t; do fi remove_containers || true run=$((run+1)) - done - if [[ "$test_ret" != 0 ]]; then + done if [[ "$test_ret" != 0 ]]; then final_ret=1 failures+=("$t (log: ${log}.*)") fi From 7d518656e4f797e3be006872e2681ef6a6282857 Mon Sep 17 00:00:00 2001 From: Tylen Wells Date: Tue, 1 Jul 2025 16:34:51 -0700 Subject: [PATCH 2/3] add function to call hooks and implement --hooks-before-symlink --- _test_tools/exechook_command_with_sleep.sh | 2 + main.go | 58 ++++++++++++++++------ test_e2e.sh | 28 ++++++++++- 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/_test_tools/exechook_command_with_sleep.sh b/_test_tools/exechook_command_with_sleep.sh index 3bc1b95..5eccbb2 100755 --- a/_test_tools/exechook_command_with_sleep.sh +++ b/_test_tools/exechook_command_with_sleep.sh @@ -19,4 +19,6 @@ sleep 3 cat file > exechook +cat exechook +if [[ "$(pwd)" != "$(pwd -P)" ]]; then echo "true" > delaycheck; fi echo "ENVKEY=$ENVKEY" > exechook-env diff --git a/main.go b/main.go index a1f3cc9..75f4746 100644 --- a/main.go +++ b/main.go @@ -240,6 +240,9 @@ func main() { flHooksAsync := pflag.Bool("hooks-async", envBool(true, "GITSYNC_HOOKS_ASYNC", "GIT_SYNC_HOOKS_ASYNC"), "run hooks asynchronously") + flHooksBeforeSymlink := pflag.Bool("hooks-before-symlink", + envBool(false, "GITSYNC_HOOKS_BEFORE_SYMLINK", "GIT_SYNC_HOOKS_BEFORE_SYMLINK"), + "run hooks before creating the symlink (defaults to false)") flUsername := pflag.String("username", envString("", "GITSYNC_USERNAME", "GIT_SYNC_USERNAME"), @@ -879,6 +882,25 @@ func main() { go exechookRunner.Run(context.Background()) } + runHooks := func(hash string) error { + var err error + if exechookRunner != nil { + log.V(3).Info("sending exechook") + err = exechookRunner.Send(hash) + if err != nil { + return err + } + } + if webhookRunner != nil { + log.V(3).Info("sending webhook") + err = webhookRunner.Send(hash) + } + if err != nil { + return err + } + return nil + } + // Setup signal notify channel sigChan := make(chan os.Signal, 1) if syncSig != 0 { @@ -919,11 +941,12 @@ func main() { failCount := 0 syncCount := uint64(0) + for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := git.SyncRepo(ctx, refreshCreds); err != nil { + if changed, hash, err := git.SyncRepo(ctx, refreshCreds, runHooks, *flHooksBeforeSymlink); err != nil { failCount++ updateSyncMetrics(metricKeyError, start) if *flMaxFailures >= 0 && failCount >= *flMaxFailures { @@ -944,11 +967,10 @@ func main() { log.V(3).Info("touched touch-file", "path", absTouchFile) } } - if webhookRunner != nil { - webhookRunner.Send(hash) - } - if exechookRunner != nil { - exechookRunner.Send(hash) + // if --hooks-before-symlink is set, these will have already been sent and completed. + // otherwise, we send them now. + if !*flHooksBeforeSymlink { + runHooks(hash) } updateSyncMetrics(metricKeySuccess, start) } else { @@ -968,14 +990,17 @@ func main() { // Assumes that if hook channels are not nil, they will have at // least one value before getting closed exitCode := 0 // is 0 if all hooks succeed, else is 1 - if exechookRunner != nil { - if err := exechookRunner.WaitForCompletion(); err != nil { - exitCode = 1 + // This will not be needed if async == false, because the Send func for the hookRunners will wait + if *flHooksAsync { + if exechookRunner != nil { + if err := exechookRunner.WaitForCompletion(); err != nil { + exitCode = 1 + } } - } - if webhookRunner != nil { - if err := webhookRunner.WaitForCompletion(); err != nil { - exitCode = 1 + if webhookRunner != nil { + if err := webhookRunner.WaitForCompletion(); err != nil { + exitCode = 1 + } } } log.DeleteErrorFile() @@ -1723,7 +1748,7 @@ func (git *repoSync) currentWorktree() (worktree, error) { // SyncRepo syncs the repository to the desired ref, publishes it via the link, // and tries to clean up any detritus. This function returns whether the // current hash has changed and what the new hash is. -func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Context) error) (bool, string, error) { +func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Context) error, runHooks func(hash string) error, flHooksBeforeSymlink bool) (bool, string, error) { git.log.V(3).Info("syncing", "repo", redactURL(git.repo)) if err := refreshCreds(ctx); err != nil { @@ -1778,6 +1803,11 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con // path was different. changed := (currentHash != remoteHash) || (currentWorktree != git.worktreeFor(currentHash)) + // Fire hooks if needed. + if flHooksBeforeSymlink { + runHooks(remoteHash) + } + // We have to do at least one fetch, to ensure that parameters like depth // are set properly. This is cheap when we already have the target hash. if changed || git.syncCount == 0 { diff --git a/test_e2e.sh b/test_e2e.sh index 6b5b87f..9331d40 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -96,7 +96,7 @@ function assert_file_eq() { if [[ $(cat "$1") == "$2" ]]; then return fi - fail "$1 does not contain '$2': $(cat "$1")" + fail "$1 does not equal '$2': $(cat "$1")" } function assert_file_contains() { @@ -2509,6 +2509,32 @@ function e2e::exechook_success_hooks_non_async() { assert_file_eq "$ROOT/link/exechook-env" "$EXECHOOK_ENVKEY=$EXECHOOK_ENVVAL" } +############################################## +# Test exechook-success with --hooks-async=false and --hooks-before-symlink +############################################## +function e2e::exechook_success_hooks_before_symlink_non_async() { + cat /dev/null > "$RUNLOG" + + GIT_SYNC \ + --hooks-async=false \ + --hooks-before-symlink \ + --period=100ms \ + --repo="file://$REPO" \ + --root="$ROOT" \ + --link="link" \ + --exechook-command="/$EXECHOOK_COMMAND_SLEEPY" \ + --exechook-backoff=1s \ + & + sleep 5 # give it time to run + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_exists "$ROOT/link/exechook" + assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]}" + assert_file_eq "$ROOT/link/exechook" "${FUNCNAME[0]}" + assert_file_eq "$ROOT/link/exechook-env" "$EXECHOOK_ENVKEY=$EXECHOOK_ENVVAL" + assert_file_absent "$ROOT/link/delaycheck" +} + ############################################## # Test webhook success ############################################## From fcbb1d1603a58ff1794cbb5829be7b1675eb0103 Mon Sep 17 00:00:00 2001 From: Tylen Wells Date: Tue, 1 Jul 2025 16:51:21 -0700 Subject: [PATCH 3/3] update man --- main.go | 7 +++++++ test_e2e.sh | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index 75f4746..4bd328e 100644 --- a/main.go +++ b/main.go @@ -2574,6 +2574,13 @@ OPTIONS -?, -h, --help Print help text and exit. + --hooks-async, $GITSYNC_HOOKS_ASYNC + Whether to run the --exechook-command asynchronously. + + --hooks-before-symlink, $GITSYNC_HOOKS_BEFORE_SYMLINK + Whether to run the --exechook-command before updating the symlink. Use in combination with --hooks-async set + to false if you need the hook to finish before the symlink is updated. + --http-bind , $GITSYNC_HTTP_BIND The bind address (including port) for git-sync's HTTP endpoint. The '/' URL of this endpoint is suitable for Kubernetes startup and diff --git a/test_e2e.sh b/test_e2e.sh index 9331d40..56cf3af 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -3846,7 +3846,8 @@ for t; do fi remove_containers || true run=$((run+1)) - done if [[ "$test_ret" != 0 ]]; then + done + if [[ "$test_ret" != 0 ]]; then final_ret=1 failures+=("$t (log: ${log}.*)") fi