diff --git a/pkg/registry/etcd/etcd.go b/pkg/registry/etcd/etcd.go index cabf7e2f59c..98b65c0bc28 100644 --- a/pkg/registry/etcd/etcd.go +++ b/pkg/registry/etcd/etcd.go @@ -157,7 +157,7 @@ func (r *Registry) CreateController(ctx api.Context, controller *api.Replication if err != nil { return err } - err = r.CreateObj(key, controller, 0) + err = r.CreateObj(key, controller, nil, 0) return etcderr.InterpretCreateError(err, "replicationController", controller.Name) } @@ -167,7 +167,7 @@ func (r *Registry) UpdateController(ctx api.Context, controller *api.Replication if err != nil { return err } - err = r.SetObj(key, controller, 0 /* ttl */) + err = r.SetObj(key, controller, nil, 0) return etcderr.InterpretUpdateError(err, "replicationController", controller.Name) } @@ -204,7 +204,7 @@ func (r *Registry) CreateService(ctx api.Context, svc *api.Service) error { if err != nil { return err } - err = r.CreateObj(key, svc, 0) + err = r.CreateObj(key, svc, nil, 0) return etcderr.InterpretCreateError(err, "service", svc.Name) } @@ -275,7 +275,7 @@ func (r *Registry) UpdateService(ctx api.Context, svc *api.Service) error { if err != nil { return err } - err = r.SetObj(key, svc, 0 /* ttl */) + err = r.SetObj(key, svc, nil, 0) return etcderr.InterpretUpdateError(err, "service", svc.Name) } @@ -362,13 +362,13 @@ func (r *Registry) ListMinions(ctx api.Context) (*api.NodeList, error) { func (r *Registry) CreateMinion(ctx api.Context, minion *api.Node) error { // TODO: Add some validations. - err := r.CreateObj(makeNodeKey(minion.Name), minion, 0) + err := r.CreateObj(makeNodeKey(minion.Name), minion, nil, 0) return etcderr.InterpretCreateError(err, "minion", minion.Name) } func (r *Registry) UpdateMinion(ctx api.Context, minion *api.Node) error { // TODO: Add some validations. - err := r.SetObj(makeNodeKey(minion.Name), minion, 0 /* ttl */) + err := r.SetObj(makeNodeKey(minion.Name), minion, nil, 0) return etcderr.InterpretUpdateError(err, "minion", minion.Name) } diff --git a/pkg/registry/generic/etcd/etcd.go b/pkg/registry/generic/etcd/etcd.go index 2b6c2b6404f..2f977e4074c 100644 --- a/pkg/registry/generic/etcd/etcd.go +++ b/pkg/registry/generic/etcd/etcd.go @@ -148,7 +148,7 @@ func (e *Etcd) CreateWithName(ctx api.Context, name string, obj runtime.Object) return err } } - err = e.Helper.CreateObj(key, obj, ttl) + err = e.Helper.CreateObj(key, obj, nil, ttl) err = etcderr.InterpretCreateError(err, e.EndpointName, name) if err == nil && e.Decorator != nil { err = e.Decorator(obj) @@ -177,7 +177,7 @@ func (e *Etcd) Create(ctx api.Context, obj runtime.Object) (runtime.Object, erro } } out := e.NewFunc() - if err := e.Helper.Create(key, obj, out, ttl); err != nil { + if err := e.Helper.CreateObj(key, obj, out, ttl); err != nil { err = etcderr.InterpretCreateError(err, e.EndpointName, name) err = rest.CheckGeneratedNameError(e.CreateStrategy, err, obj) return nil, err @@ -209,7 +209,7 @@ func (e *Etcd) UpdateWithName(ctx api.Context, name string, obj runtime.Object) return err } } - err = e.Helper.SetObj(key, obj, ttl) + err = e.Helper.SetObj(key, obj, nil, ttl) err = etcderr.InterpretUpdateError(err, e.EndpointName, name) if err == nil && e.Decorator != nil { err = e.Decorator(obj) diff --git a/pkg/tools/etcd_tools.go b/pkg/tools/etcd_tools.go index 751b03c320f..915feb21345 100644 --- a/pkg/tools/etcd_tools.go +++ b/pkg/tools/etcd_tools.go @@ -271,25 +271,9 @@ func (h *EtcdHelper) extractObj(response *etcd.Response, inErr error, objPtr run } // CreateObj adds a new object at a key unless it already exists. 'ttl' is time-to-live in seconds, -// and 0 means forever. -func (h *EtcdHelper) CreateObj(key string, obj runtime.Object, ttl uint64) error { - data, err := h.Codec.Encode(obj) - if err != nil { - return err - } - if h.ResourceVersioner != nil { - if version, err := h.ResourceVersioner.ResourceVersion(obj); err == nil && version != 0 { - return errors.New("resourceVersion may not be set on objects to be created") - } - } - - _, err = h.Client.Create(key, string(data), ttl) - return err -} - -// Create adds a new object at a key unless it already exists. 'ttl' is time-to-live in seconds, -// and 0 means forever. If no error is returned, out will be set to the read value from etcd. -func (h *EtcdHelper) Create(key string, obj, out runtime.Object, ttl uint64) error { +// and 0 means forever. If no error is returned and out is not nil, out will be set to the read value +// from etcd. +func (h *EtcdHelper) CreateObj(key string, obj, out runtime.Object, ttl uint64) error { data, err := h.Codec.Encode(obj) if err != nil { return err @@ -303,10 +287,12 @@ func (h *EtcdHelper) Create(key string, obj, out runtime.Object, ttl uint64) err if err != nil { return err } - if _, err := conversion.EnforcePtr(out); err != nil { - panic("unable to convert output object to pointer") + if out != nil { + if _, err := conversion.EnforcePtr(out); err != nil { + panic("unable to convert output object to pointer") + } + _, _, err = h.extractObj(response, err, out, false, false) } - _, _, err = h.extractObj(response, err, out, false, false) return err } @@ -332,21 +318,41 @@ func (h *EtcdHelper) DeleteObj(key string, out runtime.Object) error { } // SetObj marshals obj via json, and stores under key. Will do an atomic update if obj's ResourceVersion -// field is set. 'ttl' is time-to-live in seconds, and 0 means forever. -func (h *EtcdHelper) SetObj(key string, obj runtime.Object, ttl uint64) error { +// field is set. 'ttl' is time-to-live in seconds, and 0 means forever. If no error is returned and out is +//not nil, out will be set to the read value from etcd. +func (h *EtcdHelper) SetObj(key string, obj, out runtime.Object, ttl uint64) error { + var response *etcd.Response data, err := h.Codec.Encode(obj) if err != nil { return err } + + create := true if h.ResourceVersioner != nil { - if version, err := h.ResourceVersioner.ResourceVersion(obj); err == nil && version != 0 { - _, err = h.Client.CompareAndSwap(key, string(data), ttl, "", version) - return err // err is shadowed! + version, err := h.ResourceVersioner.ResourceVersion(obj) + if err == nil && version != 0 { + create = false + response, err = h.Client.CompareAndSwap(key, string(data), ttl, "", version) } } + if err != nil { + return err + } + if create { + // Create will fail if a key already exists. + response, err = h.Client.Create(key, string(data), ttl) + } + + if err != nil { + return err + } + if out != nil { + if _, err := conversion.EnforcePtr(out); err != nil { + panic("unable to convert output object to pointer") + } + _, _, err = h.extractObj(response, err, out, false, false) + } - // Create will fail if a key already exists. - _, err = h.Client.Create(key, string(data), ttl) return err } diff --git a/pkg/tools/etcd_tools_test.go b/pkg/tools/etcd_tools_test.go index bf68d90fbf4..472b30b9033 100644 --- a/pkg/tools/etcd_tools_test.go +++ b/pkg/tools/etcd_tools_test.go @@ -354,7 +354,7 @@ func TestCreateObj(t *testing.T) { obj := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}} fakeClient := NewFakeEtcdClient(t) helper := EtcdHelper{fakeClient, testapi.Codec(), versioner} - err := helper.CreateObj("/some/key", obj, 5) + err := helper.CreateObj("/some/key", obj, nil, 5) if err != nil { t.Errorf("Unexpected error %#v", err) } @@ -375,7 +375,7 @@ func TestSetObj(t *testing.T) { obj := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}} fakeClient := NewFakeEtcdClient(t) helper := EtcdHelper{fakeClient, testapi.Codec(), versioner} - err := helper.SetObj("/some/key", obj, 5) + err := helper.SetObj("/some/key", obj, nil, 5) if err != nil { t.Errorf("Unexpected error %#v", err) } @@ -408,7 +408,7 @@ func TestSetObjWithVersion(t *testing.T) { } helper := EtcdHelper{fakeClient, testapi.Codec(), versioner} - err := helper.SetObj("/some/key", obj, 7) + err := helper.SetObj("/some/key", obj, nil, 7) if err != nil { t.Fatalf("Unexpected error %#v", err) } @@ -430,7 +430,7 @@ func TestSetObjWithoutResourceVersioner(t *testing.T) { obj := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}} fakeClient := NewFakeEtcdClient(t) helper := EtcdHelper{fakeClient, testapi.Codec(), nil} - err := helper.SetObj("/some/key", obj, 3) + err := helper.SetObj("/some/key", obj, nil, 3) if err != nil { t.Errorf("Unexpected error %#v", err) } diff --git a/test/integration/etcd_tools_test.go b/test/integration/etcd_tools_test.go index 66c009cc6b3..1d26b6cc7ee 100644 --- a/test/integration/etcd_tools_test.go +++ b/test/integration/etcd_tools_test.go @@ -60,7 +60,7 @@ func TestSetObj(t *testing.T) { helper := tools.EtcdHelper{Client: client, Codec: stringCodec{}} withEtcdKey(func(key string) { fakeObject := fakeAPIObject("object") - if err := helper.SetObj(key, &fakeObject, 0 /* ttl */); err != nil { + if err := helper.SetObj(key, &fakeObject, nil, 0); err != nil { t.Fatalf("unexpected error: %v", err) } resp, err := client.Get(key, false, false)