From 0f9f99500c40f2a46682967ca358cd2346fd5e13 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 3 Sep 2015 20:51:04 -0400 Subject: [PATCH 1/2] Build names and links at runtime Don't rely on sqlite db for name registration and linking. Instead register names and links when the daemon starts to an in-memory store. Signed-off-by: Brian Goff --- daemon/container_operations_unix.go | 74 +++--- daemon/daemon.go | 222 +++++++----------- daemon/daemon_test.go | 41 +--- daemon/daemon_unix.go | 9 +- daemon/delete.go | 36 ++- daemon/inspect.go | 9 +- daemon/links.go | 87 +++++++ daemon/list.go | 9 +- daemon/rename.go | 8 +- integration-cli/docker_api_containers_test.go | 6 +- integration-cli/docker_cli_daemon_test.go | 101 ++++++++ pkg/registrar/registrar.go | 127 ++++++++++ pkg/registrar/registrar_test.go | 119 ++++++++++ 13 files changed, 584 insertions(+), 264 deletions(-) create mode 100644 daemon/links.go create mode 100644 pkg/registrar/registrar.go create mode 100644 pkg/registrar/registrar_test.go 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) + } +} From 2600777469b18f7133fc4d6c6c99698d6aa700fe Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Sat, 21 Nov 2015 13:45:34 -0500 Subject: [PATCH 2/2] Add migration from sqlite links back to hostConfig Before #16032, once links were setup in the sqlite db, hostConfig.Links was cleared out. This means that we need to migrate data back out of the sqlite db and put it back into hostConfig.Links so that links specified on older daemons can be used. Signed-off-by: Brian Goff --- daemon/daemon.go | 29 +++++++++++++ daemon/links.go | 41 ++++++++++++++++++ daemon/links_test.go | 101 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 daemon/links_test.go diff --git a/daemon/daemon.go b/daemon/daemon.go index f6a9d38714..cd4dffe535 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -48,6 +48,7 @@ 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" @@ -331,6 +332,7 @@ func (daemon *Daemon) restore() error { } } + var migrateLegacyLinks bool restartContainers := make(map[*container.Container]chan struct{}) for _, c := range containers { if err := daemon.registerName(c); err != nil { @@ -346,10 +348,31 @@ func (daemon *Daemon) restore() error { if daemon.configStore.AutoRestart && c.ShouldRestart() { restartContainers[c] = make(chan struct{}) } + + // 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 + } + } + + // 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() } // 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 + } + } if err := daemon.registerLinks(c, c.HostConfig); err != nil { logrus.Errorf("failed to register link for container %s: %v", c.ID, err) } @@ -1359,6 +1382,12 @@ 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 return container.ToDisk() } diff --git a/daemon/links.go b/daemon/links.go index 7f691d4f16..aaf1917d7c 100644 --- a/daemon/links.go +++ b/daemon/links.go @@ -1,9 +1,12 @@ 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 @@ -85,3 +88,41 @@ func (l *linkIndex) delete(container *container.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() +} diff --git a/daemon/links_test.go b/daemon/links_test.go new file mode 100644 index 0000000000..79a641563c --- /dev/null +++ b/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]) + } +}