Merge pull request #122850 from neolit123/1.30-v1beta4-timeouts

kubeadm: use separate context in GetConfigMapWithShortRetry

cmd/kubeadm/app/util/apiclient/idempotency.go

@@ -19,7 +19,6 @@ package apiclient
 import (
 	"context"
 	"encoding/json"
-	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -341,19 +340,16 @@ func GetConfigMapWithShortRetry(client clientset.Interface, namespace, name stri
 	var lastError error
 	err := wait.PollUntilContextTimeout(context.Background(),
 		time.Millisecond*50, time.Millisecond*350,
-		true, func(ctx context.Context) (bool, error) {
+		true, func(_ context.Context) (bool, error) {
 			var err error
-			cm, err = client.CoreV1().ConfigMaps(namespace).Get(ctx, name, metav1.GetOptions{})
+			// Intentionally pass a new context to this API call. This will let the API call run
+			// independently of the parent context timeout, which is quite short and can cause the API
+			// call to return abruptly.
+			cm, err = client.CoreV1().ConfigMaps(namespace).Get(context.Background(), name, metav1.GetOptions{})
 			if err == nil {
 				return true, nil
 			}
-			// If some code is about to go over the context deadline, "x/time/rate/rate.go" would return
-			// and untyped error with the string "would exceed context deadline". If some code already exceeded
-			// the deadline the error would be of type DeadlineExceeded. Ignore such context errors and only store
-			// API and connectivity errors.
-			if !strings.Contains(err.Error(), "would exceed context deadline") && !errors.Is(err, context.DeadlineExceeded) {
-				lastError = err
-			}
+			lastError = err
 			return false, nil
 		})
 	if err == nil {

cmd/kubeadm/app/util/apiclient/idempotency_test.go

@@ -237,54 +237,3 @@ func TestMutateConfigMapWithConflict(t *testing.T) {
 		t.Fatalf("ConfigMap mutation with conflict was invalid, has: %q", cm.Data["key"])
 	}
 }
-
-func TestGetConfigMapWithShortRetry(t *testing.T) {
-	testcases := []struct {
-		name           string
-		reactorFunc    func(core.Action) (bool, runtime.Object, error)
-		errorCheckFunc func(*testing.T, error)
-	}{
-		{
-			name: "context deadline exceeded error is handled",
-			reactorFunc: func(core.Action) (bool, runtime.Object, error) {
-				return true, nil, context.DeadlineExceeded
-			},
-			errorCheckFunc: func(t *testing.T, err error) {
-				if err != nil {
-					t.Errorf("unexpected error: %v", err)
-				}
-			},
-		},
-		{
-			name: "would exceed context deadline error is handled",
-			reactorFunc: func(core.Action) (bool, runtime.Object, error) {
-				return true, nil, errors.New("Wait returned an error: rate: Wait(n=1) would exceed context deadline")
-			},
-			errorCheckFunc: func(t *testing.T, err error) {
-				if err != nil {
-					t.Errorf("unexpected error: %v", err)
-				}
-			},
-		},
-		{
-			name: "API error is not handled",
-			reactorFunc: func(core.Action) (bool, runtime.Object, error) {
-				return true, nil, apierrors.NewNotFound(schema.GroupResource{}, "")
-			},
-			errorCheckFunc: func(t *testing.T, err error) {
-				if !apierrors.IsNotFound(err) {
-					t.Errorf("expected error: IsNotFound, got: %v", err)
-				}
-			},
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
-			client := fake.NewSimpleClientset()
-			client.PrependReactor("get", "configmaps", tc.reactorFunc)
-			_, err := GetConfigMapWithShortRetry(client, "foo", "bar")
-			tc.errorCheckFunc(t, err)
-		})
-	}
-}