From b82bb37c897183652031dba14b8b3e8122fb1b55 Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Wed, 30 Jul 2014 19:31:35 +0900 Subject: [PATCH 1/8] FakeEtcdClient: Maintain change index --- pkg/tools/fake_etcd_client.go | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/pkg/tools/fake_etcd_client.go b/pkg/tools/fake_etcd_client.go index becf7f3a8d4..3a6be8b9b29 100644 --- a/pkg/tools/fake_etcd_client.go +++ b/pkg/tools/fake_etcd_client.go @@ -41,6 +41,7 @@ type FakeEtcdClient struct { Err error t TestLogger Ix int + ChangeIndex uint64 // Will become valid after Watch is called; tester may write to it. Tester may // also read from it to verify that it's closed after injecting an error. @@ -70,6 +71,11 @@ func MakeFakeEtcdClient(t TestLogger) *FakeEtcdClient { return ret } +func (f *FakeEtcdClient) generateIndex() uint64 { + f.ChangeIndex++ + return f.ChangeIndex +} + func (f *FakeEtcdClient) AddChild(key, data string, ttl uint64) (*etcd.Response, error) { f.Ix = f.Ix + 1 return f.Set(fmt.Sprintf("%s/%d", key, f.Ix), data, ttl) @@ -86,10 +92,29 @@ func (f *FakeEtcdClient) Get(key string, sort, recursive bool) (*etcd.Response, } func (f *FakeEtcdClient) Set(key, value string, ttl uint64) (*etcd.Response, error) { + i := f.generateIndex() + + if prevResult, ok := f.Data[key]; ok && prevResult.R != nil && prevResult.R.Node != nil { + createdIndex := prevResult.R.Node.CreatedIndex + result := EtcdResponseWithError{ + R: &etcd.Response{ + Node: &etcd.Node{ + Value: value, + CreatedIndex: createdIndex, + ModifiedIndex: i, + }, + }, + } + f.Data[key] = result + return result.R, f.Err + } + result := EtcdResponseWithError{ R: &etcd.Response{ Node: &etcd.Node{ - Value: value, + Value: value, + CreatedIndex: i, + ModifiedIndex: i, }, }, } From e40cdd50edcea094cce4abacab5bca298547627a Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Thu, 31 Jul 2014 16:01:28 +0900 Subject: [PATCH 2/8] Generate modification index only if f.TestIndex is specified. This is for compatibility with existing tests. --- pkg/tools/fake_etcd_client.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/tools/fake_etcd_client.go b/pkg/tools/fake_etcd_client.go index 3a6be8b9b29..d4ae285e3a8 100644 --- a/pkg/tools/fake_etcd_client.go +++ b/pkg/tools/fake_etcd_client.go @@ -41,6 +41,7 @@ type FakeEtcdClient struct { Err error t TestLogger Ix int + TestIndex bool ChangeIndex uint64 // Will become valid after Watch is called; tester may write to it. Tester may @@ -72,6 +73,10 @@ func MakeFakeEtcdClient(t TestLogger) *FakeEtcdClient { } func (f *FakeEtcdClient) generateIndex() uint64 { + if !f.TestIndex { + return 0 + } + f.ChangeIndex++ return f.ChangeIndex } From d46bfcb132bb188e391005ff58fb4b0174b9290b Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Thu, 31 Jul 2014 16:15:03 +0900 Subject: [PATCH 3/8] Fail immediately if f.Err is set --- pkg/tools/fake_etcd_client.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/tools/fake_etcd_client.go b/pkg/tools/fake_etcd_client.go index d4ae285e3a8..452c7b14f15 100644 --- a/pkg/tools/fake_etcd_client.go +++ b/pkg/tools/fake_etcd_client.go @@ -97,6 +97,10 @@ func (f *FakeEtcdClient) Get(key string, sort, recursive bool) (*etcd.Response, } func (f *FakeEtcdClient) Set(key, value string, ttl uint64) (*etcd.Response, error) { + if f.Err != nil { + return nil, f.Err + } + i := f.generateIndex() if prevResult, ok := f.Data[key]; ok && prevResult.R != nil && prevResult.R.Node != nil { @@ -111,7 +115,7 @@ func (f *FakeEtcdClient) Set(key, value string, ttl uint64) (*etcd.Response, err }, } f.Data[key] = result - return result.R, f.Err + return result.R, nil } result := EtcdResponseWithError{ @@ -124,7 +128,7 @@ func (f *FakeEtcdClient) Set(key, value string, ttl uint64) (*etcd.Response, err }, } f.Data[key] = result - return result.R, f.Err + return result.R, nil } func (f *FakeEtcdClient) CompareAndSwap(key, value string, ttl uint64, prevValue string, prevIndex uint64) (*etcd.Response, error) { @@ -136,6 +140,10 @@ func (f *FakeEtcdClient) Create(key, value string, ttl uint64) (*etcd.Response, return f.Set(key, value, ttl) } func (f *FakeEtcdClient) Delete(key string, recursive bool) (*etcd.Response, error) { + if f.Err != nil { + return f.Err + } + f.Data[key] = EtcdResponseWithError{ R: &etcd.Response{ Node: nil, @@ -144,7 +152,7 @@ func (f *FakeEtcdClient) Delete(key string, recursive bool) (*etcd.Response, err } f.DeletedKeys = append(f.DeletedKeys, key) - return &etcd.Response{}, f.Err + return &etcd.Response{}, nil } func (f *FakeEtcdClient) WaitForWatchCompletion() { From ccb7e8793bf2aecb09d899e6242ffdc0e1163ac6 Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Thu, 31 Jul 2014 18:23:43 +0900 Subject: [PATCH 4/8] return EtcdErrorNodeExist when Create is called for existing node --- pkg/tools/etcd_tools.go | 2 ++ pkg/tools/fake_etcd_client.go | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/tools/etcd_tools.go b/pkg/tools/etcd_tools.go index 4472b32a10e..5a95c1945dc 100644 --- a/pkg/tools/etcd_tools.go +++ b/pkg/tools/etcd_tools.go @@ -30,11 +30,13 @@ import ( const ( EtcdErrorCodeNotFound = 100 + EtcdErrorCodeNodeExist = 105 EtcdErrorCodeValueRequired = 200 ) var ( EtcdErrorNotFound = &etcd.EtcdError{ErrorCode: EtcdErrorCodeNotFound} + EtcdErrorNodeExist = &etcd.EtcdError{ErrorCode: EtcdErrorCodeNodeExist} EtcdErrorValueRequired = &etcd.EtcdError{ErrorCode: EtcdErrorCodeValueRequired} ) diff --git a/pkg/tools/fake_etcd_client.go b/pkg/tools/fake_etcd_client.go index 452c7b14f15..951264fa3a9 100644 --- a/pkg/tools/fake_etcd_client.go +++ b/pkg/tools/fake_etcd_client.go @@ -137,11 +137,16 @@ func (f *FakeEtcdClient) CompareAndSwap(key, value string, ttl uint64, prevValue } func (f *FakeEtcdClient) Create(key, value string, ttl uint64) (*etcd.Response, error) { + if prevResult, ok := f.Data[key]; ok && prevResult.R != nil && prevResult.R.Node != nil { + return nil, EtcdErrorNodeExist + } + return f.Set(key, value, ttl) } + func (f *FakeEtcdClient) Delete(key string, recursive bool) (*etcd.Response, error) { if f.Err != nil { - return f.Err + return nil, f.Err } f.Data[key] = EtcdResponseWithError{ From 6dd1e9cbb5620736cf0ed93d119838f158e4f301 Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Thu, 31 Jul 2014 19:25:42 +0900 Subject: [PATCH 5/8] Implement FakeEtcdClient.CompareAndSwap --- pkg/tools/etcd_tools.go | 2 ++ pkg/tools/fake_etcd_client.go | 39 ++++++++++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/pkg/tools/etcd_tools.go b/pkg/tools/etcd_tools.go index 5a95c1945dc..c835b66fe09 100644 --- a/pkg/tools/etcd_tools.go +++ b/pkg/tools/etcd_tools.go @@ -30,12 +30,14 @@ import ( const ( EtcdErrorCodeNotFound = 100 + EtcdErrorCodeTestFailed = 101 EtcdErrorCodeNodeExist = 105 EtcdErrorCodeValueRequired = 200 ) var ( EtcdErrorNotFound = &etcd.EtcdError{ErrorCode: EtcdErrorCodeNotFound} + EtcdErrorTestFailed = &etcd.EtcdError{ErrorCode: EtcdErrorCodeTestFailed} EtcdErrorNodeExist = &etcd.EtcdError{ErrorCode: EtcdErrorCodeNodeExist} EtcdErrorValueRequired = &etcd.EtcdError{ErrorCode: EtcdErrorCodeValueRequired} ) diff --git a/pkg/tools/fake_etcd_client.go b/pkg/tools/fake_etcd_client.go index 951264fa3a9..fedf56fd789 100644 --- a/pkg/tools/fake_etcd_client.go +++ b/pkg/tools/fake_etcd_client.go @@ -17,6 +17,7 @@ limitations under the License. package tools import ( + "errors" "fmt" "github.com/coreos/go-etcd/etcd" @@ -96,6 +97,11 @@ func (f *FakeEtcdClient) Get(key string, sort, recursive bool) (*etcd.Response, return result.R, result.E } +func (f *FakeEtcdClient) nodeExists(key string) bool { + result, ok := f.Data[key] + return ok && result.R != nil && result.R.Node != nil +} + func (f *FakeEtcdClient) Set(key, value string, ttl uint64) (*etcd.Response, error) { if f.Err != nil { return nil, f.Err @@ -103,7 +109,8 @@ func (f *FakeEtcdClient) Set(key, value string, ttl uint64) (*etcd.Response, err i := f.generateIndex() - if prevResult, ok := f.Data[key]; ok && prevResult.R != nil && prevResult.R.Node != nil { + if f.nodeExists(key) { + prevResult := f.Data[key] createdIndex := prevResult.R.Node.CreatedIndex result := EtcdResponseWithError{ R: &etcd.Response{ @@ -132,12 +139,38 @@ func (f *FakeEtcdClient) Set(key, value string, ttl uint64) (*etcd.Response, err } func (f *FakeEtcdClient) CompareAndSwap(key, value string, ttl uint64, prevValue string, prevIndex uint64) (*etcd.Response, error) { - // TODO: Maybe actually implement compare and swap here? + if f.Err != nil { + return nil, f.Err + } + + if !f.TestIndex { + f.t.Errorf("Enable TestIndex for test involving CompareAndSwap") + return nil, errors.New("Enable TestIndex for test involving CompareAndSwap") + } + + if prevValue == "" && prevIndex == 0 { + return nil, errors.New("Either prevValue or prevIndex must be specified.") + } + + if !f.nodeExists(key) { + return nil, EtcdErrorNotFound + } + + prevNode := f.Data[key].R.Node + + if prevValue != "" && prevValue != prevNode.Value { + return nil, EtcdErrorTestFailed + } + + if prevIndex != 0 && prevIndex != prevNode.ModifiedIndex { + return nil, EtcdErrorTestFailed + } + return f.Set(key, value, ttl) } func (f *FakeEtcdClient) Create(key, value string, ttl uint64) (*etcd.Response, error) { - if prevResult, ok := f.Data[key]; ok && prevResult.R != nil && prevResult.R.Node != nil { + if f.nodeExists(key) { return nil, EtcdErrorNodeExist } From a3771c90425cff6809e6a4f3e6b396632bb826d8 Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Thu, 31 Jul 2014 20:32:07 +0900 Subject: [PATCH 6/8] AtomicUpdate should use api.Encode --- pkg/tools/etcd_tools.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/tools/etcd_tools.go b/pkg/tools/etcd_tools.go index c835b66fe09..2f7bce4ef5b 100644 --- a/pkg/tools/etcd_tools.go +++ b/pkg/tools/etcd_tools.go @@ -17,7 +17,6 @@ limitations under the License. package tools import ( - "encoding/json" "fmt" "reflect" @@ -225,7 +224,7 @@ func (h *EtcdHelper) AtomicUpdate(key string, ptrToType interface{}, tryUpdate E return h.SetObj(key, ret) } - data, err := json.Marshal(ret) + data, err := api.Encode(ret) if err != nil { return err } From 648b80e5d711edc7d7d77457fedf012b81a53378 Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Thu, 31 Jul 2014 20:33:18 +0900 Subject: [PATCH 7/8] Implement FakeEtcdClient.ExpectNotFoundGet --- pkg/tools/fake_etcd_client.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/pkg/tools/fake_etcd_client.go b/pkg/tools/fake_etcd_client.go index fedf56fd789..0bd88f53164 100644 --- a/pkg/tools/fake_etcd_client.go +++ b/pkg/tools/fake_etcd_client.go @@ -37,13 +37,14 @@ type TestLogger interface { type FakeEtcdClient struct { watchCompletedChan chan bool - Data map[string]EtcdResponseWithError - DeletedKeys []string - Err error - t TestLogger - Ix int - TestIndex bool - ChangeIndex uint64 + Data map[string]EtcdResponseWithError + DeletedKeys []string + expectNotFoundGetSet map[string]struct{} + Err error + t TestLogger + Ix int + TestIndex bool + ChangeIndex uint64 // Will become valid after Watch is called; tester may write to it. Tester may // also read from it to verify that it's closed after injecting an error. @@ -55,8 +56,9 @@ type FakeEtcdClient struct { func MakeFakeEtcdClient(t TestLogger) *FakeEtcdClient { ret := &FakeEtcdClient{ - t: t, - Data: map[string]EtcdResponseWithError{}, + t: t, + expectNotFoundGetSet: map[string]struct{}{}, + Data: map[string]EtcdResponseWithError{}, } // There are three publicly accessible channels in FakeEtcdClient: // - WatchResponse @@ -73,6 +75,10 @@ func MakeFakeEtcdClient(t TestLogger) *FakeEtcdClient { return ret } +func (f *FakeEtcdClient) ExpectNotFoundGet(key string) { + f.expectNotFoundGetSet[key] = struct{}{} +} + func (f *FakeEtcdClient) generateIndex() uint64 { if !f.TestIndex { return 0 @@ -90,7 +96,9 @@ func (f *FakeEtcdClient) AddChild(key, data string, ttl uint64) (*etcd.Response, func (f *FakeEtcdClient) Get(key string, sort, recursive bool) (*etcd.Response, error) { result := f.Data[key] if result.R == nil { - f.t.Errorf("Unexpected get for %s", key) + if _, ok := f.expectNotFoundGetSet[key]; !ok { + f.t.Errorf("Unexpected get for %s", key) + } return &etcd.Response{}, EtcdErrorNotFound } f.t.Logf("returning %v: %v %#v", key, result.R, result.E) From d4a5ba863ddebf03096898218e5032a059a14b74 Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Thu, 31 Jul 2014 20:33:29 +0900 Subject: [PATCH 8/8] Add test for AtomicUpdate --- pkg/tools/etcd_tools_test.go | 65 ++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/pkg/tools/etcd_tools_test.go b/pkg/tools/etcd_tools_test.go index 905e5091b48..7b210b6fc5b 100644 --- a/pkg/tools/etcd_tools_test.go +++ b/pkg/tools/etcd_tools_test.go @@ -33,6 +33,16 @@ type fakeEtcdGetSet struct { set func(key, value string, ttl uint64) (*etcd.Response, error) } +type TestResource struct { + api.JSONBase `json:",inline" yaml:",inline"` + Value int `json:"value" yaml:"value,omitempty"` +} + +func init() { + api.AddKnownTypes("", TestResource{}) + api.AddKnownTypes("v1beta1", TestResource{}) +} + func TestIsNotFoundErr(t *testing.T) { try := func(err error, isNotFound bool) { if IsEtcdNotFound(err) != isNotFound { @@ -154,6 +164,61 @@ func TestSetObj(t *testing.T) { } } +func TestAtomicUpdate(t *testing.T) { + fakeClient := MakeFakeEtcdClient(t) + fakeClient.TestIndex = true + helper := EtcdHelper{fakeClient} + + // Create a new node. + fakeClient.ExpectNotFoundGet("/some/key") + obj := TestResource{JSONBase: api.JSONBase{ID: "foo"}, Value: 1} + err := helper.AtomicUpdate("/some/key", &TestResource{}, func(in interface{}) (interface{}, error) { + return obj, nil + }) + if err != nil { + t.Errorf("Unexpected error %#v", err) + } + data, err := api.Encode(obj) + if err != nil { + t.Errorf("Unexpected error %#v", err) + } + expect := string(data) + got := fakeClient.Data["/some/key"].R.Node.Value + if expect != got { + t.Errorf("Wanted %v, got %v", expect, got) + } + return + + // Update an existing node. + callbackCalled := false + objUpdate := &TestResource{JSONBase: api.JSONBase{ID: "foo"}, Value: 2} + err = helper.AtomicUpdate("/some/key", &TestResource{}, func(in interface{}) (interface{}, error) { + callbackCalled = true + + if in.(*TestResource).Value != 1 { + t.Errorf("Callback input was not current set value") + } + + return objUpdate, nil + }) + if err != nil { + t.Errorf("Unexpected error %#v", err) + } + data, err = api.Encode(objUpdate) + if err != nil { + t.Errorf("Unexpected error %#v", err) + } + expect = string(data) + got = fakeClient.Data["/some/key"].R.Node.Value + if expect != got { + t.Errorf("Wanted %v, got %v", expect, got) + } + + if !callbackCalled { + t.Errorf("tryUpdate callback should have been called.") + } +} + func TestWatchInterpretation_ListAdd(t *testing.T) { called := false w := newEtcdWatcher(true, func(interface{}) bool {