Skip to content

Commit d9cf977

Browse files
Merge pull request #60012 from atlassian/dial-with-context
Automatic merge from submit-queue (batch tested with PRs 60012, 63692, 63977, 63960, 64008). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Use Dial with context **What this PR does / why we need it**: `net/http/Transport.Dial` field is deprecated: ```go // DialContext specifies the dial function for creating unencrypted TCP connections. // If DialContext is nil (and the deprecated Dial below is also nil), // then the transport dials using package net. DialContext func(ctx context.Context, network, addr string) (net.Conn, error) // Dial specifies the dial function for creating unencrypted TCP connections. // // Deprecated: Use DialContext instead, which allows the transport // to cancel dials as soon as they are no longer needed. // If both are set, DialContext takes priority. Dial func(network, addr string) (net.Conn, error) ``` This PR switches all `Dial` usages to `DialContext`. Fixes #63455. **Special notes for your reviewer**: Also related: kubernetes/kubernetes#59287 kubernetes/kubernetes#58532 kubernetes/kubernetes#815 kubernetes/community#1166 kubernetes/kubernetes#58677 kubernetes/kubernetes#57932 **Release note**: ```release-note HTTP transport now uses `context.Context` to cancel dial operations. k8s.io/client-go/transport/Config struct has been updated to accept a function with a `context.Context` parameter. This is a breaking change if you use this field in your code. ``` /sig api-machinery /kind enhancement /cc @sttts Kubernetes-commit: ddf551c24b7d88454f8332ce6855e53281440958
2 parents 4bb327e + 4a75b93 commit d9cf977

File tree

8 files changed

+78
-100
lines changed

8 files changed

+78
-100
lines changed

Godeps/Godeps.json

+50-50
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

discovery/discovery_client.go

-4
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,8 @@ const (
4444
defaultRetries = 2
4545
// protobuf mime type
4646
mimePb = "application/[email protected]+protobuf"
47-
)
48-
49-
var (
5047
// defaultTimeout is the maximum amount of time per request when no timeout has been set on a RESTClient.
5148
// Defaults to 32s in order to have a distinguishable length of time, relative to other timeouts that exist.
52-
// It's a variable to be able to change it in tests.
5349
defaultTimeout = 32 * time.Second
5450
)
5551

discovery/discovery_client_test.go

+6-27
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,11 @@ import (
2323
"net/http"
2424
"net/http/httptest"
2525
"reflect"
26-
"strings"
2726
"testing"
28-
"time"
2927

3028
"github.com/gogo/protobuf/proto"
3129
"github.com/googleapis/gnostic/OpenAPIv2"
30+
"github.com/stretchr/testify/assert"
3231

3332
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3433
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -131,31 +130,11 @@ func TestGetServerGroupsWithBrokenServer(t *testing.T) {
131130
}
132131
}
133132
}
134-
func TestGetServerGroupsWithTimeout(t *testing.T) {
135-
done := make(chan bool)
136-
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
137-
// first we need to write headers, otherwise http client will complain about
138-
// exceeding timeout awaiting headers, only after we can block the call
139-
w.Header().Set("Connection", "keep-alive")
140-
if wf, ok := w.(http.Flusher); ok {
141-
wf.Flush()
142-
}
143-
<-done
144-
}))
145-
defer server.Close()
146-
defer close(done)
147-
client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL, Timeout: 2 * time.Second})
148-
_, err := client.ServerGroups()
149-
// the error we're getting here is wrapped in errors.errorString which makes
150-
// it impossible to unwrap and check it's attributes, so instead we're checking
151-
// the textual output which is presenting http.httpError with timeout set to true
152-
if err == nil {
153-
t.Fatal("missing error")
154-
}
155-
if !strings.Contains(err.Error(), "timeout:true") &&
156-
!strings.Contains(err.Error(), "context.deadlineExceededError") {
157-
t.Fatalf("unexpected error: %v", err)
158-
}
133+
134+
func TestTimeoutIsSet(t *testing.T) {
135+
cfg := &restclient.Config{}
136+
setDiscoveryDefaults(cfg)
137+
assert.Equal(t, defaultTimeout, cfg.Timeout)
159138
}
160139

161140
func TestGetServerResourcesWithV1Server(t *testing.T) {

rest/config.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package rest
1818

1919
import (
20+
"context"
2021
"fmt"
2122
"io/ioutil"
2223
"net"
@@ -110,7 +111,7 @@ type Config struct {
110111
Timeout time.Duration
111112

112113
// Dial specifies the dial function for creating unencrypted TCP connections.
113-
Dial func(network, addr string) (net.Conn, error)
114+
Dial func(ctx context.Context, network, address string) (net.Conn, error)
114115

115116
// Version forces a specific version to be used (if registered)
116117
// Do we need this?

0 commit comments

Comments
 (0)