Changes from review :

- refactor protocol to proto
- fix check version for wait command available on iptables-restore
- apply "Error strings should not be capitalized"
- lower case for iptables-restore
- refactor function ApplyPartial to ApplyWithoutFlush

network/iptables.go

@@ -159,13 +159,13 @@ func ipTablesCleanAndBuild(ipt IPTables, rules []IPTablesRule) (IPTablesRestoreR
 func ipTablesBootstrap(ipt IPTables, iptRestore IPTablesRestore, rules []IPTablesRule) error {
 	tablesRules, err := ipTablesCleanAndBuild(ipt, rules)
 	if err != nil {
-		// if we can't find iptables, give up and return
-		return fmt.Errorf("Failed to setup IPTables-restore payload: %v", err)
+		// if we can't find iptables or if we can check existing rules, give up and return
+		return fmt.Errorf("failed to setup iptables-restore payload: %v", err)
 	}
 
-	log.V(6).Infof("trying to run iptable-restore < %+v", tablesRules)
+	log.V(6).Infof("trying to run iptables-restore < %+v", tablesRules)
 
-	err = iptRestore.ApplyPartial(tablesRules)
+	err = iptRestore.ApplyWithoutFlush(tablesRules)
 	if err != nil {
 		return fmt.Errorf("failed to apply partial iptables-restore %v", err)
 	}
@@ -184,8 +184,8 @@ func SetupAndEnsureIPTables(rules []IPTablesRule, resyncPeriod int) {
 	}
 	iptRestore, err := NewIPTablesRestore()
 	if err != nil {
-		// if we can't find iptables, give up and return
-		log.Errorf("Failed to setup IPTables. iptables binary was not found: %v", err)
+		// if we can't find iptables-restore, give up and return
+		log.Errorf("Failed to setup IPTables. iptables-restore binary was not found: %v", err)
 		return
 	}
 
@@ -219,7 +219,7 @@ func SetupAndEnsureIP6Tables(rules []IPTablesRule, resyncPeriod int) {
 	iptRestore, err := NewIPTablesRestore()
 	if err != nil {
 		// if we can't find iptables, give up and return
-		log.Errorf("Failed to setup IPTables-restore: %v", err)
+		log.Errorf("Failed to setup iptables-restore: %v", err)
 		return
 	}
 
@@ -254,7 +254,7 @@ func DeleteIPTables(rules []IPTablesRule) error {
 	iptRestore, err := NewIPTablesRestore()
 	if err != nil {
 		// if we can't find iptables, give up and return
-		log.Errorf("Failed to setup IPTables-restore: %v", err)
+		log.Errorf("Failed to setup iptables-restore: %v", err)
 		return err
 	}
 	err = teardownIPTables(ipt, iptRestore, rules)
@@ -277,7 +277,7 @@ func DeleteIP6Tables(rules []IPTablesRule) error {
 	iptRestore, err := NewIPTablesRestoreWithProtocol(iptables.ProtocolIPv6)
 	if err != nil {
 		// if we can't find iptables, give up and return
-		log.Errorf("Failed to setup IPTables-restore: %v", err)
+		log.Errorf("Failed to setup iptables-restore: %v", err)
 		return err
 	}
 	err = teardownIPTables(ipt, iptRestore, rules)
@@ -291,7 +291,7 @@ func DeleteIP6Tables(rules []IPTablesRule) error {
 func ensureIPTables(ipt IPTables, iptRestore IPTablesRestore, rules []IPTablesRule) error {
 	exists, err := ipTablesRulesExist(ipt, rules)
 	if err != nil {
-		return fmt.Errorf("Error checking rule existence: %v", err)
+		return fmt.Errorf("error checking rule existence: %v", err)
 	}
 	if exists {
 		// if all the rules already exist, no need to do anything
@@ -303,7 +303,7 @@ func ensureIPTables(ipt IPTables, iptRestore IPTablesRestore, rules []IPTablesRu
 	err = ipTablesBootstrap(ipt, iptRestore, rules)
 	if err != nil {
 		// if we can't find iptables, give up and return
-		return fmt.Errorf("Error setting up rules: %v", err)
+		return fmt.Errorf("error setting up rules: %v", err)
 	}
 	return nil
 }
@@ -325,7 +325,7 @@ func teardownIPTables(ipt IPTables, iptr IPTablesRestore, rules []IPTablesRule)
 			tablesRules[rule.table] = append(tablesRules[rule.table], append(IPTablesRestoreRuleSpec{"-D", rule.chain}, rule.rulespec...))
 		}
 	}
-	err := iptr.ApplyPartial(tablesRules) // ApplyPartial make a diff, Apply make a replace (desired state)
+	err := iptr.ApplyWithoutFlush(tablesRules) // ApplyWithoutFlush make a diff, Apply make a replace (desired state)
 	if err != nil {
 		return fmt.Errorf("unable to teardown iptables: %v", err)
 	}

network/iptables_restore.go

@@ -38,15 +38,15 @@ const (
 type IPTablesRestore interface {
 	// ApplyFully apply all rules and flush chains
 	ApplyFully(rules IPTablesRestoreRules) error
-	// ApplyPartial apply without flush chains
-	ApplyPartial(rules IPTablesRestoreRules) error
+	// ApplyWithoutFlush apply without flush chains
+	ApplyWithoutFlush(rules IPTablesRestoreRules) error
 }
 
 // ipTablesRestore internal type
 type ipTablesRestore struct {
-	path     string
-	protocol iptables.Protocol
-	hasWait  bool
+	path    string
+	proto   iptables.Protocol
+	hasWait bool
 }
 
 // IPTablesRestoreRules represents iptables-restore table block
@@ -60,7 +60,7 @@ func NewIPTablesRestore() (IPTablesRestore, error) {
 	return NewIPTablesRestoreWithProtocol(iptables.ProtocolIPv4)
 }
 
-// NewIPTablesRestoreWithProtocol build new IPTablesRestore for supplied protocol
+// NewIPTablesRestoreWithProtocol build new IPTablesRestore for supplied proto
 func NewIPTablesRestoreWithProtocol(protocol iptables.Protocol) (IPTablesRestore, error) {
 	cmd := getIptablesRestoreCommand(protocol)
 	path, err := exec.LookPath(cmd)
@@ -73,9 +73,9 @@ func NewIPTablesRestoreWithProtocol(protocol iptables.Protocol) (IPTablesRestore
 	}
 
 	ipt := ipTablesRestore{
-		path:     path,
-		protocol: protocol,
-		hasWait:  hasWait,
+		path:    path,
+		proto:   protocol,
+		hasWait: hasWait,
 	}
 	return &ipt, nil
 }
@@ -92,8 +92,8 @@ func (iptr *ipTablesRestore) ApplyFully(rules IPTablesRestoreRules) error {
 	return nil
 }
 
-// ApplyPartial apply without flush chains
-func (iptr *ipTablesRestore) ApplyPartial(rules IPTablesRestoreRules) error {
+// ApplyWithoutFlush apply without flush chains
+func (iptr *ipTablesRestore) ApplyWithoutFlush(rules IPTablesRestoreRules) error {
 	payload := buildIPTablesRestorePayload(rules)
 
 	log.V(6).Infof("trying to run with payload %s", payload)
@@ -158,10 +158,10 @@ func ipTablesHasWaitSupport(v1, v2, v3 int) bool {
 	if v1 > 1 {
 		return true
 	}
-	if v1 == 1 && v2 > 4 {
+	if v1 == 1 && v2 > 6 {
 		return true
 	}
-	if v1 == 1 && v2 == 4 && v3 >= 20 {
+	if v1 == 1 && v2 == 6 && v3 >= 2 {
 		return true
 	}
 	return false
@@ -205,7 +205,7 @@ func getIptablesRestoreVersionString(path string) (string, error) {
 	return out.String(), nil
 }
 
-// getIptablesRestoreCommand returns the correct command for the given protocol, either "iptables-restore" or "ip6tables-restore".
+// getIptablesRestoreCommand returns the correct command for the given proto, either "iptables-restore" or "ip6tables-restore".
 func getIptablesRestoreCommand(proto iptables.Protocol) string {
 	if proto == iptables.ProtocolIPv6 {
 		return ip6TablesRestoreCmd

network/iptables_restore_test.go

@@ -11,6 +11,7 @@
 // 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.
+//go:build !windows
 // +build !windows
 
 package network

network/iptables_test.go

@@ -75,7 +75,7 @@ func (mock *MockIPTablesRestore) ApplyFully(rules IPTablesRestoreRules) error {
 	return nil
 }
 
-func (mock *MockIPTablesRestore) ApplyPartial(rules IPTablesRestoreRules) error {
+func (mock *MockIPTablesRestore) ApplyWithoutFlush(rules IPTablesRestoreRules) error {
 	mock.rules = append(mock.rules, rules)
 	return nil
 }