From c32ef60c4be3bd982c970df85a8dd2588a55e464 Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Tue, 16 Jun 2015 09:00:55 -0700 Subject: [PATCH 1/3] Cleaning up iptables nat table on driver bootup This is required to have consistent behaviour as in 1.6.2. Signed-off-by: Madhu Venugopal Conflicts: drivers/bridge/bridge.go --- drivers/bridge/bridge.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/bridge/bridge.go b/drivers/bridge/bridge.go index ccdf204031..a0a3987ccd 100644 --- a/drivers/bridge/bridge.go +++ b/drivers/bridge/bridge.go @@ -10,6 +10,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/libnetwork/driverapi" "github.com/docker/libnetwork/ipallocator" + "github.com/docker/libnetwork/iptables" "github.com/docker/libnetwork/netlabel" "github.com/docker/libnetwork/netutils" "github.com/docker/libnetwork/options" @@ -109,6 +110,9 @@ func Init(dc driverapi.DriverCallback) error { if out, err := exec.Command("modprobe", "-va", "bridge", "nf_nat", "br_netfilter").Output(); err != nil { logrus.Warnf("Running modprobe bridge nf_nat failed with message: %s, error: %v", out, err) } + if err := iptables.RemoveExistingChain(DockerChain, iptables.Nat); err != nil { + logrus.Warnf("Failed to remove existing iptables entries in %s : %v", DockerChain, err) + } return dc.RegisterDriver(networkType, newDriver()) } From 6c07f504c3fb713e612cdbaada71ca563cafd1b9 Mon Sep 17 00:00:00 2001 From: Arnaud Porterie Date: Mon, 15 Jun 2015 23:55:29 -0700 Subject: [PATCH 2/3] Fix duplicated iptables rules The `iptables.Exists` function is wrong in two ways: 1. The iptables -C call doesn't add `-j DOCKER` and fails to match 2. The long path takes ordering into account in comparison and fails to match This patch fixes issue 1 by including `-j DOCKER` in the check. Signed-off-by: Arnaud Porterie --- iptables/iptables.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/iptables/iptables.go b/iptables/iptables.go index 481013afab..707ddb7e59 100644 --- a/iptables/iptables.go +++ b/iptables/iptables.go @@ -99,7 +99,8 @@ func NewChain(name, bridge string, table Table, hairpinMode bool) (*Chain, error case Nat: preroute := []string{ "-m", "addrtype", - "--dst-type", "LOCAL"} + "--dst-type", "LOCAL", + "-j", c.Name} if !Exists(Nat, "PREROUTING", preroute...) { if err := c.Prerouting(Append, preroute...); err != nil { return nil, fmt.Errorf("Failed to inject docker in PREROUTING chain: %s", err) @@ -107,7 +108,8 @@ func NewChain(name, bridge string, table Table, hairpinMode bool) (*Chain, error } output := []string{ "-m", "addrtype", - "--dst-type", "LOCAL"} + "--dst-type", "LOCAL", + "-j", c.Name} if !hairpinMode { output = append(output, "!", "--dst", "127.0.0.0/8") } @@ -228,7 +230,7 @@ func (c *Chain) Prerouting(action Action, args ...string) error { if len(args) > 0 { a = append(a, args...) } - if output, err := Raw(append(a, "-j", c.Name)...); err != nil { + if output, err := Raw(a...); err != nil { return err } else if len(output) != 0 { return ChainError{Chain: "PREROUTING", Output: output} @@ -242,7 +244,7 @@ func (c *Chain) Output(action Action, args ...string) error { if len(args) > 0 { a = append(a, args...) } - if output, err := Raw(append(a, "-j", c.Name)...); err != nil { + if output, err := Raw(a...); err != nil { return err } else if len(output) != 0 { return ChainError{Chain: "OUTPUT", Output: output} @@ -254,9 +256,9 @@ func (c *Chain) Output(action Action, args ...string) error { func (c *Chain) Remove() error { // Ignore errors - This could mean the chains were never set up if c.Table == Nat { - c.Prerouting(Delete, "-m", "addrtype", "--dst-type", "LOCAL") - c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL", "!", "--dst", "127.0.0.0/8") - c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL") // Created in versions <= 0.1.6 + c.Prerouting(Delete, "-m", "addrtype", "--dst-type", "LOCAL", "-j", c.Name) + c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL", "!", "--dst", "127.0.0.0/8", "-j", c.Name) + c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL", "-j", c.Name) // Created in versions <= 0.1.6 c.Prerouting(Delete) c.Output(Delete) From 78fef19fc6299838889a5f7e3e974d50f0e0b289 Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Tue, 16 Jun 2015 07:07:51 -0700 Subject: [PATCH 3/3] Fixed the tests. Signed-off-by: Madhu Venugopal --- iptables/iptables_test.go | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/iptables/iptables_test.go b/iptables/iptables_test.go index 61257d0322..63d931c8ab 100644 --- a/iptables/iptables_test.go +++ b/iptables/iptables_test.go @@ -131,16 +131,11 @@ func TestPrerouting(t *testing.T) { t.Fatal(err) } - rule := []string{ - "-j", natChain.Name} - - rule = append(rule, args...) - - if !Exists(natChain.Table, "PREROUTING", rule...) { + if !Exists(natChain.Table, "PREROUTING", args...) { t.Fatalf("rule does not exist") } - delRule := append([]string{"-D", "PREROUTING", "-t", string(Nat)}, rule...) + delRule := append([]string{"-D", "PREROUTING", "-t", string(Nat)}, args...) if _, err = Raw(delRule...); err != nil { t.Fatal(err) } @@ -156,17 +151,12 @@ func TestOutput(t *testing.T) { t.Fatal(err) } - rule := []string{ - "-j", natChain.Name} - - rule = append(rule, args...) - - if !Exists(natChain.Table, "OUTPUT", rule...) { + if !Exists(natChain.Table, "OUTPUT", args...) { t.Fatalf("rule does not exist") } delRule := append([]string{"-D", "OUTPUT", "-t", - string(natChain.Table)}, rule...) + string(natChain.Table)}, args...) if _, err = Raw(delRule...); err != nil { t.Fatal(err) }