From 53c5b1856d91093fed1c2d3f038d227f5fdef4b8 Mon Sep 17 00:00:00 2001 From: Timothy Hobbs Date: Sat, 15 Mar 2014 23:04:36 +0100 Subject: [PATCH 1/3] Add failing test case for issue #4681 Add a failing test case for an issue where docker is not creating a loopback device if networking is dissabled. Docker-DCO-1.1-Signed-off-by: Timothy Hobbs (github: https://github.com/timthelion) --- integration/container_test.go | 69 ++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/integration/container_test.go b/integration/container_test.go index 010883a709..663b350638 100644 --- a/integration/container_test.go +++ b/integration/container_test.go @@ -434,28 +434,6 @@ func TestOutput(t *testing.T) { } } -func TestContainerNetwork(t *testing.T) { - runtime := mkRuntime(t) - defer nuke(runtime) - container, _, err := runtime.Create( - &runconfig.Config{ - Image: GetTestImage(runtime).ID, - Cmd: []string{"ping", "-c", "1", "127.0.0.1"}, - }, - "", - ) - if err != nil { - t.Fatal(err) - } - defer runtime.Destroy(container) - if err := container.Run(); err != nil { - t.Fatal(err) - } - if code := container.State.GetExitCode(); code != 0 { - t.Fatalf("Unexpected ping 127.0.0.1 exit code %d (expected 0)", code) - } -} - func TestKillDifferentUser(t *testing.T) { runtime := mkRuntime(t) defer nuke(runtime) @@ -1523,6 +1501,53 @@ func TestVolumesFromWithVolumes(t *testing.T) { } } +func TestContainerNetwork(t *testing.T) { + runtime := mkRuntime(t) + defer nuke(runtime) + container, _, err := runtime.Create( + &runconfig.Config{ + Image: GetTestImage(runtime).ID, + // If I change this to ping 8.8.8.8 it fails. Any idea why? - timthelion + Cmd: []string{"ping", "-c", "1", "127.0.0.1"}, + }, + "", + ) + if err != nil { + t.Fatal(err) + } + defer runtime.Destroy(container) + if err := container.Run(); err != nil { + t.Fatal(err) + } + if code := container.State.GetExitCode(); code != 0 { + t.Fatalf("Unexpected ping 127.0.0.1 exit code %d (expected 0)", code) + } +} + +// Issue #4681 +func TestLoopbackFunctionsWhenNetworkingIsDissabled(t *testing.T) { + runtime := mkRuntime(t) + defer nuke(runtime) + container, _, err := runtime.Create( + &runconfig.Config{ + Image: GetTestImage(runtime).ID, + Cmd: []string{"ping", "-c", "1", "127.0.0.1"}, + NetworkDisabled: true, + }, + "", + ) + if err != nil { + t.Fatal(err) + } + defer runtime.Destroy(container) + if err := container.Run(); err != nil { + t.Fatal(err) + } + if code := container.State.GetExitCode(); code != 0 { + t.Fatalf("Unexpected ping 127.0.0.1 exit code %d (expected 0)", code) + } +} + func TestOnlyLoopbackExistsWhenUsingDisableNetworkOption(t *testing.T) { eng := NewTestEngine(t) runtime := mkRuntimeFromEngine(eng, t) From 353df19ab7009f6555dee506841ae0b690a08768 Mon Sep 17 00:00:00 2001 From: Timothy Hobbs Date: Sun, 16 Mar 2014 01:01:31 +0100 Subject: [PATCH 2/3] Fix issue #4681 - No loopback interface within container when networking is disabled. Docker-DCO-1.1-Signed-off-by: Timothy Hobbs (github: https://github.com/timthelion) Remove loopback code from veth strategy Docker-DCO-1.1-Signed-off-by: Timothy Hobbs (github: https://github.com/timthelion) Looback strategy: Get rid of uneeded code in Create Docker-DCO-1.1-Signed-off-by: Timothy Hobbs (github: https://github.com/timthelion) Use append when building network strategy list Docker-DCO-1.1-Signed-off-by: Timothy Hobbs (github: https://github.com/timthelion) Swap loopback and veth strategies in Networks list Docker-DCO-1.1-Signed-off-by: Timothy Hobbs (github: https://github.com/timthelion) Revert "Swap loopback and veth strategies in Networks list" This reverts commit 3b8b2c8454171d79bed5e9a80165172617e92fc7. Docker-DCO-1.1-Signed-off-by: Timothy Hobbs (github: https://github.com/timthelion) When initializing networks, only return from the loop if there is an error Docker-DCO-1.1-Signed-off-by: Timothy Hobbs (github: https://github.com/timthelion) --- pkg/libcontainer/network/loopback.go | 24 +++++++++++++ pkg/libcontainer/network/strategy.go | 3 +- pkg/libcontainer/network/veth.go | 6 ---- pkg/libcontainer/nsinit/init.go | 6 +++- runtime/execdriver/native/default_template.go | 35 +++++++++++++------ 5 files changed, 56 insertions(+), 18 deletions(-) create mode 100644 pkg/libcontainer/network/loopback.go diff --git a/pkg/libcontainer/network/loopback.go b/pkg/libcontainer/network/loopback.go new file mode 100644 index 0000000000..6215061dc2 --- /dev/null +++ b/pkg/libcontainer/network/loopback.go @@ -0,0 +1,24 @@ +package network + +import ( + "fmt" + "github.com/dotcloud/docker/pkg/libcontainer" +) + +// Loopback is a network strategy that provides a basic loopback device +type Loopback struct { +} + +func (l *Loopback) Create(n *libcontainer.Network, nspid int, context libcontainer.Context) error { + return nil +} + +func (l *Loopback) Initialize(config *libcontainer.Network, context libcontainer.Context) error { + if err := SetMtu("lo", config.Mtu); err != nil { + return fmt.Errorf("set lo mtu to %d %s", config.Mtu, err) + } + if err := InterfaceUp("lo"); err != nil { + return fmt.Errorf("lo up %s", err) + } + return nil +} diff --git a/pkg/libcontainer/network/strategy.go b/pkg/libcontainer/network/strategy.go index 234fcc0aa2..693790d280 100644 --- a/pkg/libcontainer/network/strategy.go +++ b/pkg/libcontainer/network/strategy.go @@ -10,7 +10,8 @@ var ( ) var strategies = map[string]NetworkStrategy{ - "veth": &Veth{}, + "veth": &Veth{}, + "loopback": &Loopback{}, } // NetworkStrategy represents a specific network configuration for diff --git a/pkg/libcontainer/network/veth.go b/pkg/libcontainer/network/veth.go index 3ab1b2393b..3df0cd61ee 100644 --- a/pkg/libcontainer/network/veth.go +++ b/pkg/libcontainer/network/veth.go @@ -68,12 +68,6 @@ func (v *Veth) Initialize(config *libcontainer.Network, context libcontainer.Con if err := InterfaceUp("eth0"); err != nil { return fmt.Errorf("eth0 up %s", err) } - if err := SetMtu("lo", config.Mtu); err != nil { - return fmt.Errorf("set lo mtu to %d %s", config.Mtu, err) - } - if err := InterfaceUp("lo"); err != nil { - return fmt.Errorf("lo up %s", err) - } if config.Gateway != "" { if err := SetDefaultGateway(config.Gateway); err != nil { return fmt.Errorf("set gateway to %s %s", config.Gateway, err) diff --git a/pkg/libcontainer/nsinit/init.go b/pkg/libcontainer/nsinit/init.go index e329becbdf..117ae875ed 100644 --- a/pkg/libcontainer/nsinit/init.go +++ b/pkg/libcontainer/nsinit/init.go @@ -134,7 +134,11 @@ func setupNetwork(container *libcontainer.Container, context libcontainer.Contex if err != nil { return err } - return strategy.Initialize(config, context) + + err1 := strategy.Initialize(config, context) + if err1 != nil { + return err1 + } } return nil } diff --git a/runtime/execdriver/native/default_template.go b/runtime/execdriver/native/default_template.go index e76be6ebec..47b19c9d66 100644 --- a/runtime/execdriver/native/default_template.go +++ b/runtime/execdriver/native/default_template.go @@ -19,19 +19,34 @@ func createContainer(c *execdriver.Command) *libcontainer.Container { container.WorkingDir = c.WorkingDir container.Env = c.Env + loopbackNetwork := libcontainer.Network{ + // Using constants here because + // when networking is disabled + // These settings simply don't exist: + // https://github.com/dotcloud/docker/blob/c7ea6e5da80af3d9ba7558f876efbf0801d988d8/runtime/container.go#L367 + Mtu: 1500, + Address: fmt.Sprintf("%s/%d", "127.0.0.1", 0), + Gateway: "localhost", + Type: "loopback", + Context: libcontainer.Context{}, + } + + container.Networks = []*libcontainer.Network{ + &loopbackNetwork, + } + if c.Network != nil { - container.Networks = []*libcontainer.Network{ - { - Mtu: c.Network.Mtu, - Address: fmt.Sprintf("%s/%d", c.Network.IPAddress, c.Network.IPPrefixLen), - Gateway: c.Network.Gateway, - Type: "veth", - Context: libcontainer.Context{ - "prefix": "veth", - "bridge": c.Network.Bridge, - }, + vethNetwork := libcontainer.Network{ + Mtu: c.Network.Mtu, + Address: fmt.Sprintf("%s/%d", c.Network.IPAddress, c.Network.IPPrefixLen), + Gateway: c.Network.Gateway, + Type: "veth", + Context: libcontainer.Context{ + "prefix": "veth", + "bridge": c.Network.Bridge, }, } + container.Networks = append(container.Networks, &vethNetwork) } container.Cgroups.Name = c.ID From 659b719aa66e7ed0c3104d3704fa61035050ad82 Mon Sep 17 00:00:00 2001 From: Timothy Hobbs Date: Sun, 16 Mar 2014 20:52:27 +0100 Subject: [PATCH 3/3] Refactor out interface specific information from execdriver.Network Docker-DCO-1.1-Signed-off-by: Timothy Hobbs (github: https://github.com/timthelion) --- runtime/container.go | 8 ++++++-- runtime/execdriver/driver.go | 10 +++++++--- runtime/execdriver/lxc/driver.go | 10 ++++++---- runtime/execdriver/lxc/lxc_template.go | 6 +++--- runtime/execdriver/lxc/lxc_template_unit_test.go | 8 ++++++++ runtime/execdriver/native/default_template.go | 14 +++++--------- 6 files changed, 35 insertions(+), 21 deletions(-) diff --git a/runtime/container.go b/runtime/container.go index 2a30715206..73bad67d6a 100644 --- a/runtime/container.go +++ b/runtime/container.go @@ -364,14 +364,18 @@ func populateCommand(c *Container) { driverConfig []string ) + en = &execdriver.Network{ + Mtu: c.runtime.config.Mtu, + Interface: nil, + } + if !c.Config.NetworkDisabled { network := c.NetworkSettings - en = &execdriver.Network{ + en.Interface = &execdriver.NetworkInterface{ Gateway: network.Gateway, Bridge: network.Bridge, IPAddress: network.IPAddress, IPPrefixLen: network.IPPrefixLen, - Mtu: c.runtime.config.Mtu, } } diff --git a/runtime/execdriver/driver.go b/runtime/execdriver/driver.go index ff37b6bc5b..23e31ee8d9 100644 --- a/runtime/execdriver/driver.go +++ b/runtime/execdriver/driver.go @@ -84,11 +84,15 @@ type Driver interface { // Network settings of the container type Network struct { + Interface *NetworkInterface `json:"interface"` // if interface is nil then networking is disabled + Mtu int `json:"mtu"` +} + +type NetworkInterface struct { Gateway string `json:"gateway"` IPAddress string `json:"ip"` Bridge string `json:"bridge"` IPPrefixLen int `json:"ip_prefix_len"` - Mtu int `json:"mtu"` } type Resources struct { @@ -118,8 +122,8 @@ type Command struct { WorkingDir string `json:"working_dir"` ConfigPath string `json:"config_path"` // this should be able to be removed when the lxc template is moved into the driver Tty bool `json:"tty"` - Network *Network `json:"network"` // if network is nil then networking is disabled - Config []string `json:"config"` // generic values that specific drivers can consume + Network *Network `json:"network"` + Config []string `json:"config"` // generic values that specific drivers can consume Resources *Resources `json:"resources"` Mounts []Mount `json:"mounts"` diff --git a/runtime/execdriver/lxc/driver.go b/runtime/execdriver/lxc/driver.go index b7311cc1ff..086e35f643 100644 --- a/runtime/execdriver/lxc/driver.go +++ b/runtime/execdriver/lxc/driver.go @@ -98,13 +98,15 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba DriverName, } - if c.Network != nil { + if c.Network.Interface != nil { params = append(params, - "-g", c.Network.Gateway, - "-i", fmt.Sprintf("%s/%d", c.Network.IPAddress, c.Network.IPPrefixLen), - "-mtu", strconv.Itoa(c.Network.Mtu), + "-g", c.Network.Interface.Gateway, + "-i", fmt.Sprintf("%s/%d", c.Network.Interface.IPAddress, c.Network.Interface.IPPrefixLen), ) } + params = append(params, + "-mtu", strconv.Itoa(c.Network.Mtu), + ) if c.User != "" { params = append(params, "-u", c.User) diff --git a/runtime/execdriver/lxc/lxc_template.go b/runtime/execdriver/lxc/lxc_template.go index db55287522..ce9d90469f 100644 --- a/runtime/execdriver/lxc/lxc_template.go +++ b/runtime/execdriver/lxc/lxc_template.go @@ -7,17 +7,17 @@ import ( ) const LxcTemplate = ` -{{if .Network}} +{{if .Network.Interface}} # network configuration lxc.network.type = veth -lxc.network.link = {{.Network.Bridge}} +lxc.network.link = {{.Network.Interface.Bridge}} lxc.network.name = eth0 -lxc.network.mtu = {{.Network.Mtu}} {{else}} # network is disabled (-n=false) lxc.network.type = empty lxc.network.flags = up {{end}} +lxc.network.mtu = {{.Network.Mtu}} # root filesystem {{$ROOTFS := .Rootfs}} diff --git a/runtime/execdriver/lxc/lxc_template_unit_test.go b/runtime/execdriver/lxc/lxc_template_unit_test.go index ae66371836..e613adf7a9 100644 --- a/runtime/execdriver/lxc/lxc_template_unit_test.go +++ b/runtime/execdriver/lxc/lxc_template_unit_test.go @@ -43,6 +43,10 @@ func TestLXCConfig(t *testing.T) { Memory: int64(mem), CpuShares: int64(cpu), }, + Network: &execdriver.Network{ + Mtu: 1500, + Interface: nil, + }, } p, err := driver.generateLXCConfig(command) if err != nil { @@ -75,6 +79,10 @@ func TestCustomLxcConfig(t *testing.T) { "lxc.utsname = docker", "lxc.cgroup.cpuset.cpus = 0,1", }, + Network: &execdriver.Network{ + Mtu: 1500, + Interface: nil, + }, } p, err := driver.generateLXCConfig(command) diff --git a/runtime/execdriver/native/default_template.go b/runtime/execdriver/native/default_template.go index 47b19c9d66..d744ab382f 100644 --- a/runtime/execdriver/native/default_template.go +++ b/runtime/execdriver/native/default_template.go @@ -20,11 +20,7 @@ func createContainer(c *execdriver.Command) *libcontainer.Container { container.Env = c.Env loopbackNetwork := libcontainer.Network{ - // Using constants here because - // when networking is disabled - // These settings simply don't exist: - // https://github.com/dotcloud/docker/blob/c7ea6e5da80af3d9ba7558f876efbf0801d988d8/runtime/container.go#L367 - Mtu: 1500, + Mtu: c.Network.Mtu, Address: fmt.Sprintf("%s/%d", "127.0.0.1", 0), Gateway: "localhost", Type: "loopback", @@ -35,15 +31,15 @@ func createContainer(c *execdriver.Command) *libcontainer.Container { &loopbackNetwork, } - if c.Network != nil { + if c.Network.Interface != nil { vethNetwork := libcontainer.Network{ Mtu: c.Network.Mtu, - Address: fmt.Sprintf("%s/%d", c.Network.IPAddress, c.Network.IPPrefixLen), - Gateway: c.Network.Gateway, + Address: fmt.Sprintf("%s/%d", c.Network.Interface.IPAddress, c.Network.Interface.IPPrefixLen), + Gateway: c.Network.Interface.Gateway, Type: "veth", Context: libcontainer.Context{ "prefix": "veth", - "bridge": c.Network.Bridge, + "bridge": c.Network.Interface.Bridge, }, } container.Networks = append(container.Networks, &vethNetwork)