diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 99a76eb63e..791872384b 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/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 diff --git a/pkg/util/ipset/ipset.go b/pkg/util/ipset/ipset.go index a3e4b937fa..c5ecdc45d3 100644 --- a/pkg/util/ipset/ipset.go +++ b/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) }