Store container names in memdb

Currently, names are maintained by a separate system called "registrar".
This means there is no way to atomically snapshot the state of
containers and the names associated with them.

We can add this atomicity and simplify the code by storing name
associations in the memdb. This removes the need for pkg/registrar, and
makes snapshots a lot less expensive because they no longer need to copy
all the names. This change also avoids some problematic behavior from
pkg/registrar where it returns slices which may be modified later on.

Note that while this change makes the *snapshotting* atomic, it doesn't
yet do anything to make sure containers are named at the same time that
they are added to the database. We can do that by adding a transactional
interface, either as a followup, or as part of this PR.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

container/view.go

@@ -1,6 +1,7 @@
 package container
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -8,14 +9,23 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/network"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/go-connections/nat"
 	"github.com/hashicorp/go-memdb"
 )
 
 const (
-	memdbTable   = "containers"
-	memdbIDIndex = "id"
+	memdbContainersTable = "containers"
+	memdbNamesTable      = "names"
+
+	memdbIDIndex          = "id"
+	memdbContainerIDIndex = "containerid"
+)
+
+var (
+	// ErrNameReserved is an error which is returned when a name is requested to be reserved that already is reserved
+	ErrNameReserved = errors.New("name is reserved")
+	// ErrNameNotReserved is an error which is returned when trying to find a name that is not reserved
+	ErrNameNotReserved = errors.New("name is not reserved")
 )
 
 // Snapshot is a read only view for Containers. It holds all information necessary to serve container queries in a
@@ -41,23 +51,37 @@ type Snapshot struct {
 	}
 }
 
+// nameAssociation associates a container id with a name.
+type nameAssociation struct {
+	// name is the name to associate. Note that name is the primary key
+	// ("id" in memdb).
+	name        string
+	containerID string
+}
+
 // ViewDB provides an in-memory transactional (ACID) container Store
 type ViewDB interface {
-	Snapshot(nameIndex *registrar.Registrar) View
+	Snapshot() View
 	Save(*Container) error
 	Delete(*Container) error
+
+	ReserveName(name, containerID string) error
+	ReleaseName(name string)
 }
 
 // View can be used by readers to avoid locking
 type View interface {
 	All() ([]Snapshot, error)
 	Get(id string) (*Snapshot, error)
+
+	GetID(name string) (string, error)
+	GetAllNames() map[string][]string
 }
 
 var schema = &memdb.DBSchema{
 	Tables: map[string]*memdb.TableSchema{
-		memdbTable: {
-			Name: memdbTable,
+		memdbContainersTable: {
+			Name: memdbContainersTable,
 			Indexes: map[string]*memdb.IndexSchema{
 				memdbIDIndex: {
 					Name:    memdbIDIndex,
@@ -66,6 +90,21 @@ var schema = &memdb.DBSchema{
 				},
 			},
 		},
+		memdbNamesTable: {
+			Name: memdbNamesTable,
+			Indexes: map[string]*memdb.IndexSchema{
+				// Used for names, because "id" is the primary key in memdb.
+				memdbIDIndex: {
+					Name:    memdbIDIndex,
+					Unique:  true,
+					Indexer: &namesByNameIndexer{},
+				},
+				memdbContainerIDIndex: {
+					Name:    memdbContainerIDIndex,
+					Indexer: &namesByContainerIDIndexer{},
+				},
+			},
+		},
 	},
 }
 
@@ -94,10 +133,9 @@ func NewViewDB() (ViewDB, error) {
 }
 
 // Snapshot provides a consistent read-only View of the database
-func (db *memDB) Snapshot(index *registrar.Registrar) View {
+func (db *memDB) Snapshot() View {
 	return &memdbView{
-		txn:       db.store.Txn(false),
-		nameIndex: index.GetAll(),
+		txn: db.store.Txn(false),
 	}
 }
 
@@ -106,25 +144,75 @@ func (db *memDB) Snapshot(index *registrar.Registrar) View {
 func (db *memDB) Save(c *Container) error {
 	txn := db.store.Txn(true)
 	defer txn.Commit()
-	return txn.Insert(memdbTable, c)
+	return txn.Insert(memdbContainersTable, c)
 }
 
 // Delete removes an item by ID
 func (db *memDB) Delete(c *Container) error {
 	txn := db.store.Txn(true)
 	defer txn.Commit()
-	return txn.Delete(memdbTable, NewBaseContainer(c.ID, c.Root))
+
+	// Delete any names referencing this container's ID.
+	iter, err := txn.Get(memdbNamesTable, memdbContainerIDIndex, c.ID)
+	if err != nil {
+		return err
+	}
+
+	var names []string
+	for {
+		item := iter.Next()
+		if item == nil {
+			break
+		}
+		names = append(names, item.(nameAssociation).name)
+	}
+
+	for _, name := range names {
+		txn.Delete(memdbNamesTable, nameAssociation{name: name})
+	}
+
+	return txn.Delete(memdbContainersTable, NewBaseContainer(c.ID, c.Root))
+}
+
+// ReserveName registers a container ID to a name
+// ReserveName is idempotent
+// Attempting to reserve a container ID to a name that already exists results in an `ErrNameReserved`
+// A name reservation is globally unique
+func (db *memDB) ReserveName(name, containerID string) error {
+	txn := db.store.Txn(true)
+	defer txn.Commit()
+
+	s, err := txn.First(memdbNamesTable, memdbIDIndex, name)
+	if err != nil {
+		return err
+	}
+	if s != nil {
+		if s.(nameAssociation).containerID != containerID {
+			return ErrNameReserved
+		}
+		return nil
+	}
+
+	txn.Insert(memdbNamesTable, nameAssociation{name: name, containerID: containerID})
+	return nil
+}
+
+// ReleaseName releases the reserved name
+// Once released, a name can be reserved again
+func (db *memDB) ReleaseName(name string) {
+	txn := db.store.Txn(true)
+	txn.Delete(memdbNamesTable, nameAssociation{name: name})
+	txn.Commit()
 }
 
 type memdbView struct {
-	txn       *memdb.Txn
-	nameIndex map[string][]string
+	txn *memdb.Txn
 }
 
 // All returns a all items in this snapshot. Returned objects must never be modified.
 func (v *memdbView) All() ([]Snapshot, error) {
 	var all []Snapshot
-	iter, err := v.txn.Get(memdbTable, memdbIDIndex)
+	iter, err := v.txn.Get(memdbContainersTable, memdbIDIndex)
 	if err != nil {
 		return nil, err
 	}
@@ -141,7 +229,7 @@ func (v *memdbView) All() ([]Snapshot, error) {
 
 // Get returns an item by id. Returned objects must never be modified.
 func (v *memdbView) Get(id string) (*Snapshot, error) {
-	s, err := v.txn.First(memdbTable, memdbIDIndex, id)
+	s, err := v.txn.First(memdbContainersTable, memdbIDIndex, id)
 	if err != nil {
 		return nil, err
 	}
@@ -151,13 +239,64 @@ func (v *memdbView) Get(id string) (*Snapshot, error) {
 	return v.transform(s.(*Container)), nil
 }
 
+// getNames lists all the reserved names for the given container ID.
+func (v *memdbView) getNames(containerID string) []string {
+	iter, err := v.txn.Get(memdbNamesTable, memdbContainerIDIndex, containerID)
+	if err != nil {
+		return nil
+	}
+
+	var names []string
+	for {
+		item := iter.Next()
+		if item == nil {
+			break
+		}
+		names = append(names, item.(nameAssociation).name)
+	}
+
+	return names
+}
+
+// GetID returns the container ID that the passed in name is reserved to.
+func (v *memdbView) GetID(name string) (string, error) {
+	s, err := v.txn.First(memdbNamesTable, memdbIDIndex, name)
+	if err != nil {
+		return "", err
+	}
+	if s == nil {
+		return "", ErrNameNotReserved
+	}
+	return s.(nameAssociation).containerID, nil
+}
+
+// GetAllNames returns all registered names.
+func (v *memdbView) GetAllNames() map[string][]string {
+	iter, err := v.txn.Get(memdbNamesTable, memdbContainerIDIndex)
+	if err != nil {
+		return nil
+	}
+
+	out := make(map[string][]string)
+	for {
+		item := iter.Next()
+		if item == nil {
+			break
+		}
+		assoc := item.(nameAssociation)
+		out[assoc.containerID] = append(out[assoc.containerID], assoc.name)
+	}
+
+	return out
+}
+
 // transform maps a (deep) copied Container object to what queries need.
 // A lock on the Container is not held because these are immutable deep copies.
 func (v *memdbView) transform(container *Container) *Snapshot {
 	snapshot := &Snapshot{
 		Container: types.Container{
 			ID:      container.ID,
-			Names:   v.nameIndex[container.ID],
+			Names:   v.getNames(container.ID),
 			ImageID: container.ImageID.String(),
 			Ports:   []types.Port{},
 			Mounts:  container.GetMountPoints(),
@@ -300,3 +439,55 @@ func (e *containerByIDIndexer) FromArgs(args ...interface{}) ([]byte, error) {
 	arg += "\x00"
 	return []byte(arg), nil
 }
+
+// namesByNameIndexer is used to index container name associations by name.
+type namesByNameIndexer struct{}
+
+func (e *namesByNameIndexer) FromObject(obj interface{}) (bool, []byte, error) {
+	n, ok := obj.(nameAssociation)
+	if !ok {
+		return false, nil, fmt.Errorf(`%T does not have type "nameAssociation"`, obj)
+	}
+
+	// Add the null character as a terminator
+	return true, []byte(n.name + "\x00"), nil
+}
+
+func (e *namesByNameIndexer) FromArgs(args ...interface{}) ([]byte, error) {
+	if len(args) != 1 {
+		return nil, fmt.Errorf("must provide only a single argument")
+	}
+	arg, ok := args[0].(string)
+	if !ok {
+		return nil, fmt.Errorf("argument must be a string: %#v", args[0])
+	}
+	// Add the null character as a terminator
+	arg += "\x00"
+	return []byte(arg), nil
+}
+
+// namesByContainerIDIndexer is used to index container names by container ID.
+type namesByContainerIDIndexer struct{}
+
+func (e *namesByContainerIDIndexer) FromObject(obj interface{}) (bool, []byte, error) {
+	n, ok := obj.(nameAssociation)
+	if !ok {
+		return false, nil, fmt.Errorf(`%T does not have type "nameAssocation"`, obj)
+	}
+
+	// Add the null character as a terminator
+	return true, []byte(n.containerID + "\x00"), nil
+}
+
+func (e *namesByContainerIDIndexer) FromArgs(args ...interface{}) ([]byte, error) {
+	if len(args) != 1 {
+		return nil, fmt.Errorf("must provide only a single argument")
+	}
+	arg, ok := args[0].(string)
+	if !ok {
+		return nil, fmt.Errorf("argument must be a string: %#v", args[0])
+	}
+	// Add the null character as a terminator
+	arg += "\x00"
+	return []byte(arg), nil
+}

container/view_test.go

@@ -7,8 +7,8 @@ import (
 	"testing"
 
 	containertypes "github.com/docker/docker/api/types/container"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/pborman/uuid"
+	"github.com/stretchr/testify/assert"
 )
 
 var root string
@@ -54,7 +54,6 @@ func TestViewSaveDelete(t *testing.T) {
 func TestViewAll(t *testing.T) {
 	var (
 		db, _ = NewViewDB()
-		names = registrar.NewRegistrar()
 		one   = newContainer(t)
 		two   = newContainer(t)
 	)
@@ -67,7 +66,7 @@ func TestViewAll(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	all, err := db.Snapshot(names).All()
+	all, err := db.Snapshot().All()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -89,14 +88,13 @@ func TestViewAll(t *testing.T) {
 func TestViewGet(t *testing.T) {
 	var (
 		db, _ = NewViewDB()
-		names = registrar.NewRegistrar()
 		one   = newContainer(t)
 	)
 	one.ImageID = "some-image-123"
 	if err := one.CheckpointTo(db); err != nil {
 		t.Fatal(err)
 	}
-	s, err := db.Snapshot(names).Get(one.ID)
+	s, err := db.Snapshot().Get(one.ID)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -104,3 +102,52 @@ func TestViewGet(t *testing.T) {
 		t.Fatalf("expected ImageID=some-image-123. Got: %v", s)
 	}
 }
+
+func TestNames(t *testing.T) {
+	db, err := NewViewDB()
+	if err != nil {
+		t.Fatal(err)
+	}
+	assert.NoError(t, db.ReserveName("name1", "containerid1"))
+	assert.NoError(t, db.ReserveName("name1", "containerid1")) // idempotent
+	assert.NoError(t, db.ReserveName("name2", "containerid2"))
+	assert.EqualError(t, db.ReserveName("name2", "containerid3"), ErrNameReserved.Error())
+
+	// Releasing a name allows the name to point to something else later.
+	db.ReleaseName("name2")
+	assert.NoError(t, db.ReserveName("name2", "containerid3"))
+
+	view := db.Snapshot()
+
+	id, err := view.GetID("name1")
+	assert.NoError(t, err)
+	assert.Equal(t, "containerid1", id)
+
+	id, err = view.GetID("name2")
+	assert.NoError(t, err)
+	assert.Equal(t, "containerid3", id)
+
+	_, err = view.GetID("notreserved")
+	assert.EqualError(t, err, ErrNameNotReserved.Error())
+
+	// Releasing and re-reserving a name doesn't affect the snapshot.
+	db.ReleaseName("name2")
+	assert.NoError(t, db.ReserveName("name2", "containerid4"))
+
+	id, err = view.GetID("name1")
+	assert.NoError(t, err)
+	assert.Equal(t, "containerid1", id)
+
+	id, err = view.GetID("name2")
+	assert.NoError(t, err)
+	assert.Equal(t, "containerid3", id)
+
+	// GetAllNames
+	assert.Equal(t, map[string][]string{"containerid1": {"name1"}, "containerid3": {"name2"}}, view.GetAllNames())
+
+	assert.NoError(t, db.ReserveName("name3", "containerid1"))
+	assert.NoError(t, db.ReserveName("name4", "containerid1"))
+
+	view = db.Snapshot()
+	assert.Equal(t, map[string][]string{"containerid1": {"name1", "name3", "name4"}, "containerid4": {"name2"}}, view.GetAllNames())
+}

daemon/container.go

@@ -168,7 +168,7 @@ func (daemon *Daemon) GetByName(name string) (*container.Container, error) {
 	if name[0] != '/' {
 		fullName = "/" + name
 	}
-	id, err := daemon.nameIndex.Get(fullName)
+	id, err := daemon.containersReplica.Snapshot().GetID(fullName)
 	if err != nil {
 		return nil, fmt.Errorf("Could not find entity for %s", name)
 	}

daemon/daemon.go

@@ -42,7 +42,6 @@ import (
 	"github.com/docker/docker/migrate/v1"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/plugingetter"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/docker/pkg/sysinfo"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/truncindex"
@@ -104,7 +103,6 @@ type Daemon struct {
 	stores                map[string]daemonStore // By container target platform
 	PluginStore           *plugin.Store          // todo: remove
 	pluginManager         *plugin.Manager
-	nameIndex             *registrar.Registrar
 	linkIndex             *linkIndex
 	containerd            libcontainerd.Client
 	containerdRemote      libcontainerd.Remote
@@ -448,8 +446,8 @@ func (daemon *Daemon) parents(c *container.Container) map[string]*container.Cont
 
 func (daemon *Daemon) registerLink(parent, child *container.Container, alias string) error {
 	fullName := path.Join(parent.Name, alias)
-	if err := daemon.nameIndex.Reserve(fullName, child.ID); err != nil {
-		if err == registrar.ErrNameReserved {
+	if err := daemon.containersReplica.ReserveName(fullName, child.ID); err != nil {
+		if err == container.ErrNameReserved {
 			logrus.Warnf("error registering link for %s, to %s, as alias %s, ignoring: %v", parent.ID, child.ID, alias, err)
 			return nil
 		}
@@ -780,7 +778,6 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
 	d.seccompEnabled = sysInfo.Seccomp
 	d.apparmorEnabled = sysInfo.AppArmor
 
-	d.nameIndex = registrar.NewRegistrar()
 	d.linkIndex = newLinkIndex()
 	d.containerdRemote = containerdRemote
 

daemon/daemon_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/docker/docker/container"
 	_ "github.com/docker/docker/pkg/discovery/memory"
 	"github.com/docker/docker/pkg/idtools"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/docker/pkg/truncindex"
 	"github.com/docker/docker/volume"
 	volumedrivers "github.com/docker/docker/volume/drivers"
@@ -65,10 +64,15 @@ func TestGetContainer(t *testing.T) {
 	index.Add(c4.ID)
 	index.Add(c5.ID)
 
+	containersReplica, err := container.NewViewDB()
+	if err != nil {
+		t.Fatalf("could not create ViewDB: %v", err)
+	}
+
 	daemon := &Daemon{
-		containers: store,
-		idIndex:    index,
-		nameIndex:  registrar.NewRegistrar(),
+		containers:        store,
+		containersReplica: containersReplica,
+		idIndex:           index,
 	}
 
 	daemon.reserveName(c1.ID, c1.Name)

daemon/delete.go

@@ -60,7 +60,7 @@ func (daemon *Daemon) rmLink(container *container.Container, name string) error
 	}
 
 	parent = strings.TrimSuffix(parent, "/")
-	pe, err := daemon.nameIndex.Get(parent)
+	pe, err := daemon.containersReplica.Snapshot().GetID(parent)
 	if err != nil {
 		return fmt.Errorf("Cannot get parent %s for name %s", parent, name)
 	}
@@ -128,7 +128,6 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
 		return errors.Wrapf(err, "unable to remove filesystem for %s", container.ID)
 	}
 
-	daemon.nameIndex.Delete(container.ID)
 	daemon.linkIndex.delete(container)
 	selinuxFreeLxcContexts(container.ProcessLabel)
 	daemon.idIndex.Delete(container.ID)

daemon/list.go

@@ -182,7 +182,7 @@ func (daemon *Daemon) filterByNameIDMatches(view container.View, ctx *listContex
 // reduceContainers parses the user's filtering options and generates the list of containers to return based on a reducer.
 func (daemon *Daemon) reduceContainers(config *types.ContainerListOptions, reducer containerReducer) ([]*types.Container, error) {
 	var (
-		view       = daemon.containersReplica.Snapshot(daemon.nameIndex)
+		view       = daemon.containersReplica.Snapshot()
 		containers = []*types.Container{}
 	)
 
@@ -361,7 +361,7 @@ func (daemon *Daemon) foldFilter(view container.View, config *types.ContainerLis
 		publish:              publishFilter,
 		expose:               exposeFilter,
 		ContainerListOptions: config,
-		names:                daemon.nameIndex.GetAll(),
+		names:                view.GetAllNames(),
 	}, nil
 }
 func portOp(key string, filter map[nat.Port]bool) func(value string) error {

daemon/names.go

@@ -8,7 +8,6 @@ import (
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/pkg/namesgenerator"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/docker/pkg/stringid"
 )
 
@@ -31,7 +30,7 @@ func (daemon *Daemon) registerName(container *container.Container) error {
 		}
 		container.Name = name
 	}
-	return daemon.nameIndex.Reserve(container.Name, container.ID)
+	return daemon.containersReplica.ReserveName(container.Name, container.ID)
 }
 
 func (daemon *Daemon) generateIDAndName(name string) (string, string, error) {
@@ -62,9 +61,9 @@ func (daemon *Daemon) reserveName(id, name string) (string, error) {
 		name = "/" + name
 	}
 
-	if err := daemon.nameIndex.Reserve(name, id); err != nil {
-		if err == registrar.ErrNameReserved {
-			id, err := daemon.nameIndex.Get(name)
+	if err := daemon.containersReplica.ReserveName(name, id); err != nil {
+		if err == container.ErrNameReserved {
+			id, err := daemon.containersReplica.Snapshot().GetID(name)
 			if err != nil {
 				logrus.Errorf("got unexpected error while looking up reserved name: %v", err)
 				return "", err
@@ -77,7 +76,7 @@ func (daemon *Daemon) reserveName(id, name string) (string, error) {
 }
 
 func (daemon *Daemon) releaseName(name string) {
-	daemon.nameIndex.Release(name)
+	daemon.containersReplica.ReleaseName(name)
 }
 
 func (daemon *Daemon) generateNewName(id string) (string, error) {
@@ -88,8 +87,8 @@ func (daemon *Daemon) generateNewName(id string) (string, error) {
 			name = "/" + name
 		}
 
-		if err := daemon.nameIndex.Reserve(name, id); err != nil {
-			if err == registrar.ErrNameReserved {
+		if err := daemon.containersReplica.ReserveName(name, id); err != nil {
+			if err == container.ErrNameReserved {
 				continue
 			}
 			return "", err
@@ -98,7 +97,7 @@ func (daemon *Daemon) generateNewName(id string) (string, error) {
 	}
 
 	name = "/" + stringid.TruncateID(id)
-	if err := daemon.nameIndex.Reserve(name, id); err != nil {
+	if err := daemon.containersReplica.ReserveName(name, id); err != nil {
 		return "", err
 	}
 	return name, nil

daemon/rename.go

@@ -55,7 +55,7 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error {
 	}
 
 	for k, v := range links {
-		daemon.nameIndex.Reserve(newName+k, v.ID)
+		daemon.containersReplica.ReserveName(newName+k, v.ID)
 		daemon.linkIndex.link(container, v, newName+k)
 	}
 
@@ -68,10 +68,10 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error {
 			container.NetworkSettings.IsAnonymousEndpoint = oldIsAnonymousEndpoint
 			daemon.reserveName(container.ID, oldName)
 			for k, v := range links {
-				daemon.nameIndex.Reserve(oldName+k, v.ID)
+				daemon.containersReplica.ReserveName(oldName+k, v.ID)
 				daemon.linkIndex.link(container, v, oldName+k)
 				daemon.linkIndex.unlink(newName+k, v, container)
-				daemon.nameIndex.Release(newName + k)
+				daemon.containersReplica.ReleaseName(newName + k)
 			}
 			daemon.releaseName(newName)
 		}
@@ -79,7 +79,7 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error {
 
 	for k, v := range links {
 		daemon.linkIndex.unlink(oldName+k, v, container)
-		daemon.nameIndex.Release(oldName + k)
+		daemon.containersReplica.ReleaseName(oldName + k)
 	}
 	daemon.releaseName(oldName)
 	if err = container.CheckpointTo(daemon.containersReplica); err != nil {

pkg/registrar/registrar.go

@@ -1,130 +0,0 @@
-// Package registrar provides name registration. It reserves a name to a given key.
-package registrar
-
-import (
-	"errors"
-	"sync"
-)
-
-var (
-	// ErrNameReserved is an error which is returned when a name is requested to be reserved that already is reserved
-	ErrNameReserved = errors.New("name is reserved")
-	// ErrNameNotReserved is an error which is returned when trying to find a name that is not reserved
-	ErrNameNotReserved = errors.New("name is not reserved")
-	// ErrNoSuchKey is returned when trying to find the names for a key which is not known
-	ErrNoSuchKey = errors.New("provided key does not exist")
-)
-
-// Registrar stores indexes a list of keys and their registered names as well as indexes names and the key that they are registered to
-// Names must be unique.
-// Registrar is safe for concurrent access.
-type Registrar struct {
-	idx   map[string][]string
-	names map[string]string
-	mu    sync.Mutex
-}
-
-// NewRegistrar creates a new Registrar with the an empty index
-func NewRegistrar() *Registrar {
-	return &Registrar{
-		idx:   make(map[string][]string),
-		names: make(map[string]string),
-	}
-}
-
-// Reserve registers a key to a name
-// Reserve is idempotent
-// Attempting to reserve a key to a name that already exists results in an `ErrNameReserved`
-// A name reservation is globally unique
-func (r *Registrar) Reserve(name, key string) error {
-	r.mu.Lock()
-	defer r.mu.Unlock()
-
-	if k, exists := r.names[name]; exists {
-		if k != key {
-			return ErrNameReserved
-		}
-		return nil
-	}
-
-	r.idx[key] = append(r.idx[key], name)
-	r.names[name] = key
-	return nil
-}
-
-// Release releases the reserved name
-// Once released, a name can be reserved again
-func (r *Registrar) Release(name string) {
-	r.mu.Lock()
-	defer r.mu.Unlock()
-
-	key, exists := r.names[name]
-	if !exists {
-		return
-	}
-
-	for i, n := range r.idx[key] {
-		if n != name {
-			continue
-		}
-		r.idx[key] = append(r.idx[key][:i], r.idx[key][i+1:]...)
-		break
-	}
-
-	delete(r.names, name)
-
-	if len(r.idx[key]) == 0 {
-		delete(r.idx, key)
-	}
-}
-
-// Delete removes all reservations for the passed in key.
-// All names reserved to this key are released.
-func (r *Registrar) Delete(key string) {
-	r.mu.Lock()
-	for _, name := range r.idx[key] {
-		delete(r.names, name)
-	}
-	delete(r.idx, key)
-	r.mu.Unlock()
-}
-
-// GetNames lists all the reserved names for the given key
-func (r *Registrar) GetNames(key string) ([]string, error) {
-	r.mu.Lock()
-	defer r.mu.Unlock()
-
-	names, exists := r.idx[key]
-	if !exists {
-		return nil, ErrNoSuchKey
-	}
-
-	ls := make([]string, 0, len(names))
-	ls = append(ls, names...)
-	return ls, nil
-}
-
-// Get returns the key that the passed in name is reserved to
-func (r *Registrar) Get(name string) (string, error) {
-	r.mu.Lock()
-	key, exists := r.names[name]
-	r.mu.Unlock()
-
-	if !exists {
-		return "", ErrNameNotReserved
-	}
-	return key, nil
-}
-
-// GetAll returns all registered names
-func (r *Registrar) GetAll() map[string][]string {
-	out := make(map[string][]string)
-
-	r.mu.Lock()
-	// copy index into out
-	for id, names := range r.idx {
-		out[id] = names
-	}
-	r.mu.Unlock()
-	return out
-}

pkg/registrar/registrar_test.go

@@ -1,119 +0,0 @@
-package registrar
-
-import (
-	"reflect"
-	"testing"
-)
-
-func TestReserve(t *testing.T) {
-	r := NewRegistrar()
-
-	obj := "test1"
-	if err := r.Reserve("test", obj); err != nil {
-		t.Fatal(err)
-	}
-
-	if err := r.Reserve("test", obj); err != nil {
-		t.Fatal(err)
-	}
-
-	obj2 := "test2"
-	err := r.Reserve("test", obj2)
-	if err == nil {
-		t.Fatalf("expected error when reserving an already reserved name to another object")
-	}
-	if err != ErrNameReserved {
-		t.Fatal("expected `ErrNameReserved` error when attempting to reserve an already reserved name")
-	}
-}
-
-func TestRelease(t *testing.T) {
-	r := NewRegistrar()
-	obj := "testing"
-
-	if err := r.Reserve("test", obj); err != nil {
-		t.Fatal(err)
-	}
-	r.Release("test")
-	r.Release("test") // Ensure there is no panic here
-
-	if err := r.Reserve("test", obj); err != nil {
-		t.Fatal(err)
-	}
-}
-
-func TestGetNames(t *testing.T) {
-	r := NewRegistrar()
-	obj := "testing"
-	names := []string{"test1", "test2"}
-
-	for _, name := range names {
-		if err := r.Reserve(name, obj); err != nil {
-			t.Fatal(err)
-		}
-	}
-	r.Reserve("test3", "other")
-
-	names2, err := r.GetNames(obj)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if !reflect.DeepEqual(names, names2) {
-		t.Fatalf("Expected: %v, Got: %v", names, names2)
-	}
-}
-
-func TestDelete(t *testing.T) {
-	r := NewRegistrar()
-	obj := "testing"
-	names := []string{"test1", "test2"}
-	for _, name := range names {
-		if err := r.Reserve(name, obj); err != nil {
-			t.Fatal(err)
-		}
-	}
-
-	r.Reserve("test3", "other")
-	r.Delete(obj)
-
-	_, err := r.GetNames(obj)
-	if err == nil {
-		t.Fatal("expected error getting names for deleted key")
-	}
-
-	if err != ErrNoSuchKey {
-		t.Fatal("expected `ErrNoSuchKey`")
-	}
-}
-
-func TestGet(t *testing.T) {
-	r := NewRegistrar()
-	obj := "testing"
-	name := "test"
-
-	_, err := r.Get(name)
-	if err == nil {
-		t.Fatal("expected error when key does not exist")
-	}
-	if err != ErrNameNotReserved {
-		t.Fatal(err)
-	}
-
-	if err := r.Reserve(name, obj); err != nil {
-		t.Fatal(err)
-	}
-
-	if _, err = r.Get(name); err != nil {
-		t.Fatal(err)
-	}
-
-	r.Delete(obj)
-	_, err = r.Get(name)
-	if err == nil {
-		t.Fatal("expected error when key does not exist")
-	}
-	if err != ErrNameNotReserved {
-		t.Fatal(err)
-	}
-}