Merge pull request #33886 from aaronlehmann/names-in-memdb

Store container names in memdb

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) error
 }
 
 // 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,37 +133,91 @@ 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),
+	}
+}
+
+func (db *memDB) withTxn(cb func(*memdb.Txn) error) error {
+	txn := db.store.Txn(true)
+	err := cb(txn)
+	if err != nil {
+		txn.Abort()
+		return err
 	}
+	txn.Commit()
+	return nil
 }
 
 // Save atomically updates the in-memory store state for a Container.
 // Only read only (deep) copies of containers may be passed in.
 func (db *memDB) Save(c *Container) error {
-	txn := db.store.Txn(true)
-	defer txn.Commit()
-	return txn.Insert(memdbTable, c)
+	return db.withTxn(func(txn *memdb.Txn) error {
+		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))
+	return db.withTxn(func(txn *memdb.Txn) error {
+		view := &memdbView{txn: txn}
+		names := view.getNames(c.ID)
+
+		for _, name := range names {
+			txn.Delete(memdbNamesTable, nameAssociation{name: name})
+		}
+
+		if err := txn.Delete(memdbContainersTable, NewBaseContainer(c.ID, c.Root)); err != nil {
+			return err
+		}
+		return nil
+	})
+}
+
+// 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 {
+	return db.withTxn(func(txn *memdb.Txn) error {
+		s, err := txn.First(memdbNamesTable, memdbIDIndex, name)
+		if err != nil {
+			return err
+		}
+		if s != nil {
+			if s.(nameAssociation).containerID != containerID {
+				return ErrNameReserved
+			}
+			return nil
+		}
+
+		if err := txn.Insert(memdbNamesTable, nameAssociation{name: name, containerID: containerID}); err != nil {
+			return err
+		}
+		return nil
+	})
+}
+
+// ReleaseName releases the reserved name
+// Once released, a name can be reserved again
+func (db *memDB) ReleaseName(name string) error {
+	return db.withTxn(func(txn *memdb.Txn) error {
+		if err := txn.Delete(memdbNamesTable, nameAssociation{name: name}); err != nil {
+			return err
+		}
+		return nil
+	})
 }
 
 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 +234,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 +244,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 +444,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.
+	assert.NoError(t, 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.
+	assert.NoError(t, 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)
-	}
-}