Be more agressive acquiring the iptables lock

iptables has two options to modify the behaviour trying to
acquire the lock.

--wait  -w [seconds]    maximum wait to acquire xtables lock
                        before give up
--wait-interval -W [usecs]  wait time to try to acquire xtables
                            lock
                            interval to wait for xtables lock
                            default is 1 second

Kubernetes uses -w 5 that means that wait 5 seconds to try to
acquire the lock. If we are not able to acquire it, kube-proxy
fails and retries in 30 seconds, that is an important penalty
on sensitive applications.
We can be a bit more aggresive and try to acquire the lock every
100 msec, that means that we have to fail 50 times to not being
able to succeed.
This commit is contained in:
Antonio Ojea 2019-11-29 07:07:13 +01:00
parent 82ee37f3e1
commit 51814ae189
No known key found for this signature in database
GPG Key ID: E4833AA228D4E824
3 changed files with 49 additions and 7 deletions

View File

@ -163,6 +163,9 @@ var RandomFullyMinVersion = utilversion.MustParseGeneric("1.6.2")
// WaitMinVersion a minimum iptables versions supporting the -w and -w<seconds> flags
var WaitMinVersion = utilversion.MustParseGeneric("1.4.20")
// WaitIntervalMinVersion a minimum iptables versions supporting the wait interval useconds
var WaitIntervalMinVersion = utilversion.MustParseGeneric("1.6.1")
// WaitSecondsMinVersion a minimum iptables versions supporting the wait seconds
var WaitSecondsMinVersion = utilversion.MustParseGeneric("1.4.22")
@ -175,6 +178,12 @@ const WaitString = "-w"
// WaitSecondsValue a constant for specifying the default wait seconds
const WaitSecondsValue = "5"
// WaitIntervalString a constant for specifying the wait interval flag
const WaitIntervalString = "-W"
// WaitIntervalUsecondsValue a constant for specifying the default wait interval useconds
const WaitIntervalUsecondsValue = "100000"
// LockfilePath16x is the iptables lock file acquired by any process that's making any change in the iptable rule
const LockfilePath16x = "/run/xtables.lock"
@ -638,6 +647,8 @@ func getIPTablesVersion(exec utilexec.Interface, protocol Protocol) (*utilversio
// Checks if iptables version has a "wait" flag
func getIPTablesWaitFlag(version *utilversion.Version) []string {
switch {
case version.AtLeast(WaitIntervalMinVersion):
return []string{WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue}
case version.AtLeast(WaitSecondsMinVersion):
return []string{WaitString, WaitSecondsValue}
case version.AtLeast(WaitMinVersion):
@ -650,7 +661,7 @@ func getIPTablesWaitFlag(version *utilversion.Version) []string {
// Checks if iptables-restore has a "wait" flag
func getIPTablesRestoreWaitFlag(version *utilversion.Version, exec utilexec.Interface, protocol Protocol) []string {
if version.AtLeast(WaitRestoreMinVersion) {
return []string{WaitString, WaitSecondsValue}
return []string{WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue}
}
// Older versions may have backported features; if iptables-restore supports

View File

@ -620,7 +620,7 @@ func TestIPTablesWaitFlag(t *testing.T) {
{"1.4.21", []string{WaitString}},
{"1.4.22", []string{WaitString, WaitSecondsValue}},
{"1.5.0", []string{WaitString, WaitSecondsValue}},
{"2.0.0", []string{WaitString, WaitSecondsValue}},
{"2.0.0", []string{WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue}},
}
for _, testCase := range testCases {
@ -729,6 +729,37 @@ func TestWaitFlagNew(t *testing.T) {
}
}
func TestWaitIntervalFlagNew(t *testing.T) {
fcmd := fakeexec.FakeCmd{
CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{
// iptables version check
func() ([]byte, error) { return []byte("iptables v1.6.1"), nil },
// iptables-restore version check
func() ([]byte, error) { return []byte{}, nil },
// Success.
func() ([]byte, error) { return []byte{}, nil },
},
}
fexec := fakeexec.FakeExec{
CommandScript: []fakeexec.FakeCommandAction{
func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
},
}
runner := New(&fexec, ProtocolIpv4)
err := runner.DeleteChain(TableNAT, Chain("FOOBAR"))
if err != nil {
t.Errorf("expected success, got %v", err)
}
if fcmd.CombinedOutputCalls != 3 {
t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
}
if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue) {
t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2])
}
}
func testSaveInto(t *testing.T, protocol Protocol) {
version := " v1.9.22"
iptablesCmd := iptablesCommand(protocol)
@ -963,7 +994,7 @@ func TestRestoreAllWait(t *testing.T) {
}
commandSet := sets.NewString(fcmd.CombinedOutputLog[1]...)
if !commandSet.HasAll("iptables-restore", WaitString, WaitSecondsValue, "--counters", "--noflush") {
if !commandSet.HasAll("iptables-restore", WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue, "--counters", "--noflush") {
t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1])
}

View File

@ -100,12 +100,12 @@ func (mfc *monitorFakeCmd) CombinedOutput() ([]byte, error) {
return []byte("iptables v1.6.2"), nil
}
if len(mfc.args) != 6 || mfc.args[0] != WaitString || mfc.args[1] != WaitSecondsValue || mfc.args[4] != "-t" {
if len(mfc.args) != 8 || mfc.args[0] != WaitString || mfc.args[1] != WaitSecondsValue || mfc.args[2] != WaitIntervalString || mfc.args[3] != WaitIntervalUsecondsValue || mfc.args[6] != "-t" {
panic(fmt.Sprintf("bad args %#v", mfc.args))
}
op := operation(mfc.args[2])
chainName := mfc.args[3]
tableName := mfc.args[5]
op := operation(mfc.args[4])
chainName := mfc.args[5]
tableName := mfc.args[7]
mfc.mfe.Lock()
defer mfc.mfe.Unlock()