SetObj and CreateObj optionally accept an object to fill with the res…

…ult of the get
duda · Mar 4, 2015 · e1ca63f · e1ca63f
1 parent da46d04
commit e1ca63f
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 43 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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)