From 5cbf89c060a790994b2484b2dffe2f42fd7d862e Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Fri, 27 Feb 2015 16:37:04 -0800 Subject: [PATCH] plumb through changes on ReplicationController and Service --- pkg/registry/controller/registry.go | 4 +-- pkg/registry/controller/rest.go | 13 +++------ pkg/registry/etcd/etcd.go | 36 ++++++++++++++----------- pkg/registry/etcd/etcd_test.go | 12 ++++----- pkg/registry/registrytest/controller.go | 8 +++--- pkg/registry/registrytest/service.go | 8 +++--- pkg/registry/service/registry.go | 4 +-- pkg/registry/service/rest.go | 12 +++------ 8 files changed, 46 insertions(+), 51 deletions(-) diff --git a/pkg/registry/controller/registry.go b/pkg/registry/controller/registry.go index 9943eb4f007..7ce8eb8e5fe 100644 --- a/pkg/registry/controller/registry.go +++ b/pkg/registry/controller/registry.go @@ -27,7 +27,7 @@ type Registry interface { ListControllers(ctx api.Context) (*api.ReplicationControllerList, error) WatchControllers(ctx api.Context, label, field labels.Selector, resourceVersion string) (watch.Interface, error) GetController(ctx api.Context, controllerID string) (*api.ReplicationController, error) - CreateController(ctx api.Context, controller *api.ReplicationController) error - UpdateController(ctx api.Context, controller *api.ReplicationController) error + CreateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) + UpdateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) DeleteController(ctx api.Context, controllerID string) error } diff --git a/pkg/registry/controller/rest.go b/pkg/registry/controller/rest.go index 4ea56e49618..ef2c2f65422 100644 --- a/pkg/registry/controller/rest.go +++ b/pkg/registry/controller/rest.go @@ -23,7 +23,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" - "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" rc "github.com/GoogleCloudPlatform/kubernetes/pkg/controller" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" @@ -60,11 +59,11 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err return nil, err } - if err := rs.registry.CreateController(ctx, controller); err != nil { + out, err := rs.registry.CreateController(ctx, controller) + if err != nil { err = rest.CheckGeneratedNameError(rest.ReplicationControllers, err, controller) - return apiserver.RESTResult{}, err } - return rs.registry.GetController(ctx, controller.Name) + return out, err } // Delete asynchronously deletes the ReplicationController specified by its id. @@ -124,11 +123,7 @@ func (rs *REST) Update(ctx api.Context, obj runtime.Object) (runtime.Object, boo if errs := validation.ValidateReplicationController(controller); len(errs) > 0 { return nil, false, errors.NewInvalid("replicationController", controller.Name, errs) } - err := rs.registry.UpdateController(ctx, controller) - if err != nil { - return nil, false, err - } - out, err := rs.registry.GetController(ctx, controller.Name) + out, err := rs.registry.UpdateController(ctx, controller) return out, false, err } diff --git a/pkg/registry/etcd/etcd.go b/pkg/registry/etcd/etcd.go index 98b65c0bc28..fced78a806e 100644 --- a/pkg/registry/etcd/etcd.go +++ b/pkg/registry/etcd/etcd.go @@ -152,23 +152,25 @@ func (r *Registry) GetController(ctx api.Context, controllerID string) (*api.Rep } // CreateController creates a new ReplicationController. -func (r *Registry) CreateController(ctx api.Context, controller *api.ReplicationController) error { +func (r *Registry) CreateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) { key, err := makeControllerKey(ctx, controller.Name) if err != nil { - return err + return nil, err } - err = r.CreateObj(key, controller, nil, 0) - return etcderr.InterpretCreateError(err, "replicationController", controller.Name) + out := &api.ReplicationController{} + err = r.CreateObj(key, controller, out, 0) + return out, etcderr.InterpretCreateError(err, "replicationController", controller.Name) } // UpdateController replaces an existing ReplicationController. -func (r *Registry) UpdateController(ctx api.Context, controller *api.ReplicationController) error { +func (r *Registry) UpdateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) { key, err := makeControllerKey(ctx, controller.Name) if err != nil { - return err + return nil, err } - err = r.SetObj(key, controller, nil, 0) - return etcderr.InterpretUpdateError(err, "replicationController", controller.Name) + out := &api.ReplicationController{} + err = r.SetObj(key, controller, out, 0) + return out, etcderr.InterpretUpdateError(err, "replicationController", controller.Name) } // DeleteController deletes a ReplicationController specified by its ID. @@ -199,13 +201,14 @@ func (r *Registry) ListServices(ctx api.Context) (*api.ServiceList, error) { } // CreateService creates a new Service. -func (r *Registry) CreateService(ctx api.Context, svc *api.Service) error { +func (r *Registry) CreateService(ctx api.Context, svc *api.Service) (*api.Service, error) { key, err := makeServiceKey(ctx, svc.Name) if err != nil { - return err + return nil, err } - err = r.CreateObj(key, svc, nil, 0) - return etcderr.InterpretCreateError(err, "service", svc.Name) + out := &api.Service{} + err = r.CreateObj(key, svc, out, 0) + return out, etcderr.InterpretCreateError(err, "service", svc.Name) } // GetService obtains a Service specified by its name. @@ -270,13 +273,14 @@ func (r *Registry) DeleteService(ctx api.Context, name string) error { } // UpdateService replaces an existing Service. -func (r *Registry) UpdateService(ctx api.Context, svc *api.Service) error { +func (r *Registry) UpdateService(ctx api.Context, svc *api.Service) (*api.Service, error) { key, err := makeServiceKey(ctx, svc.Name) if err != nil { - return err + return nil, err } - err = r.SetObj(key, svc, nil, 0) - return etcderr.InterpretUpdateError(err, "service", svc.Name) + out := &api.Service{} + err = r.SetObj(key, svc, out, 0) + return out, etcderr.InterpretUpdateError(err, "service", svc.Name) } // WatchServices begins watching for new, changed, or deleted service configurations. diff --git a/pkg/registry/etcd/etcd_test.go b/pkg/registry/etcd/etcd_test.go index 0990bad0057..c07a8257ca5 100644 --- a/pkg/registry/etcd/etcd_test.go +++ b/pkg/registry/etcd/etcd_test.go @@ -212,7 +212,7 @@ func TestEtcdCreateController(t *testing.T) { fakeClient := tools.NewFakeEtcdClient(t) registry := NewTestEtcdRegistry(fakeClient) key, _ := makeControllerKey(ctx, "foo") - err := registry.CreateController(ctx, &api.ReplicationController{ + _, err := registry.CreateController(ctx, &api.ReplicationController{ ObjectMeta: api.ObjectMeta{ Name: "foo", }, @@ -242,7 +242,7 @@ func TestEtcdCreateControllerAlreadyExisting(t *testing.T) { fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &api.ReplicationController{ObjectMeta: api.ObjectMeta{Name: "foo"}}), 0) registry := NewTestEtcdRegistry(fakeClient) - err := registry.CreateController(ctx, &api.ReplicationController{ + _, err := registry.CreateController(ctx, &api.ReplicationController{ ObjectMeta: api.ObjectMeta{ Name: "foo", }, @@ -259,7 +259,7 @@ func TestEtcdUpdateController(t *testing.T) { key, _ := makeControllerKey(ctx, "foo") resp, _ := fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &api.ReplicationController{ObjectMeta: api.ObjectMeta{Name: "foo"}}), 0) registry := NewTestEtcdRegistry(fakeClient) - err := registry.UpdateController(ctx, &api.ReplicationController{ + _, err := registry.UpdateController(ctx, &api.ReplicationController{ ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: strconv.FormatUint(resp.Node.ModifiedIndex, 10)}, Spec: api.ReplicationControllerSpec{ Replicas: 2, @@ -417,7 +417,7 @@ func TestEtcdCreateService(t *testing.T) { ctx := api.NewDefaultContext() fakeClient := tools.NewFakeEtcdClient(t) registry := NewTestEtcdRegistry(fakeClient) - err := registry.CreateService(ctx, &api.Service{ + _, err := registry.CreateService(ctx, &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo"}, }) if err != nil { @@ -447,7 +447,7 @@ func TestEtcdCreateServiceAlreadyExisting(t *testing.T) { key, _ := makeServiceKey(ctx, "foo") fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &api.Service{ObjectMeta: api.ObjectMeta{Name: "foo"}}), 0) registry := NewTestEtcdRegistry(fakeClient) - err := registry.CreateService(ctx, &api.Service{ + _, err := registry.CreateService(ctx, &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo"}, }) if !errors.IsAlreadyExists(err) { @@ -572,7 +572,7 @@ func TestEtcdUpdateService(t *testing.T) { SessionAffinity: "None", }, } - err := registry.UpdateService(ctx, &testService) + _, err := registry.UpdateService(ctx, &testService) if err != nil { t.Errorf("unexpected error: %v", err) } diff --git a/pkg/registry/registrytest/controller.go b/pkg/registry/registrytest/controller.go index b36fd836a4c..40ff0335235 100644 --- a/pkg/registry/registrytest/controller.go +++ b/pkg/registry/registrytest/controller.go @@ -49,16 +49,16 @@ func (r *ControllerRegistry) GetController(ctx api.Context, ID string) (*api.Rep return &api.ReplicationController{}, r.Err } -func (r *ControllerRegistry) CreateController(ctx api.Context, controller *api.ReplicationController) error { +func (r *ControllerRegistry) CreateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) { r.Lock() defer r.Unlock() - return r.Err + return controller, r.Err } -func (r *ControllerRegistry) UpdateController(ctx api.Context, controller *api.ReplicationController) error { +func (r *ControllerRegistry) UpdateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) { r.Lock() defer r.Unlock() - return r.Err + return controller, r.Err } func (r *ControllerRegistry) DeleteController(ctx api.Context, ID string) error { diff --git a/pkg/registry/registrytest/service.go b/pkg/registry/registrytest/service.go index d1cb2ff63a0..28349b3f5bd 100644 --- a/pkg/registry/registrytest/service.go +++ b/pkg/registry/registrytest/service.go @@ -71,14 +71,14 @@ func (r *ServiceRegistry) ListServices(ctx api.Context) (*api.ServiceList, error return res, r.Err } -func (r *ServiceRegistry) CreateService(ctx api.Context, svc *api.Service) error { +func (r *ServiceRegistry) CreateService(ctx api.Context, svc *api.Service) (*api.Service, error) { r.mu.Lock() defer r.mu.Unlock() r.Service = new(api.Service) *r.Service = *svc r.List.Items = append(r.List.Items, *svc) - return r.Err + return svc, r.Err } func (r *ServiceRegistry) GetService(ctx api.Context, id string) (*api.Service, error) { @@ -98,13 +98,13 @@ func (r *ServiceRegistry) DeleteService(ctx api.Context, id string) error { return r.Err } -func (r *ServiceRegistry) UpdateService(ctx api.Context, svc *api.Service) error { +func (r *ServiceRegistry) UpdateService(ctx api.Context, svc *api.Service) (*api.Service, error) { r.mu.Lock() defer r.mu.Unlock() r.UpdatedID = svc.Name *r.Service = *svc - return r.Err + return svc, r.Err } func (r *ServiceRegistry) WatchServices(ctx api.Context, label labels.Selector, field labels.Selector, resourceVersion string) (watch.Interface, error) { diff --git a/pkg/registry/service/registry.go b/pkg/registry/service/registry.go index dc6e95eef42..478e6633415 100644 --- a/pkg/registry/service/registry.go +++ b/pkg/registry/service/registry.go @@ -26,10 +26,10 @@ import ( // Registry is an interface for things that know how to store services. type Registry interface { ListServices(ctx api.Context) (*api.ServiceList, error) - CreateService(ctx api.Context, svc *api.Service) error + CreateService(ctx api.Context, svc *api.Service) (*api.Service, error) GetService(ctx api.Context, name string) (*api.Service, error) DeleteService(ctx api.Context, name string) error - UpdateService(ctx api.Context, svc *api.Service) error + UpdateService(ctx api.Context, svc *api.Service) (*api.Service, error) WatchServices(ctx api.Context, labels, fields labels.Selector, resourceVersion string) (watch.Interface, error) // TODO: endpoints and their implementation should be separated, setting endpoints should be diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index 2aa907726ea..9d48e96a23d 100644 --- a/pkg/registry/service/rest.go +++ b/pkg/registry/service/rest.go @@ -115,12 +115,12 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err } } - if err := rs.registry.CreateService(ctx, service); err != nil { + out, err := rs.registry.CreateService(ctx, service) + if err != nil { rs.portalMgr.Release(net.ParseIP(service.Spec.PortalIP)) err = rest.CheckGeneratedNameError(rest.Services, err, service) - return nil, err } - return rs.registry.GetService(ctx, service.Name) + return out, err } func hostsFromMinionList(list *api.NodeList) []string { @@ -213,11 +213,7 @@ func (rs *REST) Update(ctx api.Context, obj runtime.Object) (runtime.Object, boo } } } - err = rs.registry.UpdateService(ctx, service) - if err != nil { - return nil, false, err - } - out, err := rs.registry.GetService(ctx, service.Name) + out, err := rs.registry.UpdateService(ctx, service) return out, false, err }