refactor: imporved efficiency and code style

* changed sendCompletedOnceMessageIfApplicable so that it kills thread
  after executing
* moved logic for waiting on HookRunner into function defined in
  HookRunner itself
This commit is contained in:
ChrisERo 2022-01-14 13:32:01 -05:00
parent 043c356c03
commit 8827bf7489
2 changed files with 37 additions and 27 deletions

View File

@ -174,11 +174,6 @@ var (
}, []string{"status"}) }, []string{"status"})
) )
// Channels for ensuring hooks execute at least once before terminating when
// --one-time flag is set. Should be nil if and only if corresponding hook is
// not defined and if initialised, will only ever get written to once.
var exechookChannel, webhookChannel chan bool
const ( const (
metricKeySuccess = "success" metricKeySuccess = "success"
metricKeyError = "error" metricKeyError = "error"
@ -364,10 +359,6 @@ func main() {
if *flExechookBackoff < time.Second { if *flExechookBackoff < time.Second {
handleError(log, true, "ERROR: --exechook-backoff must be at least 1s") handleError(log, true, "ERROR: --exechook-backoff must be at least 1s")
} }
if *flOneTime {
exechookChannel = make(chan bool, 1)
}
} }
if *flWebhookURL != "" { if *flWebhookURL != "" {
@ -380,10 +371,6 @@ func main() {
if *flWebhookBackoff < time.Second { if *flWebhookBackoff < time.Second {
handleError(log, true, "ERROR: --webhook-backoff must be at least 1s") handleError(log, true, "ERROR: --webhook-backoff must be at least 1s")
} }
if *flOneTime {
webhookChannel = make(chan bool, 1)
}
} }
if *flPassword != "" && *flPasswordFile != "" { if *flPassword != "" && *flPasswordFile != "" {
@ -557,6 +544,10 @@ func main() {
// Startup webhooks goroutine // Startup webhooks goroutine
var webhookRunner *hook.HookRunner var webhookRunner *hook.HookRunner
if *flWebhookURL != "" { if *flWebhookURL != "" {
var webhookChannel chan bool
if *flOneTime {
webhookChannel = make(chan bool, 1)
}
webhook := hook.NewWebhook( webhook := hook.NewWebhook(
*flWebhookURL, *flWebhookURL,
*flWebhookMethod, *flWebhookMethod,
@ -577,6 +568,10 @@ func main() {
// Startup exechooks goroutine // Startup exechooks goroutine
var exechookRunner *hook.HookRunner var exechookRunner *hook.HookRunner
if *flExechookCommand != "" { if *flExechookCommand != "" {
var exechookChannel chan bool
if *flOneTime {
exechookChannel = make(chan bool, 1)
}
exechook := hook.NewExechook( exechook := hook.NewExechook(
cmd.NewRunner(log), cmd.NewRunner(log),
*flExechookCommand, *flExechookCommand,
@ -642,19 +637,13 @@ func main() {
// checking whether to stop program. // checking whether to stop program.
// Assumes that if hook channels are not nil, they will have at // Assumes that if hook channels are not nil, they will have at
// least one value before getting closed // least one value before getting closed
exitCode := 0 // is 0 if all hooks succeed, else is 1 exitCode := 0 // is 0 if all hooks succeed, else is 1
if exechookChannel != nil { if err = exechookRunner.WaitForCompletion(); err != nil {
exechookChannelFinishedSuccessfully := <-exechookChannel log.Error(err, "exechook completed with error")
if !exechookChannelFinishedSuccessfully { exitCode = 1
log.Error(nil, "exechook completed with error")
exitCode = 1
}
} }
if webhookChannel != nil { if err = webhookRunner.WaitForCompletion(); err != nil {
webhookChannelFinishedSuccessfully := <-webhookChannel log.Error(err, "webhook completed with error")
if !webhookChannelFinishedSuccessfully {
log.Error(nil, "webhook completed with error")
}
exitCode = 1 exitCode = 1
} }
log.DeleteErrorFile() log.DeleteErrorFile()

View File

@ -18,6 +18,8 @@ package hook
import ( import (
"context" "context"
"fmt"
"runtime"
"sync" "sync"
"time" "time"
@ -143,17 +145,36 @@ func (r *HookRunner) Run(ctx context.Context) {
// If hasCompletedOnce is nil, does nothing. Otherwise, forwards the caller // If hasCompletedOnce is nil, does nothing. Otherwise, forwards the caller
// provided success status (as a boolean) of HookRunner to receivers of // provided success status (as a boolean) of HookRunner to receivers of
// hasCompletedOnce, closes said chanel, and sets hasCompletedOnce to nil. // hasCompletedOnce, 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 hasCompletedOnce ensures it is only ever
// written to once. // written to once.
func (r *HookRunner) sendCompletedOnceMessageIfApplicable(completedSuccessfully bool) { func (r *HookRunner) sendCompletedOnceMessageIfApplicable(completedSuccessfully bool) {
if r.hasCompletedOnce != nil { if r.hasCompletedOnce != nil {
r.hasCompletedOnce <- completedSuccessfully r.hasCompletedOnce <- completedSuccessfully
close(r.hasCompletedOnce) close(r.hasCompletedOnce)
r.hasCompletedOnce = nil 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
exechookChannelFinishedSuccessfully := <-r.hasCompletedOnce
if !exechookChannelFinishedSuccessfully {
return fmt.Errorf("exechook completed with error")
}
return nil
}
func updateHookRunCountMetric(name, status string) { func updateHookRunCountMetric(name, status string) {
hookRunCount.WithLabelValues(name, status).Inc() hookRunCount.WithLabelValues(name, status).Inc()
} }