[Feat] Pre exechook#990
Conversation
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Mohamed-elg The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @Mohamed-elg! |
k8s-ci-robot
added
cncf-cla: no
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
| The timeout for the --exechook-command. If not specifid, this | ||
| defaults to 30 seconds ("30s"). | ||
|
|
||
| --pre-exechook-backoff <duration>, $GITSYNC_PRE_EXECHOOK_BACKOFF |
There was a problem hiding this comment.
Flags should be in alphabetic-sorted order, sorry
| 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 |
There was a problem hiding this comment.
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?
| start := time.Now() | ||
| ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) | ||
|
|
||
| if preExechookRunner != nil { |
There was a problem hiding this comment.
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
}
3 participants
/kind feature
What this PR does:
Fixes #937
adds flags for
--pre-exechook-command,--pre-exechook-timeoutand--pre-exechook-backoffto run a command similarly to the exechook but before the syncFollows #937 (comment)