discovery: Attribute support for global discovery.
The suffix at which advertisements are mounted in global discovery
is now approximately:
Ad.Id.String() + "," + vomEncode(Ad.Attributes) // with necessary escaping.
This is a prerequisite get global discovery working in the todos app.
Change-Id: I51d06d8b82a431a271caffed3f69d47631cd751d
diff --git a/lib/discovery/global/advertise.go b/lib/discovery/global/advertise.go
index da3c45a..312c68e 100644
--- a/lib/discovery/global/advertise.go
+++ b/lib/discovery/global/advertise.go
@@ -11,7 +11,6 @@
"v.io/v23/naming"
"v.io/v23/security"
"v.io/v23/security/access"
-
idiscovery "v.io/x/ref/lib/discovery"
)
@@ -43,7 +42,10 @@
// TODO(jhahn): There is no atomic way to check and reserve the name under mounttable.
// For example, the name can be overwritten by other applications of the same owner.
// But this would be OK for now.
- name := ad.Id.String()
+ name, err := encodeAdToSuffix(ad)
+ if err != nil {
+ return nil, err
+ }
if err := d.ns.SetPermissions(ctx, name, perms, "", naming.IsLeaf(true)); err != nil {
d.removeAd(ad)
return nil, err
@@ -58,8 +60,8 @@
defer d.removeAd(ad)
// We need a context that is detached from the deadlines and cancellation
// of ctx since we have to unmount after ctx is canceled.
- rctx, _ := context.WithRootCancel(ctx)
- defer d.unmount(rctx, name)
+ rctx, rcancel := context.WithRootCancel(ctx)
+ defer d.unmount(rctx, rcancel, name)
for {
d.mount(ctx, name, ad.Addresses)
@@ -99,8 +101,9 @@
}
}
-func (d *gdiscovery) unmount(ctx *context.T, name string) {
+func (d *gdiscovery) unmount(ctx *context.T, cancel context.CancelFunc, name string) {
if err := d.ns.Delete(ctx, name, true); err != nil {
ctx.Infof("unmount(%q) failed: %v", name, err)
}
+ cancel()
}
diff --git a/lib/discovery/global/encoding.go b/lib/discovery/global/encoding.go
new file mode 100644
index 0000000..774f5aa
--- /dev/null
+++ b/lib/discovery/global/encoding.go
@@ -0,0 +1,57 @@
+// Copyright 2015 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package global
+
+import (
+ "strings"
+
+ "v.io/v23/discovery"
+ "v.io/v23/naming"
+ "v.io/v23/vom"
+)
+
+const suffixDelim = ","
+
+// encodeAdToSuffix encodes the ad.Id and the ad.Attributes into the suffix at
+// which we mount the advertisement.
+//
+// TODO(suharshs): Currently only the id and the attributes are encoded; we may
+// want to encode the rest of the advertisement someday?
+func encodeAdToSuffix(ad *discovery.Advertisement) (string, error) {
+ b, err := vom.Encode(ad.Attributes)
+ if err != nil {
+ return "", err
+ }
+ // Escape suffixDelim to use it as our delimeter between the id and the attrs.
+ id := ad.Id.String()
+ attr := naming.Escape(string(b), suffixDelim)
+ return naming.EncodeAsNameElement(id + suffixDelim + attr), nil
+}
+
+// decodeAdFromSuffix decodes s into an advertisement.
+func decodeAdFromSuffix(in string) (*discovery.Advertisement, error) {
+ s, ok := naming.DecodeFromNameElement(in)
+ if !ok {
+ return nil, NewErrAdInvalidEncoding(nil, in)
+ }
+ parts := strings.Split(s, suffixDelim)
+ if len(parts) != 2 {
+ return nil, NewErrAdInvalidEncoding(nil, in)
+ }
+ id, attrs := parts[0], parts[1]
+ ad := &discovery.Advertisement{}
+ var err error
+ if ad.Id, err = discovery.ParseAdId(id); err != nil {
+ return nil, err
+ }
+ attrs, ok = naming.Unescape(attrs)
+ if !ok {
+ return nil, NewErrAdInvalidEncoding(nil, in)
+ }
+ if err = vom.Decode([]byte(attrs), &ad.Attributes); err != nil {
+ return nil, err
+ }
+ return ad, nil
+}
diff --git a/lib/discovery/global/encoding_test.go b/lib/discovery/global/encoding_test.go
new file mode 100644
index 0000000..cf4b0db
--- /dev/null
+++ b/lib/discovery/global/encoding_test.go
@@ -0,0 +1,34 @@
+// Copyright 2015 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package global
+
+import (
+ "reflect"
+ "testing"
+
+ "v.io/v23/discovery"
+)
+
+func TestAdSuffix(t *testing.T) {
+ testCases := []discovery.Advertisement{
+ {},
+ {Id: discovery.AdId{1, 2, 3}},
+ {Attributes: discovery.Attributes{"k": "v"}},
+ {Id: discovery.AdId{1, 2, 3}, Attributes: discovery.Attributes{"k": "v"}},
+ }
+ for _, want := range testCases {
+ encAd, err := encodeAdToSuffix(&want)
+ if err != nil {
+ t.Error(err)
+ }
+ got, err := decodeAdFromSuffix(encAd)
+ if err != nil {
+ t.Error(err)
+ }
+ if !reflect.DeepEqual(*got, want) {
+ t.Errorf("got %v, want %v", *got, want)
+ }
+ }
+}
diff --git a/lib/discovery/global/errors.vdl b/lib/discovery/global/errors.vdl
index f11c648..4bd5228 100644
--- a/lib/discovery/global/errors.vdl
+++ b/lib/discovery/global/errors.vdl
@@ -5,7 +5,6 @@
package global
error (
- NoNamespace() {
- "en": " namespace not found",
- }
+ NoNamespace() {"en": "namespace not found"}
+ AdInvalidEncoding(ad string) {"en": "ad ({ad}) has invalid encoding"}
)
diff --git a/lib/discovery/global/global.vdl.go b/lib/discovery/global/global.vdl.go
index f0fbd9a..2e725ee 100644
--- a/lib/discovery/global/global.vdl.go
+++ b/lib/discovery/global/global.vdl.go
@@ -19,7 +19,8 @@
// Error definitions
var (
- ErrNoNamespace = verror.Register("v.io/x/ref/lib/discovery/global.NoNamespace", verror.NoRetry, "{1:}{2:} namespace not found")
+ ErrNoNamespace = verror.Register("v.io/x/ref/lib/discovery/global.NoNamespace", verror.NoRetry, "{1:}{2:} namespace not found")
+ ErrAdInvalidEncoding = verror.Register("v.io/x/ref/lib/discovery/global.AdInvalidEncoding", verror.NoRetry, "{1:}{2:} ad ({3}) has invalid encoding")
)
// NewErrNoNamespace returns an error with the ErrNoNamespace ID.
@@ -27,6 +28,11 @@
return verror.New(ErrNoNamespace, ctx)
}
+// NewErrAdInvalidEncoding returns an error with the ErrAdInvalidEncoding ID.
+func NewErrAdInvalidEncoding(ctx *context.T, ad string) error {
+ return verror.New(ErrAdInvalidEncoding, ctx, ad)
+}
+
var __VDLInitCalled bool
// __VDLInit performs vdl initialization. It is safe to call multiple times.
@@ -49,7 +55,8 @@
__VDLInitCalled = true
// Set error format strings.
- i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(ErrNoNamespace.ID), "{1:}{2:} namespace not found")
+ i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(ErrNoNamespace.ID), "{1:}{2:} namespace not found")
+ i18n.Cat().SetWithBase(i18n.LangID("en"), i18n.MsgID(ErrAdInvalidEncoding.ID), "{1:}{2:} ad ({3}) has invalid encoding")
return struct{}{}
}
diff --git a/lib/discovery/global/global_test.go b/lib/discovery/global/global_test.go
index 2aa1d02..b889e7f 100644
--- a/lib/discovery/global/global_test.go
+++ b/lib/discovery/global/global_test.go
@@ -29,8 +29,9 @@
ads := []discovery.Advertisement{
{
- Id: discovery.AdId{1, 2, 3},
- Addresses: []string{"/h1:123/x", "/h2:123/y"},
+ Id: discovery.AdId{1, 2, 3},
+ Addresses: []string{"/h1:123/x", "/h2:123/y"},
+ Attributes: discovery.Attributes{"k": "v"},
},
{
Addresses: []string{"/h1:123/x", "/h2:123/z"},
diff --git a/lib/discovery/global/scan.go b/lib/discovery/global/scan.go
index 26dab17..f336b77 100644
--- a/lib/discovery/global/scan.go
+++ b/lib/discovery/global/scan.go
@@ -21,10 +21,6 @@
if err != nil {
return nil, err
}
- target := matcher.TargetKey()
- if len(target) == 0 {
- target = "*"
- }
updateCh := make(chan discovery.Update, 10)
go func() {
@@ -32,7 +28,7 @@
var prevFound map[discovery.AdId]*discovery.Advertisement
for {
- found, err := d.doScan(ctx, target, matcher)
+ found, err := d.doScan(ctx, matcher.TargetKey(), matcher)
if found == nil {
if err != nil {
ctx.Error(err)
@@ -53,7 +49,19 @@
}
func (d *gdiscovery) doScan(ctx *context.T, target string, matcher idiscovery.Matcher) (map[discovery.AdId]*discovery.Advertisement, error) {
- scanCh, err := d.ns.Glob(ctx, target)
+ // If the target is neither empty nor a valid AdId, we return without an error,
+ // since there will be not entries with the requested target length in the namespace.
+ if len(target) > 0 {
+ if _, err := discovery.ParseAdId(target); err != nil {
+ return nil, nil
+ }
+ }
+
+ // Now that the key is either empty or an AdId, we suffix it with "*".
+ // In the case of empty, we need to scan for everything.
+ // In the case where target is a AdId we need to scan for entries prefixed with
+ // the AdId with any encoded attributes afterwards.
+ scanCh, err := d.ns.Glob(ctx, target+"*")
if err != nil {
return nil, err
}
@@ -74,7 +82,7 @@
ctx.Error(err)
continue
}
- // Since mount operation is not atomic, we may not have addresses yet.
+ // Since mount operations are not atomic, we may not have addresses yet.
// Ignore it. It will be re-scanned in the next cycle.
if len(ad.Addresses) == 0 {
continue
@@ -108,7 +116,7 @@
func convToAd(glob naming.GlobReply) (*discovery.Advertisement, error) {
switch g := glob.(type) {
case *naming.GlobReplyEntry:
- id, err := discovery.ParseAdId(g.Value.Name)
+ ad, err := decodeAdFromSuffix(g.Value.Name)
if err != nil {
return nil, err
}
@@ -116,9 +124,10 @@
for _, server := range g.Value.Servers {
addrs = append(addrs, server.Server)
}
- // We sort the addresses to avoid false update.
+ // We sort the addresses to avoid false updates.
sort.Strings(addrs)
- return &discovery.Advertisement{Id: id, Addresses: addrs}, nil
+ ad.Addresses = addrs
+ return ad, nil
case *naming.GlobReplyError:
return nil, fmt.Errorf("glob error on %s: %v", g.Value.Name, g.Value.Error)
default:
diff --git a/lib/discovery/global/validate.go b/lib/discovery/global/validate.go
index e7e0efa..83eef8e 100644
--- a/lib/discovery/global/validate.go
+++ b/lib/discovery/global/validate.go
@@ -21,11 +21,8 @@
if len(ad.Addresses) == 0 {
return errors.New("address not provided")
}
- if len(ad.Attributes) > 0 {
- return errors.New("attributes name not supported")
- }
if len(ad.Attachments) > 0 {
- return errors.New("attachments name not supported")
+ return errors.New("attachments not supported")
}
return nil
}
diff --git a/lib/discovery/global/validate_test.go b/lib/discovery/global/validate_test.go
index 792165f..e9a4a6b 100644
--- a/lib/discovery/global/validate_test.go
+++ b/lib/discovery/global/validate_test.go
@@ -17,8 +17,9 @@
}{
{
discovery.Advertisement{
- Id: discovery.AdId{1, 2, 3},
- Addresses: []string{"/h:123/x"},
+ Id: discovery.AdId{1, 2, 3},
+ Addresses: []string{"/h:123/x"},
+ Attributes: discovery.Attributes{"k": "v"},
},
true,
},
@@ -47,16 +48,6 @@
discovery.Advertisement{
Id: discovery.AdId{1, 2, 3},
Addresses: []string{"/h:123/x"},
- Attributes: discovery.Attributes{ // Has attributes.
- "k": "v",
- },
- },
- false,
- },
- {
- discovery.Advertisement{
- Id: discovery.AdId{1, 2, 3},
- Addresses: []string{"/h:123/x"},
Attachments: discovery.Attachments{ // Has attachments.
"k": []byte{1},
},
