Merge pull request #4766 from jupdec/release-2.17

fixed#4680 wrapping last retryable err with timeout err

Author: k8s-ci-robot

pkg/runtime/retry_utils.go

@@ -1,20 +1,29 @@
 package runtime
 
 import (
-	"k8s.io/apimachinery/pkg/util/wait"
+	"fmt"
 	"time"
+
+	"k8s.io/apimachinery/pkg/util/wait"
 )
 
 // RetryImmediateOnError tries to run fn every interval until it succeeds, a non-retryable error occurs or the timeout is reached.
+// If the timeout is reached while retrying, the returned error wraps both the timeout and the last retryable error.
 func RetryImmediateOnError(interval time.Duration, timeout time.Duration, retryable func(error) bool, fn func() error) error {
-	return wait.PollImmediate(interval, timeout, func() (bool, error) {
+	var lastRetryableErr error
+	err := wait.PollImmediate(interval, timeout, func() (bool, error) {
 		err := fn()
 		if err != nil {
 			if retryable(err) {
+				lastRetryableErr = err
 				return false, nil
 			}
 			return false, err
 		}
 		return true, nil
 	})
+	if wait.Interrupted(err) && lastRetryableErr != nil {
+		return fmt.Errorf("%w: %v", err, lastRetryableErr)
+	}
+	return err
 }

pkg/runtime/retry_utils_test.go

@@ -2,9 +2,10 @@ package runtime
 
 import (
 	"errors"
-	"github.com/stretchr/testify/assert"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func Test_RetryImmediateOnError(t *testing.T) {
@@ -38,54 +39,63 @@ func Test_RetryImmediateOnError(t *testing.T) {
 		fn        func() error
 	}
 	tests := []struct {
-		name      string
-		args      args
-		wantCount int
-		wantErr   error
+		name         string
+		args         args
+		wantMinCount int
+		wantMaxCount int
+		wantErr      error
 	}{
 		{
 			name: "retry 4 times before failure",
 			args: args{
-				interval:  10 * time.Millisecond,
-				timeout:   100 * time.Millisecond,
+				interval:  50 * time.Millisecond,
+				timeout:   500 * time.Millisecond,
 				retryable: retryable,
 				fn:        failureAfterRetryCountFnGen(4),
 			},
-			wantCount: 5,
-			wantErr:   errors.New("failure"),
+			wantMinCount: 5,
+			wantMaxCount: 5,
+			wantErr:      errors.New("failure"),
 		},
 		{
 			name: "retry 4 times before failure - but timeout after 2nd retry",
 			args: args{
-				interval:  10 * time.Millisecond,
-				timeout:   19 * time.Millisecond,
+				interval:  50 * time.Millisecond,
+				timeout:   120 * time.Millisecond,
 				retryable: retryable,
 				fn:        failureAfterRetryCountFnGen(4),
 			},
-			wantCount: 3,
-			wantErr:   errors.New("timed out waiting for the condition"),
+			// PollImmediate calls fn at t=0, t=50ms, t=100ms. Timeout at 120ms
+			// may race with the next tick, so count can be 3 or 4.
+			wantMinCount: 3,
+			wantMaxCount: 4,
+			wantErr:      errors.New("timed out waiting for the condition: retryable"),
 		},
 		{
 			name: "retry 4 times before success",
 			args: args{
-				interval:  10 * time.Millisecond,
-				timeout:   100 * time.Millisecond,
+				interval:  50 * time.Millisecond,
+				timeout:   500 * time.Millisecond,
 				retryable: retryable,
 				fn:        successAfterRetryCountFnGen(4),
 			},
-			wantCount: 5,
-			wantErr:   nil,
+			wantMinCount: 5,
+			wantMaxCount: 5,
+			wantErr:      nil,
 		},
 		{
 			name: "retry 4 times before success - but timeout after 2nd retry",
 			args: args{
-				interval:  10 * time.Millisecond,
-				timeout:   19 * time.Millisecond,
+				interval:  50 * time.Millisecond,
+				timeout:   120 * time.Millisecond,
 				retryable: retryable,
 				fn:        successAfterRetryCountFnGen(4),
 			},
-			wantCount: 3,
-			wantErr:   errors.New("timed out waiting for the condition"),
+			// PollImmediate calls fn at t=0, t=50ms, t=100ms. Timeout at 120ms
+			// may race with the next tick, so count can be 3 or 4.
+			wantMinCount: 3,
+			wantMaxCount: 4,
+			wantErr:      errors.New("timed out waiting for the condition: retryable"),
 		},
 	}
 	for _, tt := range tests {
@@ -100,7 +110,8 @@ func Test_RetryImmediateOnError(t *testing.T) {
 			} else {
 				assert.NoError(t, err)
 			}
-			assert.Equal(t, tt.wantCount, count)
+			assert.GreaterOrEqual(t, count, tt.wantMinCount, "retry count below minimum")
+			assert.LessOrEqual(t, count, tt.wantMaxCount, "retry count above maximum")
 		})
 	}
 }