Merge pull request moby#14773 from runcom/cleanup-links

Cleanup links top level pkg

daemon/container.go

@@ -341,8 +341,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)
 	}

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,41 +81,19 @@ 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,
 				child.Config.Env,
 				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)
 			}
@@ -672,6 +650,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 {
@@ -1094,29 +1074,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 {

daemon/container_windows.go

@@ -155,17 +155,15 @@ func (container *Container) ExportRw() (archive.Archive, error) {
 	return nil, nil
 }
 
-func (container *Container) ReleaseNetwork() {
-}
-
-func (container *Container) RestoreNetwork() error {
+func (container *Container) UpdateNetwork() error {
 	return nil
 }
 
-func disableAllActiveLinks(container *Container) {
+func (container *Container) ReleaseNetwork() {
 }
 
-func (container *Container) DisableLink(name string) {
+func (container *Container) RestoreNetwork() error {
+	return nil
 }
 
 func (container *Container) UnmountVolumes(forceSyscall bool) error {

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

links/links.go → 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
-}

links/links_test.go → 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))