diff --git a/api/internal/git/gitrunner.go b/api/internal/git/gitrunner.go index edf3dfa0c..94bb78c60 100644 --- a/api/internal/git/gitrunner.go +++ b/api/internal/git/gitrunner.go @@ -9,7 +9,7 @@ import ( "github.com/pkg/errors" "sigs.k8s.io/kustomize/api/filesys" - "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/api/internal/utils" ) // Arbitrary, but non-infinite, timeout for running commands. @@ -41,25 +41,18 @@ func newCmdRunner() (*gitRunner, error) { } // run a command with a timeout. -func (r gitRunner) run(args ...string) (err error) { - ch := make(chan bool, 1) - defer close(ch) +func (r gitRunner) run(args ...string) error { //nolint: gosec cmd := exec.Command(r.gitProgram, args...) cmd.Dir = r.dir.String() - timer := time.NewTimer(r.duration) - defer timer.Stop() - go func() { - _, err = cmd.CombinedOutput() - ch <- true - }() - select { - case <-ch: - if err != nil { - return errors.Wrapf(err, "git cmd = '%s'", cmd.String()) - } - return nil - case <-timer.C: - return types.NewErrTimeOut(r.duration, cmd.String()) - } + return utils.TimedCall( + cmd.String(), + r.duration, + func() error { + _, err := cmd.CombinedOutput() + if err != nil { + return errors.Wrapf(err, "git cmd = '%s'", cmd.String()) + } + return err + }) } diff --git a/api/types/errtimeout.go b/api/internal/utils/errtimeout.go similarity index 97% rename from api/types/errtimeout.go rename to api/internal/utils/errtimeout.go index 29fe31834..24b8abe66 100644 --- a/api/types/errtimeout.go +++ b/api/internal/utils/errtimeout.go @@ -1,7 +1,7 @@ // Copyright 2020 The Kubernetes Authors. // SPDX-License-Identifier: Apache-2.0 -package types +package utils import ( "fmt" diff --git a/api/internal/utils/timedcall.go b/api/internal/utils/timedcall.go new file mode 100644 index 000000000..0afadd0c3 --- /dev/null +++ b/api/internal/utils/timedcall.go @@ -0,0 +1,23 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + "time" +) + +// TimedCall runs fn, failing if it doesn't complete in the given duration. +// The description is used in the timeout error message. +func TimedCall(description string, d time.Duration, fn func() error) error { + done := make(chan error) + timer := time.NewTimer(d) + defer timer.Stop() + go func() { done <- fn() }() + select { + case err := <-done: + return err + case <-timer.C: + return NewErrTimeOut(d, description) + } +} diff --git a/api/internal/utils/timedcall_test.go b/api/internal/utils/timedcall_test.go new file mode 100644 index 000000000..192edc48c --- /dev/null +++ b/api/internal/utils/timedcall_test.go @@ -0,0 +1,64 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package utils_test + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + . "sigs.k8s.io/kustomize/api/internal/utils" +) + +const ( + timeToWait = 10 * time.Millisecond + tooSlow = 2 * timeToWait +) + +func errMsg(msg string) string { + return fmt.Sprintf("hit %s timeout running '%s'", timeToWait, msg) +} + +func TestTimedCallFastNoError(t *testing.T) { + err := TimedCall( + "fast no error", timeToWait, + func() error { return nil }) + if !assert.NoError(t, err) { + t.Fatal(err) + } +} + +func TestTimedCallFastWithError(t *testing.T) { + err := TimedCall( + "fast with error", timeToWait, + func() error { return assert.AnError }) + if assert.Error(t, err) { + assert.EqualError(t, err, assert.AnError.Error()) + } else { + t.Fail() + } +} + +func TestTimedCallSlowNoError(t *testing.T) { + err := TimedCall( + "slow no error", timeToWait, + func() error { time.Sleep(tooSlow); return nil }) + if assert.Error(t, err) { + assert.EqualError(t, err, errMsg("slow no error")) + } else { + t.Fail() + } +} + +func TestTimedCallSlowWithError(t *testing.T) { + err := TimedCall( + "slow with error", timeToWait, + func() error { time.Sleep(tooSlow); return assert.AnError }) + if assert.Error(t, err) { + assert.EqualError(t, err, errMsg("slow with error")) + } else { + t.Fail() + } +} diff --git a/api/krusty/remoteload_test.go b/api/krusty/remoteload_test.go index 37caae9a7..0c33fec10 100644 --- a/api/krusty/remoteload_test.go +++ b/api/krusty/remoteload_test.go @@ -8,8 +8,8 @@ import ( "github.com/stretchr/testify/assert" "sigs.k8s.io/kustomize/api/filesys" + "sigs.k8s.io/kustomize/api/internal/utils" "sigs.k8s.io/kustomize/api/krusty" - "sigs.k8s.io/kustomize/api/types" ) func TestRemoteLoad(t *testing.T) { @@ -17,7 +17,7 @@ func TestRemoteLoad(t *testing.T) { b := krusty.MakeKustomizer(fSys, krusty.MakeDefaultOptions()) m, err := b.Run( "github.com/kubernetes-sigs/kustomize/examples/multibases/dev/?ref=v1.0.6") - if types.IsErrTimeout(err) { + if utils.IsErrTimeout(err) { // Don't fail on timeouts. t.SkipNow() } diff --git a/api/loader/getter.go b/api/loader/getter.go index fd45b512d..257ad4208 100644 --- a/api/loader/getter.go +++ b/api/loader/getter.go @@ -13,7 +13,7 @@ import ( "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/internal/git" - "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/api/internal/utils" ) type remoteTargetSpec struct { @@ -93,22 +93,7 @@ func getRemoteTarget(rs *remoteTargetSpec) error { }, Options: opts, } - - ch := make(chan bool, 1) - defer close(ch) - d := 21 * time.Second // arbitrary - timer := time.NewTimer(d) - defer timer.Stop() - go func() { - err = client.Get() - ch <- true - }() - select { - case <-ch: - case <-timer.C: - err = types.NewErrTimeOut(d, "go-getter client.Get") - } - return err + return utils.TimedCall("go-getter client.Get", 21*time.Second, client.Get) } func getNothing(rs *remoteTargetSpec) error {