From b6e329654e43402423b2cbc58d149d95dee13cae Mon Sep 17 00:00:00 2001 From: Hongchao Deng Date: Sat, 12 Nov 2016 19:21:00 -0800 Subject: [PATCH 1/2] etcd2: have prefix always prepended --- pkg/storage/etcd/etcd_helper.go | 24 ++++------ pkg/storage/etcd/etcd_helper_test.go | 68 ++++++++------------------- pkg/storage/etcd/etcd_watcher_test.go | 14 +++--- 3 files changed, 35 insertions(+), 71 deletions(-) diff --git a/pkg/storage/etcd/etcd_helper.go b/pkg/storage/etcd/etcd_helper.go index d7ddabb65c6..1601ca1eeb6 100644 --- a/pkg/storage/etcd/etcd_helper.go +++ b/pkg/storage/etcd/etcd_helper.go @@ -21,7 +21,6 @@ import ( "fmt" "path" "reflect" - "strings" "time" "k8s.io/kubernetes/pkg/api" @@ -97,7 +96,7 @@ func (h *etcdHelper) Create(ctx context.Context, key string, obj, out runtime.Ob if ctx == nil { glog.Errorf("Context is nil") } - key = h.prefixEtcdKey(key) + key = path.Join(h.pathPrefix, key) data, err := runtime.Encode(h.codec, obj) trace.Step("Object encoded") if err != nil { @@ -148,7 +147,7 @@ func (h *etcdHelper) Delete(ctx context.Context, key string, out runtime.Object, if ctx == nil { glog.Errorf("Context is nil") } - key = h.prefixEtcdKey(key) + key = path.Join(h.pathPrefix, key) v, err := conversion.EnforcePtr(out) if err != nil { panic("unable to convert output object to pointer") @@ -210,7 +209,7 @@ func (h *etcdHelper) Watch(ctx context.Context, key string, resourceVersion stri if err != nil { return nil, err } - key = h.prefixEtcdKey(key) + key = path.Join(h.pathPrefix, key) w := newEtcdWatcher(false, h.quorum, nil, storage.SimpleFilter(pred), h.codec, h.versioner, nil, h) go w.etcdWatch(ctx, h.etcdKeysAPI, key, watchRV) return w, nil @@ -225,7 +224,7 @@ func (h *etcdHelper) WatchList(ctx context.Context, key string, resourceVersion if err != nil { return nil, err } - key = h.prefixEtcdKey(key) + key = path.Join(h.pathPrefix, key) w := newEtcdWatcher(true, h.quorum, exceptKey(key), storage.SimpleFilter(pred), h.codec, h.versioner, nil, h) go w.etcdWatch(ctx, h.etcdKeysAPI, key, watchRV) return w, nil @@ -236,7 +235,7 @@ func (h *etcdHelper) Get(ctx context.Context, key string, resourceVersion string if ctx == nil { glog.Errorf("Context is nil") } - key = h.prefixEtcdKey(key) + key = path.Join(h.pathPrefix, key) _, _, _, err := h.bodyAndExtractObj(ctx, key, objPtr, ignoreNotFound) return err } @@ -306,7 +305,7 @@ func (h *etcdHelper) GetToList(ctx context.Context, key string, resourceVersion if err != nil { return err } - key = h.prefixEtcdKey(key) + key = path.Join(h.pathPrefix, key) startTime := time.Now() trace.Step("About to read etcd node") @@ -389,7 +388,7 @@ func (h *etcdHelper) List(ctx context.Context, key string, resourceVersion strin if err != nil { return err } - key = h.prefixEtcdKey(key) + key = path.Join(h.pathPrefix, key) startTime := time.Now() trace.Step("About to list etcd node") nodes, index, err := h.listEtcdNode(ctx, key) @@ -446,7 +445,7 @@ func (h *etcdHelper) GuaranteedUpdate( // Panic is appropriate, because this is a programming error. panic("need ptr to type") } - key = h.prefixEtcdKey(key) + key = path.Join(h.pathPrefix, key) for { obj := reflect.New(v.Type()).Interface().(runtime.Object) origBody, node, res, err := h.bodyAndExtractObj(ctx, key, obj, ignoreNotFound) @@ -540,13 +539,6 @@ func (h *etcdHelper) GuaranteedUpdate( } } -func (h *etcdHelper) prefixEtcdKey(key string) string { - if strings.HasPrefix(key, h.pathPrefix) { - return key - } - return path.Join(h.pathPrefix, key) -} - // etcdCache defines interface used for caching objects stored in etcd. Objects are keyed by // their Node.ModifiedIndex, which is unique across all types. // All implementations must be thread-safe. diff --git a/pkg/storage/etcd/etcd_helper_test.go b/pkg/storage/etcd/etcd_helper_test.go index 65d6a557f9a..dd72b1f2083 100644 --- a/pkg/storage/etcd/etcd_helper_test.go +++ b/pkg/storage/etcd/etcd_helper_test.go @@ -24,7 +24,6 @@ import ( "time" etcd "github.com/coreos/etcd/client" - "github.com/stretchr/testify/assert" "golang.org/x/net/context" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/testapi" @@ -99,8 +98,7 @@ func createPodList(t *testing.T, helper etcdHelper, list *api.PodList) error { func TestList(t *testing.T) { server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - key := etcdtest.AddPrefix("/some/key") - helper := newEtcdHelper(server.Client, testapi.Default.Codec(), key) + helper := newEtcdHelper(server.Client, testapi.Default.Codec(), etcdtest.PathPrefix()) list := api.PodList{ Items: []api.Pod{ @@ -123,7 +121,7 @@ func TestList(t *testing.T) { var got api.PodList // TODO: a sorted filter function could be applied such implied // ordering on the returned list doesn't matter. - err := helper.List(context.TODO(), key, "", storage.Everything, &got) + err := helper.List(context.TODO(), "/", "", storage.Everything, &got) if err != nil { t.Errorf("Unexpected error %v", err) } @@ -136,8 +134,7 @@ func TestList(t *testing.T) { func TestListFiltered(t *testing.T) { server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - key := etcdtest.AddPrefix("/some/key") - helper := newEtcdHelper(server.Client, testapi.Default.Codec(), key) + helper := newEtcdHelper(server.Client, testapi.Default.Codec(), etcdtest.PathPrefix()) list := api.PodList{ Items: []api.Pod{ @@ -167,7 +164,7 @@ func TestListFiltered(t *testing.T) { }, } var got api.PodList - err := helper.List(context.TODO(), key, "", p, &got) + err := helper.List(context.TODO(), "/", "", p, &got) if err != nil { t.Errorf("Unexpected error %v", err) } @@ -181,13 +178,10 @@ func TestListFiltered(t *testing.T) { func TestListAcrossDirectories(t *testing.T) { server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - rootkey := etcdtest.AddPrefix("/some/key") - key1 := etcdtest.AddPrefix("/some/key/directory1") - key2 := etcdtest.AddPrefix("/some/key/directory2") - roothelper := newEtcdHelper(server.Client, testapi.Default.Codec(), rootkey) - helper1 := newEtcdHelper(server.Client, testapi.Default.Codec(), key1) - helper2 := newEtcdHelper(server.Client, testapi.Default.Codec(), key2) + roothelper := newEtcdHelper(server.Client, testapi.Default.Codec(), etcdtest.PathPrefix()) + helper1 := newEtcdHelper(server.Client, testapi.Default.Codec(), etcdtest.PathPrefix()+"/dir1") + helper2 := newEtcdHelper(server.Client, testapi.Default.Codec(), etcdtest.PathPrefix()+"/dir2") list := api.PodList{ Items: []api.Pod{ @@ -217,7 +211,7 @@ func TestListAcrossDirectories(t *testing.T) { list.Items[2] = *returnedObj var got api.PodList - err := roothelper.List(context.TODO(), rootkey, "", storage.Everything, &got) + err := roothelper.List(context.TODO(), "/", "", storage.Everything, &got) if err != nil { t.Errorf("Unexpected error %v", err) } @@ -229,8 +223,8 @@ func TestListAcrossDirectories(t *testing.T) { func TestGet(t *testing.T) { server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - key := etcdtest.AddPrefix("/some/key") - helper := newEtcdHelper(server.Client, testapi.Default.Codec(), key) + key := "/some/key" + helper := newEtcdHelper(server.Client, testapi.Default.Codec(), etcdtest.PathPrefix()) expect := api.Pod{ ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: apitesting.DeepEqualSafePodSpec(), @@ -251,14 +245,13 @@ func TestGet(t *testing.T) { func TestGetNotFoundErr(t *testing.T) { server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - key := etcdtest.AddPrefix("/some/key") - boguskey := etcdtest.AddPrefix("/some/boguskey") - helper := newEtcdHelper(server.Client, testapi.Default.Codec(), key) + boguskey := "/some/boguskey" + helper := newEtcdHelper(server.Client, testapi.Default.Codec(), etcdtest.PathPrefix()) var got api.Pod err := helper.Get(context.TODO(), boguskey, "", &got, false) if !storage.IsNotFound(err) { - t.Errorf("Unexpected reponse on key=%v, err=%v", key, err) + t.Errorf("Unexpected reponse on key=%v, err=%v", boguskey, err) } } @@ -304,8 +297,8 @@ func TestGuaranteedUpdate(t *testing.T) { _, codec := testScheme(t) server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - key := etcdtest.AddPrefix("/some/key") - helper := newEtcdHelper(server.Client, codec, key) + key := "/some/key" + helper := newEtcdHelper(server.Client, codec, etcdtest.PathPrefix()) obj := &storagetesting.TestResource{ObjectMeta: api.ObjectMeta{Name: "foo"}, Value: 1} err := helper.GuaranteedUpdate(context.TODO(), key, &storagetesting.TestResource{}, true, nil, storage.SimpleUpdate(func(in runtime.Object) (runtime.Object, error) { @@ -349,8 +342,8 @@ func TestGuaranteedUpdateNoChange(t *testing.T) { _, codec := testScheme(t) server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - key := etcdtest.AddPrefix("/some/key") - helper := newEtcdHelper(server.Client, codec, key) + key := "/some/key" + helper := newEtcdHelper(server.Client, codec, etcdtest.PathPrefix()) obj := &storagetesting.TestResource{ObjectMeta: api.ObjectMeta{Name: "foo"}, Value: 1} err := helper.GuaranteedUpdate(context.TODO(), key, &storagetesting.TestResource{}, true, nil, storage.SimpleUpdate(func(in runtime.Object) (runtime.Object, error) { @@ -379,8 +372,8 @@ func TestGuaranteedUpdateKeyNotFound(t *testing.T) { _, codec := testScheme(t) server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - key := etcdtest.AddPrefix("/some/key") - helper := newEtcdHelper(server.Client, codec, key) + key := "/some/key" + helper := newEtcdHelper(server.Client, codec, etcdtest.PathPrefix()) // Create a new node. obj := &storagetesting.TestResource{ObjectMeta: api.ObjectMeta{Name: "foo"}, Value: 1} @@ -406,7 +399,7 @@ func TestGuaranteedUpdate_CreateCollision(t *testing.T) { _, codec := testScheme(t) server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - key := etcdtest.AddPrefix("/some/key") + key := "/some/key" helper := newEtcdHelper(server.Client, codec, etcdtest.PathPrefix()) const concurrency = 10 @@ -471,27 +464,6 @@ func TestGuaranteedUpdateUIDMismatch(t *testing.T) { } } -func TestPrefixEtcdKey(t *testing.T) { - server := etcdtesting.NewEtcdTestClientServer(t) - defer server.Terminate(t) - prefix := path.Join("/", etcdtest.PathPrefix()) - helper := newEtcdHelper(server.Client, testapi.Default.Codec(), prefix) - - baseKey := "/some/key" - - // Verify prefix is added - keyBefore := baseKey - keyAfter := helper.prefixEtcdKey(keyBefore) - - assert.Equal(t, keyAfter, path.Join(prefix, baseKey), "Prefix incorrectly added by EtcdHelper") - - // Verify prefix is not added - keyBefore = path.Join(prefix, baseKey) - keyAfter = helper.prefixEtcdKey(keyBefore) - - assert.Equal(t, keyBefore, keyAfter, "Prefix incorrectly added by EtcdHelper") -} - func TestDeleteUIDMismatch(t *testing.T) { server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) diff --git a/pkg/storage/etcd/etcd_watcher_test.go b/pkg/storage/etcd/etcd_watcher_test.go index ce05a3920e4..5dd87640d91 100644 --- a/pkg/storage/etcd/etcd_watcher_test.go +++ b/pkg/storage/etcd/etcd_watcher_test.go @@ -341,7 +341,7 @@ func makeSubsets(ip string, port int) []api.EndpointSubset { func TestWatchEtcdState(t *testing.T) { codec := testapi.Default.Codec() - key := etcdtest.AddPrefix("/somekey/foo") + key := "/somekey/foo" server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) @@ -399,7 +399,7 @@ func TestWatchFromZeroIndex(t *testing.T) { codec := testapi.Default.Codec() pod := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}} - key := etcdtest.AddPrefix("/somekey/foo") + key := "/somekey/foo" server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) @@ -458,18 +458,18 @@ func TestWatchFromZeroIndex(t *testing.T) { func TestWatchListFromZeroIndex(t *testing.T) { codec := testapi.Default.Codec() - key := etcdtest.AddPrefix("/some/key") + prefix := "/some/key" server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) - h := newEtcdHelper(server.Client, codec, key) + h := newEtcdHelper(server.Client, codec, prefix) - watching, err := h.WatchList(context.TODO(), key, "0", storage.Everything) + watching, err := h.WatchList(context.TODO(), "/", "0", storage.Everything) if err != nil { t.Fatalf("Unexpected error: %v", err) } defer watching.Stop() - // creates key/foo which should trigger the WatchList for "key" + // creates foo which should trigger the WatchList for "/" pod := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}} err = h.Create(context.TODO(), pod.Name, pod, pod, 0) if err != nil { @@ -489,7 +489,7 @@ func TestWatchListFromZeroIndex(t *testing.T) { func TestWatchListIgnoresRootKey(t *testing.T) { codec := testapi.Default.Codec() pod := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}} - key := etcdtest.AddPrefix("/some/key") + key := "/some/key" server := etcdtesting.NewEtcdTestClientServer(t) defer server.Terminate(t) h := newEtcdHelper(server.Client, codec, key) From 14ed02d0dde2fdf58d2a32ea4d43f82009d344f3 Mon Sep 17 00:00:00 2001 From: Hongchao Deng Date: Mon, 14 Nov 2016 11:28:48 -0800 Subject: [PATCH 2/2] pkg/storage/etcd: update bazel --- pkg/storage/etcd/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/storage/etcd/BUILD b/pkg/storage/etcd/BUILD index bb53f21869f..e426be52dd2 100644 --- a/pkg/storage/etcd/BUILD +++ b/pkg/storage/etcd/BUILD @@ -63,7 +63,6 @@ go_test( "//pkg/storage/testing:go_default_library", "//pkg/watch:go_default_library", "//vendor:github.com/coreos/etcd/client", - "//vendor:github.com/stretchr/testify/assert", "//vendor:golang.org/x/net/context", ], )