Skip to content

Commit f18b8e9

Browse files
kunalmemanekunalmemane
committed
Fix test panics from *http.Transport type assertion with client-go v0.36.0-beta.0
Signed-off-by: Kunal Memane <kmemane@redhat.com>
1 parent 6bbceaf commit f18b8e9

File tree

8 files changed

+35
-45
lines changed

8 files changed

+35
-45
lines changed

‎pkg/cluster/cluster_suite_test.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ var testenv *envtest.Environment
3838
var cfg *rest.Config
3939
var clientset *kubernetes.Clientset
4040

41-
// clientTransport is used to force-close keep-alives in tests that check for leaks.
42-
var clientTransport *http.Transport
41+
// clientRoundTripper is used to force-close keep-alives in tests that check for leaks.
42+
var clientRoundTripper http.RoundTripper
4343

4444
var _ = BeforeSuite(func() {
4545
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
@@ -51,11 +51,7 @@ var _ = BeforeSuite(func() {
5151
Expect(err).NotTo(HaveOccurred())
5252

5353
cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
54-
// NB(directxman12): we can't set Transport *and* use TLS options,
55-
// so we grab the transport right after it gets created so that we can
56-
// type-assert on it (hopefully)?
57-
// hopefully this doesn't break 🤞
58-
clientTransport = rt.(*http.Transport)
54+
clientRoundTripper = rt
5955
return rt
6056
}
6157

‎pkg/cluster/cluster_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ import (
2828
"go.uber.org/goleak"
2929
"k8s.io/apimachinery/pkg/api/meta"
3030
"k8s.io/apimachinery/pkg/runtime"
31+
utilnet "k8s.io/apimachinery/pkg/util/net"
32+
3133
"k8s.io/client-go/rest"
3234
"sigs.k8s.io/controller-runtime/pkg/cache"
3335
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -118,7 +120,7 @@ var _ = Describe("cluster.Cluster", func() {
118120

119121
// force-close keep-alive connections. These'll time anyway (after
120122
// like 30s or so) but force it to speed up the tests.
121-
clientTransport.CloseIdleConnections()
123+
utilnet.CloseIdleConnectionsFor(clientRoundTripper)
122124
Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed())
123125
})
124126

‎pkg/controller/controller_suite_test.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ var testenv *envtest.Environment
4444
var cfg *rest.Config
4545
var clientset *kubernetes.Clientset
4646

47-
// clientTransport is used to force-close keep-alives in tests that check for leaks.
48-
var clientTransport *http.Transport
47+
// clientRoundTripper is used to force-close keep-alives in tests that check for leaks.
48+
var clientRoundTripper http.RoundTripper
4949

5050
var _ = BeforeSuite(func() {
5151
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
@@ -63,11 +63,7 @@ var _ = BeforeSuite(func() {
6363
Expect(err).NotTo(HaveOccurred())
6464

6565
cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
66-
// NB(directxman12): we can't set Transport *and* use TLS options,
67-
// so we grab the transport right after it gets created so that we can
68-
// type-assert on it (hopefully)?
69-
// hopefully this doesn't break 🤞
70-
clientTransport = rt.(*http.Transport)
66+
clientRoundTripper = rt
7167
return rt
7268
}
7369

‎pkg/controller/controller_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
. "github.com/onsi/gomega"
2525
"go.uber.org/goleak"
2626
corev1 "k8s.io/api/core/v1"
27+
utilnet "k8s.io/apimachinery/pkg/util/net"
2728
"k8s.io/client-go/util/workqueue"
2829
"k8s.io/utils/ptr"
2930

@@ -178,7 +179,7 @@ var _ = Describe("controller.Controller", func() {
178179

179180
// force-close keep-alive connections. These'll time anyway (after
180181
// like 30s or so) but force it to speed up the tests.
181-
clientTransport.CloseIdleConnections()
182+
utilnet.CloseIdleConnectionsFor(clientRoundTripper)
182183
Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed())
183184
})
184185

@@ -193,7 +194,7 @@ var _ = Describe("controller.Controller", func() {
193194

194195
// force-close keep-alive connections. These'll time anyway (after
195196
// like 30s or so) but force it to speed up the tests.
196-
clientTransport.CloseIdleConnections()
197+
utilnet.CloseIdleConnectionsFor(clientRoundTripper)
197198
Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed())
198199
})
199200

‎pkg/internal/controller/controller_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"net/http"
2324
"strconv"
2425
"sync"
2526
"sync/atomic"
@@ -35,6 +36,7 @@ import (
3536
corev1 "k8s.io/api/core/v1"
3637
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3738
"k8s.io/apimachinery/pkg/types"
39+
utilnet "k8s.io/apimachinery/pkg/util/net"
3840
"k8s.io/client-go/util/workqueue"
3941
"k8s.io/utils/ptr"
4042
"sigs.k8s.io/controller-runtime/pkg/cache"
@@ -1803,6 +1805,13 @@ var _ = Describe("controller", func() {
18031805
testenv = &envtest.Environment{}
18041806
cfg, err := testenv.Start()
18051807
Expect(err).NotTo(HaveOccurred())
1808+
1809+
var clientRoundTripper http.RoundTripper
1810+
cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
1811+
clientRoundTripper = rt
1812+
return rt
1813+
}
1814+
18061815
m, err := manager.New(cfg, manager.Options{
18071816
LeaderElection: leaderElection,
18081817
LeaderElectionID: "some-leader-election-id",
@@ -1832,6 +1841,9 @@ var _ = Describe("controller", func() {
18321841
By("stopping the manager via context")
18331842
cancel()
18341843

1844+
// Force-close keep-alive connections to prevent goroutine leaks
1845+
// from lingering HTTP/2 connections.
1846+
utilnet.CloseIdleConnectionsFor(clientRoundTripper)
18351847
Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed())
18361848
<-waitChan
18371849
},

‎pkg/manager/manager_suite_test.go‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package manager
1818

1919
import (
20-
"fmt"
2120
"net/http"
2221
"testing"
2322

@@ -40,8 +39,8 @@ var testenv *envtest.Environment
4039
var cfg *rest.Config
4140
var clientset *kubernetes.Clientset
4241

43-
// clientTransport is used to force-close keep-alives in tests that check for leaks.
44-
var clientTransport *http.Transport
42+
// clientRoundTripper is used to force-close keep-alives in tests that check for leaks.
43+
var clientRoundTripper http.RoundTripper
4544

4645
var _ = BeforeSuite(func() {
4746
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
@@ -53,15 +52,7 @@ var _ = BeforeSuite(func() {
5352
Expect(err).NotTo(HaveOccurred())
5453

5554
cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
56-
// NB(directxman12): we can't set Transport *and* use TLS options,
57-
// so we grab the transport right after it gets created so that we can
58-
// type-assert on it (hopefully)?
59-
// hopefully this doesn't break 🤞
60-
transport, isTransport := rt.(*http.Transport)
61-
if !isTransport {
62-
panic(fmt.Sprintf("wasn't able to grab underlying transport from REST client's RoundTripper, can't figure out how to close keep-alives: expected an *http.Transport, got %#v", rt))
63-
}
64-
clientTransport = transport
55+
clientRoundTripper = rt
6556
return rt
6657
}
6758

‎pkg/manager/manager_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"k8s.io/apimachinery/pkg/fields"
4343
"k8s.io/apimachinery/pkg/runtime"
4444
"k8s.io/apimachinery/pkg/runtime/schema"
45+
utilnet "k8s.io/apimachinery/pkg/util/net"
4546
"k8s.io/client-go/rest"
4647
"k8s.io/client-go/tools/leaderelection/resourcelock"
4748
"sigs.k8s.io/controller-runtime/pkg/cache"
@@ -1803,7 +1804,7 @@ var _ = Describe("manger.Manager", func() {
18031804

18041805
// force-close keep-alive connections. These'll time anyway (after
18051806
// like 30s or so) but force it to speed up the tests.
1806-
clientTransport.CloseIdleConnections()
1807+
utilnet.CloseIdleConnectionsFor(clientRoundTripper)
18071808
Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed())
18081809
})
18091810

@@ -1851,7 +1852,7 @@ var _ = Describe("manger.Manager", func() {
18511852

18521853
// force-close keep-alive connections. These'll time anyway (after
18531854
// like 30s or so) but force it to speed up the tests.
1854-
clientTransport.CloseIdleConnections()
1855+
utilnet.CloseIdleConnectionsFor(clientRoundTripper)
18551856
Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed())
18561857
})
18571858

@@ -1905,7 +1906,7 @@ var _ = Describe("manger.Manager", func() {
19051906

19061907
// force-close keep-alive connections. These'll time anyway (after
19071908
// like 30s or so) but force it to speed up the tests.
1908-
clientTransport.CloseIdleConnections()
1909+
utilnet.CloseIdleConnectionsFor(clientRoundTripper)
19091910
Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed())
19101911
})
19111912

@@ -1953,7 +1954,7 @@ var _ = Describe("manger.Manager", func() {
19531954
time.Sleep(300 * time.Millisecond)
19541955

19551956
// force-close keep-alive connections
1956-
clientTransport.CloseIdleConnections()
1957+
utilnet.CloseIdleConnectionsFor(clientRoundTripper)
19571958
Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed())
19581959
})
19591960

‎pkg/metrics/filters/filter_suite_test.go‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package filters
1818

1919
import (
20-
"fmt"
2120
"net/http"
2221
"testing"
2322

@@ -40,8 +39,8 @@ var testenv *envtest.Environment
4039
var cfg *rest.Config
4140
var clientset *kubernetes.Clientset
4241

43-
// clientTransport is used to force-close keep-alives in tests that check for leaks.
44-
var clientTransport *http.Transport
42+
// clientRoundTripper is used to force-close keep-alives in tests that check for leaks.
43+
var clientRoundTripper http.RoundTripper
4544

4645
var _ = BeforeSuite(func() {
4746
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
@@ -53,15 +52,7 @@ var _ = BeforeSuite(func() {
5352
Expect(err).NotTo(HaveOccurred())
5453

5554
cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
56-
// NB(directxman12): we can't set Transport *and* use TLS options,
57-
// so we grab the transport right after it gets created so that we can
58-
// type-assert on it (hopefully)?
59-
// hopefully this doesn't break 🤞
60-
transport, isTransport := rt.(*http.Transport)
61-
if !isTransport {
62-
panic(fmt.Sprintf("wasn't able to grab underlying transport from REST client's RoundTripper, can't figure out how to close keep-alives: expected an *http.Transport, got %#v", rt))
63-
}
64-
clientTransport = transport
55+
clientRoundTripper = rt
6556
return rt
6657
}
6758

0 commit comments

Comments
 (0)