From 1128fc1add66a849c12d2045aed39605e673abc6 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 29 Jun 2017 18:56:22 -0700 Subject: [PATCH] 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 --- container/view.go | 223 +++++++++++++++++++++++++++++--- container/view_test.go | 57 +++++++- daemon/container.go | 2 +- daemon/daemon.go | 7 +- daemon/daemon_test.go | 12 +- daemon/delete.go | 3 +- daemon/list.go | 4 +- daemon/names.go | 17 ++- daemon/rename.go | 8 +- pkg/registrar/registrar.go | 130 ------------------- pkg/registrar/registrar_test.go | 119 ----------------- 11 files changed, 285 insertions(+), 297 deletions(-) delete mode 100644 pkg/registrar/registrar.go delete mode 100644 pkg/registrar/registrar_test.go diff --git a/container/view.go b/container/view.go index f605b4f483..449cade149 100644 --- a/container/view.go +++ b/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 +} diff --git a/container/view_test.go b/container/view_test.go index 9b872998bd..2e81998ca4 100644 --- a/container/view_test.go +++ b/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()) +} diff --git a/daemon/container.go b/daemon/container.go index 149df0dec6..4c015b70de 100644 --- a/daemon/container.go +++ b/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) } diff --git a/daemon/daemon.go b/daemon/daemon.go index 8359ef31ca..93d871d6df 100644 --- a/daemon/daemon.go +++ b/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 diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 6f07d0d1ee..13d1059c1c 100644 --- a/daemon/daemon_test.go +++ b/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) diff --git a/daemon/delete.go b/daemon/delete.go index 2d3cd0f90f..c57a89654b 100644 --- a/daemon/delete.go +++ b/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) diff --git a/daemon/list.go b/daemon/list.go index b854be7549..6889c55889 100644 --- a/daemon/list.go +++ b/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 { diff --git a/daemon/names.go b/daemon/names.go index ec6ac2924f..7cdabeba9f 100644 --- a/daemon/names.go +++ b/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 diff --git a/daemon/rename.go b/daemon/rename.go index 2a8d0b22c7..686fbd3b99 100644 --- a/daemon/rename.go +++ b/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 { diff --git a/pkg/registrar/registrar.go b/pkg/registrar/registrar.go deleted file mode 100644 index df12db7eeb..0000000000 --- a/pkg/registrar/registrar.go +++ /dev/null @@ -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 -} diff --git a/pkg/registrar/registrar_test.go b/pkg/registrar/registrar_test.go deleted file mode 100644 index 70f8084b30..0000000000 --- a/pkg/registrar/registrar_test.go +++ /dev/null @@ -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) - } -}