Merge pull request #46166 from thaJeztah/pkg_plugin_cleanup_STEP3

pkg/plugins: override timeouts during tests

pkg/plugins/client.go

@@ -105,6 +105,9 @@ type Client struct {
 // RequestOpts is the set of options that can be passed into a request
 type RequestOpts struct {
 	Timeout time.Duration
+
+	// testTimeOut is used during tests to limit the max timeout in [abort]
+	testTimeOut int
 }
 
 // WithRequestTimeout sets a timeout duration for plugin requests
@@ -195,7 +198,7 @@ func (c *Client) callWithRetry(serviceMethod string, data io.Reader, retry bool,
 			}
 
 			timeOff := backoff(retries)
-			if abort(start, timeOff) {
+			if abort(start, timeOff, opts.testTimeOut) {
 				return nil, err
 			}
 			retries++
@@ -247,8 +250,15 @@ func backoff(retries int) time.Duration {
 	return time.Duration(b) * time.Second
 }
 
-func abort(start time.Time, timeOff time.Duration) bool {
-	return timeOff+time.Since(start) >= time.Duration(defaultTimeOut)*time.Second
+// testNonExistingPlugin is a special plugin-name, which overrides defaultTimeOut in tests.
+const testNonExistingPlugin = "this-plugin-does-not-exist"
+
+func abort(start time.Time, timeOff time.Duration, overrideTimeout int) bool {
+	to := defaultTimeOut
+	if overrideTimeout > 0 {
+		to = overrideTimeout
+	}
+	return timeOff+time.Since(start) >= time.Duration(to)*time.Second
 }
 
 func httpScheme(u *url.URL) string {

pkg/plugins/client_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"os"
 	"strings"
 	"testing"
 	"time"
@@ -132,7 +133,7 @@ func TestAbortRetry(t *testing.T) {
 		tc := tc
 		t.Run(fmt.Sprintf("duration: %v", tc.timeOff), func(t *testing.T) {
 			s := tc.timeOff * time.Second
-			if a := abort(time.Now(), s); a != tc.expAbort {
+			if a := abort(time.Now(), s, 0); a != tc.expAbort {
 				t.Fatalf("Duration %v, expected %v, was %v\n", tc.timeOff, s, a)
 			}
 		})
@@ -168,18 +169,15 @@ func TestNewClientWithTimeout(t *testing.T) {
 	m := Manifest{[]string{"VolumeDriver", "NetworkDriver"}}
 
 	mux.HandleFunc("/Test.Echo", func(w http.ResponseWriter, r *http.Request) {
-		time.Sleep(time.Duration(600) * time.Millisecond)
+		time.Sleep(20 * time.Millisecond)
 		io.Copy(w, r.Body)
 	})
 
-	// setting timeout of 500ms
-	timeout := time.Duration(500) * time.Millisecond
+	timeout := 10 * time.Millisecond
 	c, _ := NewClientWithTimeout(addr, &tlsconfig.Options{InsecureSkipVerify: true}, timeout)
 	var output Manifest
-	err := c.Call("Test.Echo", m, &output)
-	if err == nil {
-		t.Fatal("Expected timeout error")
-	}
+	err := c.CallWithOptions("Test.Echo", m, &output, func(opts *RequestOpts) { opts.testTimeOut = 1 })
+	assert.ErrorType(t, err, os.IsTimeout)
 }
 
 func TestClientStream(t *testing.T) {

pkg/plugins/plugin_test.go

@@ -93,7 +93,7 @@ func TestGet(t *testing.T) {
 
 	// check negative case where plugin vegetable doesn't exist
 	t.Run("not exists", func(t *testing.T) {
-		_, err := Get("vegetable", "potato")
+		_, err := Get(testNonExistingPlugin, "no-such-implementation")
 		assert.Assert(t, errors.Is(err, ErrNotFound))
 	})
 }

pkg/plugins/plugins.go

@@ -204,7 +204,11 @@ func (p *Plugin) implements(kind string) bool {
 func loadWithRetry(name string, retry bool) (*Plugin, error) {
 	registry := NewLocalRegistry()
 	start := time.Now()
-
+	var testTimeOut int
+	if name == testNonExistingPlugin {
+		// override the timeout in tests
+		testTimeOut = 2
+	}
 	var retries int
 	for {
 		pl, err := registry.Plugin(name)
@@ -214,7 +218,7 @@ func loadWithRetry(name string, retry bool) (*Plugin, error) {
 			}
 
 			timeOff := backoff(retries)
-			if abort(start, timeOff) {
+			if abort(start, timeOff, testTimeOut) {
 				return nil, err
 			}
 			retries++