clean up code

pkg/proxy/ipvs/proxier.go

@@ -970,8 +970,8 @@ func (proxier *Proxier) OnEndpointsSynced() {
 	proxier.syncProxyRules()
 }
 
-// EntryInvalidErr indiates if an ipset entry is invalid or not
-const EntryInvalidErr = "error adding entry %s to ipset %s since entry is invalid"
+// EntryInvalidErr indicates if an ipset entry is invalid or not
+const EntryInvalidErr = "error adding entry %s to ipset %s"
 
 // This is where all of the ipvs calls happen.
 // assumes proxier.mu is held
@@ -1145,13 +1145,7 @@ func (proxier *Proxier) syncProxyRules() {
 		// add service Cluster IP:Port to kubeServiceAccess ip set for the purpose of solving hairpin.
 		// proxier.kubeServiceAccessSet.activeEntries.Insert(entry.String())
 		// Install masquerade rules if 'masqueradeAll' or 'clusterCIDR' is specified.
-		if proxier.masqueradeAll {
-			if valid := proxier.clusterIPSet.validateEntry(entry); !valid {
-				glog.Errorf("%s", fmt.Sprintf(EntryInvalidErr, entry, proxier.clusterIPSet.Name))
-				continue
-			}
-			proxier.clusterIPSet.activeEntries.Insert(entry.String())
-		} else if len(proxier.clusterCIDR) > 0 {
+		if proxier.masqueradeAll || len(proxier.clusterCIDR) > 0 {
 			if valid := proxier.clusterIPSet.validateEntry(entry); !valid {
 				glog.Errorf("%s", fmt.Sprintf(EntryInvalidErr, entry, proxier.clusterIPSet.Name))
 				continue
@@ -1380,23 +1374,23 @@ func (proxier *Proxier) syncProxyRules() {
 					Protocol: protocol,
 					SetType:  utilipset.BitmapPort,
 				}
+				var nodePortSet *IPSet
 				switch protocol {
 				case "tcp":
-					if valid := proxier.nodePortSetTCP.validateEntry(entry); !valid {
-						glog.Errorf("%s", fmt.Sprintf(EntryInvalidErr, entry, proxier.nodePortSetTCP.Name))
-						continue
-					}
-					proxier.nodePortSetTCP.activeEntries.Insert(entry.String())
+					nodePortSet = proxier.nodePortSetTCP
 				case "udp":
-					if valid := proxier.nodePortSetUDP.validateEntry(entry); !valid {
-						glog.Errorf("%s", fmt.Sprintf(EntryInvalidErr, entry, proxier.nodePortSetUDP.Name))
-						continue
-					}
-					proxier.nodePortSetUDP.activeEntries.Insert(entry.String())
+					nodePortSet = proxier.nodePortSetUDP
 				default:
 					// It should never hit
 					glog.Errorf("Unsupported protocol type: %s", protocol)
 				}
+				if nodePortSet != nil {
+					if valid := nodePortSet.validateEntry(entry); !valid {
+						glog.Errorf("%s", fmt.Sprintf(EntryInvalidErr, entry, nodePortSet.Name))
+						continue
+					}
+					nodePortSet.activeEntries.Insert(entry.String())
+				}
 			}
 
 			// Build ipvs kernel routes for each node ip address

pkg/util/ipset/ipset.go

@@ -50,7 +50,6 @@ type Interface interface {
 	ListSets() ([]string, error)
 	// GetVersion returns the "X.Y" version string for ipset.
 	GetVersion() (string, error)
-	// TODO: add comment message for ipset
 }
 
 // IPSetCmd represents the ipset util.  We use ipset command for ipset execute.
@@ -144,6 +143,10 @@ type Entry struct {
 
 // Validate checks if a given ipset entry is valid or not.  The set parameter is the ipset that entry belongs to.
 func (e *Entry) Validate(set *IPSet) bool {
+	if e.Port < 0 {
+		glog.Errorf("Entry %v port number %d should be >=0 for ipset %v", e, e.Port, set)
+		return false
+	}
 	switch e.SetType {
 	case HashIPPort:
 		// set default protocol to tcp if empty
@@ -159,11 +162,6 @@ func (e *Entry) Validate(set *IPSet) bool {
 		if valid := validateProtocol(e.Protocol); !valid {
 			return false
 		}
-
-		if e.Port < 0 {
-			glog.Errorf("Entry %v port number %d should be >=0 for ipset %v", e, e.Port, set)
-			return false
-		}
 	case HashIPPortIP:
 		// set default protocol to tcp if empty
 		if len(e.Protocol) == 0 {
@@ -179,11 +177,6 @@ func (e *Entry) Validate(set *IPSet) bool {
 			return false
 		}
 
-		if e.Port < 0 {
-			glog.Errorf("Entry %v port number %d should be >=0 for ipset %v ", e, e.Port, set)
-			return false
-		}
-
 		// IP2 can not be empty for `hash:ip,port,ip` type ip set
 		if net.ParseIP(e.IP2) == nil {
 			glog.Errorf("Error parsing entry %v second ip address %v for ipset %v", e, e.IP2, set)
@@ -204,22 +197,12 @@ func (e *Entry) Validate(set *IPSet) bool {
 			return false
 		}
 
-		if e.Port < 0 {
-			glog.Errorf("Entry %v port number %d should be >=0 for ipset %v", e, e.Port, set)
-			return false
-		}
-
 		// Net can not be empty for `hash:ip,port,net` type ip set
 		if _, ipNet, _ := net.ParseCIDR(e.Net); ipNet == nil {
 			glog.Errorf("Error parsing entry %v ip net %v for ipset %v", e, e.Net, set)
 			return false
 		}
 	case BitmapPort:
-		if e.Port < 0 {
-			glog.Errorf("Entry %v port number %d should be >=0 for ipset %v", e, e.Port, set)
-			return false
-		}
-
 		// check if port number satisfies its ipset's requirement of port range
 		if set == nil {
 			glog.Errorf("Unable to reference ip set where the entry %v exists", e)
@@ -297,7 +280,7 @@ func (runner *runner) CreateSet(set *IPSet, ignoreExistErr bool) error {
 	// Validate ipset before creating
 	valid := set.Validate()
 	if !valid {
-		return fmt.Errorf("Error creating ipset since it's invalid")
+		return fmt.Errorf("error creating ipset since it's invalid")
 	}
 	return runner.createSet(set, ignoreExistErr)
 }