namespace proxy #7549

heyitsanthony · 2017-03-21T06:36:34Z

No description provided.

xiang90 · 2017-03-21T13:08:05Z

clientv3/op.go

@@ -69,6 +69,15 @@ type Op struct {
 	leaseID LeaseID
 }

+// accesors / mutators
+
+func (op Op) KeyBytes() []byte         { return op.key }


let's document these public funcs.

xiang90 · 2017-03-21T13:08:20Z

clientv3/compare.go

@@ -82,6 +82,16 @@ func ModRevision(key string) Cmp {
 	return Cmp{Key: []byte(key), Target: pb.Compare_MOD}
 }

+func (cmp *Cmp) KeyBytes() []byte        { return cmp.Key }


let us document these public funcs.

xiang90 · 2017-03-21T13:10:22Z

clientv3/namespace/watch.go

+import (
+	"sync"
+
+	"golang.org/x/net/context"


use context instead?

./watch.go:35: cannot use watcherPrefix literal (type *watcherPrefix) as type clientv3.Watcher in return argument: *watcherPrefix does not implement clientv3.Watcher (wrong type for Watch method) have Watch("context".Context, string, ...clientv3.OpOption) clientv3.WatchChan want Watch("golang.org/x/net/context".Context, string, ...clientv3.OpOption) clientv3.WatchChan

xiang90 · 2017-03-21T13:11:34Z

clientv3/namespace/kv.go

+package namespace
+
+import (
+	"golang.org/x/net/context"


just context?

./kv.go:33: cannot use kvPrefix literal (type *kvPrefix) as type clientv3.KV in return argument: *kvPrefix does not implement clientv3.KV (wrong type for Delete method) have Delete("context".Context, string, ...clientv3.OpOption) (*clientv3.DeleteResponse, error) want Delete("golang.org/x/net/context".Context, string, ...clientv3.OpOption) (*clientv3.DeleteResponse, error)

ok. got it.

xiang90 · 2017-03-21T13:14:21Z

clientv3/namespace/lease.go

+import (
+	"bytes"
+
+	"golang.org/x/net/context"


use context?

./lease.go:33: cannot use leasePrefix literal (type *leasePrefix) as type clientv3.Lease in return argument: *leasePrefix does not implement clientv3.Lease (wrong type for TimeToLive method) have TimeToLive("context".Context, clientv3.LeaseID, ...clientv3.LeaseOption) (*clientv3.LeaseTimeToLiveResponse, error) want TimeToLive("golang.org/x/net/context".Context, clientv3.LeaseID, ...clientv3.LeaseOption) (*clientv3.LeaseTimeToLiveResponse, error)

xiang90 · 2017-03-21T13:15:34Z

clientv3/namespace/util.go

+
+package namespace
+
+func prefixInterval(pfx string, key, end []byte) (pfxKey []byte, pfxEnd []byte) {


add a unit test for this func?

xiang90 · 2017-03-21T13:17:10Z

clientv3/integration/namespace_test.go

+	"github.com/coreos/etcd/integration"
+	"github.com/coreos/etcd/mvcc/mvccpb"
+	"github.com/coreos/etcd/pkg/testutil"
+	"golang.org/x/net/context"


use context?

xiang90 · 2017-03-21T13:22:28Z

clientv3/namespace/watch.go

+
+func (w *watcherPrefix) Watch(ctx context.Context, key string, opts ...clientv3.OpOption) clientv3.WatchChan {
+	// add prefix
+	op := clientv3.OpGet("abc", opts...)


the use of this dummy op is not obvious. probably need a line of more specific comment.

xiang90 · 2017-03-21T13:24:17Z

Looks good in general.

xiang90 · 2017-03-21T13:26:27Z

clientv3/namespace/doc.go

+// Next, override the client interfaces:
+//
+//	unprefixedKV := cli.KV
+//	cli.KV = namespace.NewKV(cli.KV, "my-prefix/")


my initial thought was to add a WithPrefix option to NewKV function. But I guess a new pkg might be cleaner though. The only problem is discoverability of this new pkg. we probably also needs to add a one line doc somewhere in the root doc.

trying to observe separation of concerns with this; a monster client that does everything seems much more difficult to maintain than having composable wrappers-- docs are no big deal by comparison...

xiang90 · 2017-03-21T13:46:41Z

@heyitsanthony

namespaced proxy is great! It can help to limit the access of an etcd cluster to a prefix without changing any client logic.

In the long term, I also want the proxy to support multiple etcd clusters through auto discovery.

Say we have etcd-us, etcd-cn, etcd-jp, etcd-kr, etcd-uk.

Users can do client.Put("abc", WithCluster("us")) and the gRPC proxy should forward the request to etcd-us (optionally auto fail back to a default cluster one if the requested Cluster does not exist).

This might be useful for supporting sharded etcd cluster for different Kubernetes namespace without introducing too much client side changes. For example, Kubernetes API server can still accept only a set of symmetric endpoints (gRPC proxy endpoints). All we need to do is to attach different namespace as cluster option in client.

/cc @wojtek-t @timothysc @heyitsanthony @smarterclayton

gyuho · 2017-03-21T17:35:15Z

clientv3/namespace/util.go

+			pfxEnd = []byte{0}
+		}
+	} else if len(end) >= 1 {
+		pfxKey = make([]byte, len(pfx)+len(key))


aren't these redundant with line 18-19?

this was a bug; fixed while writing the unit test

fanminshi · 2017-03-21T19:51:32Z

Documentation/op-guide/grpc_proxy.md

+```bash
+$ etcd grpc-proxy start --endpoints=localhost:2379 \
+  --listen-addr=127.0.0.1:23790 \
+  --namespace-prefix=my-prefix/


should we also test grpc-proxy with namespace?

Like, -tags cluster_proxy? I don't think it'll exercise any behavior that's not already covered by the clientv3/integration tests.

Although, it probably should because it might miss some update to the protocol and not translate the full response like the grpcproxy proper has done in the past. The patch to test it is relatively unobtrusive, I'll throw it on...

xiang90 · 2017-03-21T19:53:57Z

Documentation/op-guide/grpc_proxy.md

+
+Suppose an application expects full control over the entire key space, but the etcd cluster is shared with other applications. To let all appications run without interfering with each other, the proxy can partition the etcd keyspace so clients appear to have access to the complete keyspace. When the proxy is given the flag `--namespace-prefix`, all client requests going into the proxy are translated to have a user-defined prefix on the keys. Accesses to the etcd cluster will be under the prefix and responses from the proxy will strip away the prefix; to the client, it appears as if there is no prefix at all.
+
+To namespace a proxy, start it with `--namespace-prefix`:


i feel namespace is clear enough. it is shorter than namespace-prefix.

xiang90 · 2017-03-21T19:59:22Z

clientv3/namespace/doc.go

@@ -0,0 +1,43 @@
+// Copyright 2017 The etcd Authors


can we also add a link to this godoc in https://github.com/coreos/etcd/tree/master/clientv3#etcdclientv3? I think this should be useful for a lot of users.

xiang90 · 2017-03-21T19:59:33Z

LGTM

fanminshi · 2017-03-21T20:51:52Z

integration/cluster_proxy.go

@@ -44,9 +46,16 @@ func toGRPC(c *clientv3.Client) grpcAPI {
 	if v, ok := proxies[c]; ok {
 		return v.grpc
 	}
+
+	// test namespacing proxy
+	c.KV = namespace.NewKV(c.KV, ns)


where is ns var defined? I don't see it.

Fixes etcd-io#7250

fanminshi · 2017-03-22T22:35:33Z

lgtm

Hardcode a namespace over the testing grpcproxy.

Fixes etcd-io#6577

xiang90 reviewed Mar 21, 2017

View reviewed changes

gyuho added the kind/release-note label Mar 21, 2017

gyuho reviewed Mar 21, 2017

View reviewed changes

heyitsanthony force-pushed the namespace-proxy branch from 18b347c to 08aede2 Compare March 21, 2017 19:18

fanminshi reviewed Mar 21, 2017

View reviewed changes

xiang90 reviewed Mar 21, 2017

View reviewed changes

heyitsanthony force-pushed the namespace-proxy branch from 08aede2 to 324383e Compare March 21, 2017 20:30

fanminshi reviewed Mar 21, 2017

View reviewed changes

heyitsanthony force-pushed the namespace-proxy branch 6 times, most recently from 3b7a41d to c3e3508 Compare March 22, 2017 19:17

Anthony Romano added 2 commits March 22, 2017 14:08

clientv3: make ops and compares non-opaque and mutable

f9b6066

Fixes etcd-io#7250

namespace: a wrapper for clientv3 to namespace requests

1f8c7b3

heyitsanthony force-pushed the namespace-proxy branch from c3e3508 to 978118b Compare March 22, 2017 21:11

gyuho approved these changes Mar 22, 2017

View reviewed changes

Anthony Romano added 4 commits March 22, 2017 16:45

clientv3/integration: simple namespace wrapper tests

66d1477

integration: test namespacing on proxy layer

f35d7d9

Hardcode a namespace over the testing grpcproxy.

etcdmain: add prefixing support to grpc proxy

397a42e

Fixes etcd-io#6577

Documentation, op-guide, clientv3: add documentation for namespacing

85f989a

heyitsanthony force-pushed the namespace-proxy branch from 978118b to 85f989a Compare March 22, 2017 23:45

heyitsanthony merged commit 049ca87 into etcd-io:master Mar 23, 2017

heyitsanthony deleted the namespace-proxy branch March 23, 2017 06:26

gyuho changed the title ~~namespace proxy~~ *: namespace proxy Apr 25, 2017

gyuho changed the title *: namespace proxy namespace proxy Apr 25, 2017

gyuho removed the kind/need-release-note-v3.3 label Nov 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

namespace proxy #7549

namespace proxy #7549

heyitsanthony commented Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 Mar 21, 2017

heyitsanthony Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 Mar 21, 2017

heyitsanthony Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 Mar 21, 2017

heyitsanthony Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 commented Mar 21, 2017

xiang90 Mar 21, 2017

heyitsanthony Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 commented Mar 21, 2017 •

edited

Loading

gyuho Mar 21, 2017

heyitsanthony Mar 21, 2017

fanminshi Mar 21, 2017

heyitsanthony Mar 21, 2017

heyitsanthony Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 Mar 21, 2017

xiang90 commented Mar 21, 2017

fanminshi Mar 21, 2017

fanminshi commented Mar 22, 2017


		package namespace

		func prefixInterval(pfx string, key, end []byte) (pfxKey []byte, pfxEnd []byte) {


		Suppose an application expects full control over the entire key space, but the etcd cluster is shared with other applications. To let all appications run without interfering with each other, the proxy can partition the etcd keyspace so clients appear to have access to the complete keyspace. When the proxy is given the flag `--namespace-prefix`, all client requests going into the proxy are translated to have a user-defined prefix on the keys. Accesses to the etcd cluster will be under the prefix and responses from the proxy will strip away the prefix; to the client, it appears as if there is no prefix at all.

		To namespace a proxy, start it with `--namespace-prefix`:

namespace proxy #7549

namespace proxy #7549

Conversation

heyitsanthony commented Mar 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiang90 commented Mar 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiang90 commented Mar 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiang90 commented Mar 21, 2017

Choose a reason for hiding this comment

fanminshi commented Mar 22, 2017

xiang90 commented Mar 21, 2017 •

edited

Loading