Skip to content

Commit

Permalink
Merge pull request #60012 from atlassian/dial-with-context
Browse files Browse the repository at this point in the history
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
  • Loading branch information
k8s-publishing-bot committed May 19, 2018
2 parents 4bb327e + 4a75b93 commit d9cf977
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 100 deletions.
100 changes: 50 additions & 50 deletions Godeps/Godeps.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions discovery/discovery_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,8 @@ const (
defaultRetries = 2
// protobuf mime type
mimePb = "application/com.github.proto-openapi.spec.v2@v1.0+protobuf"
)

var (
// defaultTimeout is the maximum amount of time per request when no timeout has been set on a RESTClient.
// Defaults to 32s in order to have a distinguishable length of time, relative to other timeouts that exist.
// It's a variable to be able to change it in tests.
defaultTimeout = 32 * time.Second
)

Expand Down
33 changes: 6 additions & 27 deletions discovery/discovery_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,11 @@ import (
"net/http"
"net/http/httptest"
"reflect"
"strings"
"testing"
"time"

"github.com/gogo/protobuf/proto"
"github.com/googleapis/gnostic/OpenAPIv2"
"github.com/stretchr/testify/assert"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -131,31 +130,11 @@ func TestGetServerGroupsWithBrokenServer(t *testing.T) {
}
}
}
func TestGetServerGroupsWithTimeout(t *testing.T) {
done := make(chan bool)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
// first we need to write headers, otherwise http client will complain about
// exceeding timeout awaiting headers, only after we can block the call
w.Header().Set("Connection", "keep-alive")
if wf, ok := w.(http.Flusher); ok {
wf.Flush()
}
<-done
}))
defer server.Close()
defer close(done)
client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL, Timeout: 2 * time.Second})
_, err := client.ServerGroups()
// the error we're getting here is wrapped in errors.errorString which makes
// it impossible to unwrap and check it's attributes, so instead we're checking
// the textual output which is presenting http.httpError with timeout set to true
if err == nil {
t.Fatal("missing error")
}
if !strings.Contains(err.Error(), "timeout:true") &&
!strings.Contains(err.Error(), "context.deadlineExceededError") {
t.Fatalf("unexpected error: %v", err)
}

func TestTimeoutIsSet(t *testing.T) {
cfg := &restclient.Config{}
setDiscoveryDefaults(cfg)
assert.Equal(t, defaultTimeout, cfg.Timeout)
}

func TestGetServerResourcesWithV1Server(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion rest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package rest

import (
"context"
"fmt"
"io/ioutil"
"net"
Expand Down Expand Up @@ -110,7 +111,7 @@ type Config struct {
Timeout time.Duration

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

// Version forces a specific version to be used (if registered)
// Do we need this?
Expand Down
Loading

0 comments on commit d9cf977

Please sign in to comment.