From d5ee171410ba2b6af1b1f4f483edced2548525b8 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Fri, 24 Oct 2014 13:16:02 -0400 Subject: [PATCH] Allow clients to determine the difference between create or update on PUT PUT allows an object to be created (http 201). This allows REST code to indicate an object has been created and clients to react to it. APIServer now deals with <-chan RESTResult instead of <-chan runtime.Object, allowing more data to be passed through. --- pkg/apiserver/apiserver_test.go | 6 +-- pkg/apiserver/async.go | 31 ++++++++++++-- pkg/apiserver/handlers.go | 1 + pkg/apiserver/interfaces.go | 23 +++++++++-- pkg/apiserver/operation.go | 17 ++++---- pkg/apiserver/operation_test.go | 8 ++-- pkg/apiserver/resthandler.go | 15 ++++--- pkg/apiserver/resthandler_test.go | 69 +++++++++++++++++++++++++++++++ pkg/client/client_test.go | 1 + pkg/client/request.go | 17 ++++++-- pkg/client/request_test.go | 50 +++++++++++++++++++++- pkg/client/restclient.go | 15 +++---- pkg/client/restclient_test.go | 49 ++++++++++++++++++---- pkg/registry/binding/rest.go | 6 +-- pkg/registry/binding/rest_test.go | 2 +- pkg/registry/controller/rest.go | 6 +-- pkg/registry/endpoint/rest.go | 6 +-- pkg/registry/event/rest.go | 6 +-- pkg/registry/event/rest_test.go | 4 +- pkg/registry/minion/rest.go | 6 +-- pkg/registry/minion/rest_test.go | 4 +- pkg/registry/pod/rest.go | 6 +-- pkg/registry/pod/rest_test.go | 10 ++--- pkg/registry/service/rest.go | 6 +-- pkg/registry/service/rest_test.go | 20 ++++----- 25 files changed, 297 insertions(+), 87 deletions(-) create mode 100644 pkg/apiserver/resthandler_test.go diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 487c906b2d1..bcb1aab6cdb 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -103,7 +103,7 @@ func (storage *SimpleRESTStorage) Get(ctx api.Context, id string) (runtime.Objec return api.Scheme.CopyOrDie(&storage.item), storage.errors["get"] } -func (storage *SimpleRESTStorage) Delete(ctx api.Context, id string) (<-chan runtime.Object, error) { +func (storage *SimpleRESTStorage) Delete(ctx api.Context, id string) (<-chan RESTResult, error) { storage.deleted = id if err := storage.errors["delete"]; err != nil { return nil, err @@ -120,7 +120,7 @@ func (storage *SimpleRESTStorage) New() runtime.Object { return &Simple{} } -func (storage *SimpleRESTStorage) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (storage *SimpleRESTStorage) Create(ctx api.Context, obj runtime.Object) (<-chan RESTResult, error) { storage.created = obj.(*Simple) if err := storage.errors["create"]; err != nil { return nil, err @@ -133,7 +133,7 @@ func (storage *SimpleRESTStorage) Create(ctx api.Context, obj runtime.Object) (< }), nil } -func (storage *SimpleRESTStorage) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (storage *SimpleRESTStorage) Update(ctx api.Context, obj runtime.Object) (<-chan RESTResult, error) { storage.updated = obj.(*Simple) if err := storage.errors["update"]; err != nil { return nil, err diff --git a/pkg/apiserver/async.go b/pkg/apiserver/async.go index 97a5919d070..a96a151be34 100644 --- a/pkg/apiserver/async.go +++ b/pkg/apiserver/async.go @@ -28,13 +28,38 @@ type WorkFunc func() (result runtime.Object, err error) // MakeAsync takes a function and executes it, delivering the result in the way required // by RESTStorage's Update, Delete, and Create methods. -func MakeAsync(fn WorkFunc) <-chan runtime.Object { - channel := make(chan runtime.Object) +func MakeAsync(fn WorkFunc) <-chan RESTResult { + channel := make(chan RESTResult) go func() { defer util.HandleCrash() obj, err := fn() if err != nil { - channel <- errToAPIStatus(err) + channel <- RESTResult{Object: errToAPIStatus(err)} + } else { + channel <- RESTResult{Object: obj} + } + // 'close' is used to signal that no further values will + // be written to the channel. Not strictly necessary, but + // also won't hurt. + close(channel) + }() + return channel +} + +// WorkFunc is used to perform any time consuming work for an api call, after +// the input has been validated. Pass one of these to MakeAsync to create an +// appropriate return value for the Update, Delete, and Create methods. +type WorkResultFunc func() (result RESTResult, err error) + +// MakeAsync takes a function and executes it, delivering the result in the way required +// by RESTStorage's Update, Delete, and Create methods. +func MakeAsyncResult(fn WorkResultFunc) <-chan RESTResult { + channel := make(chan RESTResult) + go func() { + defer util.HandleCrash() + obj, err := fn() + if err != nil { + channel <- RESTResult{Object: errToAPIStatus(err)} } else { channel <- obj } diff --git a/pkg/apiserver/handlers.go b/pkg/apiserver/handlers.go index 9d5d0b94ab5..43336de7a24 100644 --- a/pkg/apiserver/handlers.go +++ b/pkg/apiserver/handlers.go @@ -65,6 +65,7 @@ func RecoverPanics(handler http.Handler) http.Handler { defer httplog.NewLogged(req, &w).StacktraceWhen( httplog.StatusIsNot( http.StatusOK, + http.StatusCreated, http.StatusAccepted, http.StatusMovedPermanently, http.StatusTemporaryRedirect, diff --git a/pkg/apiserver/interfaces.go b/pkg/apiserver/interfaces.go index e700765fa0f..72d5ebb1021 100644 --- a/pkg/apiserver/interfaces.go +++ b/pkg/apiserver/interfaces.go @@ -41,10 +41,27 @@ type RESTStorage interface { // Delete finds a resource in the storage and deletes it. // Although it can return an arbitrary error value, IsNotFound(err) is true for the // returned error value err when the specified resource is not found. - Delete(ctx api.Context, id string) (<-chan runtime.Object, error) + Delete(ctx api.Context, id string) (<-chan RESTResult, error) - Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) - Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) + // Create creates a new version of a resource. + Create(ctx api.Context, obj runtime.Object) (<-chan RESTResult, error) + + // Update finds a resource in the storage and updates it. Some implementations + // may allow updates creates the object - they should set the Created flag of + // the returned RESTResultto true. In the event of an asynchronous error returned + // via an api.Status object, the Created flag is ignored. + Update(ctx api.Context, obj runtime.Object) (<-chan RESTResult, error) +} + +// RESTResult indicates the result of a REST transformation. +type RESTResult struct { + // The result of this operation. May be nil if the operation has no meaningful + // result (like Delete) + runtime.Object + + // May be set true to indicate that the Update operation resulted in the object + // being created. + Created bool } // ResourceWatcher should be implemented by all RESTStorage objects that diff --git a/pkg/apiserver/operation.go b/pkg/apiserver/operation.go index dcb27864225..38451eb5b1e 100644 --- a/pkg/apiserver/operation.go +++ b/pkg/apiserver/operation.go @@ -53,7 +53,8 @@ func (h *OperationHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } - obj, complete := op.StatusOrResult() + result, complete := op.StatusOrResult() + obj := result.Object if complete { writeJSON(http.StatusOK, h.codec, obj, w) } else { @@ -64,9 +65,9 @@ func (h *OperationHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { // Operation represents an ongoing action which the server is performing. type Operation struct { ID string - result runtime.Object - onReceive func(runtime.Object) - awaiting <-chan runtime.Object + result RESTResult + onReceive func(RESTResult) + awaiting <-chan RESTResult finished *time.Time lock sync.Mutex notify chan struct{} @@ -93,7 +94,7 @@ func NewOperations() *Operations { // NewOperation adds a new operation. It is lock-free. 'onReceive' will be called // with the value read from 'from', when it is read. -func (ops *Operations) NewOperation(from <-chan runtime.Object, onReceive func(runtime.Object)) *Operation { +func (ops *Operations) NewOperation(from <-chan RESTResult, onReceive func(RESTResult)) *Operation { id := atomic.AddInt64(&ops.lastID, 1) op := &Operation{ ID: strconv.FormatInt(id, 10), @@ -192,16 +193,16 @@ func (op *Operation) expired(limitTime time.Time) bool { // StatusOrResult returns status information or the result of the operation if it is complete, // with a bool indicating true in the latter case. -func (op *Operation) StatusOrResult() (description runtime.Object, finished bool) { +func (op *Operation) StatusOrResult() (description RESTResult, finished bool) { op.lock.Lock() defer op.lock.Unlock() if op.finished == nil { - return &api.Status{ + return RESTResult{Object: &api.Status{ Status: api.StatusWorking, Reason: api.StatusReasonWorking, Details: &api.StatusDetails{ID: op.ID, Kind: "operation"}, - }, false + }}, false } return op.result, true } diff --git a/pkg/apiserver/operation_test.go b/pkg/apiserver/operation_test.go index d0bcc7d3a68..a9670634386 100644 --- a/pkg/apiserver/operation_test.go +++ b/pkg/apiserver/operation_test.go @@ -34,16 +34,16 @@ import ( func TestOperation(t *testing.T) { ops := NewOperations() - c := make(chan runtime.Object) + c := make(chan RESTResult) called := make(chan struct{}) - op := ops.NewOperation(c, func(runtime.Object) { go close(called) }) + op := ops.NewOperation(c, func(RESTResult) { go close(called) }) // Allow context switch, so that op's ID can get added to the map and Get will work. // This is just so we can test Get. Ordinary users have no need to call Get immediately // after calling NewOperation, because it returns the operation directly. time.Sleep(time.Millisecond) go func() { time.Sleep(500 * time.Millisecond) - c <- &Simple{ObjectMeta: api.ObjectMeta{Name: "All done"}} + c <- RESTResult{Object: &Simple{ObjectMeta: api.ObjectMeta{Name: "All done"}}} }() if op.expired(time.Now().Add(-time.Minute)) { @@ -96,7 +96,7 @@ func TestOperation(t *testing.T) { t.Errorf("expire failed to remove the operation %#v", ops) } - if op.result.(*Simple).Name != "All done" { + if op.result.Object.(*Simple).Name != "All done" { t.Errorf("Got unexpected result: %#v", op.result) } } diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index edbcaa52371..822751c8660 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -29,6 +29,7 @@ import ( "github.com/golang/glog" ) +// RESTHandler implements HTTP verbs on a set of RESTful resources identified by name. type RESTHandler struct { storage map[string]RESTStorage codec runtime.Codec @@ -78,9 +79,9 @@ func (h *RESTHandler) setSelfLinkAddName(obj runtime.Object, req *http.Request) } // curry adapts either of the self link setting functions into a function appropriate for operation's hook. -func curry(f func(runtime.Object, *http.Request) error, req *http.Request) func(runtime.Object) { - return func(obj runtime.Object) { - if err := f(obj, req); err != nil { +func curry(f func(runtime.Object, *http.Request) error, req *http.Request) func(RESTResult) { + return func(obj RESTResult) { + if err := f(obj.Object, req); err != nil { glog.Errorf("unable to set self link for %#v: %v", obj, err) } } @@ -217,7 +218,7 @@ func (h *RESTHandler) handleRESTStorage(parts []string, req *http.Request, w htt } // createOperation creates an operation to process a channel response. -func (h *RESTHandler) createOperation(out <-chan runtime.Object, sync bool, timeout time.Duration, onReceive func(runtime.Object)) *Operation { +func (h *RESTHandler) createOperation(out <-chan RESTResult, sync bool, timeout time.Duration, onReceive func(RESTResult)) *Operation { op := h.ops.NewOperation(out, onReceive) if sync { op.WaitFor(timeout) @@ -230,9 +231,13 @@ func (h *RESTHandler) createOperation(out <-chan runtime.Object, sync bool, time // finishReq finishes up a request, waiting until the operation finishes or, after a timeout, creating an // Operation to receive the result and returning its ID down the writer. func (h *RESTHandler) finishReq(op *Operation, req *http.Request, w http.ResponseWriter) { - obj, complete := op.StatusOrResult() + result, complete := op.StatusOrResult() + obj := result.Object if complete { status := http.StatusOK + if result.Created { + status = http.StatusCreated + } switch stat := obj.(type) { case *api.Status: if stat.Code != 0 { diff --git a/pkg/apiserver/resthandler_test.go b/pkg/apiserver/resthandler_test.go new file mode 100644 index 00000000000..2f471beda19 --- /dev/null +++ b/pkg/apiserver/resthandler_test.go @@ -0,0 +1,69 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apiserver + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" +) + +func TestFinishReq(t *testing.T) { + handler := &RESTHandler{codec: api.Codec} + op := &Operation{finished: &time.Time{}, result: RESTResult{Object: &api.Status{Code: http.StatusNotFound}}} + resp := httptest.NewRecorder() + handler.finishReq(op, nil, resp) + status := &api.Status{} + if err := json.Unmarshal([]byte(resp.Body.String()), status); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.Code != http.StatusNotFound || status.Code != http.StatusNotFound { + t.Errorf("unexpected status: %#v", status) + } +} + +func TestFinishReqUnwrap(t *testing.T) { + handler := &RESTHandler{codec: api.Codec} + op := &Operation{finished: &time.Time{}, result: RESTResult{Created: true, Object: &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}}} + resp := httptest.NewRecorder() + handler.finishReq(op, nil, resp) + obj := &api.Pod{} + if err := json.Unmarshal([]byte(resp.Body.String()), obj); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.Code != http.StatusCreated || obj.Name != "foo" { + t.Errorf("unexpected object: %#v", obj) + } +} + +func TestFinishReqUnwrapStatus(t *testing.T) { + handler := &RESTHandler{codec: api.Codec} + op := &Operation{finished: &time.Time{}, result: RESTResult{Created: true, Object: &api.Status{Code: http.StatusNotFound}}} + resp := httptest.NewRecorder() + handler.finishReq(op, nil, resp) + obj := &api.Status{} + if err := json.Unmarshal([]byte(resp.Body.String()), obj); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.Code != http.StatusNotFound || obj.Code != http.StatusNotFound { + t.Errorf("unexpected object: %#v", obj) + } +} diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index ba91061d881..3b08dfba902 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -57,6 +57,7 @@ type testClient struct { Request testRequest Response Response Error bool + Created bool server *httptest.Server handler *util.FakeHandler // For query args, an optional function to validate the contents diff --git a/pkg/client/request.go b/pkg/client/request.go index 1678ef3ed3b..965e5846872 100644 --- a/pkg/client/request.go +++ b/pkg/client/request.go @@ -272,7 +272,7 @@ func (r *Request) Do() Result { if err != nil { return Result{err: err} } - respBody, err := r.c.doRequest(req) + respBody, created, err := r.c.doRequest(req) if err != nil { if s, ok := err.(APIStatus); ok { status := s.Status() @@ -292,14 +292,16 @@ func (r *Request) Do() Result { } } } - return Result{respBody, err, r.c.Codec} + return Result{respBody, created, err, r.c.Codec} } } // Result contains the result of calling Request.Do(). type Result struct { - body []byte - err error + body []byte + created bool + err error + codec runtime.Codec } @@ -324,6 +326,13 @@ func (r Result) Into(obj runtime.Object) error { return r.codec.DecodeInto(r.body, obj) } +// WasCreated updates the provided bool pointer to whether the server returned +// 201 created or a different response. +func (r Result) WasCreated(wasCreated *bool) Result { + *wasCreated = r.created + return r +} + // Error returns the error executing the request, nil if no error occurred. func (r Result) Error() error { return r.err diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go index d61da5561a7..9183c820353 100644 --- a/pkg/client/request_test.go +++ b/pkg/client/request_test.go @@ -168,13 +168,14 @@ func TestDoRequestNewWayFile(t *testing.T) { } testServer := httptest.NewServer(&fakeHandler) c := NewOrDie(&Config{Host: testServer.URL, Version: "v1beta1", Username: "user", Password: "pass"}) + wasCreated := true obj, err := c.Verb("POST"). Path("foo/bar"). Path("baz"). ParseSelectorParam("labels", "name=foo"). Timeout(time.Second). Body(file.Name()). - Do().Get() + Do().WasCreated(&wasCreated).Get() if err != nil { t.Errorf("Unexpected error: %v %#v", err, err) return @@ -184,6 +185,9 @@ func TestDoRequestNewWayFile(t *testing.T) { } else if !reflect.DeepEqual(obj, expectedObj) { t.Errorf("Expected: %#v, got %#v", expectedObj, obj) } + if wasCreated { + t.Errorf("expected object was not created") + } tmpStr := string(reqBodyExpected) fakeHandler.ValidateRequest(t, "/api/v1beta1/foo/bar/baz?labels=name%3Dfoo", "POST", &tmpStr) if fakeHandler.RequestReceived.Header["Authorization"] == nil { @@ -191,6 +195,50 @@ func TestDoRequestNewWayFile(t *testing.T) { } } +func TestWasCreated(t *testing.T) { + reqObj := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}} + reqBodyExpected, err := v1beta1.Codec.Encode(reqObj) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + expectedObj := &api.Service{Port: 12345} + expectedBody, _ := v1beta1.Codec.Encode(expectedObj) + fakeHandler := util.FakeHandler{ + StatusCode: 201, + ResponseBody: string(expectedBody), + T: t, + } + testServer := httptest.NewServer(&fakeHandler) + c := NewOrDie(&Config{Host: testServer.URL, Version: "v1beta1", Username: "user", Password: "pass"}) + wasCreated := false + obj, err := c.Verb("PUT"). + Path("foo/bar"). + Path("baz"). + ParseSelectorParam("labels", "name=foo"). + Timeout(time.Second). + Body(reqBodyExpected). + Do().WasCreated(&wasCreated).Get() + if err != nil { + t.Errorf("Unexpected error: %v %#v", err, err) + return + } + if obj == nil { + t.Error("nil obj") + } else if !reflect.DeepEqual(obj, expectedObj) { + t.Errorf("Expected: %#v, got %#v", expectedObj, obj) + } + if !wasCreated { + t.Errorf("Expected object was created") + } + + tmpStr := string(reqBodyExpected) + fakeHandler.ValidateRequest(t, "/api/v1beta1/foo/bar/baz?labels=name%3Dfoo", "PUT", &tmpStr) + if fakeHandler.RequestReceived.Header["Authorization"] == nil { + t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived) + } +} + func TestVerbs(t *testing.T) { c := NewOrDie(&Config{}) if r := c.Post(); r.verb != "POST" { diff --git a/pkg/client/restclient.go b/pkg/client/restclient.go index d714cc59e16..caa510fc928 100644 --- a/pkg/client/restclient.go +++ b/pkg/client/restclient.go @@ -76,7 +76,7 @@ func NewRESTClient(baseURL *url.URL, c runtime.Codec) *RESTClient { } // doRequest executes a request against a server -func (c *RESTClient) doRequest(request *http.Request) ([]byte, error) { +func (c *RESTClient) doRequest(request *http.Request) ([]byte, bool, error) { client := c.Client if client == nil { client = http.DefaultClient @@ -84,12 +84,12 @@ func (c *RESTClient) doRequest(request *http.Request) ([]byte, error) { response, err := client.Do(request) if err != nil { - return nil, err + return nil, false, err } defer response.Body.Close() body, err := ioutil.ReadAll(response.Body) if err != nil { - return body, err + return body, false, err } // Did the server give us a status response? @@ -102,19 +102,20 @@ func (c *RESTClient) doRequest(request *http.Request) ([]byte, error) { switch { case response.StatusCode < http.StatusOK || response.StatusCode > http.StatusPartialContent: if !isStatusResponse { - return nil, fmt.Errorf("request [%#v] failed (%d) %s: %s", request, response.StatusCode, response.Status, string(body)) + return nil, false, fmt.Errorf("request [%#v] failed (%d) %s: %s", request, response.StatusCode, response.Status, string(body)) } - return nil, errors.FromObject(&status) + return nil, false, errors.FromObject(&status) } // If the server gave us a status back, look at what it was. if isStatusResponse && status.Status != api.StatusSuccess { // "Working" requests need to be handled specially. // "Failed" requests are clearly just an error and it makes sense to return them as such. - return nil, errors.FromObject(&status) + return nil, false, errors.FromObject(&status) } - return body, err + created := response.StatusCode == http.StatusCreated + return body, created, err } // Verb begins a request with a verb (GET, POST, PUT, DELETE). diff --git a/pkg/client/restclient_test.go b/pkg/client/restclient_test.go index 8c1df0d0ae8..63b742b1655 100644 --- a/pkg/client/restclient_test.go +++ b/pkg/client/restclient_test.go @@ -105,6 +105,7 @@ func TestDoRequest(t *testing.T) { uri, _ := url.Parse("http://localhost") testClients := []testClient{ {Request: testRequest{Method: "GET", Path: "good"}, Response: Response{StatusCode: 200}}, + {Request: testRequest{Method: "GET", Path: "good"}, Response: Response{StatusCode: 201}, Created: true}, {Request: testRequest{Method: "GET", Path: "bad%ZZ"}, Error: true}, {Request: testRequest{Method: "GET", Path: "error"}, Response: Response{StatusCode: 500}, Error: true}, {Request: testRequest{Method: "POST", Path: "faildecode"}, Response: Response{StatusCode: 200, RawBody: &invalid}}, @@ -120,7 +121,10 @@ func TestDoRequest(t *testing.T) { Header: make(http.Header), URL: &prefix, } - response, err := client.doRequest(request) + response, created, err := client.doRequest(request) + if c.Created != created { + t.Errorf("expected created %f, got %f", c.Created, created) + } //t.Logf("dorequest: %#v\n%#v\n%v", request.URL, response, err) c.ValidateRaw(t, response, err) } @@ -160,18 +164,16 @@ func TestDoRequestAccepted(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - body, err := c.doRequest(request) + body, _, err := c.doRequest(request) if fakeHandler.RequestReceived.Header["Authorization"] == nil { t.Errorf("Request is missing authorization header: %#v", *request) } if err == nil { - t.Error("Unexpected non-error") - return + t.Fatalf("Unexpected non-error") } se, ok := err.(APIStatus) if !ok { - t.Errorf("Unexpected kind of error: %#v", err) - return + t.Fatalf("Unexpected kind of error: %#v", err) } if !reflect.DeepEqual(se.Status(), *status) { t.Errorf("Unexpected status: %#v %#v", se.Status(), status) @@ -196,7 +198,7 @@ func TestDoRequestAcceptedSuccess(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - body, err := c.doRequest(request) + body, _, err := c.doRequest(request) if fakeHandler.RequestReceived.Header["Authorization"] == nil { t.Errorf("Request is missing authorization header: %#v", *request) } @@ -227,7 +229,7 @@ func TestDoRequestFailed(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - body, err := c.doRequest(request) + body, _, err := c.doRequest(request) if err == nil || body != nil { t.Errorf("unexpected non-error: %#v", body) } @@ -240,3 +242,34 @@ func TestDoRequestFailed(t *testing.T) { t.Errorf("Unexpected mis-match. Expected %#v. Saw %#v", status, actual) } } + +func TestDoRequestCreated(t *testing.T) { + status := &api.Status{Status: api.StatusSuccess} + expectedBody, _ := latest.Codec.Encode(status) + fakeHandler := util.FakeHandler{ + StatusCode: 201, + ResponseBody: string(expectedBody), + T: t, + } + testServer := httptest.NewServer(&fakeHandler) + request, _ := http.NewRequest("GET", testServer.URL+"/foo/bar", nil) + c, err := RESTClientFor(&Config{Host: testServer.URL, Username: "user", Password: "pass"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + body, created, err := c.doRequest(request) + if err != nil { + t.Errorf("Unexpected error %#v", err) + } + if !created { + t.Errorf("Expected object to be created") + } + statusOut, err := latest.Codec.Decode(body) + if err != nil { + t.Errorf("Unexpected error %#v", err) + } + if !reflect.DeepEqual(status, statusOut) { + t.Errorf("Unexpected mis-match. Expected %#v. Saw %#v", status, statusOut) + } + fakeHandler.ValidateRequest(t, "/foo/bar", "GET", nil) +} diff --git a/pkg/registry/binding/rest.go b/pkg/registry/binding/rest.go index 8c02a920e6f..47c40b6a295 100644 --- a/pkg/registry/binding/rest.go +++ b/pkg/registry/binding/rest.go @@ -51,7 +51,7 @@ func (*REST) Get(ctx api.Context, id string) (runtime.Object, error) { } // Delete returns an error because bindings are write-only objects. -func (*REST) Delete(ctx api.Context, id string) (<-chan runtime.Object, error) { +func (*REST) Delete(ctx api.Context, id string) (<-chan apiserver.RESTResult, error) { return nil, errors.NewNotFound("binding", id) } @@ -61,7 +61,7 @@ func (*REST) New() runtime.Object { } // Create attempts to make the assignment indicated by the binding it recieves. -func (b *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (b *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { binding, ok := obj.(*api.Binding) if !ok { return nil, fmt.Errorf("incorrect type: %#v", obj) @@ -75,6 +75,6 @@ func (b *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Objec } // Update returns an error-- this object may not be updated. -func (b *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (b *REST) Update(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { return nil, fmt.Errorf("Bindings may not be changed.") } diff --git a/pkg/registry/binding/rest_test.go b/pkg/registry/binding/rest_test.go index dc080ed19b3..13206ec006e 100644 --- a/pkg/registry/binding/rest_test.go +++ b/pkg/registry/binding/rest_test.go @@ -111,7 +111,7 @@ func TestRESTPost(t *testing.T) { Message: item.err.Error(), } } - if e, a := expect, <-resultChan; !reflect.DeepEqual(e, a) { + if e, a := expect, (<-resultChan).Object; !reflect.DeepEqual(e, a) { t.Errorf("%v: expected %#v, got %#v", i, e, a) } } diff --git a/pkg/registry/controller/rest.go b/pkg/registry/controller/rest.go index b45910c2169..61310d749c9 100644 --- a/pkg/registry/controller/rest.go +++ b/pkg/registry/controller/rest.go @@ -54,7 +54,7 @@ func NewREST(registry Registry, podLister PodLister) *REST { } // Create registers the given ReplicationController. -func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { controller, ok := obj.(*api.ReplicationController) if !ok { return nil, fmt.Errorf("not a replication controller: %#v", obj) @@ -84,7 +84,7 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Obje } // Delete asynchronously deletes the ReplicationController specified by its id. -func (rs *REST) Delete(ctx api.Context, id string) (<-chan runtime.Object, error) { +func (rs *REST) Delete(ctx api.Context, id string) (<-chan apiserver.RESTResult, error) { return apiserver.MakeAsync(func() (runtime.Object, error) { return &api.Status{Status: api.StatusSuccess}, rs.registry.DeleteController(ctx, id) }), nil @@ -127,7 +127,7 @@ func (*REST) New() runtime.Object { // Update replaces a given ReplicationController instance with an existing // instance in storage.registry. -func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { controller, ok := obj.(*api.ReplicationController) if !ok { return nil, fmt.Errorf("not a replication controller: %#v", obj) diff --git a/pkg/registry/endpoint/rest.go b/pkg/registry/endpoint/rest.go index 7a2c15e8e78..aba485f8e34 100644 --- a/pkg/registry/endpoint/rest.go +++ b/pkg/registry/endpoint/rest.go @@ -60,7 +60,7 @@ func (rs *REST) Watch(ctx api.Context, label, field labels.Selector, resourceVer } // Create satisfies the RESTStorage interface. -func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { endpoints, ok := obj.(*api.Endpoints) if !ok { return nil, fmt.Errorf("not an endpoints: %#v", obj) @@ -79,7 +79,7 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Obje } // Update satisfies the RESTStorage interface. -func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { endpoints, ok := obj.(*api.Endpoints) if !ok { return nil, fmt.Errorf("not an endpoints: %#v", obj) @@ -94,7 +94,7 @@ func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Obje } // Delete satisfies the RESTStorage interface but is unimplemented. -func (rs *REST) Delete(ctx api.Context, id string) (<-chan runtime.Object, error) { +func (rs *REST) Delete(ctx api.Context, id string) (<-chan apiserver.RESTResult, error) { return nil, errors.New("unimplemented") } diff --git a/pkg/registry/event/rest.go b/pkg/registry/event/rest.go index b5da79baa44..eb01f17950e 100644 --- a/pkg/registry/event/rest.go +++ b/pkg/registry/event/rest.go @@ -41,7 +41,7 @@ func NewREST(registry generic.Registry) *REST { } } -func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { event, ok := obj.(*api.Event) if !ok { return nil, fmt.Errorf("invalid object type") @@ -57,7 +57,7 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Obje }), nil } -func (rs *REST) Delete(ctx api.Context, id string) (<-chan runtime.Object, error) { +func (rs *REST) Delete(ctx api.Context, id string) (<-chan apiserver.RESTResult, error) { obj, err := rs.registry.Get(ctx, id) if err != nil { return nil, err @@ -116,6 +116,6 @@ func (*REST) New() runtime.Object { } // Update returns an error: Events are not mutable. -func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { return nil, fmt.Errorf("not allowed: 'Event' objects are not mutable") } diff --git a/pkg/registry/event/rest_test.go b/pkg/registry/event/rest_test.go index 518d82921c3..24748964c28 100644 --- a/pkg/registry/event/rest_test.go +++ b/pkg/registry/event/rest_test.go @@ -47,7 +47,7 @@ func TestRESTCreate(t *testing.T) { if err != nil { t.Fatalf("Unexpected error %v", err) } - if e, a := eventA, <-c; !reflect.DeepEqual(e, a) { + if e, a := eventA, (<-c).Object; !reflect.DeepEqual(e, a) { t.Errorf("diff: %s", util.ObjectDiff(e, a)) } } @@ -67,7 +67,7 @@ func TestRESTDelete(t *testing.T) { if err != nil { t.Fatalf("Unexpected error %v", err) } - if stat := (<-c).(*api.Status); stat.Status != api.StatusSuccess { + if stat := (<-c).Object.(*api.Status); stat.Status != api.StatusSuccess { t.Errorf("unexpected status: %v", stat) } } diff --git a/pkg/registry/minion/rest.go b/pkg/registry/minion/rest.go index ed1bbf48b07..792c383f94a 100644 --- a/pkg/registry/minion/rest.go +++ b/pkg/registry/minion/rest.go @@ -45,7 +45,7 @@ func NewREST(m Registry) *REST { var ErrDoesNotExist = errors.New("The requested resource does not exist.") var ErrNotHealty = errors.New("The requested minion is not healthy.") -func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { minion, ok := obj.(*api.Minion) if !ok { return nil, fmt.Errorf("not a minion: %#v", obj) @@ -72,7 +72,7 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Obje }), nil } -func (rs *REST) Delete(ctx api.Context, id string) (<-chan runtime.Object, error) { +func (rs *REST) Delete(ctx api.Context, id string) (<-chan apiserver.RESTResult, error) { minion, err := rs.registry.GetMinion(ctx, id) if minion == nil { return nil, ErrDoesNotExist @@ -101,7 +101,7 @@ func (rs *REST) New() runtime.Object { return &api.Minion{} } -func (rs *REST) Update(ctx api.Context, minion runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Update(ctx api.Context, minion runtime.Object) (<-chan apiserver.RESTResult, error) { return nil, fmt.Errorf("Minions can only be created (inserted) and deleted.") } diff --git a/pkg/registry/minion/rest_test.go b/pkg/registry/minion/rest_test.go index 122cf909a25..06fcdf750b9 100644 --- a/pkg/registry/minion/rest_test.go +++ b/pkg/registry/minion/rest_test.go @@ -42,7 +42,7 @@ func TestMinionREST(t *testing.T) { t.Errorf("insert failed") } obj := <-c - if m, ok := obj.(*api.Minion); !ok || m.Name != "baz" { + if m, ok := obj.Object.(*api.Minion); !ok || m.Name != "baz" { t.Errorf("insert return value was weird: %#v", obj) } if obj, err := ms.Get(ctx, "baz"); err != nil || obj.(*api.Minion).Name != "baz" { @@ -54,7 +54,7 @@ func TestMinionREST(t *testing.T) { t.Errorf("delete failed") } obj = <-c - if s, ok := obj.(*api.Status); !ok || s.Status != api.StatusSuccess { + if s, ok := obj.Object.(*api.Status); !ok || s.Status != api.StatusSuccess { t.Errorf("delete return value was weird: %#v", obj) } if _, err := ms.Get(ctx, "bar"); err != ErrDoesNotExist { diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go index f6715a768bb..66d7e1c67de 100644 --- a/pkg/registry/pod/rest.go +++ b/pkg/registry/pod/rest.go @@ -88,7 +88,7 @@ func NewREST(config *RESTConfig) *REST { } } -func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { pod := obj.(*api.Pod) if !api.ValidNamespace(ctx, &pod.ObjectMeta) { return nil, errors.NewConflict("pod", pod.Namespace, fmt.Errorf("Pod.Namespace does not match the provided context")) @@ -111,7 +111,7 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Obje }), nil } -func (rs *REST) Delete(ctx api.Context, id string) (<-chan runtime.Object, error) { +func (rs *REST) Delete(ctx api.Context, id string) (<-chan apiserver.RESTResult, error) { return apiserver.MakeAsync(func() (runtime.Object, error) { return &api.Status{Status: api.StatusSuccess}, rs.registry.DeletePod(ctx, id) }), nil @@ -184,7 +184,7 @@ func (*REST) New() runtime.Object { return &api.Pod{} } -func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { pod := obj.(*api.Pod) if !api.ValidNamespace(ctx, &pod.ObjectMeta) { return nil, errors.NewConflict("pod", pod.Namespace, fmt.Errorf("Pod.Namespace does not match the provided context")) diff --git a/pkg/registry/pod/rest_test.go b/pkg/registry/pod/rest_test.go index 69123be3285..6e73a5c92b8 100644 --- a/pkg/registry/pod/rest_test.go +++ b/pkg/registry/pod/rest_test.go @@ -26,17 +26,17 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/fake" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" - "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) -func expectApiStatusError(t *testing.T, ch <-chan runtime.Object, msg string) { +func expectApiStatusError(t *testing.T, ch <-chan apiserver.RESTResult, msg string) { out := <-ch - status, ok := out.(*api.Status) + status, ok := out.Object.(*api.Status) if !ok { t.Errorf("Expected an api.Status object, was %#v", out) return @@ -46,9 +46,9 @@ func expectApiStatusError(t *testing.T, ch <-chan runtime.Object, msg string) { } } -func expectPod(t *testing.T, ch <-chan runtime.Object) (*api.Pod, bool) { +func expectPod(t *testing.T, ch <-chan apiserver.RESTResult) (*api.Pod, bool) { out := <-ch - pod, ok := out.(*api.Pod) + pod, ok := out.Object.(*api.Pod) if !ok || pod == nil { t.Errorf("Expected an api.Pod object, was %#v", out) return nil, false diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index 8ed59b5c413..d946e1e7e6c 100644 --- a/pkg/registry/service/rest.go +++ b/pkg/registry/service/rest.go @@ -79,7 +79,7 @@ func reloadIPsFromStorage(ipa *ipAllocator, registry Registry) { } } -func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { service := obj.(*api.Service) if !api.ValidNamespace(ctx, &service.ObjectMeta) { return nil, errors.NewConflict("service", service.Namespace, fmt.Errorf("Service.Namespace does not match the provided context")) @@ -144,7 +144,7 @@ func hostsFromMinionList(list *api.MinionList) []string { return result } -func (rs *REST) Delete(ctx api.Context, id string) (<-chan runtime.Object, error) { +func (rs *REST) Delete(ctx api.Context, id string) (<-chan apiserver.RESTResult, error) { service, err := rs.registry.GetService(ctx, id) if err != nil { return nil, err @@ -211,7 +211,7 @@ func GetServiceEnvironmentVariables(ctx api.Context, registry Registry, machine return result, nil } -func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) { +func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { service := obj.(*api.Service) if !api.ValidNamespace(ctx, &service.ObjectMeta) { return nil, errors.NewConflict("service", service.Namespace, fmt.Errorf("Service.Namespace does not match the provided context")) diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index 5f0a3fae827..5de1ca3a73f 100644 --- a/pkg/registry/service/rest_test.go +++ b/pkg/registry/service/rest_test.go @@ -52,7 +52,7 @@ func TestServiceRegistryCreate(t *testing.T) { ctx := api.NewDefaultContext() c, _ := storage.Create(ctx, svc) created_svc := <-c - created_service := created_svc.(*api.Service) + created_service := created_svc.Object.(*api.Service) if created_service.Name != "foo" { t.Errorf("Expected foo, but got %v", created_service.Name) } @@ -125,7 +125,7 @@ func TestServiceRegistryUpdate(t *testing.T) { t.Errorf("Expected no error") } updated_svc := <-c - updated_service := updated_svc.(*api.Service) + updated_service := updated_svc.Object.(*api.Service) if updated_service.Name != "foo" { t.Errorf("Expected foo, but got %v", updated_service.Name) } @@ -423,7 +423,7 @@ func TestServiceRegistryIPAllocation(t *testing.T) { ctx := api.NewDefaultContext() c1, _ := rest.Create(ctx, svc1) created_svc1 := <-c1 - created_service_1 := created_svc1.(*api.Service) + created_service_1 := created_svc1.Object.(*api.Service) if created_service_1.Name != "foo" { t.Errorf("Expected foo, but got %v", created_service_1.Name) } @@ -439,7 +439,7 @@ func TestServiceRegistryIPAllocation(t *testing.T) { ctx = api.NewDefaultContext() c2, _ := rest.Create(ctx, svc2) created_svc2 := <-c2 - created_service_2 := created_svc2.(*api.Service) + created_service_2 := created_svc2.Object.(*api.Service) if created_service_2.Name != "bar" { t.Errorf("Expected bar, but got %v", created_service_2.Name) } @@ -462,7 +462,7 @@ func TestServiceRegistryIPReallocation(t *testing.T) { ctx := api.NewDefaultContext() c1, _ := rest.Create(ctx, svc1) created_svc1 := <-c1 - created_service_1 := created_svc1.(*api.Service) + created_service_1 := created_svc1.Object.(*api.Service) if created_service_1.Name != "foo" { t.Errorf("Expected foo, but got %v", created_service_1.Name) } @@ -481,7 +481,7 @@ func TestServiceRegistryIPReallocation(t *testing.T) { ctx = api.NewDefaultContext() c2, _ := rest.Create(ctx, svc2) created_svc2 := <-c2 - created_service_2 := created_svc2.(*api.Service) + created_service_2 := created_svc2.Object.(*api.Service) if created_service_2.Name != "bar" { t.Errorf("Expected bar, but got %v", created_service_2.Name) } @@ -504,7 +504,7 @@ func TestServiceRegistryIPUpdate(t *testing.T) { ctx := api.NewDefaultContext() c, _ := rest.Create(ctx, svc) created_svc := <-c - created_service := created_svc.(*api.Service) + created_service := created_svc.Object.(*api.Service) if created_service.Port != 6502 { t.Errorf("Expected port 6502, but got %v", created_service.Port) } @@ -523,7 +523,7 @@ func TestServiceRegistryIPUpdate(t *testing.T) { c, _ = rest.Update(ctx, update) updated_svc := <-c - updated_service := updated_svc.(*api.Service) + updated_service := updated_svc.Object.(*api.Service) if updated_service.Port != 6503 { t.Errorf("Expected port 6503, but got %v", updated_service.Port) } @@ -550,7 +550,7 @@ func TestServiceRegistryIPExternalLoadBalancer(t *testing.T) { ctx := api.NewDefaultContext() c, _ := rest.Create(ctx, svc) created_svc := <-c - created_service := created_svc.(*api.Service) + created_service := created_svc.Object.(*api.Service) if created_service.Port != 6502 { t.Errorf("Expected port 6502, but got %v", created_service.Port) } @@ -594,7 +594,7 @@ func TestServiceRegistryIPReloadFromStorage(t *testing.T) { } c, _ = rest2.Create(ctx, svc) created_svc := <-c - created_service := created_svc.(*api.Service) + created_service := created_svc.Object.(*api.Service) if created_service.PortalIP != "1.2.3.3" { t.Errorf("Unexpected PortalIP: %s", created_service.PortalIP) }