Merge pull request #3066 from derekwaynecarr/client_update

Do not use namespace in url paths pre v1beta3 from client
pull/6/head
Daniel Smith 2014-12-19 18:02:55 -08:00
commit 36cfc02c6c
10 changed files with 109 additions and 42 deletions

View File

@ -130,6 +130,7 @@ func (c *testClient) ValidateCommon(t *testing.T, err error) {
requestBody := body(c.Request.Body, c.Request.RawBody)
actualQuery := c.handler.RequestReceived.URL.Query()
t.Logf("got query: %v", actualQuery)
t.Logf("path: %v", c.Request.Path)
// We check the query manually, so blank it out so that FakeHandler.ValidateRequest
// won't check it.
c.handler.RequestReceived.URL.RawQuery = ""
@ -161,18 +162,38 @@ func (c *testClient) ValidateCommon(t *testing.T, err error) {
}
}
// convenience function to build paths
// buildResourcePath is a convenience function for knowing if a namespace should in a path param or not
func buildResourcePath(namespace, resource string) string {
if len(namespace) > 0 {
return path.Join("ns", namespace, resource)
if NamespaceInPathFor(testapi.Version()) {
return path.Join("ns", namespace, resource)
}
}
return resource
}
// buildQueryValues is a convenience function for knowing if a namespace should go in a query param or not
func buildQueryValues(namespace string, query url.Values) url.Values {
v := url.Values{}
if query != nil {
for key, values := range query {
for _, value := range values {
v.Add(key, value)
}
}
}
if len(namespace) > 0 {
if !NamespaceInPathFor(testapi.Version()) {
v.Set("namespace", namespace)
}
}
return v
}
func TestListEmptyPods(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods")},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200, Body: &api.PodList{}},
}
podList, err := c.Setup().Pods(ns).List(labels.Everything())
@ -182,7 +203,7 @@ func TestListEmptyPods(t *testing.T) {
func TestListPods(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods")},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200,
Body: &api.PodList{
Items: []api.Pod{
@ -214,7 +235,7 @@ func validateLabels(a, b string) bool {
func TestListPodsLabels(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods"), Query: url.Values{"labels": []string{"foo=bar,name=baz"}}},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods"), Query: buildQueryValues(ns, url.Values{"labels": []string{"foo=bar,name=baz"}})},
Response: Response{
StatusCode: 200,
Body: &api.PodList{
@ -244,7 +265,7 @@ func TestListPodsLabels(t *testing.T) {
func TestGetPod(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods/foo")},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods/foo"), Query: buildQueryValues(ns, nil)},
Response: Response{
StatusCode: 200,
Body: &api.Pod{
@ -278,7 +299,7 @@ func TestGetPodWithNoName(t *testing.T) {
func TestDeletePod(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "DELETE", Path: buildResourcePath(ns, "/pods/foo")},
Request: testRequest{Method: "DELETE", Path: buildResourcePath(ns, "/pods/foo"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200},
}
err := c.Setup().Pods(ns).Delete("foo")
@ -299,7 +320,7 @@ func TestCreatePod(t *testing.T) {
},
}
c := &testClient{
Request: testRequest{Method: "POST", Path: buildResourcePath(ns, "/pods"), Body: requestPod},
Request: testRequest{Method: "POST", Path: buildResourcePath(ns, "/pods"), Query: buildQueryValues(ns, nil), Body: requestPod},
Response: Response{
StatusCode: 200,
Body: requestPod,
@ -325,7 +346,7 @@ func TestUpdatePod(t *testing.T) {
},
}
c := &testClient{
Request: testRequest{Method: "PUT", Path: buildResourcePath(ns, "/pods/foo")},
Request: testRequest{Method: "PUT", Path: buildResourcePath(ns, "/pods/foo"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200, Body: requestPod},
}
receivedPod, err := c.Setup().Pods(ns).Update(requestPod)
@ -363,7 +384,7 @@ func TestListControllers(t *testing.T) {
func TestGetController(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/replicationControllers/foo")},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/replicationControllers/foo"), Query: buildQueryValues(ns, nil)},
Response: Response{
StatusCode: 200,
Body: &api.ReplicationController{
@ -402,7 +423,7 @@ func TestUpdateController(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"},
}
c := &testClient{
Request: testRequest{Method: "PUT", Path: buildResourcePath(ns, "/replicationControllers/foo")},
Request: testRequest{Method: "PUT", Path: buildResourcePath(ns, "/replicationControllers/foo"), Query: buildQueryValues(ns, nil)},
Response: Response{
StatusCode: 200,
Body: &api.ReplicationController{
@ -427,7 +448,7 @@ func TestUpdateController(t *testing.T) {
func TestDeleteController(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "DELETE", Path: buildResourcePath(ns, "/replicationControllers/foo")},
Request: testRequest{Method: "DELETE", Path: buildResourcePath(ns, "/replicationControllers/foo"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200},
}
err := c.Setup().ReplicationControllers(ns).Delete("foo")
@ -440,7 +461,7 @@ func TestCreateController(t *testing.T) {
ObjectMeta: api.ObjectMeta{Name: "foo"},
}
c := &testClient{
Request: testRequest{Method: "POST", Path: buildResourcePath(ns, "/replicationControllers"), Body: requestController},
Request: testRequest{Method: "POST", Path: buildResourcePath(ns, "/replicationControllers"), Body: requestController, Query: buildQueryValues(ns, nil)},
Response: Response{
StatusCode: 200,
Body: &api.ReplicationController{
@ -474,7 +495,7 @@ func body(obj runtime.Object, raw *string) *string {
func TestListServices(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/services")},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/services"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200,
Body: &api.ServiceList{
Items: []api.Service{
@ -504,7 +525,7 @@ func TestListServices(t *testing.T) {
func TestListServicesLabels(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/services"), Query: url.Values{"labels": []string{"foo=bar,name=baz"}}},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/services"), Query: buildQueryValues(ns, url.Values{"labels": []string{"foo=bar,name=baz"}})},
Response: Response{StatusCode: 200,
Body: &api.ServiceList{
Items: []api.Service{
@ -536,7 +557,7 @@ func TestListServicesLabels(t *testing.T) {
func TestGetService(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/services/1")},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/services/1"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200, Body: &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}},
}
response, err := c.Setup().Services(ns).Get("1")
@ -557,7 +578,7 @@ func TestGetServiceWithNoName(t *testing.T) {
func TestCreateService(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "POST", Path: buildResourcePath(ns, "/services"), Body: &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}},
Request: testRequest{Method: "POST", Path: buildResourcePath(ns, "/services"), Body: &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}, Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200, Body: &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}},
}
response, err := c.Setup().Services(ns).Create(&api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}})
@ -568,7 +589,7 @@ func TestUpdateService(t *testing.T) {
ns := api.NamespaceDefault
svc := &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1", ResourceVersion: "1"}}
c := &testClient{
Request: testRequest{Method: "PUT", Path: buildResourcePath(ns, "/services/service-1"), Body: svc},
Request: testRequest{Method: "PUT", Path: buildResourcePath(ns, "/services/service-1"), Body: svc, Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200, Body: svc},
}
response, err := c.Setup().Services(ns).Update(svc)
@ -578,7 +599,7 @@ func TestUpdateService(t *testing.T) {
func TestDeleteService(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "DELETE", Path: buildResourcePath(ns, "/services/1")},
Request: testRequest{Method: "DELETE", Path: buildResourcePath(ns, "/services/1"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200},
}
err := c.Setup().Services(ns).Delete("1")
@ -588,7 +609,7 @@ func TestDeleteService(t *testing.T) {
func TestListEndpooints(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/endpoints")},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/endpoints"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200,
Body: &api.EndpointsList{
Items: []api.Endpoints{
@ -607,7 +628,7 @@ func TestListEndpooints(t *testing.T) {
func TestGetEndpoints(t *testing.T) {
ns := api.NamespaceDefault
c := &testClient{
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/endpoints/endpoint-1")},
Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/endpoints/endpoint-1"), Query: buildQueryValues(ns, nil)},
Response: Response{StatusCode: 200, Body: &api.Endpoints{ObjectMeta: api.ObjectMeta{Name: "endpoint-1"}}},
}
response, err := c.Setup().Endpoints(ns).Get("endpoint-1")

View File

@ -96,16 +96,16 @@ type FakeRESTClient struct {
}
func (c *FakeRESTClient) Get() *Request {
return NewRequest(c, "GET", &url.URL{Host: "localhost"}, c.Codec)
return NewRequest(c, "GET", &url.URL{Host: "localhost"}, c.Codec, true)
}
func (c *FakeRESTClient) Put() *Request {
return NewRequest(c, "PUT", &url.URL{Host: "localhost"}, c.Codec)
return NewRequest(c, "PUT", &url.URL{Host: "localhost"}, c.Codec, true)
}
func (c *FakeRESTClient) Post() *Request {
return NewRequest(c, "POST", &url.URL{Host: "localhost"}, c.Codec)
return NewRequest(c, "POST", &url.URL{Host: "localhost"}, c.Codec, true)
}
func (c *FakeRESTClient) Delete() *Request {
return NewRequest(c, "DELETE", &url.URL{Host: "localhost"}, c.Codec)
return NewRequest(c, "DELETE", &url.URL{Host: "localhost"}, c.Codec, true)
}
func (c *FakeRESTClient) Do(req *http.Request) (*http.Response, error) {
c.Req = req

View File

@ -117,7 +117,7 @@ func RESTClientFor(config *Config) (*RESTClient, error) {
return nil, err
}
client := NewRESTClient(baseURL, versionInterfaces.Codec)
client := NewRESTClient(baseURL, versionInterfaces.Codec, NamespaceInPathFor(version))
transport, err := TransportFor(config)
if err != nil {
@ -252,3 +252,9 @@ func defaultVersionFor(config *Config) string {
}
return version
}
// namespaceInPathFor is used to control what api version should use namespace in url paths
func NamespaceInPathFor(version string) bool {
// we use query param for v1beta1/v1beta2, v1beta3+ will use path param
return (version != "v1beta1" && version != "v1beta2")
}

View File

@ -96,20 +96,23 @@ type Request struct {
sync bool
timeout time.Duration
// flag to control how to use namespace in urls
namespaceAsPath bool
// output
err error
body io.Reader
}
// NewRequest creates a new request with the core attributes.
func NewRequest(client HTTPClient, verb string, baseURL *url.URL, codec runtime.Codec) *Request {
func NewRequest(client HTTPClient, verb string, baseURL *url.URL, codec runtime.Codec, namespaceAsPath bool) *Request {
return &Request{
client: client,
verb: verb,
baseURL: baseURL,
codec: codec,
path: baseURL.Path,
client: client,
verb: verb,
baseURL: baseURL,
codec: codec,
namespaceAsPath: namespaceAsPath,
path: baseURL.Path,
}
}
@ -136,8 +139,14 @@ func (r *Request) Namespace(namespace string) *Request {
if r.err != nil {
return r
}
if len(namespace) > 0 {
return r.Path("ns").Path(namespace)
if r.namespaceAsPath {
return r.Path("ns").Path(namespace)
} else {
return r.setParam("namespace", namespace)
}
}
return r
}

View File

@ -137,7 +137,7 @@ func TestTransformResponse(t *testing.T) {
{Response: &http.Response{StatusCode: 200, Body: ioutil.NopCloser(bytes.NewReader(invalid))}, Data: invalid},
}
for i, test := range testCases {
r := NewRequest(nil, "", uri, testapi.Codec())
r := NewRequest(nil, "", uri, testapi.Codec(), true)
if test.Response.Body == nil {
test.Response.Body = ioutil.NopCloser(bytes.NewReader([]byte{}))
}

View File

@ -36,6 +36,10 @@ import (
type RESTClient struct {
baseURL *url.URL
// namespaceInPath controls if URLs should encode the namespace as path param instead of query param
// needed for backward compatibility
namespaceInPath bool
// Codec is the encoding and decoding scheme that applies to a particular set of
// REST resources.
Codec runtime.Codec
@ -56,7 +60,7 @@ type RESTClient struct {
// NewRESTClient creates a new RESTClient. This client performs generic REST functions
// such as Get, Put, Post, and Delete on specified paths. Codec controls encoding and
// decoding of responses from the server.
func NewRESTClient(baseURL *url.URL, c runtime.Codec) *RESTClient {
func NewRESTClient(baseURL *url.URL, c runtime.Codec, namespaceInPath bool) *RESTClient {
base := *baseURL
if !strings.HasSuffix(base.Path, "/") {
base.Path += "/"
@ -68,6 +72,8 @@ func NewRESTClient(baseURL *url.URL, c runtime.Codec) *RESTClient {
baseURL: &base,
Codec: c,
namespaceInPath: namespaceInPath,
// Make asynchronous requests by default
Sync: false,
@ -98,7 +104,7 @@ func (c *RESTClient) Verb(verb string) *Request {
if poller == nil {
poller = c.DefaultPoll
}
return NewRequest(c.Client, verb, c.baseURL, c.Codec).Poller(poller).Sync(c.Sync).Timeout(c.Timeout)
return NewRequest(c.Client, verb, c.baseURL, c.Codec, c.namespaceInPath).Poller(poller).Sync(c.Sync).Timeout(c.Timeout)
}
// Post begins a POST request. Short for c.Verb("POST").

View File

@ -48,7 +48,7 @@ func getFakeClient(t *testing.T, validURLs []string) (ClientPosterFunc, *httptes
return func(mapping *meta.RESTMapping) (RESTClientPoster, error) {
fakeCodec := runtime.CodecFor(api.Scheme, "v1beta1")
fakeUri, _ := url.Parse(server.URL + "/api/v1beta1")
return client.NewRESTClient(fakeUri, fakeCodec), nil
return client.NewRESTClient(fakeUri, fakeCodec, false), nil
}, server
}

View File

@ -37,6 +37,13 @@ import (
"github.com/coreos/go-etcd/etcd"
)
func makeNamespaceURL(namespace, suffix string) string {
if client.NamespaceInPathFor(testapi.Version()) {
return makeURL("/ns/" + namespace + suffix)
}
return makeURL(suffix + "?namespace=" + namespace)
}
func makeURL(suffix string) string {
return path.Join("/api", testapi.Version(), suffix)
}
@ -221,7 +228,7 @@ func TestCreateReplica(t *testing.T) {
},
Spec: controllerSpec.Spec.Template.Spec,
}
fakeHandler.ValidateRequest(t, makeURL("/ns/default/pods"), "POST", nil)
fakeHandler.ValidateRequest(t, makeNamespaceURL("default", "/pods"), "POST", nil)
actualPod, err := client.Codec.Decode([]byte(fakeHandler.RequestBody))
if err != nil {
t.Errorf("Unexpected error: %#v", err)

View File

@ -204,7 +204,7 @@ func (factory *ConfigFactory) pollMinions() (cache.Enumerator, error) {
func (factory *ConfigFactory) makeDefaultErrorFunc(backoff *podBackoff, podQueue *cache.FIFO) func(pod *api.Pod, err error) {
return func(pod *api.Pod, err error) {
glog.Errorf("Error scheduling %v: %v; retrying", pod.Name, err)
glog.Errorf("Error scheduling %v %v: %v; retrying", pod.Namespace, pod.Name, err)
backoff.gc()
// Retry asynchronously.
// Note that this is extremely rudimentary and we need a more real error handling path.

View File

@ -19,6 +19,7 @@ package factory
import (
"net/http"
"net/http/httptest"
"path"
"reflect"
"testing"
"time"
@ -183,6 +184,22 @@ func TestPollMinions(t *testing.T) {
}
}
func makeNamespaceURL(namespace, suffix string, isClient bool) string {
if client.NamespaceInPathFor(testapi.Version()) {
return makeURL("/ns/" + namespace + suffix)
}
// if this is a url the client should call, encode the url
if isClient {
return makeURL(suffix + "?namespace=" + namespace)
}
// its not a client url, so its what the server needs to listen on
return makeURL(suffix)
}
func makeURL(suffix string) string {
return path.Join("/api", testapi.Version(), suffix)
}
func TestDefaultErrorFunc(t *testing.T) {
testPod := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "bar"}}
handler := util.FakeHandler{
@ -191,8 +208,9 @@ func TestDefaultErrorFunc(t *testing.T) {
T: t,
}
mux := http.NewServeMux()
// FakeHandler musn't be sent requests other than the one you want to test.
mux.Handle("/api/"+testapi.Version()+"/ns/bar/pods/foo", &handler)
mux.Handle(makeNamespaceURL("bar", "/pods/foo", false), &handler)
server := httptest.NewServer(mux)
defer server.Close()
factory := NewConfigFactory(client.NewOrDie(&client.Config{Host: server.URL, Version: testapi.Version()}))
@ -213,7 +231,7 @@ func TestDefaultErrorFunc(t *testing.T) {
if !exists {
continue
}
handler.ValidateRequest(t, "/api/"+testapi.Version()+"/ns/bar/pods/foo", "GET", nil)
handler.ValidateRequest(t, makeNamespaceURL("bar", "/pods/foo", true), "GET", nil)
if e, a := testPod, got; !reflect.DeepEqual(e, a) {
t.Errorf("Expected %v, got %v", e, a)
}