Merge pull request #3400 from monopole/dropClose

Drop channel close in timed call.

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) {
+func (r gitRunner) run(args ...string) error {
-	ch := make(chan bool, 1)
-	defer close(ch)
 	//nolint: gosec
 	cmd := exec.Command(r.gitProgram, args...)
 	cmd.Dir = r.dir.String()
-	timer := time.NewTimer(r.duration)
+	return utils.TimedCall(
-	defer timer.Stop()
+		cmd.String(),
-	go func() {
+		r.duration,
-		_, err = cmd.CombinedOutput()
+		func() error {
-		ch <- true
+			_, err := cmd.CombinedOutput()
-	}()
-	select {
-	case <-ch:
 			if err != nil {
 				return errors.Wrapf(err, "git cmd = '%s'", cmd.String())
 			}
-		return nil
+			return err
-	case <-timer.C:
+		})
-		return types.NewErrTimeOut(r.duration, cmd.String())
-	}
 }

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"

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)
+	}
+}

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()
+	}
+}

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()
 	}

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,
 	}
-
+	return utils.TimedCall("go-getter client.Get", 21*time.Second, client.Get)
-	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
 }
 
 func getNothing(rs *remoteTargetSpec) error {