Use logger.WithName() for web/exechook

This makes logs easier to comprehend, since hooks are async.
This commit is contained in:
Tim Hockin 2022-03-17 09:48:35 -07:00
parent adf37ef5fd
commit 3ca8099d2d
5 changed files with 37 additions and 25 deletions

View File

@ -574,6 +574,7 @@ func main() {
// Startup webhooks goroutine // Startup webhooks goroutine
var webhookRunner *hook.HookRunner var webhookRunner *hook.HookRunner
if *flWebhookURL != "" { if *flWebhookURL != "" {
log := log.WithName("webhook")
webhook := hook.NewWebhook( webhook := hook.NewWebhook(
*flWebhookURL, *flWebhookURL,
*flWebhookMethod, *flWebhookMethod,
@ -594,6 +595,7 @@ func main() {
// Startup exechooks goroutine // Startup exechooks goroutine
var exechookRunner *hook.HookRunner var exechookRunner *hook.HookRunner
if *flExechookCommand != "" { if *flExechookCommand != "" {
log := log.WithName("exechook")
exechook := hook.NewExechook( exechook := hook.NewExechook(
cmd.NewRunner(log), cmd.NewRunner(log),
*flExechookCommand, *flExechookCommand,

View File

@ -23,18 +23,24 @@ import (
"os/exec" "os/exec"
"strings" "strings"
"k8s.io/git-sync/pkg/logging" "github.com/go-logr/logr"
) )
// Runner is an API to run commands and log them in a consistent way. // Runner is an API to run commands and log them in a consistent way.
type Runner struct { type Runner struct {
// Logger log logintf
logger *logging.Logger }
// Just the logr methods we need in this package.
type logintf interface {
Info(msg string, keysAndValues ...interface{})
Error(err error, msg string, keysAndValues ...interface{})
V(level int) logr.Logger
} }
// NewRunner returns a new CommandRunner // NewRunner returns a new CommandRunner
func NewRunner(logger *logging.Logger) *Runner { func NewRunner(log logintf) *Runner {
return &Runner{logger: logger} return &Runner{log: log}
} }
// Run runs given command // Run runs given command
@ -45,7 +51,7 @@ func (r *Runner) Run(ctx context.Context, cwd string, env []string, command stri
// RunWithStdin runs given command with stardart input // RunWithStdin runs given command with stardart input
func (r *Runner) RunWithStdin(ctx context.Context, cwd string, env []string, stdin, command string, args ...string) (string, error) { func (r *Runner) RunWithStdin(ctx context.Context, cwd string, env []string, stdin, command string, args ...string) (string, error) {
cmdStr := cmdForLog(command, args...) cmdStr := cmdForLog(command, args...)
r.logger.V(5).Info("running command", "cwd", cwd, "cmd", cmdStr) r.log.V(5).Info("running command", "cwd", cwd, "cmd", cmdStr)
cmd := exec.CommandContext(ctx, command, args...) cmd := exec.CommandContext(ctx, command, args...)
if cwd != "" { if cwd != "" {
@ -61,15 +67,15 @@ func (r *Runner) RunWithStdin(ctx context.Context, cwd string, env []string, std
cmd.Stdin = bytes.NewBufferString(stdin) cmd.Stdin = bytes.NewBufferString(stdin)
err := cmd.Run() err := cmd.Run()
stdout := outbuf.String() stdout := strings.TrimSpace(outbuf.String())
stderr := errbuf.String() stderr := strings.TrimSpace(errbuf.String())
if ctx.Err() == context.DeadlineExceeded { if ctx.Err() == context.DeadlineExceeded {
return "", fmt.Errorf("Run(%s): %w: { stdout: %q, stderr: %q }", cmdStr, ctx.Err(), stdout, stderr) return "", fmt.Errorf("Run(%s): %w: { stdout: %q, stderr: %q }", cmdStr, ctx.Err(), stdout, stderr)
} }
if err != nil { if err != nil {
return "", fmt.Errorf("Run(%s): %w: { stdout: %q, stderr: %q }", cmdStr, err, stdout, stderr) return "", fmt.Errorf("Run(%s): %w: { stdout: %q, stderr: %q }", cmdStr, err, stdout, stderr)
} }
r.logger.V(6).Info("command result", "stdout", stdout, "stderr", stderr) r.log.V(6).Info("command result", "stdout", stdout, "stderr", stderr)
return stdout, nil return stdout, nil
} }

View File

@ -23,7 +23,6 @@ import (
"time" "time"
"k8s.io/git-sync/pkg/cmd" "k8s.io/git-sync/pkg/cmd"
"k8s.io/git-sync/pkg/logging"
) )
// Exechook structure, implements Hook // Exechook structure, implements Hook
@ -39,18 +38,18 @@ type Exechook struct {
// Timeout for the command // Timeout for the command
timeout time.Duration timeout time.Duration
// Logger // Logger
logger *logging.Logger log logintf
} }
// NewExechook returns a new Exechook // NewExechook returns a new Exechook
func NewExechook(cmdrunner *cmd.Runner, command, gitroot string, args []string, timeout time.Duration, logger *logging.Logger) *Exechook { func NewExechook(cmdrunner *cmd.Runner, command, gitroot string, args []string, timeout time.Duration, log logintf) *Exechook {
return &Exechook{ return &Exechook{
cmdrunner: cmdrunner, cmdrunner: cmdrunner,
command: command, command: command,
gitRoot: gitroot, gitRoot: gitroot,
args: args, args: args,
timeout: timeout, timeout: timeout,
logger: logger, log: log,
} }
} }
@ -66,7 +65,7 @@ func (h *Exechook) Do(ctx context.Context, hash string) error {
worktreePath := filepath.Join(h.gitRoot, hash) worktreePath := filepath.Join(h.gitRoot, hash)
h.logger.V(0).Info("running exechook", "command", h.command, "timeout", h.timeout) h.log.V(0).Info("running exechook", "command", h.command, "timeout", h.timeout)
_, err := h.cmdrunner.Run(ctx, worktreePath, []string{envKV("GITSYNC_HASH", hash)}, h.command, h.args...) _, err := h.cmdrunner.Run(ctx, worktreePath, []string{envKV("GITSYNC_HASH", hash)}, h.command, h.args...)
return err return err
} }

View File

@ -23,8 +23,8 @@ import (
"sync" "sync"
"time" "time"
"github.com/go-logr/logr"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"k8s.io/git-sync/pkg/logging"
) )
var ( var (
@ -88,8 +88,8 @@ func (d *hookData) send(newHash string) {
} }
// NewHookRunner returns a new HookRunner // NewHookRunner returns a new HookRunner
func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log *logging.Logger, oneTime bool) *HookRunner { func NewHookRunner(hook Hook, backoff time.Duration, data *hookData, log logintf, oneTime bool) *HookRunner {
hr := &HookRunner{hook: hook, backoff: backoff, data: data, logger: log} hr := &HookRunner{hook: hook, backoff: backoff, data: data, log: log}
if oneTime { if oneTime {
hr.oneTimeResult = make(chan bool, 1) hr.oneTimeResult = make(chan bool, 1)
} }
@ -105,12 +105,19 @@ type HookRunner struct {
// Holds the data as it crosses from producer to consumer. // Holds the data as it crosses from producer to consumer.
data *hookData data *hookData
// Logger // Logger
logger *logging.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 mode.
// Should be initialised to a buffered channel of size 1. // Should be initialised to a buffered channel of size 1.
oneTimeResult chan bool oneTimeResult chan bool
} }
// Just the logr methods we need in this package.
type logintf interface {
Info(msg string, keysAndValues ...interface{})
Error(err error, msg string, keysAndValues ...interface{})
V(level int) logr.Logger
}
// Send sends hash to hookdata // Send sends hash to hookdata
func (r *HookRunner) Send(hash string) { func (r *HookRunner) Send(hash string) {
r.data.send(hash) r.data.send(hash)
@ -133,7 +140,7 @@ func (r *HookRunner) Run(ctx context.Context) {
} }
if err := r.hook.Do(ctx, hash); err != nil { if err := r.hook.Do(ctx, hash); err != nil {
r.logger.Error(err, "hook failed") r.log.Error(err, "hook failed")
updateHookRunCountMetric(r.hook.Name(), "error") updateHookRunCountMetric(r.hook.Name(), "error")
// don't want to sleep unnecessarily terminating anyways // don't want to sleep unnecessarily terminating anyways
r.sendOneTimeResultAndTerminate(false) r.sendOneTimeResultAndTerminate(false)

View File

@ -21,8 +21,6 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"time" "time"
"k8s.io/git-sync/pkg/logging"
) )
// WebHook structure, implements Hook // WebHook structure, implements Hook
@ -37,17 +35,17 @@ type Webhook struct {
// Timeout for the http/s request // Timeout for the http/s request
timeout time.Duration timeout time.Duration
// Logger // Logger
logger *logging.Logger log logintf
} }
// NewWebhook returns a new WebHook // NewWebhook returns a new WebHook
func NewWebhook(url, method string, success int, timeout time.Duration, logger *logging.Logger) *Webhook { func NewWebhook(url, method string, success int, timeout time.Duration, log logintf) *Webhook {
return &Webhook{ return &Webhook{
url: url, url: url,
method: method, method: method,
success: success, success: success,
timeout: timeout, timeout: timeout,
logger: logger, log: log,
} }
} }
@ -68,7 +66,7 @@ func (w *Webhook) Do(ctx context.Context, hash string) error {
defer cancel() defer cancel()
req = req.WithContext(ctx) req = req.WithContext(ctx)
w.logger.V(0).Info("sending webhook", "hash", hash, "url", w.url, "method", w.method, "timeout", w.timeout) w.log.V(0).Info("sending webhook", "hash", hash, "url", w.url, "method", w.method, "timeout", w.timeout)
resp, err := http.DefaultClient.Do(req) resp, err := http.DefaultClient.Do(req)
if err != nil { if err != nil {
return err return err