Skip to content

Commit 4cd59e6

Browse files
lavalamplavalamp
committed
Merge pull request kubernetes#5010 from smarterclayton/update_resource_version
genericetcd.Etcd should test resourceVersion
2 parents 9ba020b + 3d52aac commit 4cd59e6

File tree

3 files changed

+268
-14
lines changed

3 files changed

+268
-14
lines changed

‎pkg/registry/generic/etcd/etcd.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ limitations under the License.
1717
package etcd
1818

1919
import (
20+
"fmt"
21+
2022
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
2123
kubeerr "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
2224
etcderr "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors/etcd"
@@ -228,6 +230,7 @@ func (e *Etcd) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool
228230
if err != nil {
229231
return nil, false, err
230232
}
233+
// TODO: expose TTL
231234
creating := false
232235
out := e.NewFunc()
233236
err = e.Helper.AtomicUpdate(key, out, true, func(existing runtime.Object) (runtime.Object, error) {
@@ -237,21 +240,30 @@ func (e *Etcd) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool
237240
}
238241
if version == 0 {
239242
if !e.UpdateStrategy.AllowCreateOnUpdate() {
240-
return nil, kubeerr.NewAlreadyExists(e.EndpointName, name)
243+
return nil, kubeerr.NewNotFound(e.EndpointName, name)
241244
}
242245
creating = true
243246
if err := rest.BeforeCreate(e.CreateStrategy, ctx, obj); err != nil {
244247
return nil, err
245248
}
246249
return obj, nil
247250
}
251+
248252
creating = false
253+
newVersion, err := e.Helper.ResourceVersioner.ResourceVersion(obj)
254+
if err != nil {
255+
return nil, err
256+
}
257+
if newVersion != version {
258+
// TODO: return the most recent version to a client?
259+
return nil, kubeerr.NewConflict(e.EndpointName, name, fmt.Errorf("the resource was updated to %d", version))
260+
}
249261
if err := rest.BeforeUpdate(e.UpdateStrategy, ctx, obj, existing); err != nil {
250262
return nil, err
251263
}
252-
// TODO: expose TTL
253264
return obj, nil
254265
})
266+
255267
if err != nil {
256268
if creating {
257269
err = etcderr.InterpretCreateError(err, e.EndpointName, name)

‎pkg/registry/generic/etcd/etcd_test.go‎

Lines changed: 252 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,52 @@ import (
3232
"github.com/coreos/go-etcd/etcd"
3333
)
3434

35+
type testRESTStrategy struct {
36+
runtime.ObjectTyper
37+
api.NameGenerator
38+
namespaceScoped bool
39+
allowCreateOnUpdate bool
40+
}
41+
42+
func (t *testRESTStrategy) NamespaceScoped() bool { return t.namespaceScoped }
43+
func (t *testRESTStrategy) AllowCreateOnUpdate() bool { return t.allowCreateOnUpdate }
44+
45+
func (t *testRESTStrategy) ResetBeforeCreate(obj runtime.Object) {}
46+
func (t *testRESTStrategy) Validate(obj runtime.Object) errors.ValidationErrorList {
47+
return nil
48+
}
49+
func (t *testRESTStrategy) ValidateUpdate(obj, old runtime.Object) errors.ValidationErrorList {
50+
return nil
51+
}
52+
53+
func hasCreated(t *testing.T, pod *api.Pod) func(runtime.Object) bool {
54+
return func(obj runtime.Object) bool {
55+
actualPod := obj.(*api.Pod)
56+
if !api.Semantic.DeepDerivative(pod.Status, actualPod.Status) {
57+
t.Errorf("not a deep derivative %#v", actualPod)
58+
return false
59+
}
60+
return api.HasObjectMetaSystemFieldValues(&actualPod.ObjectMeta)
61+
}
62+
}
63+
3564
func NewTestGenericEtcdRegistry(t *testing.T) (*tools.FakeEtcdClient, *Etcd) {
3665
f := tools.NewFakeEtcdClient(t)
3766
f.TestIndex = true
3867
h := tools.EtcdHelper{f, testapi.Codec(), tools.RuntimeVersionAdapter{testapi.MetadataAccessor()}}
68+
strategy := &testRESTStrategy{api.Scheme, api.SimpleNameGenerator, true, false}
3969
return f, &Etcd{
40-
NewFunc: func() runtime.Object { return &api.Pod{} },
41-
NewListFunc: func() runtime.Object { return &api.PodList{} },
42-
EndpointName: "pods",
43-
KeyRootFunc: func(ctx api.Context) string { return "/registry/pods" },
70+
NewFunc: func() runtime.Object { return &api.Pod{} },
71+
NewListFunc: func() runtime.Object { return &api.PodList{} },
72+
EndpointName: "pods",
73+
CreateStrategy: strategy,
74+
UpdateStrategy: strategy,
75+
KeyRootFunc: func(ctx api.Context) string { return "/registry/pods" },
4476
KeyFunc: func(ctx api.Context, id string) (string, error) {
4577
return path.Join("/registry/pods", id), nil
4678
},
47-
Helper: h,
79+
ObjectNameFunc: func(obj runtime.Object) (string, error) { return obj.(*api.Pod).Name, nil },
80+
Helper: h,
4881
}
4982
}
5083

@@ -153,11 +186,11 @@ func TestEtcdList(t *testing.T) {
153186

154187
func TestEtcdCreate(t *testing.T) {
155188
podA := &api.Pod{
156-
ObjectMeta: api.ObjectMeta{Name: "foo"},
189+
ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault},
157190
Status: api.PodStatus{Host: "machine"},
158191
}
159192
podB := &api.Pod{
160-
ObjectMeta: api.ObjectMeta{Name: "foo"},
193+
ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault},
161194
Status: api.PodStatus{Host: "machine2"},
162195
}
163196

@@ -178,18 +211,98 @@ func TestEtcdCreate(t *testing.T) {
178211
}
179212

180213
path := "/registry/pods/foo"
181-
key := "foo"
182214

183215
table := map[string]struct {
184216
existing tools.EtcdResponseWithError
185217
expect tools.EtcdResponseWithError
186218
toCreate runtime.Object
219+
objOK func(obj runtime.Object) bool
187220
errOK func(error) bool
188221
}{
189222
"normal": {
190223
existing: emptyNode,
224+
toCreate: podA,
225+
objOK: hasCreated(t, podA),
226+
errOK: func(err error) bool { return err == nil },
227+
},
228+
"preExisting": {
229+
existing: nodeWithPodA,
191230
expect: nodeWithPodA,
231+
toCreate: podB,
232+
errOK: errors.IsAlreadyExists,
233+
},
234+
}
235+
236+
for name, item := range table {
237+
fakeClient, registry := NewTestGenericEtcdRegistry(t)
238+
fakeClient.Data[path] = item.existing
239+
obj, err := registry.Create(api.NewDefaultContext(), item.toCreate)
240+
if !item.errOK(err) {
241+
t.Errorf("%v: unexpected error: %v", name, err)
242+
}
243+
244+
actual := fakeClient.Data[path]
245+
if item.objOK != nil {
246+
if !item.objOK(obj) {
247+
t.Errorf("%v: unexpected returned: %v", name, obj)
248+
}
249+
actualObj, err := api.Scheme.Decode([]byte(actual.R.Node.Value))
250+
if err != nil {
251+
t.Errorf("unable to decode stored value for %#v", actual)
252+
continue
253+
}
254+
if !item.objOK(actualObj) {
255+
t.Errorf("%v: unexpected response: %v", name, actual)
256+
}
257+
} else {
258+
if e, a := item.expect, actual; !api.Semantic.DeepDerivative(e, a) {
259+
t.Errorf("%v:\n%s", name, util.ObjectDiff(e, a))
260+
}
261+
}
262+
}
263+
}
264+
265+
// DEPRECATED
266+
func TestEtcdCreateWithName(t *testing.T) {
267+
podA := &api.Pod{
268+
ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault},
269+
Status: api.PodStatus{Host: "machine"},
270+
}
271+
podB := &api.Pod{
272+
ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault},
273+
Status: api.PodStatus{Host: "machine2"},
274+
}
275+
276+
nodeWithPodA := tools.EtcdResponseWithError{
277+
R: &etcd.Response{
278+
Node: &etcd.Node{
279+
Value: runtime.EncodeOrDie(testapi.Codec(), podA),
280+
ModifiedIndex: 1,
281+
CreatedIndex: 1,
282+
},
283+
},
284+
E: nil,
285+
}
286+
287+
emptyNode := tools.EtcdResponseWithError{
288+
R: &etcd.Response{},
289+
E: tools.EtcdErrorNotFound,
290+
}
291+
292+
path := "/registry/pods/foo"
293+
key := "foo"
294+
295+
table := map[string]struct {
296+
existing tools.EtcdResponseWithError
297+
expect tools.EtcdResponseWithError
298+
toCreate runtime.Object
299+
objOK func(obj runtime.Object) bool
300+
errOK func(error) bool
301+
}{
302+
"normal": {
303+
existing: emptyNode,
192304
toCreate: podA,
305+
objOK: hasCreated(t, podA),
193306
errOK: func(err error) bool { return err == nil },
194307
},
195308
"preExisting": {
@@ -203,18 +316,146 @@ func TestEtcdCreate(t *testing.T) {
203316
for name, item := range table {
204317
fakeClient, registry := NewTestGenericEtcdRegistry(t)
205318
fakeClient.Data[path] = item.existing
206-
err := registry.CreateWithName(api.NewContext(), key, item.toCreate)
319+
err := registry.CreateWithName(api.NewDefaultContext(), key, item.toCreate)
207320
if !item.errOK(err) {
208321
t.Errorf("%v: unexpected error: %v", name, err)
209322
}
210323

211-
if e, a := item.expect, fakeClient.Data[path]; !api.Semantic.DeepDerivative(e, a) {
212-
t.Errorf("%v:\n%s", name, util.ObjectDiff(e, a))
324+
actual := fakeClient.Data[path]
325+
if item.objOK != nil {
326+
obj, err := api.Scheme.Decode([]byte(actual.R.Node.Value))
327+
if err != nil {
328+
t.Errorf("unable to decode stored value for %#v", actual)
329+
continue
330+
}
331+
if !item.objOK(obj) {
332+
t.Errorf("%v: unexpected response: %v", name, actual)
333+
}
334+
} else {
335+
if e, a := item.expect, actual; !api.Semantic.DeepDerivative(e, a) {
336+
t.Errorf("%v:\n%s", name, util.ObjectDiff(e, a))
337+
}
213338
}
214339
}
215340
}
216341

217342
func TestEtcdUpdate(t *testing.T) {
343+
podA := &api.Pod{
344+
ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault},
345+
Status: api.PodStatus{Host: "machine"},
346+
}
347+
podB := &api.Pod{
348+
ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault, ResourceVersion: "1"},
349+
Status: api.PodStatus{Host: "machine2"},
350+
}
351+
352+
nodeWithPodA := tools.EtcdResponseWithError{
353+
R: &etcd.Response{
354+
Node: &etcd.Node{
355+
Value: runtime.EncodeOrDie(testapi.Codec(), podA),
356+
ModifiedIndex: 1,
357+
CreatedIndex: 1,
358+
},
359+
},
360+
E: nil,
361+
}
362+
363+
newerNodeWithPodA := tools.EtcdResponseWithError{
364+
R: &etcd.Response{
365+
Node: &etcd.Node{
366+
Value: runtime.EncodeOrDie(testapi.Codec(), podA),
367+
ModifiedIndex: 2,
368+
CreatedIndex: 1,
369+
},
370+
},
371+
E: nil,
372+
}
373+
374+
nodeWithPodB := tools.EtcdResponseWithError{
375+
R: &etcd.Response{
376+
Node: &etcd.Node{
377+
Value: runtime.EncodeOrDie(testapi.Codec(), podB),
378+
ModifiedIndex: 1,
379+
CreatedIndex: 1,
380+
},
381+
},
382+
E: nil,
383+
}
384+
385+
emptyNode := tools.EtcdResponseWithError{
386+
R: &etcd.Response{},
387+
E: tools.EtcdErrorNotFound,
388+
}
389+
390+
path := "/registry/pods/foo"
391+
392+
table := map[string]struct {
393+
existing tools.EtcdResponseWithError
394+
expect tools.EtcdResponseWithError
395+
toUpdate runtime.Object
396+
allowCreate bool
397+
objOK func(obj runtime.Object) bool
398+
errOK func(error) bool
399+
}{
400+
"normal": {
401+
existing: nodeWithPodA,
402+
expect: nodeWithPodB,
403+
toUpdate: podB,
404+
errOK: func(err error) bool { return err == nil },
405+
},
406+
"notExisting": {
407+
existing: emptyNode,
408+
expect: emptyNode,
409+
toUpdate: podA,
410+
errOK: func(err error) bool { return errors.IsNotFound(err) },
411+
},
412+
"createIfNotFound": {
413+
existing: emptyNode,
414+
toUpdate: podA,
415+
allowCreate: true,
416+
objOK: hasCreated(t, podA),
417+
errOK: func(err error) bool { return err == nil },
418+
},
419+
"outOfDate": {
420+
existing: newerNodeWithPodA,
421+
expect: newerNodeWithPodA,
422+
toUpdate: podB,
423+
errOK: func(err error) bool { return errors.IsConflict(err) },
424+
},
425+
}
426+
427+
for name, item := range table {
428+
fakeClient, registry := NewTestGenericEtcdRegistry(t)
429+
registry.UpdateStrategy.(*testRESTStrategy).allowCreateOnUpdate = item.allowCreate
430+
fakeClient.Data[path] = item.existing
431+
obj, _, err := registry.Update(api.NewDefaultContext(), item.toUpdate)
432+
if !item.errOK(err) {
433+
t.Errorf("%v: unexpected error: %v", name, err)
434+
}
435+
436+
actual := fakeClient.Data[path]
437+
if item.objOK != nil {
438+
if !item.objOK(obj) {
439+
t.Errorf("%v: unexpected returned: %#v", name, obj)
440+
}
441+
actualObj, err := api.Scheme.Decode([]byte(actual.R.Node.Value))
442+
if err != nil {
443+
t.Errorf("unable to decode stored value for %#v", actual)
444+
continue
445+
}
446+
if !item.objOK(actualObj) {
447+
t.Errorf("%v: unexpected response: %#v", name, actual)
448+
}
449+
} else {
450+
if e, a := item.expect, actual; !api.Semantic.DeepDerivative(e, a) {
451+
t.Errorf("%v:\n%s", name, util.ObjectDiff(e, a))
452+
}
453+
}
454+
}
455+
}
456+
457+
// DEPRECATED
458+
func TestEtcdUpdateWithName(t *testing.T) {
218459
podA := &api.Pod{
219460
ObjectMeta: api.ObjectMeta{Name: "foo"},
220461
Status: api.PodStatus{Host: "machine"},

‎pkg/tools/etcd_tools.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,10 +404,11 @@ func (h *EtcdHelper) AtomicUpdate(key string, ptrToType runtime.Object, ignoreNo
404404

405405
// First time this key has been used, try creating new value.
406406
if index == 0 {
407-
_, err = h.Client.Create(key, string(data), 0)
407+
response, err := h.Client.Create(key, string(data), 0)
408408
if IsEtcdNodeExist(err) {
409409
continue
410410
}
411+
_, _, err = h.extractObj(response, err, ptrToType, false, false)
411412
return err
412413
}
413414

0 commit comments

Comments
 (0)