mirror of
				https://github.com/optim-enterprises-bv/kubernetes.git
				synced 2025-11-04 04:08:16 +00:00 
			
		
		
		
	Fixup #1 addressing review comments
This commit is contained in:
		@@ -294,14 +294,11 @@ func NewProxier(ipt utiliptables.Interface,
 | 
				
			|||||||
		ipFamily = v1.IPv6Protocol
 | 
							ipFamily = v1.IPv6Protocol
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	isCIDR := true
 | 
						ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses)
 | 
				
			||||||
	ipFamilyIPMap := utilproxy.MapIPsToIPFamily(nodePortAddresses, isCIDR)
 | 
						nodePortAddresses = ipFamilyMap[ipFamily]
 | 
				
			||||||
	nodePortAddresses = ipFamilyIPMap[ipFamily]
 | 
						// Log the IPs not matching the ipFamily
 | 
				
			||||||
	for ipFam, ips := range ipFamilyIPMap {
 | 
						if ips, ok := ipFamilyMap[utilproxy.OtherIPFamily(ipFamily)]; ok && len(ips) > 0 {
 | 
				
			||||||
		// Log the IPs not matching the ipFamily
 | 
							klog.Warningf("IP Family: %s, NodePortAddresses of wrong family; %s", ipFamily, strings.Join(ips, ","))
 | 
				
			||||||
		if ipFam != ipFamily && len(ips) > 0 {
 | 
					 | 
				
			||||||
			klog.Warningf("IP Family: %s, NodePortAddresses of wrong family; %s", ipFamily, strings.Join(ips, ","))
 | 
					 | 
				
			||||||
		}
 | 
					 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	proxier := &Proxier{
 | 
						proxier := &Proxier{
 | 
				
			||||||
@@ -371,18 +368,17 @@ func NewDualStackProxier(
 | 
				
			|||||||
	nodePortAddresses []string,
 | 
						nodePortAddresses []string,
 | 
				
			||||||
) (proxy.Provider, error) {
 | 
					) (proxy.Provider, error) {
 | 
				
			||||||
	// Create an ipv4 instance of the single-stack proxier
 | 
						// Create an ipv4 instance of the single-stack proxier
 | 
				
			||||||
	isCIDR := true
 | 
						ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses)
 | 
				
			||||||
	ipFamilyIPMap := utilproxy.MapIPsToIPFamily(nodePortAddresses, isCIDR)
 | 
					 | 
				
			||||||
	ipv4Proxier, err := NewProxier(ipt[0], sysctl,
 | 
						ipv4Proxier, err := NewProxier(ipt[0], sysctl,
 | 
				
			||||||
		exec, syncPeriod, minSyncPeriod, masqueradeAll, masqueradeBit, localDetectors[0], hostname,
 | 
							exec, syncPeriod, minSyncPeriod, masqueradeAll, masqueradeBit, localDetectors[0], hostname,
 | 
				
			||||||
		nodeIP[0], recorder, healthzServer, ipFamilyIPMap[v1.IPv4Protocol])
 | 
							nodeIP[0], recorder, healthzServer, ipFamilyMap[v1.IPv4Protocol])
 | 
				
			||||||
	if err != nil {
 | 
						if err != nil {
 | 
				
			||||||
		return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err)
 | 
							return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	ipv6Proxier, err := NewProxier(ipt[1], sysctl,
 | 
						ipv6Proxier, err := NewProxier(ipt[1], sysctl,
 | 
				
			||||||
		exec, syncPeriod, minSyncPeriod, masqueradeAll, masqueradeBit, localDetectors[1], hostname,
 | 
							exec, syncPeriod, minSyncPeriod, masqueradeAll, masqueradeBit, localDetectors[1], hostname,
 | 
				
			||||||
		nodeIP[1], recorder, healthzServer, ipFamilyIPMap[v1.IPv6Protocol])
 | 
							nodeIP[1], recorder, healthzServer, ipFamilyMap[v1.IPv6Protocol])
 | 
				
			||||||
	if err != nil {
 | 
						if err != nil {
 | 
				
			||||||
		return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err)
 | 
							return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -450,15 +450,13 @@ func NewProxier(ipt utiliptables.Interface,
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
	endpointSlicesEnabled := utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceProxying)
 | 
						endpointSlicesEnabled := utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceProxying)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	isCIDR := true
 | 
						ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses)
 | 
				
			||||||
	ipFamilyIPMap := utilproxy.MapIPsToIPFamily(nodePortAddresses, isCIDR)
 | 
						nodePortAddresses = ipFamilyMap[ipFamily]
 | 
				
			||||||
	nodePortAddresses = ipFamilyIPMap[ipFamily]
 | 
						// Log the IPs not matching the ipFamily
 | 
				
			||||||
	for ipFam, ips := range ipFamilyIPMap {
 | 
						if ips, ok := ipFamilyMap[utilproxy.OtherIPFamily(ipFamily)]; ok && len(ips) > 0 {
 | 
				
			||||||
		// Log the IPs not matching the ipFamily
 | 
							klog.Warningf("IP Family: %s, NodePortAddresses of wrong family; %s", ipFamily, strings.Join(ips, ","))
 | 
				
			||||||
		if ipFam != ipFamily && len(ips) > 0 {
 | 
					 | 
				
			||||||
			klog.Warningf("IP Family: %s, NodePortAddresses of wrong family; %s", ipFamily, strings.Join(ips, ","))
 | 
					 | 
				
			||||||
		}
 | 
					 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	proxier := &Proxier{
 | 
						proxier := &Proxier{
 | 
				
			||||||
		ipFamily:              ipFamily,
 | 
							ipFamily:              ipFamily,
 | 
				
			||||||
		portsMap:              make(map[utilproxy.LocalPort]utilproxy.Closeable),
 | 
							portsMap:              make(map[utilproxy.LocalPort]utilproxy.Closeable),
 | 
				
			||||||
@@ -536,15 +534,14 @@ func NewDualStackProxier(
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
	safeIpset := newSafeIpset(ipset)
 | 
						safeIpset := newSafeIpset(ipset)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	isCIDR := true
 | 
						ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses)
 | 
				
			||||||
	ipFamilyIPMap := utilproxy.MapIPsToIPFamily(nodePortAddresses, isCIDR)
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
	// Create an ipv4 instance of the single-stack proxier
 | 
						// Create an ipv4 instance of the single-stack proxier
 | 
				
			||||||
	ipv4Proxier, err := NewProxier(ipt[0], ipvs, safeIpset, sysctl,
 | 
						ipv4Proxier, err := NewProxier(ipt[0], ipvs, safeIpset, sysctl,
 | 
				
			||||||
		exec, syncPeriod, minSyncPeriod, filterCIDRs(false, excludeCIDRs), strictARP,
 | 
							exec, syncPeriod, minSyncPeriod, filterCIDRs(false, excludeCIDRs), strictARP,
 | 
				
			||||||
		tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit,
 | 
							tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit,
 | 
				
			||||||
		localDetectors[0], hostname, nodeIP[0],
 | 
							localDetectors[0], hostname, nodeIP[0],
 | 
				
			||||||
		recorder, healthzServer, scheduler, ipFamilyIPMap[v1.IPv4Protocol], kernelHandler)
 | 
							recorder, healthzServer, scheduler, ipFamilyMap[v1.IPv4Protocol], kernelHandler)
 | 
				
			||||||
	if err != nil {
 | 
						if err != nil {
 | 
				
			||||||
		return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err)
 | 
							return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
@@ -553,7 +550,7 @@ func NewDualStackProxier(
 | 
				
			|||||||
		exec, syncPeriod, minSyncPeriod, filterCIDRs(true, excludeCIDRs), strictARP,
 | 
							exec, syncPeriod, minSyncPeriod, filterCIDRs(true, excludeCIDRs), strictARP,
 | 
				
			||||||
		tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit,
 | 
							tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit,
 | 
				
			||||||
		localDetectors[1], hostname, nodeIP[1],
 | 
							localDetectors[1], hostname, nodeIP[1],
 | 
				
			||||||
		nil, nil, scheduler, ipFamilyIPMap[v1.IPv6Protocol], kernelHandler)
 | 
							nil, nil, scheduler, ipFamilyMap[v1.IPv6Protocol], kernelHandler)
 | 
				
			||||||
	if err != nil {
 | 
						if err != nil {
 | 
				
			||||||
		return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err)
 | 
							return nil, fmt.Errorf("unable to create ipv6 proxier: %v", err)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -156,25 +156,19 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
 | 
				
			|||||||
	// services, this is actually expected. Hence we downgraded from reporting by events
 | 
						// services, this is actually expected. Hence we downgraded from reporting by events
 | 
				
			||||||
	// to just log lines with high verbosity
 | 
						// to just log lines with high verbosity
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	isCIDR := false
 | 
						ipFamilyMap := utilproxy.MapIPsByIPFamily(service.Spec.ExternalIPs)
 | 
				
			||||||
	ipFamilyIPMap := utilproxy.MapIPsToIPFamily(service.Spec.ExternalIPs, isCIDR)
 | 
						info.externalIPs = ipFamilyMap[sct.ipFamily]
 | 
				
			||||||
	info.externalIPs = ipFamilyIPMap[sct.ipFamily]
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
	for ipFam, ips := range ipFamilyIPMap {
 | 
						// Log the IPs not matching the ipFamily
 | 
				
			||||||
		// Log the IPs not matching the ipFamily
 | 
						if ips, ok := ipFamilyMap[utilproxy.OtherIPFamily(sct.ipFamily)]; ok && len(ips) > 0 {
 | 
				
			||||||
		if ipFam != sct.ipFamily && len(ips) > 0 {
 | 
							klog.V(4).Infof("service change tracker(%v) ignored the following external IPs(%s) for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(ips, ","), service.Namespace, service.Name)
 | 
				
			||||||
			klog.V(4).Infof("service change tracker(%v) ignored the following external IPs(%s) for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(ips, ","), service.Namespace, service.Name)
 | 
					 | 
				
			||||||
		}
 | 
					 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	isCIDR = true
 | 
						ipFamilyMap = utilproxy.MapCIDRsByIPFamily(loadBalancerSourceRanges)
 | 
				
			||||||
	ipFamilyCIDRMap := utilproxy.MapIPsToIPFamily(loadBalancerSourceRanges, isCIDR)
 | 
						info.loadBalancerSourceRanges = ipFamilyMap[sct.ipFamily]
 | 
				
			||||||
	info.loadBalancerSourceRanges = ipFamilyCIDRMap[sct.ipFamily]
 | 
						// Log the CIDRs not matching the ipFamily
 | 
				
			||||||
	for ipFam, cidrs := range ipFamilyCIDRMap {
 | 
						if cidrs, ok := ipFamilyMap[utilproxy.OtherIPFamily(sct.ipFamily)]; ok && len(cidrs) > 0 {
 | 
				
			||||||
		// Log the CIDRs not matching the ipFamily
 | 
							klog.V(4).Infof("service change tracker(%v) ignored the following load balancer source ranges(%s) for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(cidrs, ","), service.Namespace, service.Name)
 | 
				
			||||||
		if ipFam != sct.ipFamily && len(cidrs) > 0 {
 | 
					 | 
				
			||||||
			klog.V(4).Infof("service change tracker(%v) ignored the following load balancer source ranges(%s) for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(cidrs, ","), service.Namespace, service.Name)
 | 
					 | 
				
			||||||
		}
 | 
					 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	// Obtain Load Balancer Ingress IPs
 | 
						// Obtain Load Balancer Ingress IPs
 | 
				
			||||||
@@ -184,17 +178,14 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
 | 
				
			|||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if len(ips) > 0 {
 | 
						if len(ips) > 0 {
 | 
				
			||||||
		isCIDR = false
 | 
							ipFamilyMap = utilproxy.MapIPsByIPFamily(ips)
 | 
				
			||||||
		ipFamilyIPMap = utilproxy.MapIPsToIPFamily(ips, isCIDR)
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
		for ipFam, ipList := range ipFamilyIPMap {
 | 
							if ipList, ok := ipFamilyMap[utilproxy.OtherIPFamily(sct.ipFamily)]; ok && len(ipList) > 0 {
 | 
				
			||||||
			if ipFam != sct.ipFamily && len(ipList) > 0 {
 | 
								klog.V(4).Infof("service change tracker(%v) ignored the following load balancer(%s) ingress ips for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(ipList, ","), service.Namespace, service.Name)
 | 
				
			||||||
				klog.V(4).Infof("service change tracker(%v) ignored the following load balancer(%s) ingress ips for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(ipList, ","), service.Namespace, service.Name)
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
			}
 | 
					 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
		// Create the LoadBalancerStatus with the filtered IPs
 | 
							// Create the LoadBalancerStatus with the filtered IPs
 | 
				
			||||||
		for _, ip := range ipFamilyIPMap[sct.ipFamily] {
 | 
							for _, ip := range ipFamilyMap[sct.ipFamily] {
 | 
				
			||||||
			info.loadBalancerStatus.Ingress = append(info.loadBalancerStatus.Ingress, v1.LoadBalancerIngress{IP: ip})
 | 
								info.loadBalancerStatus.Ingress = append(info.loadBalancerStatus.Ingress, v1.LoadBalancerIngress{IP: ip})
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -255,23 +255,50 @@ func LogAndEmitIncorrectIPVersionEvent(recorder record.EventRecorder, fieldName,
 | 
				
			|||||||
	}
 | 
						}
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
// MapIPsToIPFamily maps a slice of IPs to their respective IP families (v4 or v6)
 | 
					// MapIPsByIPFamily maps a slice of IPs to their respective IP families (v4 or v6)
 | 
				
			||||||
func MapIPsToIPFamily(ipStrings []string, isCIDR bool) map[v1.IPFamily][]string {
 | 
					func MapIPsByIPFamily(ipStrings []string) map[v1.IPFamily][]string {
 | 
				
			||||||
	ipFamilyMap := map[v1.IPFamily][]string{}
 | 
						ipFamilyMap := map[v1.IPFamily][]string{}
 | 
				
			||||||
	for _, ip := range ipStrings {
 | 
						for _, ip := range ipStrings {
 | 
				
			||||||
		ipFamily := getIPFamilyFromIP(ip, isCIDR)
 | 
							// Handle only the valid IPs
 | 
				
			||||||
		ipFamilyMap[ipFamily] = append(ipFamilyMap[ipFamily], ip)
 | 
							if net.ParseIP(ip) != nil {
 | 
				
			||||||
 | 
								ipFamily := getIPFamilyFromIP(ip)
 | 
				
			||||||
 | 
								ipFamilyMap[ipFamily] = append(ipFamilyMap[ipFamily], ip)
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	return ipFamilyMap
 | 
						return ipFamilyMap
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func getIPFamilyFromIP(ip string, isCIDR bool) v1.IPFamily {
 | 
					// MapCIDRsByIPFamily maps a slice of IPs to their respective IP families (v4 or v6)
 | 
				
			||||||
	conditionFunc := utilnet.IsIPv6String
 | 
					func MapCIDRsByIPFamily(ipStrings []string) map[v1.IPFamily][]string {
 | 
				
			||||||
	if isCIDR {
 | 
						ipFamilyMap := map[v1.IPFamily][]string{}
 | 
				
			||||||
		conditionFunc = utilnet.IsIPv6CIDRString
 | 
						for _, cidr := range ipStrings {
 | 
				
			||||||
 | 
							// Handle only the valid CIDRs
 | 
				
			||||||
 | 
							if _, _, err := net.ParseCIDR(cidr); err == nil {
 | 
				
			||||||
 | 
								ipFamily := getIPFamilyFromCIDR(cidr)
 | 
				
			||||||
 | 
								ipFamilyMap[ipFamily] = append(ipFamilyMap[ipFamily], cidr)
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
						return ipFamilyMap
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					func getIPFamilyFromIP(ip string) v1.IPFamily {
 | 
				
			||||||
 | 
						if utilnet.IsIPv6String(ip) {
 | 
				
			||||||
 | 
							return v1.IPv6Protocol
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
						return v1.IPv4Protocol
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					// OtherIPFamily returns the other ip family
 | 
				
			||||||
 | 
					func OtherIPFamily(ipFamily v1.IPFamily) v1.IPFamily {
 | 
				
			||||||
 | 
						if ipFamily == v1.IPv6Protocol {
 | 
				
			||||||
 | 
							return v1.IPv4Protocol
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if conditionFunc(ip) {
 | 
						return v1.IPv6Protocol
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					func getIPFamilyFromCIDR(ip string) v1.IPFamily {
 | 
				
			||||||
 | 
						if utilnet.IsIPv6CIDRString(ip) {
 | 
				
			||||||
		return v1.IPv6Protocol
 | 
							return v1.IPv6Protocol
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	return v1.IPv4Protocol
 | 
						return v1.IPv4Protocol
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -567,7 +567,7 @@ func TestShuffleStrings(t *testing.T) {
 | 
				
			|||||||
	}
 | 
						}
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func TestMapIPsToIPFamily(t *testing.T) {
 | 
					func TestMapIPsByIPFamily(t *testing.T) {
 | 
				
			||||||
	testCases := []struct {
 | 
						testCases := []struct {
 | 
				
			||||||
		desc            string
 | 
							desc            string
 | 
				
			||||||
		ipString        []string
 | 
							ipString        []string
 | 
				
			||||||
@@ -657,8 +657,7 @@ func TestMapIPsToIPFamily(t *testing.T) {
 | 
				
			|||||||
				otherIPFamily = v1.IPv4Protocol
 | 
									otherIPFamily = v1.IPv4Protocol
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			isCIDR := false
 | 
								ipMap := MapIPsByIPFamily(testcase.ipString)
 | 
				
			||||||
			ipMap := MapIPsToIPFamily(testcase.ipString, isCIDR)
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
			if !reflect.DeepEqual(testcase.expectCorrect, ipMap[ipFamily]) {
 | 
								if !reflect.DeepEqual(testcase.expectCorrect, ipMap[ipFamily]) {
 | 
				
			||||||
				t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectCorrect, ipMap[ipFamily])
 | 
									t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectCorrect, ipMap[ipFamily])
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user