diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index bc8e4294fd..be43eade38 100644 --- a/daemon/container_operations_unix.go +++ b/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 } diff --git a/daemon/daemon.go b/daemon/daemon.go index b23543a52a..f6a9d38714 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -12,9 +12,9 @@ import ( "io/ioutil" "net" "os" + "path" "path/filepath" "runtime" - "strings" "sync" "syscall" "time" @@ -48,11 +48,11 @@ import ( "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/discovery" "github.com/docker/docker/pkg/fileutils" - "github.com/docker/docker/pkg/graphdb" "github.com/docker/docker/pkg/idtools" "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 +147,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 +161,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 +246,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 @@ -257,10 +257,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 +289,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 +325,48 @@ 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(".") - } + 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] - - if c, ok := containers[e.ID()]; ok { - c.registered = true - } + // get list of containers we need to restart + if daemon.configStore.AutoRestart && c.ShouldRestart() { + restartContainers[c] = make(chan struct{}) } } - restartContainers := make(map[*container.Container]chan struct{}) + // Now that all the containers are registered, register the links 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 - } - } - - if err := daemon.Register(c.container); err != nil { - logrus.Errorf("Failed to register container %s: %s", c.container.ID, err) - continue - } - // 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 +376,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 +431,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 + 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) } - - conflictingContainer, err := daemon.GetByName(name) - if err != nil { - 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.", 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 +461,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 +521,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 +550,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 +768,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 +799,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 +882,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 +1283,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. @@ -1421,8 +1360,7 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig * } container.HostConfig = hostConfig - container.ToDisk() - return nil + return container.ToDisk() } func (daemon *Daemon) setupInitLayer(initPath string) error { diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 0d927f76ae..e6550a44da 100644 --- a/daemon/daemon_test.go +++ b/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"} diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 78ca7595ac..100e923f9b 100644 --- a/daemon/daemon_unix.go +++ b/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 diff --git a/daemon/delete.go b/daemon/delete.go index af9903f508..c8be1bc506 100644 --- a/daemon/delete.go +++ b/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 } diff --git a/daemon/inspect.go b/daemon/inspect.go index 098d43df5d..feb7de28f1 100644 --- a/daemon/inspect.go +++ b/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 == "" { diff --git a/daemon/links.go b/daemon/links.go new file mode 100644 index 0000000000..7f691d4f16 --- /dev/null +++ b/daemon/links.go @@ -0,0 +1,87 @@ +package daemon + +import ( + "sync" + + "github.com/docker/docker/container" +) + +// 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() +} diff --git a/daemon/list.go b/daemon/list.go index c5765e387a..0e6f02c910 100644 --- a/daemon/list.go +++ b/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 } diff --git a/daemon/rename.go b/daemon/rename.go index c09888c234..eaed577ad6 100644 --- a/daemon/rename.go +++ b/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 } diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 568beca238..c33ad0ce28 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/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) diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index 5dee35f839..190480f235 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/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") +} diff --git a/pkg/registrar/registrar.go b/pkg/registrar/registrar.go new file mode 100644 index 0000000000..8910197f29 --- /dev/null +++ b/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 +} diff --git a/pkg/registrar/registrar_test.go b/pkg/registrar/registrar_test.go new file mode 100644 index 0000000000..0c1ef312ae --- /dev/null +++ b/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) + } +}