From fa0e8696e252b94a9499b6bb94572cf7d8aa57d2 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 31 Oct 2020 00:31:53 -0700 Subject: [PATCH] Change time-related flags to durations Add '--period' to replace '--wait', which is now obsolete. Add '--sync-timeout' to replace '--timeout', which is now obsolete. Both of these new flags take a Go-style time string, rather than a bare number. For example "1s" for 1 second or "1m" for one minute. The old flags have been kept and will take precedence if specified. --- README.md | 8 ++--- cmd/git-sync/main.go | 74 ++++++++++++++++++++++++----------------- cmd/git-sync/webhook.go | 4 +-- test_e2e.sh | 41 ++++++++++++----------- 4 files changed, 72 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index a7a338227..1970d204a 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ docker run -d \ registry/git-sync:tag \ --repo=https://github.com/kubernetes/git-sync \ --branch=master \ - --wait=30 + --period=30s # run an nginx container to serve the content docker run -d \ @@ -74,7 +74,7 @@ docker run -d \ registry/git-sync:tag \ --repo=https://github.com/kubernetes/git-sync \ --branch=master \ - --wait=30 \ + --period=30s \ --webhook-url="http://localhost:9090/-/reload" ``` @@ -89,8 +89,8 @@ docker run -d \ | GIT_SYNC_SUBMODULES | `--submodules` | git submodule behavior: one of 'recursive', 'shallow', or 'off' | recursive | | GIT_SYNC_ROOT | `--root` | the root directory for git-sync operations, under which --dest will be created | "$HOME/git" | | GIT_SYNC_DEST | `--dest` | the name of (a symlink to) a directory in which to check-out files under --root (defaults to the leaf dir of --repo) | "" | -| GIT_SYNC_WAIT | `--wait` | the number of seconds between syncs | 1 (second) | -| GIT_SYNC_TIMEOUT | `--timeout` | the max number of seconds allowed for a complete sync | 120 | +| GIT_SYNC_PERIOD | `--period` | how long to wait between syncs, must be >= 10ms | "10s" | +| GIT_SYNC_SYNC_TIMEOUT | `--sync-timeout` | the total time allowed for one complete sync, must be >= 10ms | "120s" | | GIT_SYNC_ONE_TIME | `--one-time` | exit after the first sync | false | | GIT_SYNC_MAX_SYNC_FAILURES | `--max-sync-failures` | the number of consecutive failures allowed before aborting (the first sync must succeed, -1 will retry forever after the initial sync) | 0 | | GIT_SYNC_PERMISSIONS | `--change-permissions` | the file permissions to apply to the checked-out files (0 will not change permissions at all) | 0 | diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 503834995..84a943e73 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -66,10 +66,10 @@ var flRoot = pflag.String("root", envString("GIT_SYNC_ROOT", envString("HOME", " "the root directory for git-sync operations, under which --dest will be created") var flDest = pflag.String("dest", envString("GIT_SYNC_DEST", ""), "the name of (a symlink to) a directory in which to check-out files under --root (defaults to the leaf dir of --repo)") -var flWait = pflag.Float64("wait", envFloat("GIT_SYNC_WAIT", 1), - "the number of seconds between syncs") -var flSyncTimeout = pflag.Int("timeout", envInt("GIT_SYNC_TIMEOUT", 120), - "the number of seconds allowed for a complete sync") +var flPeriod = pflag.Duration("period", envDuration("GIT_SYNC_PERIOD", 10*time.Second), + "how long to wait between syncs, must be >= 10ms; --wait overrides this") +var flSyncTimeout = pflag.Duration("sync-timeout", envDuration("GIT_SYNC_SYNC_TIMEOUT", 120*time.Second), + "the total time allowed for one complete sync, must be >= 10ms; --timeout overrides this") var flOneTime = pflag.Bool("one-time", envBool("GIT_SYNC_ONE_TIME", false), "exit after the first sync") var flMaxSyncFailures = pflag.Int("max-sync-failures", envInt("GIT_SYNC_MAX_SYNC_FAILURES", 0), @@ -123,6 +123,17 @@ var flHTTPMetrics = pflag.Bool("http-metrics", envBool("GIT_SYNC_HTTP_METRICS", var flHTTPprof = pflag.Bool("http-pprof", envBool("GIT_SYNC_HTTP_PPROF", false), "enable the pprof debug endpoints on git-sync's HTTP endpoint") +// Obsolete flags, kept for compat. +var flWait = pflag.Float64("wait", envFloat("GIT_SYNC_WAIT", 0), + "DEPRECATED: use --period instead") +var flTimeout = pflag.Int("timeout", envInt("GIT_SYNC_TIMEOUT", 0), + "DEPRECATED: use --sync-timeout instead") + +func init() { + pflag.CommandLine.MarkDeprecated("wait", "use --period instead") + pflag.CommandLine.MarkDeprecated("timeout", "use --sync-timeout instead") +} + var log = glogr.New() // Total pull/error, summary on pull duration @@ -150,9 +161,6 @@ const ( metricKeyNoOp = "noop" ) -// initTimeout is a timeout for initialization, like git credentials setup. -const initTimeout = time.Second * 30 - const ( submodulesRecursive = "recursive" submodulesShallow = "shallow" @@ -308,14 +316,20 @@ func main() { os.Exit(1) } - if *flWait < 0 { - fmt.Fprintf(os.Stderr, "ERROR: --wait must be greater than or equal to 0\n") + if *flWait != 0 { + *flPeriod = time.Duration(int(*flWait*1000)) * time.Millisecond + } + if *flPeriod < 10*time.Millisecond { + fmt.Fprintf(os.Stderr, "ERROR: --period must be at least 10ms\n") pflag.Usage() os.Exit(1) } - if *flSyncTimeout < 0 { - fmt.Fprintf(os.Stderr, "ERROR: --timeout must be greater than 0\n") + if *flTimeout != 0 { + *flSyncTimeout = time.Duration(*flTimeout) * time.Second + } + if *flSyncTimeout < 10*time.Millisecond { + fmt.Fprintf(os.Stderr, "ERROR: --sync-timeout must be at least 10ms\n") pflag.Usage() os.Exit(1) } @@ -382,8 +396,8 @@ func main() { } // This context is used only for git credentials initialization. There are no long-running operations like - // `git clone`, so initTimeout set to 30 seconds should be enough. - ctx, cancel := context.WithTimeout(context.Background(), initTimeout) + // `git clone`, so hopefully 30 seconds will be enough. + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) if *flUsername != "" && *flPassword != "" { if err := setupGitAuth(ctx, *flUsername, *flPassword, *flRepo); err != nil { @@ -471,7 +485,7 @@ func main() { failCount := 0 for { start := time.Now() - ctx, cancel := context.WithTimeout(context.Background(), time.Second*time.Duration(*flSyncTimeout)) + ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) if changed, hash, err := syncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, *flRoot, *flDest, *flAskPassURL, *flSubmodules); err != nil { updateSyncMetrics(metricKeyError, start) if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures { @@ -482,9 +496,9 @@ func main() { failCount++ log.Error(err, "unexpected error syncing repo, will retry") - log.V(0).Info("waiting before retrying", "waitTime", waitTime(*flWait)) + log.V(0).Info("waiting before retrying", "waitTime", flPeriod.String()) cancel() - time.Sleep(waitTime(*flWait)) + time.Sleep(*flPeriod) continue } else if changed { if webhook != nil { @@ -510,9 +524,9 @@ func main() { } failCount = 0 - log.V(1).Info("next sync", "wait_time", waitTime(*flWait)) + log.V(1).Info("next sync", "waitTime", flPeriod.String()) cancel() - time.Sleep(waitTime(*flWait)) + time.Sleep(*flPeriod) } } @@ -521,10 +535,6 @@ func updateSyncMetrics(key string, start time.Time) { syncCount.WithLabelValues(key).Inc() } -func waitTime(seconds float64) time.Duration { - return time.Duration(int(seconds*1000)) * time.Millisecond -} - // Do no work, but don't do something that triggers go's runtime into thinking // it is deadlocked. func sleepForever() { @@ -1113,6 +1123,11 @@ OPTIONS users should prefer the environment variable for specifying the password. + --period , $GIT_SYNC_PERIOD + How long to wait between sync attempts. This must be at least + 10ms. This flag obsoletes --wait, but if --wait is specifed, it + will take precedence. (default: 10s) + --repo , $GIT_SYNC_REPO The git repository to sync. @@ -1145,11 +1160,13 @@ OPTIONS An optional command to be executed after 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 - execution is subject to the overall --timeout flag and will extend - the period between syncs attempts. + execution is subject to the overall --sync-timeout flag and will + extend the effective period between sync attempts. - --timeout , $GIT_SYNC_TIMEOUT - The number of seconds allowed for a complete sync. (default: 120) + --sync-timeout , $GIT_SYNC_SYNC_TIMEOUT + The total time allowed for one complete sync. This must be at least + 10ms. This flag obsoletes --timeout, but if --timeout is specified, + it will take precedence. (default: 120s) --username , $GIT_SYNC_USERNAME The username to use for git authentication (see --password). @@ -1161,9 +1178,6 @@ OPTIONS --version Print the version and exit. - --wait , $GIT_SYNC_WAIT - The number of seconds between sync attempts. (default: 1) - --webhook-backoff , $GIT_SYNC_WEBHOOK_BACKOFF The time to wait before retrying a failed --webhook-url). (default: 3s) @@ -1188,7 +1202,7 @@ EXAMPLE USAGE --repo=https://github.com/kubernetes/git-sync \ --branch=master \ --rev=HEAD \ - --wait=10 \ + --period=10s \ --root=/mnt/git AUTHENTICATION diff --git a/cmd/git-sync/webhook.go b/cmd/git-sync/webhook.go index 267996d75..ecc389ede 100644 --- a/cmd/git-sync/webhook.go +++ b/cmd/git-sync/webhook.go @@ -81,7 +81,7 @@ func (w *Webhook) Do(hash string) error { defer cancel() req = req.WithContext(ctx) - log.V(0).Info("sending webhook", "hash", hash, "url", w.URL, "method", w.Method, "timeout", w.Timeout) + log.V(0).Info("sending webhook", "hash", hash, "url", w.URL, "method", w.Method, "timeout", w.Timeout.String()) resp, err := http.DefaultClient.Do(req) if err != nil { return err @@ -113,7 +113,7 @@ func (w *Webhook) run() { } if err := w.Do(hash); err != nil { - log.Error(err, "webhook failed", "url", w.URL, "method", w.Method, "timeout", w.Timeout) + log.Error(err, "webhook failed", "url", w.URL, "method", w.Method, "timeout", w.Timeout.String()) time.Sleep(w.Backoff) } else { lastHash = hash diff --git a/test_e2e.sh b/test_e2e.sh index 26851c7e1..dbb9bef10 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -172,7 +172,7 @@ testcase "default-sync" echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --root="$ROOT" \ --dest="link" \ @@ -205,7 +205,7 @@ testcase "head-sync" echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --branch=master \ --rev=HEAD \ @@ -243,7 +243,7 @@ echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" git -C "$REPO" checkout -q master GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --branch="$BRANCH" \ --root="$ROOT" \ @@ -283,7 +283,7 @@ echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" git -C "$REPO" tag -f "$TAG" >/dev/null GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --rev="$TAG" \ --root="$ROOT" \ @@ -328,7 +328,7 @@ echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" git -C "$REPO" tag -af "$TAG" -m "$TESTCASE 1" >/dev/null GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --rev="$TAG" \ --root="$ROOT" \ @@ -372,7 +372,7 @@ echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" REV=$(git -C "$REPO" rev-list -n1 HEAD) GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --rev="$REV" \ --root="$ROOT" \ @@ -460,7 +460,7 @@ git -C "$REPO" commit -qam "$TESTCASE 1" GIT_SYNC \ --git="$SLOW_GIT" \ --one-time \ - --timeout=1 \ + --sync-timeout=1s \ --repo="file://$REPO" \ --root="$ROOT" \ --dest="link" \ @@ -470,8 +470,8 @@ assert_file_absent "$ROOT"/link/file # run with slow_git but without timing out GIT_SYNC \ --git="$SLOW_GIT" \ - --wait=0.1 \ - --timeout=16 \ + --period=100ms \ + --sync-timeout=16s \ --repo="file://$REPO" \ --root="$ROOT" \ --dest="link" \ @@ -499,7 +499,7 @@ echo "$TESTCASE 1" > "$REPO"/file expected_depth="1" git -C "$REPO" commit -qam "$TESTCASE 1" GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --depth="$expected_depth" \ --root="$ROOT" \ @@ -634,7 +634,7 @@ testcase "sync_hook_command" echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --root="$ROOT" \ --dest="link" \ @@ -667,6 +667,7 @@ freencport echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" GIT_SYNC \ + --period=100ms \ --repo="file://$REPO" \ --root="$ROOT" \ --webhook-url="http://127.0.0.1:$NCPORT" \ @@ -709,6 +710,7 @@ freencport echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" GIT_SYNC \ + --period=100ms \ --repo="file://$REPO" \ --root="$ROOT" \ --webhook-url="http://127.0.0.1:$NCPORT" \ @@ -735,6 +737,7 @@ echo "$TESTCASE 1" > "$REPO"/file git -C "$REPO" commit -qam "$TESTCASE 1" GIT_SYNC \ --git="$SLOW_GIT" \ + --period=100ms \ --repo="file://$REPO" \ --root="$ROOT" \ --http-bind=":$BINDPORT" \ @@ -795,7 +798,7 @@ git -C "$NESTED_SUBMODULE" commit -aqm "init nested-submodule file" git -C "$REPO" submodule add -q file://$SUBMODULE git -C "$REPO" commit -aqm "add submodule" GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --root="$ROOT" \ --dest="link" \ @@ -882,7 +885,7 @@ git -C "$REPO" submodule add -q file://$SUBMODULE git -C "$REPO" config -f "$REPO"/.gitmodules submodule.$SUBMODULE_REPO_NAME.shallow true git -C "$REPO" commit -qam "$TESTCASE 1" GIT_SYNC \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --depth="$expected_depth" \ --root="$ROOT" \ @@ -957,11 +960,11 @@ git -C "$REPO" submodule add -q file://$SUBMODULE git -C "$REPO" commit -aqm "add submodule" GIT_SYNC \ - --submodules=off \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --root="$ROOT" \ --dest="link" \ + --submodules=off \ > "$DIR"/log."$TESTCASE" 2>&1 & sleep 3 assert_file_absent "$ROOT"/link/$SUBMODULE_REPO_NAME/submodule @@ -999,11 +1002,11 @@ git -C "$REPO" submodule add -q file://$SUBMODULE git -C "$REPO" commit -aqm "add submodule" GIT_SYNC \ - --submodules=shallow \ - --wait=0.1 \ + --period=100ms \ --repo="file://$REPO" \ --root="$ROOT" \ --dest="link" \ + --submodules=shallow \ > "$DIR"/log."$TESTCASE" 2>&1 & sleep 3 assert_link_exists "$ROOT"/link @@ -1032,13 +1035,13 @@ IP=$(docker inspect "$CTR" | jq -r .[0].NetworkSettings.IPAddress) git -C "$REPO" commit -qam "$TESTCASE" GIT_SYNC \ --one-time \ - --ssh \ - --ssh-known-hosts=false \ --repo="test@$IP:/src" \ --branch=master \ --rev=HEAD \ --root="$ROOT" \ --dest="link" \ + --ssh \ + --ssh-known-hosts=false \ > "$DIR"/log."$TESTCASE" 2>&1 assert_link_exists "$ROOT"/link assert_file_exists "$ROOT"/link/file