diff --git a/pkg/client/helper.go b/pkg/client/helper.go index cb658838a8..6d42c4d5bd 100644 --- a/pkg/client/helper.go +++ b/pkg/client/helper.go @@ -145,12 +145,14 @@ func New(c *Config) (*Client, error) { // MatchesServerVersion queries the server to compares the build version // (git hash) of the client with the server's build version. It returns an error // if it failed to contact the server or if the versions are not an exact match. -func MatchesServerVersion(c *Config) error { - client, err := New(c) - if err != nil { - return err +func MatchesServerVersion(client *Client, c *Config) error { + var err error + if client == nil { + client, err = New(c) + if err != nil { + return err + } } - clientVersion := version.Get() serverVersion, err := client.ServerVersion() if err != nil { @@ -165,17 +167,20 @@ func MatchesServerVersion(c *Config) error { // NegotiateVersion queries the server's supported api versions to find // a version that both client and server support. -// - If no version is provided, try the client's registered versions in order of +// - If no version is provided, try registered client versions in order of // preference. // - If version is provided, but not default config (explicitly requested via // commandline flag), and is unsupported by the server, print a warning to // stderr and try client's registered versions in order of preference. // - If version is config default, and the server does not support it, // return an error. -func NegotiateVersion(c *Config, version string) (string, error) { - client, err := New(c) - if err != nil { - return "", err +func NegotiateVersion(client *Client, c *Config, version string) (string, error) { + var err error + if client == nil { + client, err = New(c) + if err != nil { + return "", err + } } clientVersions := util.StringSet{} for _, v := range registered.RegisteredVersions { diff --git a/pkg/client/helper_test.go b/pkg/client/helper_test.go index 44a8f70ba9..ab2897c068 100644 --- a/pkg/client/helper_test.go +++ b/pkg/client/helper_test.go @@ -17,12 +17,18 @@ limitations under the License. package client import ( + "bytes" + "encoding/json" + "io" + "io/ioutil" "net/http" "reflect" "strings" "testing" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/testapi" ) const ( @@ -314,3 +320,79 @@ func TestSetKubernetesDefaultsUserAgent(t *testing.T) { t.Errorf("no user agent set: %#v", config) } } + +func objBody(object interface{}) io.ReadCloser { + output, err := json.MarshalIndent(object, "", "") + if err != nil { + panic(err) + } + return ioutil.NopCloser(bytes.NewReader([]byte(output))) +} + +func TestNegotiateVersion(t *testing.T) { + tests := []struct { + name, version, expectedVersion string + serverVersions []string + config *Config + expectErr bool + }{ + { + name: "server supports client default", + version: "v1", + expectedVersion: "v1", + config: &Config{}, + serverVersions: []string{"v1beta3", "v1"}, + expectErr: false, + }, + { + name: "server falls back to client supported", + version: "v1", + expectedVersion: "v1beta3", + config: &Config{}, + serverVersions: []string{"v1beta3"}, + expectErr: false, + }, + { + name: "explicit version supported", + version: "", + expectedVersion: "v1", + config: &Config{Version: "v1"}, + serverVersions: []string{"v1beta3", "v1"}, + expectErr: false, + }, + { + name: "explicit version not supported", + version: "", + expectedVersion: "", + config: &Config{Version: "v1"}, + serverVersions: []string{"v1beta3"}, + expectErr: true, + }, + } + codec := testapi.Codec() + + for _, test := range tests { + fakeClient := &FakeRESTClient{ + Codec: codec, + Resp: &http.Response{ + StatusCode: 200, + Body: objBody(&api.APIVersions{Versions: test.serverVersions}), + }, + Client: HTTPClientFunc(func(req *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: 200, Body: objBody(&api.APIVersions{Versions: test.serverVersions})}, nil + }), + } + c := NewOrDie(test.config) + c.Client = fakeClient.Client + response, err := NegotiateVersion(c, test.config, test.version) + if err == nil && test.expectErr { + t.Errorf("expected error, got nil for [%s].", test.name) + } + if err != nil && !test.expectErr { + t.Errorf("unexpected error for [%s]: %v.", test.name, err) + } + if response != test.expectedVersion { + t.Errorf("expected version %s, got %s.", test.expectedVersion, response) + } + } +} diff --git a/pkg/kubectl/cmd/util/clientcache.go b/pkg/kubectl/cmd/util/clientcache.go index 6d86d5d89f..9ddf8718dc 100644 --- a/pkg/kubectl/cmd/util/clientcache.go +++ b/pkg/kubectl/cmd/util/clientcache.go @@ -36,6 +36,7 @@ type clientCache struct { clients map[string]*client.Client configs map[string]*client.Config defaultConfig *client.Config + defaultClient *client.Client matchVersion bool } @@ -48,7 +49,7 @@ func (c *clientCache) ClientConfigForVersion(version string) (*client.Config, er } c.defaultConfig = config if c.matchVersion { - if err := client.MatchesServerVersion(config); err != nil { + if err := client.MatchesServerVersion(c.defaultClient, config); err != nil { return nil, err } } @@ -58,7 +59,7 @@ func (c *clientCache) ClientConfigForVersion(version string) (*client.Config, er } // TODO: have a better config copy method config := *c.defaultConfig - negotiatedVersion, err := client.NegotiateVersion(&config, version) + negotiatedVersion, err := client.NegotiateVersion(c.defaultClient, &config, version) if err != nil { return nil, err }