Merge pull request #1485 from SebTardif/fix-roundtripper-stacking

Move retryingRoundTripper wrapping to constructor

Author: matheuscscp

internal/kube/client.go

@@ -112,6 +112,11 @@ func NewMemoryRESTClientGetter(cfg *rest.Config, opts ...Option) *MemoryRESTClie
 		opts(g)
 	}
 	g.setDefaults()
+	// Add retries to fix temporary "etcdserver: leader changed" errors from kube-apiserver.
+	// Done here once rather than in ToRESTConfig to avoid stacking wrappers on repeated calls.
+	g.cfg.Wrap(func(rt http.RoundTripper) http.RoundTripper {
+		return &retryingRoundTripper{wrapped: rt}
+	})
 	return g
 }
 
@@ -131,10 +136,6 @@ func (c *MemoryRESTClientGetter) ToRESTConfig() (*rest.Config, error) {
 	if c.cfg == nil {
 		return nil, fmt.Errorf("MemoryRESTClientGetter has no REST config")
 	}
-	// add retries to fix temporary "etcdserver: leader changed" errors from kube-apiserver
-	c.cfg.Wrap(func(rt http.RoundTripper) http.RoundTripper {
-		return &retryingRoundTripper{wrapped: rt}
-	})
 	return c.cfg, nil
 }
 

internal/kube/client_test.go

@@ -184,6 +184,30 @@ func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) {
 		g.Expect(cfg).To(BeIdenticalTo(c.cfg))
 	})
 
+	t.Run("wraps transport only once across multiple calls", func(t *testing.T) {
+		g := NewWithT(t)
+
+		c := NewMemoryRESTClientGetter(&rest.Config{
+			Host: "https://example.com",
+		})
+
+		// Call ToRESTConfig multiple times.
+		for i := 0; i < 10; i++ {
+			_, err := c.ToRESTConfig()
+			g.Expect(err).ToNot(HaveOccurred())
+		}
+
+		// Verify that only one retryingRoundTripper layer exists by
+		// walking the WrapTransport chain.
+		rt := c.cfg.WrapTransport(nil)
+		_, ok := rt.(*retryingRoundTripper)
+		g.Expect(ok).To(BeTrue(), "expected retryingRoundTripper")
+
+		inner := rt.(*retryingRoundTripper).wrapped
+		_, nested := inner.(*retryingRoundTripper)
+		g.Expect(nested).To(BeFalse(), "transport should not be wrapped more than once")
+	})
+
 	t.Run("error on nil REST config", func(t *testing.T) {
 		g := NewWithT(t)