Bladeren bron

Merge pull request #16032 from cpuguy83/remove_sqlite_dep

Build names and links at runtime - no more sqlite
Alexander Morozov 9 jaren geleden
bovenliggende
commit
9a23569ecf

+ 31 - 43
daemon/container_operations_unix.go

@@ -37,40 +37,36 @@ import (
 
 func (daemon *Daemon) setupLinkedContainers(container *container.Container) ([]string, error) {
 	var env []string
-	children, err := daemon.children(container.Name)
-	if err != nil {
-		return nil, err
-	}
+	children := daemon.children(container)
 
 	bridgeSettings := container.NetworkSettings.Networks["bridge"]
 	if bridgeSettings == nil {
 		return nil, nil
 	}
 
-	if len(children) > 0 {
-		for linkAlias, child := range children {
-			if !child.IsRunning() {
-				return nil, derr.ErrorCodeLinkNotRunning.WithArgs(child.Name, linkAlias)
-			}
+	for linkAlias, child := range children {
+		if !child.IsRunning() {
+			return nil, derr.ErrorCodeLinkNotRunning.WithArgs(child.Name, linkAlias)
+		}
 
-			childBridgeSettings := child.NetworkSettings.Networks["bridge"]
-			if childBridgeSettings == nil {
-				return nil, fmt.Errorf("container %s not attached to default bridge network", child.ID)
-			}
+		childBridgeSettings := child.NetworkSettings.Networks["bridge"]
+		if childBridgeSettings == nil {
+			return nil, fmt.Errorf("container %s not attached to default bridge network", child.ID)
+		}
 
-			link := links.NewLink(
-				bridgeSettings.IPAddress,
-				childBridgeSettings.IPAddress,
-				linkAlias,
-				child.Config.Env,
-				child.Config.ExposedPorts,
-			)
+		link := links.NewLink(
+			bridgeSettings.IPAddress,
+			childBridgeSettings.IPAddress,
+			linkAlias,
+			child.Config.Env,
+			child.Config.ExposedPorts,
+		)
 
-			for _, envVar := range link.ToEnv() {
-				env = append(env, envVar)
-			}
+		for _, envVar := range link.ToEnv() {
+			env = append(env, envVar)
 		}
 	}
+
 	return env, nil
 }
 
@@ -419,11 +415,7 @@ func (daemon *Daemon) buildSandboxOptions(container *container.Container, n libn
 
 	var childEndpoints, parentEndpoints []string
 
-	children, err := daemon.children(container.Name)
-	if err != nil {
-		return nil, err
-	}
-
+	children := daemon.children(container)
 	for linkAlias, child := range children {
 		if !isLinkable(child) {
 			return nil, fmt.Errorf("Cannot link to %s, as it does not belong to the default network", child.Name)
@@ -443,23 +435,20 @@ func (daemon *Daemon) buildSandboxOptions(container *container.Container, n libn
 	}
 
 	bridgeSettings := container.NetworkSettings.Networks["bridge"]
-	refs := daemon.containerGraph().RefPaths(container.ID)
-	for _, ref := range refs {
-		if ref.ParentID == "0" {
+	for alias, parent := range daemon.parents(container) {
+		if daemon.configStore.DisableBridge || !container.HostConfig.NetworkMode.IsPrivate() {
 			continue
 		}
 
-		c, err := daemon.GetContainer(ref.ParentID)
-		if err != nil {
-			logrus.Error(err)
-		}
-
-		if c != nil && !daemon.configStore.DisableBridge && container.HostConfig.NetworkMode.IsPrivate() {
-			logrus.Debugf("Update /etc/hosts of %s for alias %s with ip %s", c.ID, ref.Name, bridgeSettings.IPAddress)
-			sboxOptions = append(sboxOptions, libnetwork.OptionParentUpdate(c.ID, ref.Name, bridgeSettings.IPAddress))
-			if ep.ID() != "" {
-				parentEndpoints = append(parentEndpoints, ep.ID())
-			}
+		_, alias = path.Split(alias)
+		logrus.Debugf("Update /etc/hosts of %s for alias %s with ip %s", parent.ID, alias, bridgeSettings.IPAddress)
+		sboxOptions = append(sboxOptions, libnetwork.OptionParentUpdate(
+			parent.ID,
+			alias,
+			bridgeSettings.IPAddress,
+		))
+		if ep.ID() != "" {
+			parentEndpoints = append(parentEndpoints, ep.ID())
 		}
 	}
 
@@ -471,7 +460,6 @@ func (daemon *Daemon) buildSandboxOptions(container *container.Container, n libn
 	}
 
 	sboxOptions = append(sboxOptions, libnetwork.OptionGeneric(linkOptions))
-
 	return sboxOptions, nil
 }
 

+ 105 - 138
daemon/daemon.go

@@ -12,9 +12,9 @@ import (
 	"io/ioutil"
 	"net"
 	"os"
+	"path"
 	"path/filepath"
 	"runtime"
-	"strings"
 	"sync"
 	"syscall"
 	"time"
@@ -53,6 +53,7 @@ import (
 	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/namesgenerator"
 	"github.com/docker/docker/pkg/progress"
+	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/streamformatter"
 	"github.com/docker/docker/pkg/stringid"
@@ -147,7 +148,6 @@ type Daemon struct {
 	trustKey                  libtrust.PrivateKey
 	idIndex                   *truncindex.TruncIndex
 	configStore               *Config
-	containerGraphDB          *graphdb.Database
 	execDriver                execdriver.Driver
 	statsCollector            *statsCollector
 	defaultLogConfig          containertypes.LogConfig
@@ -162,6 +162,8 @@ type Daemon struct {
 	gidMaps                   []idtools.IDMap
 	layerStore                layer.Store
 	imageStore                image.Store
+	nameIndex                 *registrar.Registrar
+	linkIndex                 *linkIndex
 }
 
 // GetContainer looks for a container using the provided information, which could be
@@ -245,8 +247,7 @@ func (daemon *Daemon) registerName(container *container.Container) error {
 			logrus.Errorf("Error saving container name to disk: %v", err)
 		}
 	}
-
-	return nil
+	return daemon.nameIndex.Reserve(container.Name, container.ID)
 }
 
 // Register makes a container object usable by the daemon as <container.ID>
@@ -257,10 +258,8 @@ func (daemon *Daemon) Register(container *container.Container) error {
 	} else {
 		container.NewNopInputPipe()
 	}
-	daemon.containers.Add(container.ID, container)
 
-	// don't update the Suffixarray if we're starting up
-	// we'll waste time if we update it for every container
+	daemon.containers.Add(container.ID, container)
 	daemon.idIndex.Add(container.ID)
 
 	if container.IsRunning() {
@@ -291,15 +290,10 @@ func (daemon *Daemon) Register(container *container.Container) error {
 }
 
 func (daemon *Daemon) restore() error {
-	type cr struct {
-		container  *container.Container
-		registered bool
-	}
-
 	var (
 		debug         = os.Getenv("DEBUG") != ""
 		currentDriver = daemon.GraphDriverName()
-		containers    = make(map[string]*cr)
+		containers    = make(map[string]*container.Container)
 	)
 
 	if !debug {
@@ -332,63 +326,70 @@ func (daemon *Daemon) restore() error {
 		if (container.Driver == "" && currentDriver == "aufs") || container.Driver == currentDriver {
 			logrus.Debugf("Loaded container %v", container.ID)
 
-			containers[container.ID] = &cr{container: container}
+			containers[container.ID] = container
 		} else {
 			logrus.Debugf("Cannot load container %s because it was created with another graph driver.", container.ID)
 		}
 	}
 
-	if entities := daemon.containerGraphDB.List("/", -1); entities != nil {
-		for _, p := range entities.Paths() {
-			if !debug && logrus.GetLevel() == logrus.InfoLevel {
-				fmt.Print(".")
-			}
+	var migrateLegacyLinks bool
+	restartContainers := make(map[*container.Container]chan struct{})
+	for _, c := range containers {
+		if err := daemon.registerName(c); err != nil {
+			logrus.Errorf("Failed to register container %s: %s", c.ID, err)
+			continue
+		}
+		if err := daemon.Register(c); err != nil {
+			logrus.Errorf("Failed to register container %s: %s", c.ID, err)
+			continue
+		}
 
-			e := entities[p]
+		// get list of containers we need to restart
+		if daemon.configStore.AutoRestart && c.ShouldRestart() {
+			restartContainers[c] = make(chan struct{})
+		}
 
-			if c, ok := containers[e.ID()]; ok {
-				c.registered = true
-			}
+		// if c.hostConfig.Links is nil (not just empty), then it is using the old sqlite links and needs to be migrated
+		if c.HostConfig != nil && c.HostConfig.Links == nil {
+			migrateLegacyLinks = true
 		}
 	}
 
-	restartContainers := make(map[*container.Container]chan struct{})
-	for _, c := range containers {
-		if !c.registered {
-			// Try to set the default name for a container if it exists prior to links
-			c.container.Name, err = daemon.generateNewName(c.container.ID)
-			if err != nil {
-				logrus.Debugf("Setting default id - %s", err)
-			}
-			if err := daemon.registerName(c.container); err != nil {
-				logrus.Errorf("Failed to register container %s: %s", c.container.ID, err)
-				continue
-			}
+	// migrate any legacy links from sqlite
+	linkdbFile := filepath.Join(daemon.root, "linkgraph.db")
+	var legacyLinkDB *graphdb.Database
+	if migrateLegacyLinks {
+		legacyLinkDB, err = graphdb.NewSqliteConn(linkdbFile)
+		if err != nil {
+			return fmt.Errorf("error connecting to legacy link graph DB %s, container links may be lost: %v", linkdbFile, err)
 		}
+		defer legacyLinkDB.Close()
+	}
 
-		if err := daemon.Register(c.container); err != nil {
-			logrus.Errorf("Failed to register container %s: %s", c.container.ID, err)
-			continue
+	// Now that all the containers are registered, register the links
+	for _, c := range containers {
+		if migrateLegacyLinks {
+			if err := daemon.migrateLegacySqliteLinks(legacyLinkDB, c); err != nil {
+				return err
+			}
 		}
-		// get list of containers we need to restart
-		if daemon.configStore.AutoRestart && c.container.ShouldRestart() {
-			restartContainers[c.container] = make(chan struct{})
+		if err := daemon.registerLinks(c, c.HostConfig); err != nil {
+			logrus.Errorf("failed to register link for container %s: %v", c.ID, err)
 		}
 	}
 
 	group := sync.WaitGroup{}
 	for c, notifier := range restartContainers {
 		group.Add(1)
-		go func(container *container.Container, chNotify chan struct{}) {
+
+		go func(c *container.Container, chNotify chan struct{}) {
 			defer group.Done()
-			logrus.Debugf("Starting container %s", container.ID)
+
+			logrus.Debugf("Starting container %s", c.ID)
 
 			// ignore errors here as this is a best effort to wait for children to be
 			//   running before we try to start the container
-			children, err := daemon.children(container.Name)
-			if err != nil {
-				logrus.Warnf("error getting children for %s: %v", container.Name, err)
-			}
+			children := daemon.children(c)
 			timeout := time.After(5 * time.Second)
 			for _, child := range children {
 				if notifier, exists := restartContainers[child]; exists {
@@ -398,11 +399,12 @@ func (daemon *Daemon) restore() error {
 					}
 				}
 			}
-			if err := daemon.containerStart(container); err != nil {
-				logrus.Errorf("Failed to start container %s: %s", container.ID, err)
+			if err := daemon.containerStart(c); err != nil {
+				logrus.Errorf("Failed to start container %s: %s", c.ID, err)
 			}
 			close(chNotify)
 		}(c, notifier)
+
 	}
 	group.Wait()
 
@@ -452,28 +454,28 @@ func (daemon *Daemon) reserveName(id, name string) (string, error) {
 	if !validContainerNamePattern.MatchString(name) {
 		return "", fmt.Errorf("Invalid container name (%s), only %s are allowed", name, validContainerNameChars)
 	}
-
 	if name[0] != '/' {
 		name = "/" + name
 	}
 
-	if _, err := daemon.containerGraphDB.Set(name, id); err != nil {
-		if !graphdb.IsNonUniqueNameError(err) {
-			return "", err
-		}
-
-		conflictingContainer, err := daemon.GetByName(name)
-		if err != nil {
-			return "", err
+	if err := daemon.nameIndex.Reserve(name, id); err != nil {
+		if err == registrar.ErrNameReserved {
+			id, err := daemon.nameIndex.Get(name)
+			if err != nil {
+				logrus.Errorf("got unexpected error while looking up reserved name: %v", err)
+				return "", err
+			}
+			return "", fmt.Errorf("Conflict. The name %q is already in use by container %s. You have to remove (or rename) that container to be able to reuse that name.", name, id)
 		}
-		return "", fmt.Errorf(
-			"Conflict. The name %q is already in use by container %s. You have to remove (or rename) that container to be able to reuse that name.", strings.TrimPrefix(name, "/"),
-			stringid.TruncateID(conflictingContainer.ID))
-
+		return "", fmt.Errorf("error reserving name: %s, error: %v", name, err)
 	}
 	return name, nil
 }
 
+func (daemon *Daemon) releaseName(name string) {
+	daemon.nameIndex.Release(name)
+}
+
 func (daemon *Daemon) generateNewName(id string) (string, error) {
 	var name string
 	for i := 0; i < 6; i++ {
@@ -482,17 +484,17 @@ func (daemon *Daemon) generateNewName(id string) (string, error) {
 			name = "/" + name
 		}
 
-		if _, err := daemon.containerGraphDB.Set(name, id); err != nil {
-			if !graphdb.IsNonUniqueNameError(err) {
-				return "", err
+		if err := daemon.nameIndex.Reserve(name, id); err != nil {
+			if err == registrar.ErrNameReserved {
+				continue
 			}
-			continue
+			return "", err
 		}
 		return name, nil
 	}
 
 	name = "/" + stringid.TruncateID(id)
-	if _, err := daemon.containerGraphDB.Set(name, id); err != nil {
+	if err := daemon.nameIndex.Reserve(name, id); err != nil {
 		return "", err
 	}
 	return name, nil
@@ -542,32 +544,19 @@ func (daemon *Daemon) newContainer(name string, config *containertypes.Config, i
 	return base, err
 }
 
-// GetFullContainerName returns a constructed container name. I think
-// it has to do with the fact that a container is a file on disk and
-// this is sort of just creating a file name.
-func GetFullContainerName(name string) (string, error) {
-	if name == "" {
-		return "", fmt.Errorf("Container name cannot be empty")
-	}
-	if name[0] != '/' {
-		name = "/" + name
-	}
-	return name, nil
-}
-
 // GetByName returns a container given a name.
 func (daemon *Daemon) GetByName(name string) (*container.Container, error) {
-	fullName, err := GetFullContainerName(name)
-	if err != nil {
-		return nil, err
+	fullName := name
+	if name[0] != '/' {
+		fullName = "/" + name
 	}
-	entity := daemon.containerGraphDB.Get(fullName)
-	if entity == nil {
+	id, err := daemon.nameIndex.Get(fullName)
+	if err != nil {
 		return nil, fmt.Errorf("Could not find entity for %s", name)
 	}
-	e := daemon.containers.Get(entity.ID())
+	e := daemon.containers.Get(id)
 	if e == nil {
-		return nil, fmt.Errorf("Could not find container for entity id %s", entity.ID())
+		return nil, fmt.Errorf("Could not find container for entity id %s", id)
 	}
 	return e, nil
 }
@@ -584,48 +573,37 @@ func (daemon *Daemon) UnsubscribeFromEvents(listener chan interface{}) {
 	daemon.EventsService.Evict(listener)
 }
 
-// children returns all child containers of the container with the
-// given name. The containers are returned as a map from the container
-// name to a pointer to Container.
-func (daemon *Daemon) children(name string) (map[string]*container.Container, error) {
-	name, err := GetFullContainerName(name)
-	if err != nil {
-		return nil, err
+// GetLabels for a container or image id
+func (daemon *Daemon) GetLabels(id string) map[string]string {
+	// TODO: TestCase
+	container := daemon.containers.Get(id)
+	if container != nil {
+		return container.Config.Labels
 	}
-	children := make(map[string]*container.Container)
 
-	err = daemon.containerGraphDB.Walk(name, func(p string, e *graphdb.Entity) error {
-		c, err := daemon.GetContainer(e.ID())
-		if err != nil {
-			return err
-		}
-		children[p] = c
-		return nil
-	}, 0)
-
-	if err != nil {
-		return nil, err
+	img, err := daemon.GetImage(id)
+	if err == nil {
+		return img.ContainerConfig.Labels
 	}
-	return children, nil
+	return nil
+}
+
+func (daemon *Daemon) children(c *container.Container) map[string]*container.Container {
+	return daemon.linkIndex.children(c)
 }
 
 // parents returns the names of the parent containers of the container
 // with the given name.
-func (daemon *Daemon) parents(name string) ([]string, error) {
-	name, err := GetFullContainerName(name)
-	if err != nil {
-		return nil, err
-	}
-
-	return daemon.containerGraphDB.Parents(name)
+func (daemon *Daemon) parents(c *container.Container) map[string]*container.Container {
+	return daemon.linkIndex.parents(c)
 }
 
 func (daemon *Daemon) registerLink(parent, child *container.Container, alias string) error {
-	fullName := filepath.Join(parent.Name, alias)
-	if !daemon.containerGraphDB.Exists(fullName) {
-		_, err := daemon.containerGraphDB.Set(fullName, child.ID)
+	fullName := path.Join(parent.Name, alias)
+	if err := daemon.nameIndex.Reserve(fullName, child.ID); err != nil {
 		return err
 	}
+	daemon.linkIndex.link(parent, child, fullName)
 	return nil
 }
 
@@ -813,14 +791,6 @@ func NewDaemon(config *Config, registryService *registry.Service) (daemon *Daemo
 		return nil, fmt.Errorf("Error initializing network controller: %v", err)
 	}
 
-	graphdbPath := filepath.Join(config.Root, "linkgraph.db")
-	graph, err := graphdb.NewSqliteConn(graphdbPath)
-	if err != nil {
-		return nil, err
-	}
-
-	d.containerGraphDB = graph
-
 	sysInfo := sysinfo.New(false)
 	// Check if Devices cgroup is mounted, it is hard requirement for container security,
 	// on Linux/FreeBSD.
@@ -852,10 +822,12 @@ func NewDaemon(config *Config, registryService *registry.Service) (daemon *Daemo
 	d.uidMaps = uidMaps
 	d.gidMaps = gidMaps
 
+	d.nameIndex = registrar.NewRegistrar()
+	d.linkIndex = newLinkIndex()
+
 	if err := d.cleanupMounts(); err != nil {
 		return nil, err
 	}
-
 	go d.execCommandGC()
 
 	if err := d.restore(); err != nil {
@@ -933,12 +905,6 @@ func (daemon *Daemon) Shutdown() error {
 		daemon.netController.Stop()
 	}
 
-	if daemon.containerGraphDB != nil {
-		if err := daemon.containerGraphDB.Close(); err != nil {
-			logrus.Errorf("Error during container graph.Close(): %v", err)
-		}
-	}
-
 	if daemon.layerStore != nil {
 		if err := daemon.layerStore.Cleanup(); err != nil {
 			logrus.Errorf("Error during layer Store.Cleanup(): %v", err)
@@ -1340,10 +1306,6 @@ func (daemon *Daemon) ExecutionDriver() execdriver.Driver {
 	return daemon.execDriver
 }
 
-func (daemon *Daemon) containerGraph() *graphdb.Database {
-	return daemon.containerGraphDB
-}
-
 // GetUIDGIDMaps returns the current daemon's user namespace settings
 // for the full uid and gid maps which will be applied to containers
 // started in this instance.
@@ -1420,9 +1382,14 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig *
 		return err
 	}
 
+	// make sure links is not nil
+	// this ensures that on the next daemon restart we don't try to migrate from legacy sqlite links
+	if hostConfig.Links == nil {
+		hostConfig.Links = []string{}
+	}
+
 	container.HostConfig = hostConfig
-	container.ToDisk()
-	return nil
+	return container.ToDisk()
 }
 
 func (daemon *Daemon) setupInitLayer(initPath string) error {

+ 10 - 31
daemon/daemon_test.go

@@ -3,12 +3,11 @@ package daemon
 import (
 	"io/ioutil"
 	"os"
-	"path"
 	"path/filepath"
 	"testing"
 
 	"github.com/docker/docker/container"
-	"github.com/docker/docker/pkg/graphdb"
+	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/docker/pkg/truncindex"
 	"github.com/docker/docker/volume"
 	volumedrivers "github.com/docker/docker/volume/drivers"
@@ -75,23 +74,18 @@ func TestGetContainer(t *testing.T) {
 	index.Add(c4.ID)
 	index.Add(c5.ID)
 
-	daemonTestDbPath := path.Join(os.TempDir(), "daemon_test.db")
-	graph, err := graphdb.NewSqliteConn(daemonTestDbPath)
-	if err != nil {
-		t.Fatalf("Failed to create daemon test sqlite database at %s", daemonTestDbPath)
-	}
-	graph.Set(c1.Name, c1.ID)
-	graph.Set(c2.Name, c2.ID)
-	graph.Set(c3.Name, c3.ID)
-	graph.Set(c4.Name, c4.ID)
-	graph.Set(c5.Name, c5.ID)
-
 	daemon := &Daemon{
-		containers:       store,
-		idIndex:          index,
-		containerGraphDB: graph,
+		containers: store,
+		idIndex:    index,
+		nameIndex:  registrar.NewRegistrar(),
 	}
 
+	daemon.reserveName(c1.ID, c1.Name)
+	daemon.reserveName(c2.ID, c2.Name)
+	daemon.reserveName(c3.ID, c3.Name)
+	daemon.reserveName(c4.ID, c4.Name)
+	daemon.reserveName(c5.ID, c5.Name)
+
 	if container, _ := daemon.GetContainer("3cdbd1aa394fd68559fd1441d6eff2ab7c1e6363582c82febfaa8045df3bd8de"); container != c2 {
 		t.Fatal("Should explicitly match full container IDs")
 	}
@@ -120,8 +114,6 @@ func TestGetContainer(t *testing.T) {
 	if _, err := daemon.GetContainer("nothing"); err == nil {
 		t.Fatal("Should return an error when provided a prefix that is neither a name or a partial match to an ID")
 	}
-
-	os.Remove(daemonTestDbPath)
 }
 
 func initDaemonWithVolumeStore(tmp string) (*Daemon, error) {
@@ -206,19 +198,6 @@ func TestNetworkOptions(t *testing.T) {
 	}
 }
 
-func TestGetFullName(t *testing.T) {
-	name, err := GetFullContainerName("testing")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if name != "/testing" {
-		t.Fatalf("Expected /testing got %s", name)
-	}
-	if _, err := GetFullContainerName(""); err == nil {
-		t.Fatal("Error should not be nil")
-	}
-}
-
 func TestValidContainerNames(t *testing.T) {
 	invalidNames := []string{"-rm", "&sdfsfd", "safd%sd"}
 	validNames := []string{"word-word", "word_word", "1weoid"}

+ 2 - 7
daemon/daemon_unix.go

@@ -676,7 +676,7 @@ func setupInitLayer(initLayer string, rootUID, rootGID int) error {
 
 // registerLinks writes the links to a file.
 func (daemon *Daemon) registerLinks(container *container.Container, hostConfig *containertypes.HostConfig) error {
-	if hostConfig == nil || hostConfig.Links == nil {
+	if hostConfig == nil {
 		return nil
 	}
 
@@ -707,12 +707,7 @@ func (daemon *Daemon) registerLinks(container *container.Container, hostConfig *
 
 	// After we load all the links into the daemon
 	// set them to nil on the hostconfig
-	hostConfig.Links = nil
-	if err := container.WriteHostConfig(); err != nil {
-		return err
-	}
-
-	return nil
+	return container.WriteHostConfig()
 }
 
 // conditionalMountOnStart is a platform specific helper function during the

+ 16 - 20
daemon/delete.go

@@ -1,8 +1,10 @@
 package daemon
 
 import (
+	"fmt"
 	"os"
 	"path"
+	"strings"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/container"
@@ -38,11 +40,10 @@ func (daemon *Daemon) ContainerRm(name string, config *types.ContainerRmConfig)
 	}
 
 	if config.RemoveLink {
-		return daemon.rmLink(name)
+		return daemon.rmLink(container, name)
 	}
 
 	if err := daemon.cleanupContainer(container, config.ForceRemove); err != nil {
-		// return derr.ErrorCodeCantDestroy.WithArgs(name, utils.GetErrorMessage(err))
 		return err
 	}
 
@@ -53,32 +54,29 @@ func (daemon *Daemon) ContainerRm(name string, config *types.ContainerRmConfig)
 	return nil
 }
 
-// rmLink removes link by name from other containers
-func (daemon *Daemon) rmLink(name string) error {
-	name, err := GetFullContainerName(name)
-	if err != nil {
-		return err
+func (daemon *Daemon) rmLink(container *container.Container, name string) error {
+	if name[0] != '/' {
+		name = "/" + name
 	}
 	parent, n := path.Split(name)
 	if parent == "/" {
-		return derr.ErrorCodeDefaultName
-	}
-	pe := daemon.containerGraph().Get(parent)
-	if pe == nil {
-		return derr.ErrorCodeNoParent.WithArgs(parent, name)
+		return fmt.Errorf("Conflict, cannot remove the default name of the container")
 	}
 
-	if err := daemon.containerGraph().Delete(name); err != nil {
-		return err
+	parent = strings.TrimSuffix(parent, "/")
+	pe, err := daemon.nameIndex.Get(parent)
+	if err != nil {
+		return fmt.Errorf("Cannot get parent %s for name %s", parent, name)
 	}
 
-	parentContainer, _ := daemon.GetContainer(pe.ID())
+	daemon.releaseName(name)
+	parentContainer, _ := daemon.GetContainer(pe)
 	if parentContainer != nil {
+		daemon.linkIndex.unlink(name, container, parentContainer)
 		if err := daemon.updateNetwork(parentContainer); err != nil {
 			logrus.Debugf("Could not update network to remove link %s: %v", n, err)
 		}
 	}
-
 	return nil
 }
 
@@ -116,9 +114,8 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
 	// indexes even if removal failed.
 	defer func() {
 		if err == nil || forceRemove {
-			if _, err := daemon.containerGraphDB.Purge(container.ID); err != nil {
-				logrus.Debugf("Unable to remove container from link graph: %s", err)
-			}
+			daemon.nameIndex.Delete(container.ID)
+			daemon.linkIndex.delete(container)
 			selinuxFreeLxcContexts(container.ProcessLabel)
 			daemon.idIndex.Delete(container.ID)
 			daemon.containers.Delete(container.ID)
@@ -139,7 +136,6 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
 	if err = daemon.execDriver.Clean(container.ID); err != nil {
 		return derr.ErrorCodeRmExecDriver.WithArgs(container.ID, err)
 	}
-
 	return nil
 }
 

+ 5 - 4
daemon/inspect.go

@@ -102,11 +102,12 @@ func (daemon *Daemon) getInspectData(container *container.Container, size bool)
 	// make a copy to play with
 	hostConfig := *container.HostConfig
 
-	if children, err := daemon.children(container.Name); err == nil {
-		for linkAlias, child := range children {
-			hostConfig.Links = append(hostConfig.Links, fmt.Sprintf("%s:%s", child.Name, linkAlias))
-		}
+	children := daemon.children(container)
+	hostConfig.Links = nil // do not expose the internal structure
+	for linkAlias, child := range children {
+		hostConfig.Links = append(hostConfig.Links, fmt.Sprintf("%s:%s", child.Name, linkAlias))
 	}
+
 	// we need this trick to preserve empty log driver, so
 	// container will use daemon defaults even if daemon change them
 	if hostConfig.LogConfig.Type == "" {

+ 128 - 0
daemon/links.go

@@ -0,0 +1,128 @@
+package daemon
+
+import (
+	"strings"
+	"sync"
+
+	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker/container"
+	"github.com/docker/docker/pkg/graphdb"
+)
+
+// linkIndex stores link relationships between containers, including their specified alias
+// The alias is the name the parent uses to reference the child
+type linkIndex struct {
+	// idx maps a parent->alias->child relationship
+	idx map[*container.Container]map[string]*container.Container
+	// childIdx maps  child->parent->aliases
+	childIdx map[*container.Container]map[*container.Container]map[string]struct{}
+	mu       sync.Mutex
+}
+
+func newLinkIndex() *linkIndex {
+	return &linkIndex{
+		idx:      make(map[*container.Container]map[string]*container.Container),
+		childIdx: make(map[*container.Container]map[*container.Container]map[string]struct{}),
+	}
+}
+
+// link adds indexes for the passed in parent/child/alias relationships
+func (l *linkIndex) link(parent, child *container.Container, alias string) {
+	l.mu.Lock()
+
+	if l.idx[parent] == nil {
+		l.idx[parent] = make(map[string]*container.Container)
+	}
+	l.idx[parent][alias] = child
+	if l.childIdx[child] == nil {
+		l.childIdx[child] = make(map[*container.Container]map[string]struct{})
+	}
+	if l.childIdx[child][parent] == nil {
+		l.childIdx[child][parent] = make(map[string]struct{})
+	}
+	l.childIdx[child][parent][alias] = struct{}{}
+
+	l.mu.Unlock()
+}
+
+// unlink removes the requested alias for the given parent/child
+func (l *linkIndex) unlink(alias string, child, parent *container.Container) {
+	l.mu.Lock()
+	delete(l.idx[parent], alias)
+	delete(l.childIdx[child], parent)
+	l.mu.Unlock()
+}
+
+// children maps all the aliases-> children for the passed in parent
+// aliases here are the aliases the parent uses to refer to the child
+func (l *linkIndex) children(parent *container.Container) map[string]*container.Container {
+	l.mu.Lock()
+	children := l.idx[parent]
+	l.mu.Unlock()
+	return children
+}
+
+// parents maps all the aliases->parent for the passed in child
+// aliases here are the aliases the parents use to refer to the child
+func (l *linkIndex) parents(child *container.Container) map[string]*container.Container {
+	l.mu.Lock()
+
+	parents := make(map[string]*container.Container)
+	for parent, aliases := range l.childIdx[child] {
+		for alias := range aliases {
+			parents[alias] = parent
+		}
+	}
+
+	l.mu.Unlock()
+	return parents
+}
+
+// delete deletes all link relationships referencing this container
+func (l *linkIndex) delete(container *container.Container) {
+	l.mu.Lock()
+	for _, child := range l.idx[container] {
+		delete(l.childIdx[child], container)
+	}
+	delete(l.idx, container)
+	delete(l.childIdx, container)
+	l.mu.Unlock()
+}
+
+// migrateLegacySqliteLinks migrates sqlite links to use links from HostConfig
+// when sqlite links were used, hostConfig.Links was set to nil
+func (daemon *Daemon) migrateLegacySqliteLinks(db *graphdb.Database, container *container.Container) error {
+	// if links is populated (or an empty slice), then this isn't using sqlite links and can be skipped
+	if container.HostConfig == nil || container.HostConfig.Links != nil {
+		return nil
+	}
+
+	logrus.Debugf("migrating legacy sqlite link info for container: %s", container.ID)
+
+	fullName := container.Name
+	if fullName[0] != '/' {
+		fullName = "/" + fullName
+	}
+
+	// don't use a nil slice, this ensures that the check above will skip once the migration has completed
+	links := []string{}
+	children, err := db.Children(fullName, 0)
+	if err != nil {
+		if !strings.Contains(err.Error(), "Cannot find child for") {
+			return err
+		}
+		// else continue... it's ok if we didn't find any children, it'll just be nil and we can continue the migration
+	}
+
+	for _, child := range children {
+		c, err := daemon.GetContainer(child.Entity.ID())
+		if err != nil {
+			return err
+		}
+
+		links = append(links, c.Name+":"+child.Edge.Name)
+	}
+
+	container.HostConfig.Links = links
+	return container.WriteHostConfig()
+}

+ 101 - 0
daemon/links_test.go

@@ -0,0 +1,101 @@
+package daemon
+
+import (
+	"encoding/json"
+	"io/ioutil"
+	"os"
+	"path"
+	"path/filepath"
+	"testing"
+
+	"github.com/docker/docker/container"
+	"github.com/docker/docker/pkg/graphdb"
+	"github.com/docker/docker/pkg/stringid"
+	containertypes "github.com/docker/engine-api/types/container"
+)
+
+func TestMigrateLegacySqliteLinks(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", "legacy-qlite-links-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	name1 := "test1"
+	c1 := &container.Container{
+		CommonContainer: container.CommonContainer{
+			ID:         stringid.GenerateNonCryptoID(),
+			Name:       name1,
+			HostConfig: &containertypes.HostConfig{},
+		},
+	}
+	c1.Root = tmpDir
+
+	name2 := "test2"
+	c2 := &container.Container{
+		CommonContainer: container.CommonContainer{
+			ID:   stringid.GenerateNonCryptoID(),
+			Name: name2,
+		},
+	}
+
+	store := &contStore{
+		s: map[string]*container.Container{
+			c1.ID: c1,
+			c2.ID: c2,
+		},
+	}
+
+	d := &Daemon{root: tmpDir, containers: store}
+	db, err := graphdb.NewSqliteConn(filepath.Join(d.root, "linkgraph.db"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := db.Set("/"+name1, c1.ID); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := db.Set("/"+name2, c2.ID); err != nil {
+		t.Fatal(err)
+	}
+
+	alias := "hello"
+	if _, err := db.Set(path.Join(c1.Name, alias), c2.ID); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := d.migrateLegacySqliteLinks(db, c1); err != nil {
+		t.Fatal(err)
+	}
+
+	if len(c1.HostConfig.Links) != 1 {
+		t.Fatal("expected links to be populated but is empty")
+	}
+
+	expected := name2 + ":" + alias
+	actual := c1.HostConfig.Links[0]
+	if actual != expected {
+		t.Fatalf("got wrong link value, expected: %q, got: %q", expected, actual)
+	}
+
+	// ensure this is persisted
+	b, err := ioutil.ReadFile(filepath.Join(c1.Root, "hostconfig.json"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	type hc struct {
+		Links []string
+	}
+	var cfg hc
+	if err := json.Unmarshal(b, &cfg); err != nil {
+		t.Fatal(err)
+	}
+
+	if len(cfg.Links) != 1 {
+		t.Fatalf("expected one entry in links, got: %d", len(cfg.Links))
+	}
+	if cfg.Links[0] != expected { // same expected as above
+		t.Fatalf("got wrong link value, expected: %q, got: %q", expected, cfg.Links[0])
+	}
+}

+ 1 - 8
daemon/list.go

@@ -9,7 +9,6 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/image"
-	"github.com/docker/docker/pkg/graphdb"
 	"github.com/docker/engine-api/types"
 	"github.com/docker/engine-api/types/filters"
 	"github.com/docker/go-connections/nat"
@@ -197,12 +196,6 @@ func (daemon *Daemon) foldFilter(config *ContainersConfig) (*listContext, error)
 		})
 	}
 
-	names := make(map[string][]string)
-	daemon.containerGraph().Walk("/", func(p string, e *graphdb.Entity) error {
-		names[e.ID()] = append(names[e.ID()], p)
-		return nil
-	}, 1)
-
 	if config.Before != "" && beforeContFilter == nil {
 		beforeContFilter, err = daemon.GetContainer(config.Before)
 		if err != nil {
@@ -220,12 +213,12 @@ func (daemon *Daemon) foldFilter(config *ContainersConfig) (*listContext, error)
 	return &listContext{
 		filters:          psFilters,
 		ancestorFilter:   ancestorFilter,
-		names:            names,
 		images:           imagesFilter,
 		exitAllowed:      filtExited,
 		beforeFilter:     beforeContFilter,
 		sinceFilter:      sinceContFilter,
 		ContainersConfig: config,
+		names:            daemon.nameIndex.GetAll(),
 	}, nil
 }
 

+ 2 - 6
daemon/rename.go

@@ -40,14 +40,11 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error {
 		if err != nil {
 			container.Name = oldName
 			daemon.reserveName(container.ID, oldName)
-			daemon.containerGraphDB.Delete(newName)
+			daemon.releaseName(newName)
 		}
 	}()
 
-	if err = daemon.containerGraphDB.Delete(oldName); err != nil {
-		return derr.ErrorCodeRenameDelete.WithArgs(oldName, err)
-	}
-
+	daemon.releaseName(oldName)
 	if err = container.ToDisk(); err != nil {
 		return err
 	}
@@ -76,7 +73,6 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error {
 	if err != nil {
 		return err
 	}
-
 	daemon.LogContainerEvent(container, "rename")
 	return nil
 }

+ 3 - 3
integration-cli/docker_api_containers_test.go

@@ -1083,9 +1083,9 @@ func (s *DockerSuite) TestContainerApiDeleteRemoveLinks(c *check.C) {
 	c.Assert(err, checker.IsNil)
 	c.Assert(links, checker.Equals, "[\"/tlink1:/tlink2/tlink1\"]", check.Commentf("expected to have links between containers"))
 
-	status, _, err := sockRequest("DELETE", "/containers/tlink2/tlink1?link=1", nil)
-	c.Assert(err, checker.IsNil)
-	c.Assert(status, checker.Equals, http.StatusNoContent)
+	status, b, err := sockRequest("DELETE", "/containers/tlink2/tlink1?link=1", nil)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusNoContent, check.Commentf(string(b)))
 
 	linksPostRm, err := inspectFieldJSON(id2, "HostConfig.Links")
 	c.Assert(err, checker.IsNil)

+ 101 - 0
integration-cli/docker_cli_daemon_test.go

@@ -1960,3 +1960,104 @@ func (s *DockerDaemonSuite) TestDaemonCgroupParent(c *check.C) {
 	}
 	c.Assert(found, checker.True, check.Commentf("Cgroup path for container (%s) doesn't found in cgroups file: %s", expectedCgroup, cgroupPaths))
 }
+
+func (s *DockerDaemonSuite) TestDaemonRestartWithLinks(c *check.C) {
+	testRequires(c, DaemonIsLinux) // Windows does not support links
+	err := s.d.StartWithBusybox()
+	c.Assert(err, check.IsNil)
+
+	out, err := s.d.Cmd("run", "-d", "--name=test", "busybox", "top")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+
+	out, err = s.d.Cmd("run", "--name=test2", "--link", "test:abc", "busybox", "sh", "-c", "ping -c 1 -w 1 abc")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+
+	c.Assert(s.d.Restart(), check.IsNil)
+
+	// should fail since test is not running yet
+	out, err = s.d.Cmd("start", "test2")
+	c.Assert(err, check.NotNil, check.Commentf(out))
+
+	out, err = s.d.Cmd("start", "test")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+	out, err = s.d.Cmd("start", "-a", "test2")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+	c.Assert(strings.Contains(out, "1 packets transmitted, 1 packets received"), check.Equals, true, check.Commentf(out))
+}
+
+func (s *DockerDaemonSuite) TestDaemonRestartWithNames(c *check.C) {
+	testRequires(c, DaemonIsLinux) // Windows does not support links
+	err := s.d.StartWithBusybox()
+	c.Assert(err, check.IsNil)
+
+	out, err := s.d.Cmd("create", "--name=test", "busybox")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+
+	out, err = s.d.Cmd("run", "-d", "--name=test2", "busybox", "top")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+	test2ID := strings.TrimSpace(out)
+
+	out, err = s.d.Cmd("run", "-d", "--name=test3", "--link", "test2:abc", "busybox", "top")
+	test3ID := strings.TrimSpace(out)
+
+	c.Assert(s.d.Restart(), check.IsNil)
+
+	out, err = s.d.Cmd("create", "--name=test", "busybox")
+	c.Assert(err, check.NotNil, check.Commentf("expected error trying to create container with duplicate name"))
+	// this one is no longer needed, removing simplifies the remainder of the test
+	out, err = s.d.Cmd("rm", "-f", "test")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+
+	out, err = s.d.Cmd("ps", "-a", "--no-trunc")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+
+	lines := strings.Split(strings.TrimSpace(out), "\n")[1:]
+
+	test2validated := false
+	test3validated := false
+	for _, line := range lines {
+		fields := strings.Fields(line)
+		names := fields[len(fields)-1]
+		switch fields[0] {
+		case test2ID:
+			c.Assert(names, check.Equals, "test2,test3/abc")
+			test2validated = true
+		case test3ID:
+			c.Assert(names, check.Equals, "test3")
+			test3validated = true
+		}
+	}
+
+	c.Assert(test2validated, check.Equals, true)
+	c.Assert(test3validated, check.Equals, true)
+}
+
+// TestRunLinksChanged checks that creating a new container with the same name does not update links
+// this ensures that the old, pre gh#16032 functionality continues on
+func (s *DockerDaemonSuite) TestRunLinksChanged(c *check.C) {
+	testRequires(c, DaemonIsLinux) // Windows does not support links
+	err := s.d.StartWithBusybox()
+	c.Assert(err, check.IsNil)
+
+	out, err := s.d.Cmd("run", "-d", "--name=test", "busybox", "top")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+
+	out, err = s.d.Cmd("run", "--name=test2", "--link=test:abc", "busybox", "sh", "-c", "ping -c 1 abc")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+	c.Assert(out, checker.Contains, "1 packets transmitted, 1 packets received")
+
+	out, err = s.d.Cmd("rm", "-f", "test")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+
+	out, err = s.d.Cmd("run", "-d", "--name=test", "busybox", "top")
+	c.Assert(err, check.IsNil, check.Commentf(out))
+	out, err = s.d.Cmd("start", "-a", "test2")
+	c.Assert(err, check.NotNil, check.Commentf(out))
+	c.Assert(out, check.Not(checker.Contains), "1 packets transmitted, 1 packets received")
+
+	err = s.d.Restart()
+	c.Assert(err, check.IsNil)
+	out, err = s.d.Cmd("start", "-a", "test2")
+	c.Assert(err, check.NotNil, check.Commentf(out))
+	c.Assert(out, check.Not(checker.Contains), "1 packets transmitted, 1 packets received")
+}

+ 127 - 0
pkg/registrar/registrar.go

@@ -0,0 +1,127 @@
+// 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 registred 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
+	}
+	return names, 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
+}

+ 119 - 0
pkg/registrar/registrar_test.go

@@ -0,0 +1,119 @@
+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("Exepected: %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)
+	}
+}