Merge pull request #2561 from dotcloud/add_iptabled_errors

Make iptable return an error in case of output

iptables/iptables.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"os"
 	"os/exec"
 	"strconv"
 	"strings"
@@ -27,8 +28,10 @@ type Chain struct {
 }
 
 func NewChain(name, bridge string) (*Chain, error) {
-	if err := Raw("-t", "nat", "-N", name); err != nil {
+	if output, err := Raw("-t", "nat", "-N", name); err != nil {
 		return nil, err
+	} else if len(output) != 0 {
+		return nil, fmt.Errorf("Error creating new iptables chain: %s", output)
 	}
 	chain := &Chain{
 		Name:   name,
@@ -52,13 +55,18 @@ func RemoveExistingChain(name string) error {
 }
 
 func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr string, dest_port int) error {
-	return Raw("-t", "nat", fmt.Sprint(action), c.Name,
+	if output, err := Raw("-t", "nat", fmt.Sprint(action), c.Name,
 		"-p", proto,
 		"-d", ip.String(),
 		"--dport", strconv.Itoa(port),
 		"!", "-i", c.Bridge,
 		"-j", "DNAT",
-		"--to-destination", net.JoinHostPort(dest_addr, strconv.Itoa(dest_port)))
+		"--to-destination", net.JoinHostPort(dest_addr, strconv.Itoa(dest_port))); err != nil {
+		return err
+	} else if len(output) != 0 {
+		return fmt.Errorf("Error iptables forward: %s", output)
+	}
+	return nil
 }
 
 func (c *Chain) Prerouting(action Action, args ...string) error {
@@ -66,7 +74,12 @@ func (c *Chain) Prerouting(action Action, args ...string) error {
 	if len(args) > 0 {
 		a = append(a, args...)
 	}
-	return Raw(append(a, "-j", c.Name)...)
+	if output, err := Raw(append(a, "-j", c.Name)...); err != nil {
+		return err
+	} else if len(output) != 0 {
+		return fmt.Errorf("Error iptables prerouting: %s", output)
+	}
+	return nil
 }
 
 func (c *Chain) Output(action Action, args ...string) error {
@@ -74,7 +87,12 @@ func (c *Chain) Output(action Action, args ...string) error {
 	if len(args) > 0 {
 		a = append(a, args...)
 	}
-	return Raw(append(a, "-j", c.Name)...)
+	if output, err := Raw(append(a, "-j", c.Name)...); err != nil {
+		return err
+	} else if len(output) != 0 {
+		return fmt.Errorf("Error iptables output: %s", output)
+	}
+	return nil
 }
 
 func (c *Chain) Remove() error {
@@ -94,17 +112,23 @@ func (c *Chain) Remove() error {
 
 // Check if an existing rule exists
 func Exists(args ...string) bool {
-	return Raw(append([]string{"-C"}, args...)...) == nil
+	if _, err := Raw(append([]string{"-C"}, args...)...); err != nil {
+		return false
+	}
+	return true
 }
 
-func Raw(args ...string) error {
+func Raw(args ...string) ([]byte, error) {
 	path, err := exec.LookPath("iptables")
 	if err != nil {
-		return ErrIptablesNotFound
+		return nil, ErrIptablesNotFound
 	}
-	if err := exec.Command(path, args...).Run(); err != nil {
+	if os.Getenv("DEBUG") != "" {
-		return fmt.Errorf("iptables failed: iptables %v", strings.Join(args, " "))
+		fmt.Printf("[DEBUG] [iptables]: %s, %v\n", path, args)
 	}
-	return nil
+	output, err := exec.Command(path, args...).CombinedOutput()
-
+	if err != nil {
+		return nil, fmt.Errorf("iptables failed: iptables %v: %s (%s)", strings.Join(args, " "), output, err)
+	}
+	return output, err
 }

iptables/iptables_test.go

@@ -6,13 +6,13 @@ import (
 )
 
 func TestIptables(t *testing.T) {
-	if err := Raw("-L"); err != nil {
+	if _, err := Raw("-L"); err != nil {
 		t.Fatal(err)
 	}
 	path := os.Getenv("PATH")
 	os.Setenv("PATH", "")
 	defer os.Setenv("PATH", path)
-	if err := Raw("-L"); err == nil {
+	if _, err := Raw("-L"); err == nil {
 		t.Fatal("Not finding iptables in the PATH should cause an error")
 	}
 }

links.go

@@ -120,7 +120,7 @@ func (l *Link) Disable() {
 
 func (l *Link) toggle(action string, ignoreErrors bool) error {
 	for _, p := range l.Ports {
-		if err := iptables.Raw(action, "FORWARD",
+		if output, err := iptables.Raw(action, "FORWARD",
 			"-i", l.BridgeInterface, "-o", l.BridgeInterface,
 			"-p", p.Proto(),
 			"-s", l.ParentIP,
@@ -128,9 +128,11 @@ func (l *Link) toggle(action string, ignoreErrors bool) error {
 			"-d", l.ChildIP,
 			"-j", "ACCEPT"); !ignoreErrors && err != nil {
 			return err
+		} else if len(output) != 0 {
+			return fmt.Errorf("Error toggle iptables forward: %s", output)
 		}
 
-		if err := iptables.Raw(action, "FORWARD",
+		if output, err := iptables.Raw(action, "FORWARD",
 			"-i", l.BridgeInterface, "-o", l.BridgeInterface,
 			"-p", p.Proto(),
 			"-s", l.ChildIP,
@@ -138,6 +140,8 @@ func (l *Link) toggle(action string, ignoreErrors bool) error {
 			"-d", l.ParentIP,
 			"-j", "ACCEPT"); !ignoreErrors && err != nil {
 			return err
+		} else if len(output) != 0 {
+			return fmt.Errorf("Error toggle iptables forward: %s", output)
 		}
 	}
 	return nil

network.go

@@ -168,9 +168,11 @@ func CreateBridgeIface(config *DaemonConfig) error {
 	}
 
 	if config.EnableIptables {
-		if err := iptables.Raw("-t", "nat", "-A", "POSTROUTING", "-s", ifaceAddr,
+		if output, err := iptables.Raw("-t", "nat", "-A", "POSTROUTING", "-s", ifaceAddr,
 			"!", "-d", ifaceAddr, "-j", "MASQUERADE"); err != nil {
 			return fmt.Errorf("Unable to enable network bridge NAT: %s", err)
+		} else if len(output) != 0 {
+			return fmt.Errorf("Error iptables postrouting: %s", output)
 		}
 	}
 	return nil
@@ -683,8 +685,10 @@ func newNetworkManager(config *DaemonConfig) (*NetworkManager, error) {
 		if !config.InterContainerCommunication {
 			if !iptables.Exists(args...) {
 				utils.Debugf("Disable inter-container communication")
-				if err := iptables.Raw(append([]string{"-A"}, args...)...); err != nil {
+				if output, err := iptables.Raw(append([]string{"-A"}, args...)...); err != nil {
 					return nil, fmt.Errorf("Unable to prevent intercontainer communication: %s", err)
+				} else if len(output) != 0 {
+					return nil, fmt.Errorf("Error enabling iptables: %s", output)
 				}
 			}
 		} else {