From 0e0c7e521c996bc18a9e602122135b07d4d4469e Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Tue, 21 Jul 2015 00:19:05 +0200 Subject: [PATCH] Cleanup links top level pkg - Move top level pkg links under daemon - Refactor code accordingly - golint pkg Signed-off-by: Antonio Murdaca --- daemon/container.go | 2 - daemon/container_unix.go | 51 ++----------------- daemon/container_windows.go | 10 ++-- daemon/delete.go | 6 ++- {links => daemon/links}/links.go | 72 ++++++++++++--------------- {links => daemon/links}/links_test.go | 31 ++---------- 6 files changed, 50 insertions(+), 122 deletions(-) rename {links => daemon/links}/links.go (84%) rename {links => daemon/links}/links_test.go (89%) diff --git a/daemon/container.go b/daemon/container.go index 855900f061..9ac3e46037 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -333,8 +333,6 @@ func (container *Container) isNetworkAllocated() bool { func (container *Container) cleanup() { container.ReleaseNetwork() - disableAllActiveLinks(container) - if err := container.CleanupStorage(); err != nil { logrus.Errorf("%v: Failed to cleanup storage: %v", container.ID, err) } diff --git a/daemon/container_unix.go b/daemon/container_unix.go index b5163a1110..3ffd65a433 100644 --- a/daemon/container_unix.go +++ b/daemon/container_unix.go @@ -16,8 +16,8 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/daemon/execdriver" + "github.com/docker/docker/daemon/links" "github.com/docker/docker/daemon/network" - "github.com/docker/docker/links" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/directory" "github.com/docker/docker/pkg/ioutils" @@ -81,23 +81,12 @@ func (container *Container) setupLinkedContainers() ([]string, error) { } if len(children) > 0 { - container.activeLinks = make(map[string]*links.Link, len(children)) - - // If we encounter an error make sure that we rollback any network - // config and iptables changes - rollback := func() { - for _, link := range container.activeLinks { - link.Disable() - } - container.activeLinks = nil - } - for linkAlias, child := range children { if !child.IsRunning() { return nil, fmt.Errorf("Cannot link to a non running container: %s AS %s", child.Name, linkAlias) } - link, err := links.NewLink( + link := links.NewLink( container.NetworkSettings.IPAddress, child.NetworkSettings.IPAddress, linkAlias, @@ -105,17 +94,6 @@ func (container *Container) setupLinkedContainers() ([]string, error) { child.Config.ExposedPorts, ) - if err != nil { - rollback() - return nil, err - } - - container.activeLinks[link.Alias()] = link - if err := link.Enable(); err != nil { - rollback() - return nil, err - } - for _, envVar := range link.ToEnv() { env = append(env, envVar) } @@ -668,6 +646,8 @@ func (container *Container) updateNetworkSettings(n libnetwork.Network, ep libne return nil } +// UpdateNetwork is used to update the container's network (e.g. when linked containers +// get removed/unlinked). func (container *Container) UpdateNetwork() error { n, err := container.daemon.netController.NetworkByID(container.NetworkSettings.NetworkID) if err != nil { @@ -1090,29 +1070,6 @@ func (container *Container) ReleaseNetwork() { logrus.Errorf("deleting endpoint failed: %v", err) } } - -} - -func disableAllActiveLinks(container *Container) { - if container.activeLinks != nil { - for _, link := range container.activeLinks { - link.Disable() - } - } -} - -func (container *Container) DisableLink(name string) { - if container.activeLinks != nil { - if link, exists := container.activeLinks[name]; exists { - link.Disable() - delete(container.activeLinks, name) - if err := container.UpdateNetwork(); err != nil { - logrus.Debugf("Could not update network to remove link: %v", err) - } - } else { - logrus.Debugf("Could not find active link for %s", name) - } - } } func (container *Container) UnmountVolumes(forceSyscall bool) error { diff --git a/daemon/container_windows.go b/daemon/container_windows.go index 2a64f68238..410f7d660f 100644 --- a/daemon/container_windows.go +++ b/daemon/container_windows.go @@ -155,6 +155,10 @@ func (container *Container) ExportRw() (archive.Archive, error) { return nil, nil } +func (container *Container) UpdateNetwork() error { + return nil +} + func (container *Container) ReleaseNetwork() { } @@ -162,12 +166,6 @@ func (container *Container) RestoreNetwork() error { return nil } -func disableAllActiveLinks(container *Container) { -} - -func (container *Container) DisableLink(name string) { -} - func (container *Container) UnmountVolumes(forceSyscall bool) error { return nil } diff --git a/daemon/delete.go b/daemon/delete.go index 9990019bf1..30bfeddd82 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -32,14 +32,16 @@ func (daemon *Daemon) ContainerRm(name string, config *ContainerRmConfig) error if pe == nil { return fmt.Errorf("Cannot get parent %s for name %s", parent, name) } - parentContainer, _ := daemon.Get(pe.ID()) if err := daemon.ContainerGraph().Delete(name); err != nil { return err } + parentContainer, _ := daemon.Get(pe.ID()) if parentContainer != nil { - parentContainer.DisableLink(n) + if err := parentContainer.UpdateNetwork(); err != nil { + logrus.Debugf("Could not update network to remove link %s: %v", n, err) + } } return nil diff --git a/links/links.go b/daemon/links/links.go similarity index 84% rename from links/links.go rename to daemon/links/links.go index d8e9730eed..e513fe06e7 100644 --- a/links/links.go +++ b/daemon/links/links.go @@ -8,17 +8,22 @@ import ( "github.com/docker/docker/pkg/nat" ) +// Link struct holds informations about parent/child linked container type Link struct { - ParentIP string - ChildIP string - Name string + // Parent container IP address + ParentIP string + // Child container IP address + ChildIP string + // Link name + Name string + // Child environments variables ChildEnvironment []string - Ports []nat.Port - IsEnabled bool + // Child exposed ports + Ports []nat.Port } -func NewLink(parentIP, childIP, name string, env []string, exposedPorts map[nat.Port]struct{}) (*Link, error) { - +// NewLink initializes a new Link struct with the provided options. +func NewLink(parentIP, childIP, name string, env []string, exposedPorts map[nat.Port]struct{}) *Link { var ( i int ports = make([]nat.Port, len(exposedPorts)) @@ -29,39 +34,23 @@ func NewLink(parentIP, childIP, name string, env []string, exposedPorts map[nat. i++ } - l := &Link{ + return &Link{ Name: name, ChildIP: childIP, ParentIP: parentIP, ChildEnvironment: env, Ports: ports, } - return l, nil - -} - -func (l *Link) Alias() string { - _, alias := path.Split(l.Name) - return alias -} - -func nextContiguous(ports []nat.Port, value int, index int) int { - if index+1 == len(ports) { - return index - } - for i := index + 1; i < len(ports); i++ { - if ports[i].Int() > value+1 { - return i - 1 - } - - value++ - } - return len(ports) - 1 } +// ToEnv creates a string's slice containing child container informations in +// the form of environment variables which will be later exported on container +// startup. func (l *Link) ToEnv() []string { env := []string{} - alias := strings.Replace(strings.ToUpper(l.Alias()), "-", "_", -1) + + _, n := path.Split(l.Name) + alias := strings.Replace(strings.ToUpper(n), "-", "_", -1) if p := l.getDefaultPort(); p != nil { env = append(env, fmt.Sprintf("%s_PORT=%s://%s:%s", alias, p.Proto(), l.ChildIP, p.Port())) @@ -119,6 +108,20 @@ func (l *Link) ToEnv() []string { return env } +func nextContiguous(ports []nat.Port, value int, index int) int { + if index+1 == len(ports) { + return index + } + for i := index + 1; i < len(ports); i++ { + if ports[i].Int() > value+1 { + return i - 1 + } + + value++ + } + return len(ports) - 1 +} + // Default port rules func (l *Link) getDefaultPort() *nat.Port { var p nat.Port @@ -136,12 +139,3 @@ func (l *Link) getDefaultPort() *nat.Port { p = l.Ports[0] return &p } - -func (l *Link) Enable() error { - l.IsEnabled = true - return nil -} - -func (l *Link) Disable() { - l.IsEnabled = false -} diff --git a/links/links_test.go b/daemon/links/links_test.go similarity index 89% rename from links/links_test.go rename to daemon/links/links_test.go index 21952c3d77..3d0fb6d7c6 100644 --- a/links/links_test.go +++ b/daemon/links/links_test.go @@ -18,10 +18,7 @@ func TestLinkNaming(t *testing.T) { ports := make(nat.PortSet) ports[newPortNoError("tcp", "6379")] = struct{}{} - link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker-1", nil, ports) - if err != nil { - t.Fatal(err) - } + link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker-1", nil, ports) rawEnv := link.ToEnv() env := make(map[string]string, len(rawEnv)) @@ -48,20 +45,11 @@ func TestLinkNew(t *testing.T) { ports := make(nat.PortSet) ports[newPortNoError("tcp", "6379")] = struct{}{} - link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", nil, ports) - if err != nil { - t.Fatal(err) - } + link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", nil, ports) - if link == nil { - t.FailNow() - } if link.Name != "/db/docker" { t.Fail() } - if link.Alias() != "docker" { - t.Fail() - } if link.ParentIP != "172.0.17.3" { t.Fail() } @@ -79,10 +67,7 @@ func TestLinkEnv(t *testing.T) { ports := make(nat.PortSet) ports[newPortNoError("tcp", "6379")] = struct{}{} - link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, ports) - if err != nil { - t.Fatal(err) - } + link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, ports) rawEnv := link.ToEnv() env := make(map[string]string, len(rawEnv)) @@ -122,10 +107,7 @@ func TestLinkMultipleEnv(t *testing.T) { ports[newPortNoError("tcp", "6380")] = struct{}{} ports[newPortNoError("tcp", "6381")] = struct{}{} - link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, ports) - if err != nil { - t.Fatal(err) - } + link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, ports) rawEnv := link.ToEnv() env := make(map[string]string, len(rawEnv)) @@ -171,10 +153,7 @@ func TestLinkPortRangeEnv(t *testing.T) { ports[newPortNoError("tcp", "6380")] = struct{}{} ports[newPortNoError("tcp", "6381")] = struct{}{} - link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, ports) - if err != nil { - t.Fatal(err) - } + link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, ports) rawEnv := link.ToEnv() env := make(map[string]string, len(rawEnv))