diff --git a/pkg/kubelet/network/hostport/fake_iptables.go b/pkg/kubelet/network/hostport/fake_iptables.go index 08faa77dd8..42f7c68116 100644 --- a/pkg/kubelet/network/hostport/fake_iptables.go +++ b/pkg/kubelet/network/hostport/fake_iptables.go @@ -227,12 +227,6 @@ func saveChain(chain *fakeChain, data *bytes.Buffer) { } } -func (f *fakeIPTables) Save(tableName utiliptables.Table) ([]byte, error) { - data := bytes.NewBuffer(nil) - err := f.SaveInto(tableName, data) - return data.Bytes(), err -} - func (f *fakeIPTables) SaveInto(tableName utiliptables.Table, buffer *bytes.Buffer) error { table, err := f.getTable(tableName) if err != nil { diff --git a/pkg/kubelet/network/hostport/hostport_manager.go b/pkg/kubelet/network/hostport/hostport_manager.go index 5c18f8fce1..1499ff9c66 100644 --- a/pkg/kubelet/network/hostport/hostport_manager.go +++ b/pkg/kubelet/network/hostport/hostport_manager.go @@ -275,11 +275,12 @@ func gatherHostportMappings(podPortMapping *PodPortMapping) []*PortMapping { // getExistingHostportIPTablesRules retrieves raw data from iptables-save, parse it, // return all the hostport related chains and rules func getExistingHostportIPTablesRules(iptables utiliptables.Interface) (map[utiliptables.Chain]string, []string, error) { - iptablesSaveRaw, err := iptables.Save(utiliptables.TableNAT) + iptablesData := bytes.NewBuffer(nil) + err := iptables.SaveInto(utiliptables.TableNAT, iptablesData) if err != nil { // if we failed to get any rules return nil, nil, fmt.Errorf("failed to execute iptables-save: %v", err) } - existingNATChains := utiliptables.GetChainLines(utiliptables.TableNAT, iptablesSaveRaw) + existingNATChains := utiliptables.GetChainLines(utiliptables.TableNAT, iptablesData.Bytes()) existingHostportChains := make(map[utiliptables.Chain]string) existingHostportRules := []string{} @@ -290,7 +291,7 @@ func getExistingHostportIPTablesRules(iptables utiliptables.Interface) (map[util } } - for _, line := range strings.Split(string(iptablesSaveRaw), "\n") { + for _, line := range strings.Split(string(iptablesData.Bytes()), "\n") { if strings.HasPrefix(line, fmt.Sprintf("-A %s", kubeHostportChainPrefix)) || strings.HasPrefix(line, fmt.Sprintf("-A %s", string(kubeHostportsChain))) { existingHostportRules = append(existingHostportRules, line) diff --git a/pkg/kubelet/network/hostport/hostport_manager_test.go b/pkg/kubelet/network/hostport/hostport_manager_test.go index 3499dc2d34..2558122d42 100644 --- a/pkg/kubelet/network/hostport/hostport_manager_test.go +++ b/pkg/kubelet/network/hostport/hostport_manager_test.go @@ -17,6 +17,7 @@ limitations under the License. package hostport import ( + "bytes" "net" "testing" @@ -132,10 +133,11 @@ func TestHostportManager(t *testing.T) { } // Check Iptables-save result after adding hostports - raw, err := iptables.Save(utiliptables.TableNAT) + raw := bytes.NewBuffer(nil) + err := iptables.SaveInto(utiliptables.TableNAT, raw) assert.NoError(t, err) - lines := strings.Split(string(raw), "\n") + lines := strings.Split(string(raw.Bytes()), "\n") expectedLines := map[string]bool{ `*nat`: true, `:KUBE-HOSTPORTS - [0:0]`: true, @@ -175,9 +177,10 @@ func TestHostportManager(t *testing.T) { } // Check Iptables-save result after deleting hostports - raw, err = iptables.Save(utiliptables.TableNAT) + raw.Reset() + err = iptables.SaveInto(utiliptables.TableNAT, raw) assert.NoError(t, err) - lines = strings.Split(string(raw), "\n") + lines = strings.Split(string(raw.Bytes()), "\n") remainingChains := make(map[string]bool) for _, line := range lines { if strings.HasPrefix(line, ":") { diff --git a/pkg/kubelet/network/hostport/hostport_syncer.go b/pkg/kubelet/network/hostport/hostport_syncer.go index c72c9e16e3..d1c577dbda 100644 --- a/pkg/kubelet/network/hostport/hostport_syncer.go +++ b/pkg/kubelet/network/hostport/hostport_syncer.go @@ -192,11 +192,12 @@ func (h *hostportSyncer) SyncHostports(natInterfaceName string, activePodPortMap // Get iptables-save output so we can check for existing chains and rules. // This will be a map of chain name to chain with rules as stored in iptables-save/iptables-restore existingNATChains := make(map[utiliptables.Chain]string) - iptablesSaveRaw, err := h.iptables.Save(utiliptables.TableNAT) + iptablesData := bytes.NewBuffer(nil) + err = h.iptables.SaveInto(utiliptables.TableNAT, iptablesData) if err != nil { // if we failed to get any rules glog.Errorf("Failed to execute iptables-save, syncing all rules: %v", err) } else { // otherwise parse the output - existingNATChains = utiliptables.GetChainLines(utiliptables.TableNAT, iptablesSaveRaw) + existingNATChains = utiliptables.GetChainLines(utiliptables.TableNAT, iptablesData.Bytes()) } natChains := bytes.NewBuffer(nil) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index b1015add67..8f58ea3364 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -527,11 +527,12 @@ func CleanupLeftovers(ipt utiliptables.Interface) (encounteredError bool) { } // Flush and remove all of our chains. - if iptablesSaveRaw, err := ipt.Save(utiliptables.TableNAT); err != nil { + iptablesData := bytes.NewBuffer(nil) + if err := ipt.SaveInto(utiliptables.TableNAT, iptablesData); err != nil { glog.Errorf("Failed to execute iptables-save for %s: %v", utiliptables.TableNAT, err) encounteredError = true } else { - existingNATChains := utiliptables.GetChainLines(utiliptables.TableNAT, iptablesSaveRaw) + existingNATChains := utiliptables.GetChainLines(utiliptables.TableNAT, iptablesData.Bytes()) natChains := bytes.NewBuffer(nil) natRules := bytes.NewBuffer(nil) writeLine(natChains, "*nat") diff --git a/pkg/util/exec/fake_exec.go b/pkg/util/exec/fake_exec.go index e3741dca42..c7fcd6cecd 100644 --- a/pkg/util/exec/fake_exec.go +++ b/pkg/util/exec/fake_exec.go @@ -49,6 +49,9 @@ type FakeCmd struct { CombinedOutputScript []FakeCombinedOutputAction CombinedOutputCalls int CombinedOutputLog [][]string + RunScript []FakeRunAction + RunCalls int + RunLog [][]string Dirs []string Stdin io.Reader Stdout io.Writer @@ -61,6 +64,7 @@ func InitFakeCmd(fake *FakeCmd, cmd string, args ...string) Cmd { } type FakeCombinedOutputAction func() ([]byte, error) +type FakeRunAction func() ([]byte, []byte, error) func (fake *FakeCmd) SetDir(dir string) { fake.Dirs = append(fake.Dirs, dir) @@ -79,7 +83,23 @@ func (fake *FakeCmd) SetStderr(out io.Writer) { } func (fake *FakeCmd) Run() error { - return fmt.Errorf("unimplemented") + if fake.RunCalls > len(fake.RunScript)-1 { + panic("ran out of Run() actions") + } + if fake.RunLog == nil { + fake.RunLog = [][]string{} + } + i := fake.RunCalls + fake.RunLog = append(fake.RunLog, append([]string{}, fake.Argv...)) + fake.RunCalls++ + stdout, stderr, err := fake.RunScript[i]() + if stdout != nil { + fake.Stdout.Write(stdout) + } + if stderr != nil { + fake.Stderr.Write(stderr) + } + return err } func (fake *FakeCmd) CombinedOutput() ([]byte, error) { diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go index 57edf5f1e5..b6c08fa378 100644 --- a/pkg/util/iptables/iptables.go +++ b/pkg/util/iptables/iptables.go @@ -54,13 +54,11 @@ type Interface interface { DeleteRule(table Table, chain Chain, args ...string) error // IsIpv6 returns true if this is managing ipv6 tables IsIpv6() bool - // Save calls `iptables-save` for table. - Save(table Table) ([]byte, error) // SaveInto calls `iptables-save` for table and stores result in a given buffer. SaveInto(table Table, buffer *bytes.Buffer) error // Restore runs `iptables-restore` passing data through []byte. // table is the Table to restore - // data should be formatted like the output of Save() + // data should be formatted like the output of SaveInto() // flush sets the presence of the "--noflush" flag. see: FlushFlag // counters sets the "--counters" flag. see: RestoreCountersFlag Restore(table Table, data []byte, flush FlushFlag, counters RestoreCountersFlag) error @@ -306,17 +304,6 @@ func (runner *runner) IsIpv6() bool { return runner.protocol == ProtocolIpv6 } -// Save is part of Interface. -func (runner *runner) Save(table Table) ([]byte, error) { - runner.mu.Lock() - defer runner.mu.Unlock() - - // run and return - args := []string{"-t", string(table)} - glog.V(4).Infof("running iptables-save %v", args) - return runner.exec.Command(cmdIPTablesSave, args...).CombinedOutput() -} - // SaveInto is part of Interface. func (runner *runner) SaveInto(table Table, buffer *bytes.Buffer) error { runner.mu.Lock() diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go index 62b8416709..824cede912 100644 --- a/pkg/util/iptables/iptables_test.go +++ b/pkg/util/iptables/iptables_test.go @@ -17,6 +17,7 @@ limitations under the License. package iptables import ( + "bytes" "net" "os" "strings" @@ -831,7 +832,7 @@ func TestReload(t *testing.T) { } } -func TestSave(t *testing.T) { +func TestSaveInto(t *testing.T) { output := `# Generated by iptables-save v1.6.0 on Thu Jan 19 11:38:09 2017 *filter :INPUT ACCEPT [15079:38410730] @@ -846,8 +847,10 @@ COMMIT func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, // iptables-restore version check func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, - func() ([]byte, error) { return []byte(output), nil }, - func() ([]byte, error) { return nil, &exec.FakeExitError{Status: 1} }, + }, + RunScript: []exec.FakeRunAction{ + func() ([]byte, []byte, error) { return []byte(output), nil, nil }, + func() ([]byte, []byte, error) { return nil, nil, &exec.FakeExitError{Status: 1} }, }, } fexec := exec.FakeExec{ @@ -860,25 +863,31 @@ COMMIT } runner := New(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4) defer runner.Destroy() + buffer := bytes.NewBuffer(nil) + // Success. - o, err := runner.Save(TableNAT) + err := runner.SaveInto(TableNAT, buffer) if err != nil { t.Fatalf("expected success, got %v", err) } - if string(o[:len(output)]) != output { - t.Errorf("expected output to be equal to mocked one, got %v", o) + if string(buffer.Bytes()[:len(output)]) != output { + t.Errorf("expected output to be equal to mocked one, got %v", buffer.Bytes()) } - if fcmd.CombinedOutputCalls != 3 { - t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables-save", "-t", "nat") { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) + if fcmd.RunCalls != 1 { + t.Errorf("expected 1 Run() call, got %d", fcmd.RunCalls) + } + if !sets.NewString(fcmd.RunLog[0]...).HasAll("iptables-save", "-t", "nat") { + t.Errorf("wrong Run() log, got %s", fcmd.RunLog[0]) } // Failure. - _, err = runner.Save(TableNAT) + buffer.Reset() + err = runner.SaveInto(TableNAT, buffer) if err == nil { t.Errorf("expected failure") }