diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 459e821..d0d1d3b 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -462,8 +462,8 @@ func main() { run: cmdRunner, } - // This context is used only for git credentials initialization. There are no long-running operations like - // `git clone`, so hopefully 30 seconds will be enough. + // This context is used only for git credentials initialization. There are + // no long-running operations like `git clone`, so hopefully 30 seconds will be enough. ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) if *flUsername != "" { @@ -544,6 +544,10 @@ func main() { // Startup webhooks goroutine var webhookRunner *hook.HookRunner if *flWebhookURL != "" { + var webhookChannel chan bool + if *flOneTime { + webhookChannel = make(chan bool, 1) + } webhook := hook.NewWebhook( *flWebhookURL, *flWebhookMethod, @@ -556,6 +560,7 @@ func main() { *flWebhookBackoff, hook.NewHookData(), log, + webhookChannel, ) go webhookRunner.Run(context.Background()) } @@ -563,6 +568,10 @@ func main() { // Startup exechooks goroutine var exechookRunner *hook.HookRunner if *flExechookCommand != "" { + var exechookChannel chan bool + if *flOneTime { + exechookChannel = make(chan bool, 1) + } exechook := hook.NewExechook( cmd.NewRunner(log), *flExechookCommand, @@ -576,6 +585,7 @@ func main() { *flExechookBackoff, hook.NewHookData(), log, + exechookChannel, ) go exechookRunner.Run(context.Background()) } @@ -621,9 +631,27 @@ func main() { } if initialSync { + // Determine if git-sync should terminate for one of several reasons if *flOneTime { + // Wait for hooks to complete at least once, if not nil, before + // checking whether to stop program. + // 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 { + log.Error(err, "exechook completed with error") + exitCode = 1 + } + } + if webhookRunner != nil { + if err = webhookRunner.WaitForCompletion(); err != nil { + log.Error(err, "webhook completed with error") + exitCode = 1 + } + } log.DeleteErrorFile() - os.Exit(0) + os.Exit(exitCode) } if isHash, err := git.RevIsHash(ctx); err != nil { log.Error(err, "can't tell if rev is a git hash, exiting", "rev", git.rev) diff --git a/pkg/hook/hook.go b/pkg/hook/hook.go index d5b9857..13f92e3 100644 --- a/pkg/hook/hook.go +++ b/pkg/hook/hook.go @@ -18,6 +18,8 @@ package hook import ( "context" + "fmt" + "runtime" "sync" "time" @@ -82,8 +84,8 @@ func (d *hookData) send(newHash string) { } // NewHookRunner returns a new HookRunner -func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log *logging.Logger) *HookRunner { - return &HookRunner{hook: hook, backoff: backoff, data: data, logger: log} +func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log *logging.Logger, hasSucceededOnce chan bool) *HookRunner { + return &HookRunner{hook: hook, backoff: backoff, data: data, logger: log, hasCompletedOnce: hasSucceededOnce} } // HookRunner struct @@ -96,6 +98,11 @@ type HookRunner struct { data *hookData // Logger logger *logging.Logger + // hasCompletedOnce is used to send true if and only if first run executed + // successfully and false otherwise to some receiver. Should be + // initialised to a buffered channel of size 1. + // Is only meant for use within sendCompletedOnceMessageIfApplicable. + hasCompletedOnce chan bool } // Send sends hash to hookdata @@ -123,16 +130,51 @@ func (r *HookRunner) Run(ctx context.Context) { if err := r.hook.Do(ctx, hash); err != nil { r.logger.Error(err, "hook failed") updateHookRunCountMetric(r.hook.Name(), "error") + // don't want to sleep unnecessarily terminating anyways + r.sendCompletedOnceMessageIfApplicable(false) time.Sleep(r.backoff) } else { updateHookRunCountMetric(r.hook.Name(), "success") lastHash = hash + r.sendCompletedOnceMessageIfApplicable(true) break } } } } +// If hasCompletedOnce is nil, does nothing. Otherwise, forwards the caller +// provided success status (as a boolean) of HookRunner to receivers of +// hasCompletedOnce, closes said chanel, and terminates this goroutine. +// Using this function to write to hasCompletedOnce ensures it is only ever +// written to once. +func (r *HookRunner) sendCompletedOnceMessageIfApplicable(completedSuccessfully bool) { + if r.hasCompletedOnce != nil { + r.hasCompletedOnce <- completedSuccessfully + close(r.hasCompletedOnce) + runtime.Goexit() + } +} + +// 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.hasCompletedOnce is not nil, but if it is, returns an error +func (r *HookRunner) WaitForCompletion() error { + // Make sure function should be called + if r.hasCompletedOnce == nil { + return fmt.Errorf("HookRunner.WaitForCompletion called on async runner") + } + + // Perform wait on HookRunner + hookRunnerFinishedSuccessfully := <-r.hasCompletedOnce + if !hookRunnerFinishedSuccessfully { + return fmt.Errorf("hook completed with error") + } + + return nil +} + func updateHookRunCountMetric(name, status string) { hookRunCount.WithLabelValues(name, status).Inc() } diff --git a/test_e2e.sh b/test_e2e.sh index c69f2f6..40bb137 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -143,6 +143,8 @@ SLOW_GIT_FETCH=/slow_git_fetch.sh ASKPASS_GIT=/askpass_git.sh EXECHOOK_COMMAND=/test_exechook_command.sh EXECHOOK_COMMAND_FAIL=/test_exechook_command_fail.sh +EXECHOOK_COMMAND_SLEEPY=/test_exechook_command_with_sleep.sh +EXECHOOK_COMMAND_FAIL_SLEEPY=/test_exechook_command_fail_with_sleep.sh RUNLOG="$DIR/runlog.exechook-fail-retry" rm -f $RUNLOG touch $RUNLOG @@ -161,6 +163,8 @@ function GIT_SYNC() { -v "$(pwd)/askpass_git.sh":"$ASKPASS_GIT":ro \ -v "$(pwd)/test_exechook_command.sh":"$EXECHOOK_COMMAND":ro \ -v "$(pwd)/test_exechook_command_fail.sh":"$EXECHOOK_COMMAND_FAIL":ro \ + -v "$(pwd)/test_exechook_command_with_sleep.sh":"$EXECHOOK_COMMAND_SLEEPY":ro \ + -v "$(pwd)/test_exechook_command_fail_with_sleep.sh":"$EXECHOOK_COMMAND_FAIL_SLEEPY":ro \ -v "$RUNLOG":/var/log/runs \ -v "$DOT_SSH/id_test":"/etc/git-secret/ssh":ro \ --env XDG_CONFIG_HOME=$DIR \ @@ -1072,6 +1076,62 @@ function e2e::exechook_fail_retry() { fi } +############################################## +# Test exechook-success with GIT_SYNC_ONE_TIME +############################################## +function e2e::exechook_success_once() { + # First sync + echo "$FUNCNAME 1" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 1" + + GIT_SYNC \ + --period=100ms \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --root="$ROOT" \ + --link="link" \ + --exechook-command="$EXECHOOK_COMMAND_SLEEPY" \ + >> "$1" 2>&1 + + sleep 2 + assert_link_exists "$ROOT"/link + assert_file_exists "$ROOT"/link/file + assert_file_exists "$ROOT"/link/exechook + assert_file_exists "$ROOT"/link/link-exechook + assert_file_eq "$ROOT"/link/file "$FUNCNAME 1" + assert_file_eq "$ROOT"/link/exechook "$FUNCNAME 1" + assert_file_eq "$ROOT"/link/link-exechook "$FUNCNAME 1" +} + +############################################## +# Test exechook-fail with GIT_SYNC_ONE_TIME +############################################## +function e2e::exechook_fail_once() { + cat /dev/null > "$RUNLOG" + # First sync - return a failure to ensure that we try again + echo "$FUNCNAME 1" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 1" + + GIT_SYNC \ + --period=100ms \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --root="$ROOT" \ + --link="link" \ + --exechook-command="$EXECHOOK_COMMAND_FAIL_SLEEPY" \ + --exechook-backoff=1s \ + >> "$1" 2>&1 + + # Check that exechook was called + sleep 2 + RUNS=$(cat "$RUNLOG" | wc -l) + if [[ "$RUNS" < 1 ]]; then + fail "exechook called $RUNS times, it should be at least 1" + fi +} + ############################################## # Test webhook success ############################################## @@ -1168,6 +1228,79 @@ function e2e::webhook_fail_retry() { docker_kill "$CTR" } +############################################## +# Test webhook success with --one-time +############################################## +function e2e::webhook_success_once() { + HITLOG="$DIR/hitlog" + + # First sync + cat /dev/null > "$HITLOG" + CTR=$(docker_run \ + -v "$HITLOG":/var/log/hits \ + e2e/test/test-ncsvr \ + 80 'sleep 3 && echo -e "HTTP/1.1 200 OK\r\n"') + IP=$(docker_ip "$CTR") + echo "$FUNCNAME 1" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 1" + + GIT_SYNC \ + --period=100ms \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --root="$ROOT" \ + --webhook-url="http://$IP" \ + --webhook-success-status=200 \ + --link="link" \ + >> "$1" 2>&1 + + # check that basic call works + sleep 2 + HITS=$(cat "$HITLOG" | wc -l) + if [[ "$HITS" < 1 ]]; then + fail "webhook 1 called $HITS times" + fi + + docker_kill "$CTR" +} + +############################################## +# Test webhook fail with --one-time +############################################## +function e2e::webhook_fail_retry_once() { + HITLOG="$DIR/hitlog" + + # First sync - return a failure to ensure that we try again + cat /dev/null > "$HITLOG" + CTR=$(docker_run \ + -v "$HITLOG":/var/log/hits \ + e2e/test/test-ncsvr \ + 80 'sleep 3 && echo -e "HTTP/1.1 500 Internal Server Error\r\n"') + IP=$(docker_ip "$CTR") + echo "$FUNCNAME 1" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 1" + + GIT_SYNC \ + --period=100ms \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --root="$ROOT" \ + --webhook-url="http://$IP" \ + --webhook-success-status=200 \ + --link="link" \ + >> "$1" 2>&1 + + # Check that webhook was called + sleep 2 + HITS=$(cat "$HITLOG" | wc -l) + if [[ "$HITS" < 1 ]]; then + fail "webhook 1 called $HITS times" + fi + docker_kill "$CTR" +} + ############################################## # Test webhook fire-and-forget ############################################## diff --git a/test_exechook_command_fail_with_sleep.sh b/test_exechook_command_fail_with_sleep.sh new file mode 100755 index 0000000..321f3d5 --- /dev/null +++ b/test_exechook_command_fail_with_sleep.sh @@ -0,0 +1,22 @@ +#!/bin/sh +# +# Copyright 2021 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "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 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Use for e2e test of --exechook-command. +# This option takes no command arguments, so requires a wrapper script. + +sleep 3 +date >> /var/log/runs +exit 1 diff --git a/test_exechook_command_with_sleep.sh b/test_exechook_command_with_sleep.sh new file mode 100755 index 0000000..78d633c --- /dev/null +++ b/test_exechook_command_with_sleep.sh @@ -0,0 +1,22 @@ +#!/bin/sh +# +# Copyright 2020 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "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 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Use for e2e test of --exechook-command. +# This option takes no command arguments, so requires a wrapper script. + +sleep 3 +cat file > exechook +cat ../link/file > link-exechook