Skip to content

Commit 69f3e09

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #299 from thockin/time-flags
Change time-related flags to durations
2 parents b0eebbc + fa0e869 commit 69f3e09

File tree

4 files changed

+72
-55
lines changed

4 files changed

+72
-55
lines changed

‎README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ docker run -d \
4848
registry/git-sync:tag \
4949
--repo=https://github.com/kubernetes/git-sync \
5050
--branch=master \
51-
--wait=30
51+
--period=30s
5252
5353
# run an nginx container to serve the content
5454
docker run -d \
@@ -74,7 +74,7 @@ docker run -d \
7474
registry/git-sync:tag \
7575
--repo=https://github.com/kubernetes/git-sync \
7676
--branch=master \
77-
--wait=30 \
77+
--period=30s \
7878
--webhook-url="http://localhost:9090/-/reload"
7979
```
8080

@@ -89,8 +89,8 @@ docker run -d \
8989
| GIT_SYNC_SUBMODULES | `--submodules` | git submodule behavior: one of 'recursive', 'shallow', or 'off' | recursive |
9090
| GIT_SYNC_ROOT | `--root` | the root directory for git-sync operations, under which --dest will be created | "$HOME/git" |
9191
| 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) | "" |
92-
| GIT_SYNC_WAIT | `--wait` | the number of seconds between syncs | 1 (second) |
93-
| GIT_SYNC_TIMEOUT | `--timeout` | the max number of seconds allowed for a complete sync | 120 |
92+
| GIT_SYNC_PERIOD | `--period` | how long to wait between syncs, must be >= 10ms | "10s" |
93+
| GIT_SYNC_SYNC_TIMEOUT | `--sync-timeout` | the total time allowed for one complete sync, must be >= 10ms | "120s" |
9494
| GIT_SYNC_ONE_TIME | `--one-time` | exit after the first sync | false |
9595
| 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 |
9696
| GIT_SYNC_PERMISSIONS | `--change-permissions` | the file permissions to apply to the checked-out files (0 will not change permissions at all) | 0 |

‎cmd/git-sync/main.go

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,10 @@ var flRoot = pflag.String("root", envString("GIT_SYNC_ROOT", envString("HOME", "
6666
"the root directory for git-sync operations, under which --dest will be created")
6767
var flDest = pflag.String("dest", envString("GIT_SYNC_DEST", ""),
6868
"the name of (a symlink to) a directory in which to check-out files under --root (defaults to the leaf dir of --repo)")
69-
var flWait = pflag.Float64("wait", envFloat("GIT_SYNC_WAIT", 1),
70-
"the number of seconds between syncs")
71-
var flSyncTimeout = pflag.Int("timeout", envInt("GIT_SYNC_TIMEOUT", 120),
72-
"the number of seconds allowed for a complete sync")
69+
var flPeriod = pflag.Duration("period", envDuration("GIT_SYNC_PERIOD", 10*time.Second),
70+
"how long to wait between syncs, must be >= 10ms; --wait overrides this")
71+
var flSyncTimeout = pflag.Duration("sync-timeout", envDuration("GIT_SYNC_SYNC_TIMEOUT", 120*time.Second),
72+
"the total time allowed for one complete sync, must be >= 10ms; --timeout overrides this")
7373
var flOneTime = pflag.Bool("one-time", envBool("GIT_SYNC_ONE_TIME", false),
7474
"exit after the first sync")
7575
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",
123123
var flHTTPprof = pflag.Bool("http-pprof", envBool("GIT_SYNC_HTTP_PPROF", false),
124124
"enable the pprof debug endpoints on git-sync's HTTP endpoint")
125125

126+
// Obsolete flags, kept for compat.
127+
var flWait = pflag.Float64("wait", envFloat("GIT_SYNC_WAIT", 0),
128+
"DEPRECATED: use --period instead")
129+
var flTimeout = pflag.Int("timeout", envInt("GIT_SYNC_TIMEOUT", 0),
130+
"DEPRECATED: use --sync-timeout instead")
131+
132+
func init() {
133+
pflag.CommandLine.MarkDeprecated("wait", "use --period instead")
134+
pflag.CommandLine.MarkDeprecated("timeout", "use --sync-timeout instead")
135+
}
136+
126137
var log = glogr.New()
127138

128139
// Total pull/error, summary on pull duration
@@ -150,9 +161,6 @@ const (
150161
metricKeyNoOp = "noop"
151162
)
152163

153-
// initTimeout is a timeout for initialization, like git credentials setup.
154-
const initTimeout = time.Second * 30
155-
156164
const (
157165
submodulesRecursive = "recursive"
158166
submodulesShallow = "shallow"
@@ -308,14 +316,20 @@ func main() {
308316
os.Exit(1)
309317
}
310318

311-
if *flWait < 0 {
312-
fmt.Fprintf(os.Stderr, "ERROR: --wait must be greater than or equal to 0\n")
319+
if *flWait != 0 {
320+
*flPeriod = time.Duration(int(*flWait*1000)) * time.Millisecond
321+
}
322+
if *flPeriod < 10*time.Millisecond {
323+
fmt.Fprintf(os.Stderr, "ERROR: --period must be at least 10ms\n")
313324
pflag.Usage()
314325
os.Exit(1)
315326
}
316327

317-
if *flSyncTimeout < 0 {
318-
fmt.Fprintf(os.Stderr, "ERROR: --timeout must be greater than 0\n")
328+
if *flTimeout != 0 {
329+
*flSyncTimeout = time.Duration(*flTimeout) * time.Second
330+
}
331+
if *flSyncTimeout < 10*time.Millisecond {
332+
fmt.Fprintf(os.Stderr, "ERROR: --sync-timeout must be at least 10ms\n")
319333
pflag.Usage()
320334
os.Exit(1)
321335
}
@@ -382,8 +396,8 @@ func main() {
382396
}
383397

384398
// This context is used only for git credentials initialization. There are no long-running operations like
385-
// `git clone`, so initTimeout set to 30 seconds should be enough.
386-
ctx, cancel := context.WithTimeout(context.Background(), initTimeout)
399+
// `git clone`, so hopefully 30 seconds will be enough.
400+
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
387401

388402
if *flUsername != "" && *flPassword != "" {
389403
if err := setupGitAuth(ctx, *flUsername, *flPassword, *flRepo); err != nil {
@@ -471,7 +485,7 @@ func main() {
471485
failCount := 0
472486
for {
473487
start := time.Now()
474-
ctx, cancel := context.WithTimeout(context.Background(), time.Second*time.Duration(*flSyncTimeout))
488+
ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout)
475489
if changed, hash, err := syncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, *flRoot, *flDest, *flAskPassURL, *flSubmodules); err != nil {
476490
updateSyncMetrics(metricKeyError, start)
477491
if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures {
@@ -482,9 +496,9 @@ func main() {
482496

483497
failCount++
484498
log.Error(err, "unexpected error syncing repo, will retry")
485-
log.V(0).Info("waiting before retrying", "waitTime", waitTime(*flWait))
499+
log.V(0).Info("waiting before retrying", "waitTime", flPeriod.String())
486500
cancel()
487-
time.Sleep(waitTime(*flWait))
501+
time.Sleep(*flPeriod)
488502
continue
489503
} else if changed {
490504
if webhook != nil {
@@ -510,9 +524,9 @@ func main() {
510524
}
511525

512526
failCount = 0
513-
log.V(1).Info("next sync", "wait_time", waitTime(*flWait))
527+
log.V(1).Info("next sync", "waitTime", flPeriod.String())
514528
cancel()
515-
time.Sleep(waitTime(*flWait))
529+
time.Sleep(*flPeriod)
516530
}
517531
}
518532

@@ -521,10 +535,6 @@ func updateSyncMetrics(key string, start time.Time) {
521535
syncCount.WithLabelValues(key).Inc()
522536
}
523537

524-
func waitTime(seconds float64) time.Duration {
525-
return time.Duration(int(seconds*1000)) * time.Millisecond
526-
}
527-
528538
// Do no work, but don't do something that triggers go's runtime into thinking
529539
// it is deadlocked.
530540
func sleepForever() {
@@ -1113,6 +1123,11 @@ OPTIONS
11131123
users should prefer the environment variable for specifying the
11141124
password.
11151125
1126+
--period <duration>, $GIT_SYNC_PERIOD
1127+
How long to wait between sync attempts. This must be at least
1128+
10ms. This flag obsoletes --wait, but if --wait is specifed, it
1129+
will take precedence. (default: 10s)
1130+
11161131
--repo <string>, $GIT_SYNC_REPO
11171132
The git repository to sync.
11181133
@@ -1145,11 +1160,13 @@ OPTIONS
11451160
An optional command to be executed after syncing a new hash of the
11461161
remote repository. This command does not take any arguments and
11471162
executes with the synced repo as its working directory. The
1148-
execution is subject to the overall --timeout flag and will extend
1149-
the period between syncs attempts.
1163+
execution is subject to the overall --sync-timeout flag and will
1164+
extend the effective period between sync attempts.
11501165
1151-
--timeout <int>, $GIT_SYNC_TIMEOUT
1152-
The number of seconds allowed for a complete sync. (default: 120)
1166+
--sync-timeout <duration>, $GIT_SYNC_SYNC_TIMEOUT
1167+
The total time allowed for one complete sync. This must be at least
1168+
10ms. This flag obsoletes --timeout, but if --timeout is specified,
1169+
it will take precedence. (default: 120s)
11531170
11541171
--username <string>, $GIT_SYNC_USERNAME
11551172
The username to use for git authentication (see --password).
@@ -1161,9 +1178,6 @@ OPTIONS
11611178
--version
11621179
Print the version and exit.
11631180
1164-
--wait <float>, $GIT_SYNC_WAIT
1165-
The number of seconds between sync attempts. (default: 1)
1166-
11671181
--webhook-backoff <duration>, $GIT_SYNC_WEBHOOK_BACKOFF
11681182
The time to wait before retrying a failed --webhook-url).
11691183
(default: 3s)
@@ -1188,7 +1202,7 @@ EXAMPLE USAGE
11881202
--repo=https://github.com/kubernetes/git-sync \
11891203
--branch=master \
11901204
--rev=HEAD \
1191-
--wait=10 \
1205+
--period=10s \
11921206
--root=/mnt/git
11931207
11941208
AUTHENTICATION

‎cmd/git-sync/webhook.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func (w *Webhook) Do(hash string) error {
8181
defer cancel()
8282
req = req.WithContext(ctx)
8383

84-
log.V(0).Info("sending webhook", "hash", hash, "url", w.URL, "method", w.Method, "timeout", w.Timeout)
84+
log.V(0).Info("sending webhook", "hash", hash, "url", w.URL, "method", w.Method, "timeout", w.Timeout.String())
8585
resp, err := http.DefaultClient.Do(req)
8686
if err != nil {
8787
return err
@@ -113,7 +113,7 @@ func (w *Webhook) run() {
113113
}
114114

115115
if err := w.Do(hash); err != nil {
116-
log.Error(err, "webhook failed", "url", w.URL, "method", w.Method, "timeout", w.Timeout)
116+
log.Error(err, "webhook failed", "url", w.URL, "method", w.Method, "timeout", w.Timeout.String())
117117
time.Sleep(w.Backoff)
118118
} else {
119119
lastHash = hash

‎test_e2e.sh

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ testcase "default-sync"
172172
echo "$TESTCASE 1" > "$REPO"/file
173173
git -C "$REPO" commit -qam "$TESTCASE 1"
174174
GIT_SYNC \
175-
--wait=0.1 \
175+
--period=100ms \
176176
--repo="file://$REPO" \
177177
--root="$ROOT" \
178178
--dest="link" \
@@ -205,7 +205,7 @@ testcase "head-sync"
205205
echo "$TESTCASE 1" > "$REPO"/file
206206
git -C "$REPO" commit -qam "$TESTCASE 1"
207207
GIT_SYNC \
208-
--wait=0.1 \
208+
--period=100ms \
209209
--repo="file://$REPO" \
210210
--branch=master \
211211
--rev=HEAD \
@@ -243,7 +243,7 @@ echo "$TESTCASE 1" > "$REPO"/file
243243
git -C "$REPO" commit -qam "$TESTCASE 1"
244244
git -C "$REPO" checkout -q master
245245
GIT_SYNC \
246-
--wait=0.1 \
246+
--period=100ms \
247247
--repo="file://$REPO" \
248248
--branch="$BRANCH" \
249249
--root="$ROOT" \
@@ -283,7 +283,7 @@ echo "$TESTCASE 1" > "$REPO"/file
283283
git -C "$REPO" commit -qam "$TESTCASE 1"
284284
git -C "$REPO" tag -f "$TAG" >/dev/null
285285
GIT_SYNC \
286-
--wait=0.1 \
286+
--period=100ms \
287287
--repo="file://$REPO" \
288288
--rev="$TAG" \
289289
--root="$ROOT" \
@@ -328,7 +328,7 @@ echo "$TESTCASE 1" > "$REPO"/file
328328
git -C "$REPO" commit -qam "$TESTCASE 1"
329329
git -C "$REPO" tag -af "$TAG" -m "$TESTCASE 1" >/dev/null
330330
GIT_SYNC \
331-
--wait=0.1 \
331+
--period=100ms \
332332
--repo="file://$REPO" \
333333
--rev="$TAG" \
334334
--root="$ROOT" \
@@ -372,7 +372,7 @@ echo "$TESTCASE 1" > "$REPO"/file
372372
git -C "$REPO" commit -qam "$TESTCASE 1"
373373
REV=$(git -C "$REPO" rev-list -n1 HEAD)
374374
GIT_SYNC \
375-
--wait=0.1 \
375+
--period=100ms \
376376
--repo="file://$REPO" \
377377
--rev="$REV" \
378378
--root="$ROOT" \
@@ -460,7 +460,7 @@ git -C "$REPO" commit -qam "$TESTCASE 1"
460460
GIT_SYNC \
461461
--git="$SLOW_GIT" \
462462
--one-time \
463-
--timeout=1 \
463+
--sync-timeout=1s \
464464
--repo="file://$REPO" \
465465
--root="$ROOT" \
466466
--dest="link" \
@@ -470,8 +470,8 @@ assert_file_absent "$ROOT"/link/file
470470
# run with slow_git but without timing out
471471
GIT_SYNC \
472472
--git="$SLOW_GIT" \
473-
--wait=0.1 \
474-
--timeout=16 \
473+
--period=100ms \
474+
--sync-timeout=16s \
475475
--repo="file://$REPO" \
476476
--root="$ROOT" \
477477
--dest="link" \
@@ -499,7 +499,7 @@ echo "$TESTCASE 1" > "$REPO"/file
499499
expected_depth="1"
500500
git -C "$REPO" commit -qam "$TESTCASE 1"
501501
GIT_SYNC \
502-
--wait=0.1 \
502+
--period=100ms \
503503
--repo="file://$REPO" \
504504
--depth="$expected_depth" \
505505
--root="$ROOT" \
@@ -634,7 +634,7 @@ testcase "sync_hook_command"
634634
echo "$TESTCASE 1" > "$REPO"/file
635635
git -C "$REPO" commit -qam "$TESTCASE 1"
636636
GIT_SYNC \
637-
--wait=0.1 \
637+
--period=100ms \
638638
--repo="file://$REPO" \
639639
--root="$ROOT" \
640640
--dest="link" \
@@ -667,6 +667,7 @@ freencport
667667
echo "$TESTCASE 1" > "$REPO"/file
668668
git -C "$REPO" commit -qam "$TESTCASE 1"
669669
GIT_SYNC \
670+
--period=100ms \
670671
--repo="file://$REPO" \
671672
--root="$ROOT" \
672673
--webhook-url="http://127.0.0.1:$NCPORT" \
@@ -709,6 +710,7 @@ freencport
709710
echo "$TESTCASE 1" > "$REPO"/file
710711
git -C "$REPO" commit -qam "$TESTCASE 1"
711712
GIT_SYNC \
713+
--period=100ms \
712714
--repo="file://$REPO" \
713715
--root="$ROOT" \
714716
--webhook-url="http://127.0.0.1:$NCPORT" \
@@ -735,6 +737,7 @@ echo "$TESTCASE 1" > "$REPO"/file
735737
git -C "$REPO" commit -qam "$TESTCASE 1"
736738
GIT_SYNC \
737739
--git="$SLOW_GIT" \
740+
--period=100ms \
738741
--repo="file://$REPO" \
739742
--root="$ROOT" \
740743
--http-bind=":$BINDPORT" \
@@ -795,7 +798,7 @@ git -C "$NESTED_SUBMODULE" commit -aqm "init nested-submodule file"
795798
git -C "$REPO" submodule add -q file://$SUBMODULE
796799
git -C "$REPO" commit -aqm "add submodule"
797800
GIT_SYNC \
798-
--wait=0.1 \
801+
--period=100ms \
799802
--repo="file://$REPO" \
800803
--root="$ROOT" \
801804
--dest="link" \
@@ -882,7 +885,7 @@ git -C "$REPO" submodule add -q file://$SUBMODULE
882885
git -C "$REPO" config -f "$REPO"/.gitmodules submodule.$SUBMODULE_REPO_NAME.shallow true
883886
git -C "$REPO" commit -qam "$TESTCASE 1"
884887
GIT_SYNC \
885-
--wait=0.1 \
888+
--period=100ms \
886889
--repo="file://$REPO" \
887890
--depth="$expected_depth" \
888891
--root="$ROOT" \
@@ -957,11 +960,11 @@ git -C "$REPO" submodule add -q file://$SUBMODULE
957960
git -C "$REPO" commit -aqm "add submodule"
958961

959962
GIT_SYNC \
960-
--submodules=off \
961-
--wait=0.1 \
963+
--period=100ms \
962964
--repo="file://$REPO" \
963965
--root="$ROOT" \
964966
--dest="link" \
967+
--submodules=off \
965968
> "$DIR"/log."$TESTCASE" 2>&1 &
966969
sleep 3
967970
assert_file_absent "$ROOT"/link/$SUBMODULE_REPO_NAME/submodule
@@ -999,11 +1002,11 @@ git -C "$REPO" submodule add -q file://$SUBMODULE
9991002
git -C "$REPO" commit -aqm "add submodule"
10001003

10011004
GIT_SYNC \
1002-
--submodules=shallow \
1003-
--wait=0.1 \
1005+
--period=100ms \
10041006
--repo="file://$REPO" \
10051007
--root="$ROOT" \
10061008
--dest="link" \
1009+
--submodules=shallow \
10071010
> "$DIR"/log."$TESTCASE" 2>&1 &
10081011
sleep 3
10091012
assert_link_exists "$ROOT"/link
@@ -1032,13 +1035,13 @@ IP=$(docker inspect "$CTR" | jq -r .[0].NetworkSettings.IPAddress)
10321035
git -C "$REPO" commit -qam "$TESTCASE"
10331036
GIT_SYNC \
10341037
--one-time \
1035-
--ssh \
1036-
--ssh-known-hosts=false \
10371038
--repo="test@$IP:/src" \
10381039
--branch=master \
10391040
--rev=HEAD \
10401041
--root="$ROOT" \
10411042
--dest="link" \
1043+
--ssh \
1044+
--ssh-known-hosts=false \
10421045
> "$DIR"/log."$TESTCASE" 2>&1
10431046
assert_link_exists "$ROOT"/link
10441047
assert_file_exists "$ROOT"/link/file

0 commit comments

Comments
 (0)