Small cleanup on one-time x hooks

cmd/git-sync/main.go

@@ -544,10 +544,6 @@ 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,
@@ -560,7 +556,7 @@ func main() {
 			*flWebhookBackoff,
 			hook.NewHookData(),
 			log,
-			webhookChannel,
+			*flOneTime,
 		)
 		go webhookRunner.Run(context.Background())
 	}
@@ -568,10 +564,6 @@ 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,
@@ -585,7 +577,7 @@ func main() {
 			*flExechookBackoff,
 			hook.NewHookData(),
 			log,
-			exechookChannel,
+			*flOneTime,
 		)
 		go exechookRunner.Run(context.Background())
 	}
@@ -639,14 +631,12 @@ func main() {
 				// 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 {
+					if err := exechookRunner.WaitForCompletion(); err != nil {
-						log.Error(err, "exechook completed with error")
 						exitCode = 1
 					}
 				}
 				if webhookRunner != nil {
-					if err = webhookRunner.WaitForCompletion(); err != nil {
+					if err := webhookRunner.WaitForCompletion(); err != nil {
-						log.Error(err, "webhook completed with error")
 						exitCode = 1
 					}
 				}

pkg/hook/hook.go

@@ -84,8 +84,12 @@ func (d *hookData) send(newHash string) {
 }
 
 // NewHookRunner returns a new HookRunner
-func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log *logging.Logger, hasSucceededOnce chan bool) *HookRunner {
+func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log *logging.Logger, oneTime bool) *HookRunner {
-	return &HookRunner{hook: hook, backoff: backoff, data: data, logger: log, hasCompletedOnce: hasSucceededOnce}
+	hr := &HookRunner{hook: hook, backoff: backoff, data: data, logger: log}
+	if oneTime {
+		hr.oneTimeResult = make(chan bool, 1)
+	}
+	return hr
 }
 
 // HookRunner struct
@@ -98,11 +102,9 @@ type HookRunner struct {
 	data *hookData
 	// Logger
 	logger *logging.Logger
-	// hasCompletedOnce is used to send true if and only if first run executed
+	// Used to send a status result when running in one-time mode.
-	// successfully and false otherwise to some receiver.  Should be
+	// Should be initialised to a buffered channel of size 1.
-	// initialised to a buffered channel of size 1.
+	oneTimeResult chan bool
-	// Is only meant for use within sendCompletedOnceMessageIfApplicable.
-	hasCompletedOnce chan bool
 }
 
 // Send sends hash to hookdata
@@ -131,27 +133,27 @@ func (r *HookRunner) Run(ctx context.Context) {
 				r.logger.Error(err, "hook failed")
 				updateHookRunCountMetric(r.hook.Name(), "error")
 				// don't want to sleep unnecessarily terminating anyways
-				r.sendCompletedOnceMessageIfApplicable(false)
+				r.sendOneTimeResultAndTerminate(false)
 				time.Sleep(r.backoff)
 			} else {
 				updateHookRunCountMetric(r.hook.Name(), "success")
 				lastHash = hash
-				r.sendCompletedOnceMessageIfApplicable(true)
+				r.sendOneTimeResultAndTerminate(true)
 				break
 			}
 		}
 	}
 }
 
-// If hasCompletedOnce is nil, does nothing. Otherwise, forwards the caller
+// If oneTimeResult 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.
+// oneTimeResult, closes said chanel, and terminates this goroutine.
-// Using this function to write to hasCompletedOnce ensures it is only ever
+// Using this function to write to oneTimeResult ensures it is only ever
 // written to once.
-func (r *HookRunner) sendCompletedOnceMessageIfApplicable(completedSuccessfully bool) {
+func (r *HookRunner) sendOneTimeResultAndTerminate(completedSuccessfully bool) {
-	if r.hasCompletedOnce != nil {
+	if r.oneTimeResult != nil {
-		r.hasCompletedOnce <- completedSuccessfully
+		r.oneTimeResult <- completedSuccessfully
-		close(r.hasCompletedOnce)
+		close(r.oneTimeResult)
 		runtime.Goexit()
 	}
 }
@@ -159,15 +161,15 @@ func (r *HookRunner) sendCompletedOnceMessageIfApplicable(completedSuccessfully
 // 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
+// Assumes that r.oneTimeResult 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 {
+	if r.oneTimeResult == nil {
 		return fmt.Errorf("HookRunner.WaitForCompletion called on async runner")
 	}
 
 	// Perform wait on HookRunner
-	hookRunnerFinishedSuccessfully := <-r.hasCompletedOnce
+	hookRunnerFinishedSuccessfully := <-r.oneTimeResult
 	if !hookRunnerFinishedSuccessfully {
 		return fmt.Errorf("hook completed with error")
 	}