Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,22 @@ OPTIONS
The timeout for the --exechook-command. If not specifid, this
defaults to 30 seconds ("30s").

--pre-exechook-backoff <duration>, $GITSYNC_PRE_EXECHOOK_BACKOFF
The time to wait before retrying a failed --pre-exechook-command. If
not specified, this defaults to 3 seconds ("3s").

--pre-exechook-command <string>, $GITSYNC_PRE_EXECHOOK_COMMAND
An optional command to be executed before syncing a new hash of the
remote repository. This command does not take any arguments and
executes with the synced repo as its working directory. The
$GITSYNC_HASH environment variable will be set to the previous git hash that
was synced. This hook will always be invoked as it runs before any sync attempt.

--pre-exechook-timeout <duration>, $GITSYNC_PRE_EXECHOOK_TIMEOUT
The timeout for the --pre-exechook-command. If not specifid, this
defaults to 30 seconds ("30s").


--filter <string>, $GITSYNC_FILTER
Use partial clone with the specified filter. This can reduce
the amount of data transferred when cloning large repositories.
Expand Down
72 changes: 71 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,16 @@ func main() {
envDuration(3*time.Second, "GITSYNC_EXECHOOK_BACKOFF", "GIT_SYNC_EXECHOOK_BACKOFF"),
"the time to wait before retrying a failed exechook")

flPreExechookCommand := pflag.String("pre-exechook-command",
envString("", "GITSYNC_PRE_EXECHOOK_COMMAND", "GIT_SYNC_PRE_EXECHOOK_COMMAND"),
"an optional command to be run before syncs complete (must be idempotent)")
flPreExechookTimeout := pflag.Duration("pre-exechook-timeout",
envDuration(30*time.Second, "GITSYNC_PRE_EXECHOOK_TIMEOUT", "GIT_SYNC_PRE_EXECHOOK_TIMEOUT"),
"the timeout for the pre-exechook")
flPreExechookBackoff := pflag.Duration("pre-exechook-backoff",
envDuration(3*time.Second, "GITSYNC_PRE_EXECHOOK_BACKOFF", "GIT_SYNC_PRE_EXECHOOK_BACKOFF"),
"the time to wait before retrying a failed pre-exechook")

flWebhookURL := pflag.String("webhook-url",
envString("", "GITSYNC_WEBHOOK_URL", "GIT_SYNC_WEBHOOK_URL"),
"a URL for optional webhook notifications when syncs complete (must be idempotent)")
Expand Down Expand Up @@ -541,6 +551,15 @@ func main() {
}
}

if *flPreExechookCommand != "" {
if *flPreExechookTimeout < time.Second {
fatalConfigErrorf(log, true, "invalid flag: --pre-exechook-timeout must be at least 1s")
}
if *flPreExechookBackoff < time.Second {
fatalConfigErrorf(log, true, "invalid flag: --pre-exechook-backoff must be at least 1s")
}
}

if *flWebhookURL != "" {
if *flWebhookStatusSuccess == -1 {
// Back-compat: -1 and 0 mean the same things
Expand Down Expand Up @@ -859,8 +878,10 @@ func main() {
// Startup exechooks goroutine
var exechookRunner *hook.HookRunner
if *flExechookCommand != "" {
log := log.WithName("exechook")
logname := "exechook"
log := log.WithName(logname)
exechook := hook.NewExechook(
logname,
cmd.NewRunner(log),
*flExechookCommand,
func(hash string) string {
Expand All @@ -880,6 +901,32 @@ func main() {
go exechookRunner.Run(context.Background())
}

// Startup pre-exechooks goroutine
var preExechookRunner *hook.HookRunner
if *flPreExechookCommand != "" {
logname := "pre-exechook"
log := log.WithName(logname)
exechook := hook.NewExechook(
logname,
cmd.NewRunner(log),
*flPreExechookCommand,
func(hash string) string {
return git.worktreeFor(hash).Path().String()
},
[]string{},
*flPreExechookTimeout,
log,
)
preExechookRunner = hook.NewHookRunner(
exechook,
*flPreExechookBackoff,
hook.NewHookData(),
log,
*flOneTime,
)
go preExechookRunner.Run(context.Background())
}

// Setup signal notify channel
sigChan := make(chan os.Signal, 1)
if syncSig != 0 {
Expand Down Expand Up @@ -935,6 +982,7 @@ func main() {
syncCount := uint64(0)
initialSyncDone := false
waitTime := *flInitPeriod
prevHash := ""
// getMaxFailures returns the effective max-failure limit for the current
// phase. During the initial sync phase, --init-max-failures (if set)
// overrides --max-failures; otherwise --max-failures applies.
Expand All @@ -948,6 +996,10 @@ func main() {
start := time.Now()
ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout)

if preExechookRunner != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem right? This will send the old hash on every loop, then sync a new hash. Unless I totally misunderstood the request, you want to try to sync, and ONLY IF there is a new hash, call the hook before publishing the symlink.

Did I misunderstand?

Unfortunately, the sync and the publish are inside git.SyncRepo. You can either decompose that (which would need to share a bunch of state) or you could pass function pointers into it like we do for refreshCreds - pass a struct like:

type syncHooks struct {
    refreshCreds func(context.Context) error
    beforePublish func(context.Context, changed bool, hash string) error
    afterPublish func(context.Context, changed bool, hash string) error
}

preExechookRunner.Send(prevHash)
}

if changed, hash, err := git.SyncRepo(ctx, refreshCreds); err != nil {
failCount++
updateSyncMetrics(metricKeyError, start)
Expand Down Expand Up @@ -981,6 +1033,9 @@ func main() {
if exechookRunner != nil {
exechookRunner.Send(hash)
}
if preExechookRunner != nil {
prevHash = hash
}
updateSyncMetrics(metricKeySuccess, start)
} else {
updateSyncMetrics(metricKeyNoOp, start)
Expand Down Expand Up @@ -2528,6 +2583,21 @@ OPTIONS
The timeout for the --exechook-command. If not specifid, this
defaults to 30 seconds ("30s").

--pre-exechook-backoff <duration>, $GITSYNC_PRE_EXECHOOK_BACKOFF

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flags should be in alphabetic-sorted order, sorry

The time to wait before retrying a failed --pre-exechook-command. If
not specified, this defaults to 3 seconds ("3s").

--pre-exechook-command <string>, $GITSYNC_PRE_EXECHOOK_COMMAND
An optional command to be executed before syncing a new hash of the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite right.

I think we should update the docs on --exechook-command to say something like:

"""
An optional command to be executed after syncing a new hash of the remote repository and publishing the symlink (see --link).
"""

Then this can be:

"""
An optional command to be executed after syncing a new hash of the remote repository but before publishing the symlink (see --link).
"""

The rest should the same. What do you think?

remote repository. This command does not take any arguments and
executes with the synced repo as its working directory. The
$GITSYNC_HASH environment variable will be set to the previous git hash that
was synced. This hook will always be invoked as it runs before any sync attempt.

--pre-exechook-timeout <duration>, $GITSYNC_PRE_EXECHOOK_TIMEOUT
The timeout for the --pre-exechook-command. If not specifid, this
defaults to 30 seconds ("30s").

--filter <string>, $GITSYNC_FILTER
Use partial clone with the specified filter. This can reduce
the amount of data transferred when cloning large repositories.
Expand Down
7 changes: 5 additions & 2 deletions pkg/hook/exechook.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (

// Exechook implements Hook in terms of executing a command.
type Exechook struct {
// Name
name string
// Runner
cmdrunner cmd.Runner
// Command to run
Expand All @@ -42,8 +44,9 @@ type Exechook struct {
}

// NewExechook returns a new Exechook.
func NewExechook(cmdrunner cmd.Runner, command string, getWorktree func(string) string, args []string, timeout time.Duration, log logintf) *Exechook {
func NewExechook(name string, cmdrunner cmd.Runner, command string, getWorktree func(string) string, args []string, timeout time.Duration, log logintf) *Exechook {
return &Exechook{
name: name,
cmdrunner: cmdrunner,
command: command,
getWorktree: getWorktree,
Expand All @@ -55,7 +58,7 @@ func NewExechook(cmdrunner cmd.Runner, command string, getWorktree func(string)

// Name describes hook, implements Hook.Name.
func (h *Exechook) Name() string {
return "exechook"
return h.name
}

// Do runs exechook.command, implements Hook.Do.
Expand Down
3 changes: 3 additions & 0 deletions pkg/hook/exechook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func TestNotZeroReturnExechookDo(t *testing.T) {
t.Run("test not zero return code", func(t *testing.T) {
l := logging.New("", "", 0)
ch := NewExechook(
"exechook",
cmd.NewRunner(l),
"false",
func(string) string { return "/tmp" },
Expand All @@ -47,6 +48,7 @@ func TestZeroReturnExechookDo(t *testing.T) {
t.Run("test zero return code", func(t *testing.T) {
l := logging.New("", "", 0)
ch := NewExechook(
"exechook",
cmd.NewRunner(l),
"true",
func(string) string { return "/tmp" },
Expand All @@ -65,6 +67,7 @@ func TestTimeoutExechookDo(t *testing.T) {
t.Run("test timeout", func(t *testing.T) {
l := logging.New("", "", 0)
ch := NewExechook(
"exechook",
cmd.NewRunner(l),
"/bin/sh",
func(string) string { return "/tmp" },
Expand Down