Grab the iptables lock before running iptables-restore.

Makefile

@@ -78,7 +78,7 @@ help:
 .SUFFIXES:
 
 all: deb rpm calico/felix
-test: ut
+test: ut fv
 
 GO_BUILD_CONTAINER?=calico/go-build:v0.6
 
@@ -292,6 +292,12 @@ bin/calico-felix: $(FELIX_GO_FILES) vendor/.up-to-date
                ( ldd bin/calico-felix 2>&1 | grep -q "Not a valid dynamic program" || \
 	             ( echo "Error: bin/calico-felix was not statically linked"; false ) )'
 
+bin/iptables-locker: $(FELIX_GO_FILES) vendor/.up-to-date
+	@echo Building iptables-locker...
+	mkdir -p bin
+	$(DOCKER_GO_BUILD) \
+	    sh -c 'go build -v -i -o $@ -v $(LDFLAGS) "github.com/projectcalico/felix/fv/iptables-locker"'
+
 bin/k8sfv.test: $(K8SFV_GO_FILES) vendor/.up-to-date
 	@echo Building $@...
 	$(DOCKER_GO_BUILD) \
@@ -324,6 +330,16 @@ ut combined.coverprofile: vendor/.up-to-date $(FELIX_GO_FILES)
 	@echo Running Go UTs.
 	$(DOCKER_GO_BUILD) ./utils/run-coverage
 
+fv/fv.test: vendor/.up-to-date $(FELIX_GO_FILES)
+	$(DOCKER_GO_BUILD) go test ./fv -c --tags fvtests -o fv/fv.test
+
+.PHONY: fv
+fv: calico/felix bin/iptables-locker fv/fv.test
+	@echo Running Go FVs.
+	# For now, we pre-build the binary so that we can run it outside a container and allow it
+	# to interact with docker.
+	cd fv && ./fv.test
+
 bin/check-licenses: $(FELIX_GO_FILES)
 	$(DOCKER_GO_BUILD) go build -v -i -o $@ "github.com/projectcalico/felix/check-licenses"
 
@@ -403,6 +419,7 @@ clean:
 	       docker-image/bin \
 	       dist \
 	       build \
+	       fv/fv.test \
 	       $(GENERATED_GO_FILES) \
 	       go/docs/calc.pdf \
 	       .glide \

README.md

@@ -110,7 +110,24 @@ Ginkgo will re-run tests as files are modified and saved.
 
 After building the docker image (see above), you can run Felix and log to screen 
 with, for example:
-`docker run --privileged --net=host -e FELIX_LOGSEVERITYSCREEN=INFO calico/felix`
+
+```
+docker run --privileged \
+           --net=host \
+           -v /run:/run \
+           -e FELIX_LOGSEVERITYSCREEN=INFO \
+           calico/felix
+```
+
+Notes:
+
+- `--privileged` is required because Felix needs to execute iptables and other privileged commands.
+- `--net=host` is required so that Felix can manipulate the routes and iptables tables in the host
+  namespace (outside its container).
+- `-v /run:/run` is required so that Felix shares the global iptables file lock with other
+  processes; this allows Felix and other daemons that manipulate iptables to avoid clobbering each
+  other's updates.
+- `-e FELIX_LOGSEVERITYSCREEN=INFO` tells Felix to log at info level to stderr.
 
 ### Debs and RPMs
 

config/config_params.go

@@ -109,8 +109,14 @@ type Config struct {
 	Ipv6Support    bool `config:"bool;true"`
 	IgnoreLooseRPF bool `config:"bool;false"`
 
-	IptablesRefreshInterval            time.Duration `config:"seconds;10"`
-	IptablesPostWriteCheckIntervalSecs time.Duration `config:"seconds;1"`
+	RouteRefreshInterval               time.Duration `config:"seconds;90"`
+	IptablesRefreshInterval            time.Duration `config:"seconds;90"`
+	IptablesPostWriteCheckIntervalSecs time.Duration `config:"seconds;30"`
+	IptablesLockFilePath               string        `config:"file;/run/xtables.lock"`
+	IptablesLockTimeoutSecs            time.Duration `config:"seconds;30"`
+	IptablesLockProbeIntervalMillis    time.Duration `config:"millis;50"`
+	IpsetsRefreshInterval              time.Duration `config:"seconds;10"`
+	MaxIpsetSize                       int           `config:"int;1048576;non-zero"`
 
 	MetadataAddr string `config:"hostname;127.0.0.1;die-on-fail"`
 	MetadataPort int    `config:"int(0,65535);8775;die-on-fail"`
@@ -138,8 +144,6 @@ type Config struct {
 	EndpointReportingEnabled   bool          `config:"bool;false"`
 	EndpointReportingDelaySecs time.Duration `config:"seconds;1"`
 
-	MaxIpsetSize int `config:"int;1048576;non-zero"`
-
 	IptablesMarkMask uint32 `config:"mark-bitmask;0xff000000;non-zero,die-on-fail"`
 
 	DisableConntrackInvalidCheck bool `config:"bool;false"`
@@ -459,6 +463,8 @@ func loadParams() {
 			param = &FloatParam{}
 		case "seconds":
 			param = &SecondsParam{}
+		case "millis":
+			param = &MillisParam{}
 		case "iface-list":
 			param = &RegexpParam{Regexp: IfaceListRegexp,
 				Msg: "invalid Linux interface name"}

config/config_params_test.go

@@ -76,6 +76,14 @@ var _ = DescribeTable("Config parsing",
 
 	Entry("IptablesPostWriteCheckIntervalSecs", "IptablesPostWriteCheckIntervalSecs",
 		"1.5", 1500*time.Millisecond),
+	Entry("IptablesLockFilePath", "IptablesLockFilePath",
+		"/host/run/xtables.lock", "/host/run/xtables.lock"),
+	Entry("IptablesLockTimeoutSecs", "IptablesLockTimeoutSecs",
+		"123", 123*time.Second),
+	Entry("IptablesLockProbeIntervalMillis", "IptablesLockProbeIntervalMillis",
+		"123", 123*time.Millisecond),
+	Entry("IptablesLockProbeIntervalMillis garbage", "IptablesLockProbeIntervalMillis",
+		"garbage", 50*time.Millisecond),
 
 	Entry("DefaultEndpointToHostAction", "DefaultEndpointToHostAction",
 		"RETURN", "RETURN"),

config/param_types.go

@@ -140,6 +140,20 @@ func (p *SecondsParam) Parse(raw string) (result interface{}, err error) {
 	return
 }
 
+type MillisParam struct {
+	Metadata
+}
+
+func (p *MillisParam) Parse(raw string) (result interface{}, err error) {
+	millis, err := strconv.ParseFloat(raw, 64)
+	if err != nil {
+		err = p.parseFailed(raw, "invalid float")
+		return
+	}
+	result = time.Duration(millis * float64(time.Millisecond))
+	return
+}
+
 type RegexpParam struct {
 	Metadata
 	Regexp *regexp.Regexp

felix.go

@@ -283,8 +283,13 @@ configRetry:
 			},
 			IPIPMTU:                        configParams.IpInIpMtu,
 			IptablesRefreshInterval:        configParams.IptablesRefreshInterval,
+			RouteRefreshInterval:           configParams.RouteRefreshInterval,
+			IPSetsRefreshInterval:          configParams.IpsetsRefreshInterval,
 			IptablesPostWriteCheckInterval: configParams.IptablesPostWriteCheckIntervalSecs,
 			IptablesInsertMode:             configParams.ChainInsertMode,
+			IptablesLockFilePath:           configParams.IptablesLockFilePath,
+			IptablesLockTimeout:            configParams.IptablesLockTimeoutSecs,
+			IptablesLockProbeInterval:      configParams.IptablesLockProbeIntervalMillis,
 			MaxIPSetSize:                   configParams.MaxIpsetSize,
 			IgnoreLooseRPF:                 configParams.IgnoreLooseRPF,
 			IPv6Enabled:                    configParams.Ipv6Support,

fv/.gitignore

@@ -0,0 +1 @@
+fv.test

fv/fv.go

@@ -0,0 +1,16 @@
+// Copyright (c) 2017 Tigera, Inc. All rights reserved.
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// The fv packge contains FV tests that execute Felix for-real.
+package fv

fv/fv_suite_test.go

@@ -0,0 +1,35 @@
+// Copyright (c) 2017 Tigera, Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package fv_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/reporters"
+	. "github.com/onsi/gomega"
+
+	"github.com/projectcalico/libcalico-go/lib/testutils"
+)
+
+func init() {
+	testutils.HookLogrusForGinkgo()
+}
+
+func TestFv(t *testing.T) {
+	RegisterFailHandler(Fail)
+	junitReporter := reporters.NewJUnitReporter("junit.xml")
+	RunSpecsWithDefaultAndCustomReporters(t, "FV Suite", []Reporter{junitReporter})
+}

fv/iptables-locker/iptables-locker.go

@@ -0,0 +1,55 @@
+// Copyright (c) 2017 Tigera, Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"time"
+
+	log "github.com/Sirupsen/logrus"
+	"github.com/docopt/docopt-go"
+
+	"github.com/projectcalico/felix/iptables"
+)
+
+const usage = `iptables-locker, test tool for grabbing the iptables lock.
+
+Usage:
+  iptables-locker <duration>
+
+`
+
+func main() {
+	arguments, err := docopt.Parse(usage, nil, true, "v0.1", false)
+	if err != nil {
+		println(usage)
+		log.WithError(err).Fatal("Failed to parse usage")
+	}
+	durationStr := arguments["<duration>"].(string)
+	duration, err := time.ParseDuration(durationStr)
+	if err != nil {
+		println(usage)
+		log.WithError(err).Fatal("Failed to parse usage")
+	}
+
+	iptablesLock := iptables.NewSharedLock(
+		"/run/xtables.lock",
+		1*time.Second,
+		50*time.Millisecond,
+	)
+	iptablesLock.Lock()
+	println("LOCKED")
+	time.Sleep(duration)
+	iptablesLock.Unlock()
+}

fv/iptables_lock_test.go

@@ -0,0 +1,130 @@
+// +build fvtests
+
+// Copyright (c) 2017 Tigera, Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package fv_test
+
+import (
+	"bufio"
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
+
+	"time"
+
+	log "github.com/Sirupsen/logrus"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("with running container", func() {
+	var containerIdx int
+	var containerName string
+	var felixCmd *exec.Cmd
+
+	cmdInContainer := func(cmd ...string) *exec.Cmd {
+		arg := []string{"exec", containerName}
+		arg = append(arg, cmd...)
+		return exec.Command("docker", arg...)
+	}
+
+	BeforeEach(func() {
+		containerName = fmt.Sprintf("felix-fv-%d-%d", os.Getpid(), containerIdx)
+		containerIdx++
+		myDir, err := os.Getwd()
+		Expect(err).NotTo(HaveOccurred())
+		log.WithFields(log.Fields{
+			"name":  containerName,
+			"myDir": myDir,
+		}).Info("Starting a Felix container")
+		// Run a felix container.  The tests in this file don't actually rely on Felix
+		// but the calico/felix container has all the iptables dependencies we need to
+		// check the lock behaviour.  Note: we don't map the host's iptables lock into the
+		// container so the scope of the lock is limited to the container.
+		felixCmd = exec.Command("docker", "run",
+			"--rm",
+			"--name", containerName,
+			"-v", fmt.Sprintf("%s/..:/codebase", myDir),
+			"--privileged",
+			"calico/felix")
+		err = felixCmd.Start()
+		Expect(err).NotTo(HaveOccurred())
+
+		log.Info("Waiting for container to be listed in docker ps")
+		start := time.Now()
+		for {
+			cmd := exec.Command("docker", "ps")
+			out, err := cmd.CombinedOutput()
+			Expect(err).NotTo(HaveOccurred())
+			if strings.Contains(string(out), containerName) {
+				break
+			}
+			if time.Since(start) > 10*time.Second {
+				log.Panic("Timed out waiting for container to be listed.")
+			}
+		}
+	})
+	AfterEach(func() {
+		// Send an interrupt to ensure that docker gracefully shuts down the container.
+		// If we kill the docker process then it detaches the container.
+		log.Info("Stopping Felix container")
+		felixCmd.Process.Signal(os.Interrupt)
+	})
+
+	Describe("with the lock being held for 2s", func() {
+		var lockCmd *exec.Cmd
+		BeforeEach(func() {
+			// Start the iptables-locker, which is a simple test app that locks
+			// the iptables lock and then releases it after a timeout.
+			log.Info("Starting iptables-locker")
+			lockCmd = cmdInContainer("/codebase/bin/iptables-locker", "2s")
+			stdErr, err := lockCmd.StderrPipe()
+			Expect(err).NotTo(HaveOccurred())
+			lockCmd.Start()
+
+			// Wait for the iptables-locker to tell us that it actually acquired the
+			// lock.
+			log.Info("Waiting for iptables-locker to acquire lock")
+			scanner := bufio.NewScanner(stdErr)
+			Expect(scanner.Scan()).To(BeTrue())
+			Expect(scanner.Text()).To(Equal("LOCKED"))
+			Expect(scanner.Err()).NotTo(HaveOccurred())
+			log.Info("iptables-locker acquired lock")
+		})
+
+		It("iptables should fail to get the lock in 1s", func() {
+			iptCmd := cmdInContainer("iptables", "-w", "1", "-A", "FORWARD")
+			out, err := iptCmd.CombinedOutput()
+			Expect(string(out)).To(ContainSubstring("Stopped waiting"))
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("iptables should succeed in getting the lock after 3s", func() {
+			iptCmd := cmdInContainer("iptables", "-w", "3", "-A", "FORWARD")
+			out, err := iptCmd.CombinedOutput()
+			Expect(string(out)).To(ContainSubstring("Another app is currently holding the xtables lock"))
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		AfterEach(func() {
+			if lockCmd != nil {
+				log.Info("waiting for iptables-locker to finish")
+				err := lockCmd.Wait()
+				Expect(err).NotTo(HaveOccurred())
+			}
+		})
+	})
+})