Return errors directly from negotiation

Otherwise we're hiding problems in connectivity
pull/6/head
Clayton Coleman 2015-10-20 11:45:46 -04:00
parent 034f7ccb1d
commit 5b137a64a8
3 changed files with 38 additions and 19 deletions

View File

@ -64,9 +64,12 @@ func (c *RESTClient) Delete() *unversioned.Request {
}
func (c *RESTClient) Do(req *http.Request) (*http.Response, error) {
if c.Err != nil {
return nil, c.Err
}
c.Req = req
if c.Client != unversioned.HTTPClient(nil) {
return c.Client.Do(req)
}
return c.Resp, c.Err
return c.Resp, nil
}

View File

@ -269,7 +269,9 @@ func NegotiateVersion(client *Client, c *Config, version string, clientRegistere
}
apiVersions, err := client.ServerAPIVersions()
if err != nil {
return "", fmt.Errorf("couldn't read version from server: %v", err)
// This is almost always a connection error, and higher level code should treat this as a generic error,
// not a negotiation specific error.
return "", err
}
serverVersions := sets.String{}
for _, v := range apiVersions.Versions {
@ -283,7 +285,7 @@ func NegotiateVersion(client *Client, c *Config, version string, clientRegistere
// If server does not support warn, but try to negotiate a lower version.
if len(version) != 0 {
if !clientVersions.Has(version) {
return "", fmt.Errorf("Client does not support API version '%s'. Client supported API versions: %v", version, clientVersions)
return "", fmt.Errorf("client does not support API version %q; client supported API versions: %v", version, clientVersions)
}
if serverVersions.Has(version) {
@ -291,7 +293,7 @@ func NegotiateVersion(client *Client, c *Config, version string, clientRegistere
}
// If we are using an explicit config version the server does not support, fail.
if version == c.Version {
return "", fmt.Errorf("Server does not support API version '%s'.", version)
return "", fmt.Errorf("server does not support API version %q", version)
}
}
@ -307,7 +309,7 @@ func NegotiateVersion(client *Client, c *Config, version string, clientRegistere
return clientVersion, nil
}
}
return "", fmt.Errorf("Failed to negotiate an api version. Server supports: %v. Client supports: %v.",
return "", fmt.Errorf("failed to negotiate an api version; server supports: %v, client supports: %v",
serverVersions, clientRegisteredVersions)
}

View File

@ -19,9 +19,11 @@ package unversioned_test
import (
"bytes"
"encoding/json"
"fmt"
"io"
"io/ioutil"
"net/http"
"strings"
"testing"
"k8s.io/kubernetes/pkg/api/testapi"
@ -39,12 +41,14 @@ func objBody(object interface{}) io.ReadCloser {
}
func TestNegotiateVersion(t *testing.T) {
refusedErr := fmt.Errorf("connection refused")
tests := []struct {
name, version, expectedVersion string
serverVersions []string
clientVersions []string
config *unversioned.Config
expectErr bool
expectErr func(err error) bool
sendErr error
}{
{
name: "server supports client default",
@ -53,7 +57,6 @@ func TestNegotiateVersion(t *testing.T) {
serverVersions: []string{"version1", testapi.Default.Version()},
clientVersions: []string{"version1", testapi.Default.Version()},
expectedVersion: "version1",
expectErr: false,
},
{
name: "server falls back to client supported",
@ -62,7 +65,6 @@ func TestNegotiateVersion(t *testing.T) {
serverVersions: []string{"version1"},
clientVersions: []string{"version1", testapi.Default.Version()},
expectedVersion: "version1",
expectErr: false,
},
{
name: "explicit version supported",
@ -71,16 +73,22 @@ func TestNegotiateVersion(t *testing.T) {
serverVersions: []string{"version1", testapi.Default.Version()},
clientVersions: []string{"version1", testapi.Default.Version()},
expectedVersion: testapi.Default.Version(),
expectErr: false,
},
{
name: "explicit version not supported",
version: "",
config: &unversioned.Config{Version: testapi.Default.Version()},
serverVersions: []string{"version1"},
clientVersions: []string{"version1", testapi.Default.Version()},
expectedVersion: "",
expectErr: true,
name: "explicit version not supported",
version: "",
config: &unversioned.Config{Version: testapi.Default.Version()},
serverVersions: []string{"version1"},
clientVersions: []string{"version1", testapi.Default.Version()},
expectErr: func(err error) bool { return strings.Contains(err.Error(), `server does not support API version "v1"`) },
},
{
name: "connection refused error",
config: &unversioned.Config{Version: testapi.Default.Version()},
serverVersions: []string{"version1"},
clientVersions: []string{"version1", testapi.Default.Version()},
sendErr: refusedErr,
expectErr: func(err error) bool { return err == refusedErr },
},
}
codec := testapi.Default.Codec()
@ -93,17 +101,23 @@ func TestNegotiateVersion(t *testing.T) {
Body: objBody(&unversionedapi.APIVersions{Versions: test.serverVersions}),
},
Client: fake.HTTPClientFunc(func(req *http.Request) (*http.Response, error) {
if test.sendErr != nil {
return nil, test.sendErr
}
return &http.Response{StatusCode: 200, Body: objBody(&unversionedapi.APIVersions{Versions: test.serverVersions})}, nil
}),
}
c := unversioned.NewOrDie(test.config)
c.Client = fakeClient.Client
response, err := unversioned.NegotiateVersion(c, test.config, test.version, test.clientVersions)
if err == nil && test.expectErr {
if err == nil && test.expectErr != nil {
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 err != nil {
if test.expectErr == nil || !test.expectErr(err) {
t.Errorf("unexpected error for [%s]: %v.", test.name, err)
}
continue
}
if response != test.expectedVersion {
t.Errorf("expected version %s, got %s.", test.expectedVersion, response)