RESTStorage should not need to know about async behavior

Also make sure all POST operations return 201 by default.
Removes the remainder of the asych logic in RESTStorage and
leaves it up to the API server to expose that behavior.
This commit is contained in:
Clayton Coleman
2015-02-10 12:49:56 -05:00
parent 79cb93002e
commit 26f08b7807
19 changed files with 333 additions and 442 deletions

View File

@@ -50,7 +50,7 @@ func NewREST(registry Registry, podLister PodLister) *REST {
}
// Create registers the given ReplicationController.
func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) {
func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, error) {
controller, ok := obj.(*api.ReplicationController)
if !ok {
return nil, fmt.Errorf("not a replication controller: %#v", obj)
@@ -60,20 +60,16 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE
return nil, err
}
return apiserver.MakeAsync(func() (runtime.Object, error) {
if err := rs.registry.CreateController(ctx, controller); err != nil {
err = rest.CheckGeneratedNameError(rest.ReplicationControllers, err, controller)
return apiserver.RESTResult{}, err
}
return rs.registry.GetController(ctx, controller.Name)
}), nil
if err := rs.registry.CreateController(ctx, controller); err != nil {
err = rest.CheckGeneratedNameError(rest.ReplicationControllers, err, controller)
return apiserver.RESTResult{}, err
}
return rs.registry.GetController(ctx, controller.Name)
}
// Delete asynchronously deletes the ReplicationController specified by its id.
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
func (rs *REST) Delete(ctx api.Context, id string) (runtime.Object, error) {
return &api.Status{Status: api.StatusSuccess}, rs.registry.DeleteController(ctx, id)
}
// Get obtains the ReplicationController specified by its id.
@@ -117,24 +113,23 @@ func (*REST) NewList() 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 apiserver.RESTResult, error) {
func (rs *REST) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool, error) {
controller, ok := obj.(*api.ReplicationController)
if !ok {
return nil, fmt.Errorf("not a replication controller: %#v", obj)
return nil, false, fmt.Errorf("not a replication controller: %#v", obj)
}
if !api.ValidNamespace(ctx, &controller.ObjectMeta) {
return nil, errors.NewConflict("controller", controller.Namespace, fmt.Errorf("Controller.Namespace does not match the provided context"))
return nil, false, errors.NewConflict("controller", controller.Namespace, fmt.Errorf("Controller.Namespace does not match the provided context"))
}
if errs := validation.ValidateReplicationController(controller); len(errs) > 0 {
return nil, errors.NewInvalid("replicationController", controller.Name, errs)
return nil, false, errors.NewInvalid("replicationController", controller.Name, errs)
}
return apiserver.MakeAsync(func() (runtime.Object, error) {
err := rs.registry.UpdateController(ctx, controller)
if err != nil {
return nil, err
}
return rs.registry.GetController(ctx, controller.Name)
}), nil
err := rs.registry.UpdateController(ctx, controller)
if err != nil {
return nil, false, err
}
out, err := rs.registry.GetController(ctx, controller.Name)
return out, false, err
}
// Watch returns ReplicationController events via a watch.Interface.

View File

@@ -22,7 +22,6 @@ import (
"io/ioutil"
"strings"
"testing"
"time"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
@@ -268,23 +267,17 @@ func TestCreateController(t *testing.T) {
},
}
ctx := api.NewDefaultContext()
channel, err := storage.Create(ctx, controller)
obj, err := storage.Create(ctx, controller)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if err != nil {
t.Errorf("unexpected error: %v", err)
if obj == nil {
t.Errorf("unexpected object")
}
if !api.HasObjectMetaSystemFieldValues(&controller.ObjectMeta) {
t.Errorf("storage did not populate object meta field values")
}
select {
case <-channel:
// expected case
case <-time.After(time.Millisecond * 100):
t.Error("Unexpected timeout from async channel")
}
}
// TODO: remove, covered by TestCreate
@@ -338,9 +331,9 @@ func TestControllerStorageValidatesUpdate(t *testing.T) {
}
ctx := api.NewDefaultContext()
for _, failureCase := range failureCases {
c, err := storage.Update(ctx, &failureCase)
if c != nil {
t.Errorf("Expected nil channel")
c, created, err := storage.Update(ctx, &failureCase)
if c != nil || created {
t.Errorf("Expected nil object and not created")
}
if !errors.IsInvalid(err) {
t.Errorf("Expected to get an invalid resource error, got %v", err)
@@ -441,9 +434,9 @@ func TestUpdateControllerWithConflictingNamespace(t *testing.T) {
}
ctx := api.NewDefaultContext()
channel, err := storage.Update(ctx, controller)
if channel != nil {
t.Error("Expected a nil channel, but we got a value")
obj, created, err := storage.Update(ctx, controller)
if obj != nil || created {
t.Error("Expected a nil object, but we got a value or created was true")
}
if err == nil {
t.Errorf("Expected an error, but we didn't get one")