Skip to content

Commit b031258

Browse files
committed
Improve utiliptables error handling when there's no iptables binary
If `iptables --version` failed, utiliptables.New() would log a warning and assume that the problem was that you had an implausibly ancient version of iptables installed. Change it to instead assume that the problem is that you don't have iptables installed at all (and don't log anything; the caller will discover this later).
1 parent f1d0eb4 commit b031258

File tree

2 files changed

+15
-16
lines changed

2 files changed

+15
-16
lines changed

pkg/util/iptables/iptables.go

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -219,12 +219,6 @@ type runner struct {
219219
// newInternal returns a new Interface which will exec iptables, and allows the
220220
// caller to change the iptables-restore lockfile path
221221
func newInternal(exec utilexec.Interface, protocol Protocol, lockfilePath14x, lockfilePath16x string) Interface {
222-
version, err := getIPTablesVersion(exec, protocol)
223-
if err != nil {
224-
klog.InfoS("Error checking iptables version, assuming version at least", "version", MinCheckVersion, "err", err)
225-
version = MinCheckVersion
226-
}
227-
228222
if lockfilePath16x == "" {
229223
lockfilePath16x = LockfilePath16x
230224
}
@@ -235,13 +229,22 @@ func newInternal(exec utilexec.Interface, protocol Protocol, lockfilePath14x, lo
235229
runner := &runner{
236230
exec: exec,
237231
protocol: protocol,
238-
hasCheck: version.AtLeast(MinCheckVersion),
239-
hasRandomFully: version.AtLeast(RandomFullyMinVersion),
240-
waitFlag: getIPTablesWaitFlag(version),
241-
restoreWaitFlag: getIPTablesRestoreWaitFlag(version, exec, protocol),
242232
lockfilePath14x: lockfilePath14x,
243233
lockfilePath16x: lockfilePath16x,
244234
}
235+
236+
version, err := getIPTablesVersion(exec, protocol)
237+
if err != nil {
238+
// The only likely error is "no such file or directory", in which case any
239+
// further commands will fail the same way, so we don't need to do
240+
// anything special here.
241+
return runner
242+
}
243+
244+
runner.hasCheck = version.AtLeast(MinCheckVersion)
245+
runner.hasRandomFully = version.AtLeast(RandomFullyMinVersion)
246+
runner.waitFlag = getIPTablesWaitFlag(version)
247+
runner.restoreWaitFlag = getIPTablesRestoreWaitFlag(version, exec, protocol)
245248
return runner
246249
}
247250

pkg/util/iptables/iptables_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,13 +177,9 @@ func TestNew(t *testing.T) {
177177
command: "iptables --version",
178178
action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("no such file or directory") },
179179
},
180-
{
181-
command: "iptables-restore --version",
182-
action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("no such file or directory") },
183-
},
184180
},
185181
expected: &runner{
186-
hasCheck: true,
182+
hasCheck: false,
187183
hasRandomFully: false,
188184
waitFlag: nil,
189185
restoreWaitFlag: nil,
@@ -601,7 +597,7 @@ func TestGetIPTablesHasCheckCommand(t *testing.T) {
601597
{"iptables v1.4.11", true},
602598
{"iptables v1.4.19.1", true},
603599
{"iptables v2.0.0", true},
604-
{"total junk", true},
600+
{"total junk", false},
605601
}
606602

607603
for _, testCase := range testCases {

0 commit comments

Comments
 (0)