pkg/iptables: add logger to iptables controller

This commit adds a logger to the iptables controller using the options
pattern. It also logs when the controller needs to reset rules, to be
able to identify costly reconciliations.

Signed-off-by: Lucas Servén Marín <lserven@gmail.com>

pkg/iptables/iptables.go

@@ -21,6 +21,8 @@ import (
 	"time"
 
 	"github.com/coreos/go-iptables/iptables"
+	"github.com/go-kit/kit/log"
+	"github.com/go-kit/kit/log/level"
 )
 
 // Protocol represents an IP protocol.
@@ -199,29 +201,57 @@ type Controller struct {
 	v4     Client
 	v6     Client
 	errors chan error
+	logger log.Logger
 
 	sync.Mutex
 	rules      []Rule
 	subscribed bool
 }
 
+// ControllerOption modifies the controller's configuration.
+type ControllerOption func(h *Controller)
+
+// WithLogger adds a logger to the controller.
+func WithLogger(logger log.Logger) ControllerOption {
+	return func(c *Controller) {
+		c.logger = logger
+	}
+}
+
+// WithClients adds iptables clients to the controller.
+func WithClients(v4, v6 Client) ControllerOption {
+	return func(c *Controller) {
+		c.v4 = v4
+		c.v6 = v6
+	}
+}
+
 // New generates a new iptables rules controller.
-// It expects an IP address length to determine
+// If no options are given, IPv4 and IPv6 clients
-// whether to operate in IPv4 or IPv6 mode.
+// will be instantiated using the regular iptables backend.
-func New() (*Controller, error) {
+func New(opts ...ControllerOption) (*Controller, error) {
-	v4, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	c := &Controller{
-	if err != nil {
-		return nil, fmt.Errorf("failed to create iptables IPv4 client: %v", err)
-	}
-	v6, err := iptables.NewWithProtocol(iptables.ProtocolIPv6)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create iptables IPv6 client: %v", err)
-	}
-	return &Controller{
-		v4:     v4,
-		v6:     v6,
 		errors: make(chan error),
-	}, nil
+		logger: log.NewNopLogger(),
+	}
+	for _, o := range opts {
+		o(c)
+	}
+	if c.v4 == nil {
+		v4, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create iptables IPv4 client: %v", err)
+		}
+		c.v4 = v4
+	}
+	if c.v6 == nil {
+		v6, err := iptables.NewWithProtocol(iptables.ProtocolIPv6)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create iptables IPv6 client: %v", err)
+		}
+		c.v6 = v6
+	}
+	return c, nil
 }
 
 // Run watches for changes to iptables rules and reconciles
@@ -239,7 +269,7 @@ func (c *Controller) Run(stop <-chan struct{}) (<-chan error, error) {
 		defer close(c.errors)
 		for {
 			select {
-			case <-time.After(5 * time.Second):
+			case <-time.After(30 * time.Second):
 			case <-stop:
 				return
 			}
@@ -265,6 +295,7 @@ func (c *Controller) reconcile() error {
 			return fmt.Errorf("failed to check if rule exists: %v", err)
 		}
 		if !ok {
+			level.Info(c.logger).Log("msg", fmt.Sprintf("applying %d iptables rules", len(c.rules)-i))
 			if err := c.resetFromIndex(i, c.rules); err != nil {
 				return fmt.Errorf("failed to add rule: %v", err)
 			}

pkg/iptables/iptables_test.go

@@ -83,10 +83,11 @@ func TestSet(t *testing.T) {
 			},
 		},
 	} {
-		controller := &Controller{}
 		client := &fakeClient{}
-		controller.v4 = client
+		controller, err := New(WithClients(client, client))
-		controller.v6 = client
+		if err != nil {
+			t.Fatalf("test case %q: got unexpected error instantiating controller: %v", tc.name, err)
+		}
 		for i := range tc.sets {
 			if err := controller.Set(tc.sets[i]); err != nil {
 				t.Fatalf("test case %q: got unexpected error seting rule set %d: %v", tc.name, i, err)
@@ -139,10 +140,11 @@ func TestCleanUp(t *testing.T) {
 			rules: []Rule{rules[0], rules[1]},
 		},
 	} {
-		controller := &Controller{}
 		client := &fakeClient{}
-		controller.v4 = client
+		controller, err := New(WithClients(client, client))
-		controller.v6 = client
+		if err != nil {
+			t.Fatalf("test case %q: got unexpected error instantiating controller: %v", tc.name, err)
+		}
 		if err := controller.Set(tc.rules); err != nil {
 			t.Fatalf("test case %q: Set should not fail: %v", tc.name, err)
 		}

pkg/mesh/mesh.go

@@ -143,7 +143,7 @@ func New(backend Backend, enc encapsulation.Encapsulator, granularity Granularit
 		level.Debug(logger).Log("msg", "running without a private IP address")
 	}
 	level.Debug(logger).Log("msg", fmt.Sprintf("using %s as the public IP address", publicIP.String()))
-	ipTables, err := iptables.New()
+	ipTables, err := iptables.New(iptables.WithLogger(log.With(logger, "component", "iptables")))
 	if err != nil {
 		return nil, fmt.Errorf("failed to IP tables controller: %v", err)
 	}