Skip to content

Commit

Permalink
Add UTs for iptables lock.
Browse files Browse the repository at this point in the history
  • Loading branch information
fasaxc committed Jul 11, 2017
1 parent 788398b commit c14aed9
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 25 deletions.
5 changes: 2 additions & 3 deletions config/config_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,14 @@ type Config struct {
Ipv6Support bool `config:"bool;true"`
IgnoreLooseRPF bool `config:"bool;false"`

IPSetsRefreshInterval time.Duration `config:"seconds;10"`
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;10"`
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"`
Expand Down Expand Up @@ -143,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"`
Expand Down
2 changes: 1 addition & 1 deletion felix.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ configRetry:
IPIPMTU: configParams.IpInIpMtu,
IptablesRefreshInterval: configParams.IptablesRefreshInterval,
RouteRefreshInterval: configParams.RouteRefreshInterval,
IPSetsRefreshInterval: configParams.IPSetsRefreshInterval,
IPSetsRefreshInterval: configParams.IpsetsRefreshInterval,
IptablesPostWriteCheckInterval: configParams.IptablesPostWriteCheckIntervalSecs,
IptablesInsertMode: configParams.ChainInsertMode,
IptablesLockFilePath: configParams.IptablesLockFilePath,
Expand Down
60 changes: 39 additions & 21 deletions iptables/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ type SharedLock struct {
lockTimeout time.Duration
lockProbeInterval time.Duration

GrabIptablesLocks func(lockFilePath string, timeout, probeInterval time.Duration) (io.Closer, error)
GrabIptablesLocks func(lockFilePath, socketName string, timeout, probeInterval time.Duration) (io.Closer, error)
}

func (l *SharedLock) Lock() {
Expand All @@ -86,8 +86,15 @@ func (l *SharedLock) Lock() {

if l.referenceCount == 0 {
// The lock isn't currently held. Acquire it.
lockHandle, err := l.GrabIptablesLocks(l.lockFilePath, l.lockTimeout, l.lockProbeInterval)
lockHandle, err := l.GrabIptablesLocks(
l.lockFilePath,
"@xtables",
l.lockTimeout,
l.lockProbeInterval,
)
if err != nil {
// We give the lock plenty of time so err on the side of assuming a
// programming bug.
log.WithError(err).Panic("Failed to acquire iptables lock")
}
l.iptablesLockHandle = lockHandle
Expand All @@ -105,43 +112,53 @@ func (l *SharedLock) Unlock() {
}
if l.referenceCount == 0 {
log.Debug("Releasing iptables lock.")
l.iptablesLockHandle.Close()
err := l.iptablesLockHandle.Close()
if err != nil {
// We haven't done anything with the file or socket so we shouldn't be
// able to hit any "deferred flush" type errors from the close. Panic
// since we're not sure what's going on.
log.WithError(err).Panic("Error while closing iptables lock.")
}
l.iptablesLockHandle = nil
}
}

type locker struct {
lock16 *os.File
lock14 *net.UnixListener
type Locker struct {
Lock16 io.Closer
Lock14 io.Closer
}

func (l *locker) Close() error {
if l.lock16 != nil {
err := l.lock16.Close()
func (l *Locker) Close() error {
var err error
if l.Lock16 != nil {
err = l.Lock16.Close()
if err != nil {
log.WithError(err).Error("Error while closing iptables lock file")
log.WithError(err).Error("Error while closing lock file.")
}
}
if l.lock14 != nil {
err := l.lock14.Close()
if err != nil {
log.WithError(err).Error("Error while closing iptables lock socket")
if l.Lock14 != nil {
err14 := l.Lock14.Close()
if err14 != nil {
log.WithError(err14).Error("Error while closing lock socket.")
}
if err14 != nil && err == nil {
err = err14
}
}
return nil
return err
}

var (
Err14LockTimeout = errors.New("Timed out waiting for iptables 1.4 lock")
Err16LockTimeout = errors.New("Timed out waiting for iptables 1.6 lock")
)

func GrabIptablesLocks(lockFilePath string, timeout, probeInterval time.Duration) (io.Closer, error) {
func GrabIptablesLocks(lockFilePath, socketName string, timeout, probeInterval time.Duration) (io.Closer, error) {
var err error
var success bool

l := &locker{}
defer func(l *locker) {
l := &Locker{}
defer func(l *Locker) {
// Clean up immediately on failure
if !success {
l.Close()
Expand All @@ -153,14 +170,15 @@ func GrabIptablesLocks(lockFilePath string, timeout, probeInterval time.Duration
// can't assume which lock method it'll use.

// Roughly duplicate iptables 1.6.x xtables_lock() function.
l.lock16, err = os.OpenFile(lockFilePath, os.O_CREATE, 0600)
f, err := os.OpenFile(lockFilePath, os.O_CREATE, 0600)
l.Lock16 = f
if err != nil {
return nil, fmt.Errorf("failed to open iptables lock %s: %v", lockFilePath, err)
}

startTime := time.Now()
for {
if err := grabIptablesFileLock(l.lock16); err == nil {
if err := grabIptablesFileLock(f); err == nil {
break
}
if time.Since(startTime) > timeout {
Expand All @@ -173,7 +191,7 @@ func GrabIptablesLocks(lockFilePath string, timeout, probeInterval time.Duration

startTime14 := time.Now()
for {
l.lock14, err = net.ListenUnix("unix", &net.UnixAddr{Name: "@xtables", Net: "unix"})
l.Lock14, err = net.ListenUnix("unix", &net.UnixAddr{Name: socketName, Net: "unix"})
if err == nil {
break
}
Expand Down
175 changes: 175 additions & 0 deletions iptables/lock_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
// 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 iptables_test

import (
"io"
"io/ioutil"
"os"
"time"

. "github.com/projectcalico/felix/iptables"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

var _ = Describe("SharedLock", func() {
var lock *SharedLock
var mockIptablesLock *mockLock
var mockErr error

mockGrabLock := func(lockFilePath, socketName string, timeout, probeInterval time.Duration) (io.Closer, error) {
return mockIptablesLock, mockErr
}

BeforeEach(func() {
mockErr = nil
mockIptablesLock = &mockLock{}
lock = NewSharedLock("/foo/bar.lock", time.Second, time.Millisecond)
lock.GrabIptablesLocks = mockGrabLock
})

It("should close the lock after final unlock call", func() {
lock.Lock()
Expect(mockIptablesLock.Closed).To(BeFalse())
lock.Unlock()
Expect(mockIptablesLock.Closed).To(BeTrue())
})
It("should allow multiple holders", func() {
lock.Lock()
lock.Lock()
Expect(mockIptablesLock.Closed).To(BeFalse())
lock.Unlock()
Expect(mockIptablesLock.Closed).To(BeFalse())
lock.Unlock()
Expect(mockIptablesLock.Closed).To(BeTrue())
})
It("should panic on misuse", func() {
Expect(lock.Unlock).To(Panic())
})
It("should panic on failure to acquire", func() {
mockErr = Err14LockTimeout
Expect(lock.Lock).To(Panic())
})
It("should panic on failure to close", func() {
lock.Lock()
mockIptablesLock.Err = Err14LockTimeout
Expect(lock.Unlock).To(Panic())
})
})

type mockLock struct {
Closed bool
Err error
}

func (l *mockLock) Close() error {
Expect(l.Closed).To(BeFalse())
l.Closed = true
return l.Err
}

var _ = Describe("GrabIptablesLocks FV", func() {
var fileName string

BeforeEach(func() {
f, err := ioutil.TempFile("", "iptlocktest")
Expect(err).NotTo(HaveOccurred())
fileName = f.Name()
f.Close()
})
AfterEach(func() {
os.Remove(fileName)
})

It("should block concurrent invocations", func() {
l, err := GrabIptablesLocks(fileName, "@dummytables", 1*time.Second, 50*time.Millisecond)
Expect(err).NotTo(HaveOccurred())
defer l.Close()

l2, err := GrabIptablesLocks(fileName, "@dummytables", 100*time.Millisecond, 10*time.Millisecond)
Expect(err).To(Equal(Err16LockTimeout))
Expect(l2).To(BeNil())
})
It("should allow access after being released", func() {
l, err := GrabIptablesLocks(fileName, "@dummytables", 1*time.Second, 50*time.Millisecond)
Expect(err).NotTo(HaveOccurred())
l.Close()

l2, err := GrabIptablesLocks(fileName, "@dummytables", 1*time.Second, 50*time.Millisecond)
Expect(err).NotTo(HaveOccurred())
l2.Close()
})
It("should block concurrent invocations using only iptables 1.4 version of lock", func() {
l, err := GrabIptablesLocks(fileName, "@dummytables", 1*time.Second, 50*time.Millisecond)
// Sneakily remove the lockfile after it's been locked so that we fall through to
// the v1.4 lock.
os.Remove(fileName)
Expect(err).NotTo(HaveOccurred())
defer l.Close()

l2, err := GrabIptablesLocks(fileName, "@dummytables", 100*time.Millisecond, 10*time.Millisecond)
Expect(err).To(Equal(Err14LockTimeout))
Expect(l2).To(BeNil())
})
It("should allow access after being released using only iptables 1.4 version of lock", func() {
l, err := GrabIptablesLocks(fileName, "@dummytables", 1*time.Second, 50*time.Millisecond)
// Sneakily remove the lockfile after it's been locked so that we fall through to
// the v1.4 lock.
os.Remove(fileName)
Expect(err).NotTo(HaveOccurred())
l.Close()

l2, err := GrabIptablesLocks(fileName, "@dummytables", 1*time.Second, 50*time.Millisecond)
Expect(err).NotTo(HaveOccurred())
l2.Close()
})
})

var _ = Describe("locker", func() {
var l *Locker
var lock14 mockCloser
var lock16 mockCloser

BeforeEach(func() {
lock14 = mockCloser{}
lock16 = mockCloser{}
l = &Locker{
Lock14: &lock14,
Lock16: &lock16,
}
})

It("should return nil with no err", func() {
Expect(l.Close()).NotTo(HaveOccurred())
})
It("should return lock16 err", func() {
lock16.Err = Err16LockTimeout
Expect(l.Close()).To(Equal(Err16LockTimeout))
})
It("should return lock14 err", func() {
lock14.Err = Err14LockTimeout
Expect(l.Close()).To(Equal(Err14LockTimeout))
})
})

type mockCloser struct {
Err error
}

func (c *mockCloser) Close() error {
return c.Err
}

0 comments on commit c14aed9

Please sign in to comment.