Skip to content

Commit

Permalink
Merge pull request #3138 from arkadeepsen/leading-zeros
Browse files Browse the repository at this point in the history
Support proper parsing of IPs with leading zeros
  • Loading branch information
trozet committed Oct 14, 2022
2 parents 213c3cc + dbe0a91 commit fed2993
Show file tree
Hide file tree
Showing 19 changed files with 91 additions and 87 deletions.
20 changes: 12 additions & 8 deletions go-controller/cmd/ovnkube-trace/ovnkube-trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/tools/remotecommand"
"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"
)

const (
Expand Down Expand Up @@ -344,12 +345,13 @@ func getSvcInfo(coreclient *corev1client.CoreV1Client, restconfig *rest.Config,
if clusterIP == "" || clusterIP == "None" {
return nil, fmt.Errorf("ClusterIP for service %s in namespace %s not available", svcName, namespace)
}
klog.V(5).Infof("==> Got service %s ClusterIP is %s\n", svcName, clusterIP)
clusterIPStr := utilnet.ParseIPSloppy(clusterIP).String()
klog.V(5).Infof("==> Got service %s ClusterIP is %s\n", svcName, clusterIPStr)

svcInfo = &SvcInfo{
SvcName: svcName,
SvcNamespace: namespace,
ClusterIP: svc.Spec.ClusterIP,
ClusterIP: clusterIPStr,
}

ep, err := coreclient.Endpoints(namespace).Get(context.TODO(), svcName, metav1.GetOptions{})
Expand Down Expand Up @@ -404,7 +406,7 @@ func extractSubsetInfo(subsets []kapi.EndpointSubset, svcInfo *SvcInfo) error {
// At this point, we should have found valid pod information + a port, so set them and return nil.
svcInfo.PodName = epAddress.TargetRef.Name
svcInfo.PodNamespace = epAddress.TargetRef.Namespace
svcInfo.PodIP = epAddress.IP
svcInfo.PodIP = utilnet.ParseIPSloppy(epAddress.IP).String()
svcInfo.PodPort = podPort
klog.V(5).Infof("==> Got address and port information for service endpoint. podName: %s, podNamespace: %s, podIP: %s, podPort: %s",
svcInfo.PodName, svcInfo.PodNamespace, svcInfo.PodIP, svcInfo.PodPort)
Expand All @@ -425,7 +427,7 @@ func getPodInfo(coreclient *corev1client.CoreV1Client, restconfig *rest.Config,
return nil, err
}
podInfo = &PodInfo{
IP: pod.Status.PodIP,
IP: utilnet.ParseIPSloppy(pod.Status.PodIP).String(),
PodName: pod.Name,
ContainerName: pod.Spec.Containers[0].Name,
HostNetwork: pod.Spec.HostNetwork,
Expand Down Expand Up @@ -904,19 +906,21 @@ func displayNodeInfo(coreclient *corev1client.CoreV1Client) {
if foundMaster || foundControlPlane {
klog.V(5).Infof(" Name: %s is a master", node.Name)
for _, s := range node.Status.Addresses {
klog.V(5).Infof(" Address Type: %s - Address: %s", s.Type, s.Address)
addrStr := utilnet.ParseIPSloppy(s.Address).String()
klog.V(5).Infof(" Address Type: %s - Address: %s", s.Type, addrStr)
//if s.Type == corev1client.NodeInternalIP {
if s.Type == "InternalIP" {
masters[node.Name] = s.Address
masters[node.Name] = addrStr
}
}
} else {
klog.V(5).Infof(" Name: %s is a worker", node.Name)
for _, s := range node.Status.Addresses {
klog.V(5).Infof(" Address Type: %s - Address: %s", s.Type, s.Address)
addrStr := utilnet.ParseIPSloppy(s.Address).String()
klog.V(5).Infof(" Address Type: %s - Address: %s", s.Type, addrStr)
//if s.Type == corev1client.NodeInternalIP {
if s.Type == "InternalIP" {
workers[node.Name] = s.Address
workers[node.Name] = addrStr
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion go-controller/hybrid-overlay/pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
kapi "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
utilnet "k8s.io/utils/net"
)

// ParseHybridOverlayHostSubnet returns the parsed hybrid overlay hostsubnet if
Expand Down Expand Up @@ -53,7 +54,7 @@ func SameIPNet(a, b *net.IPNet) bool {
func GetNodeInternalIP(node *kapi.Node) (string, error) {
for _, addr := range node.Status.Addresses {
if addr.Type == kapi.NodeInternalIP {
return addr.Address, nil
return utilnet.ParseIPSloppy(addr.Address).String(), nil
}
}
return "", fmt.Errorf("failed to read node %q InternalIP", node.Name)
Expand Down
14 changes: 5 additions & 9 deletions go-controller/pkg/node/gateway_iptables.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,13 +457,8 @@ func getGatewayIPTRules(service *kapi.Service, svcHasLocalHostNetEndPnt bool) []
rules = append(rules, getNodePortIPTRules(svcPort, clusterIP, svcPort.Port, svcHasLocalHostNetEndPnt, false)...)
}
}
externalIPs := make([]string, 0, len(service.Spec.ExternalIPs)+len(service.Status.LoadBalancer.Ingress))
externalIPs = append(externalIPs, service.Spec.ExternalIPs...)
for _, ingress := range service.Status.LoadBalancer.Ingress {
if len(ingress.IP) > 0 {
externalIPs = append(externalIPs, ingress.IP)
}
}

externalIPs := util.GetExternalAndLBIPs(service)

for _, externalIP := range externalIPs {
err := util.ValidatePort(svcPort.Protocol, svcPort.Port)
Expand Down Expand Up @@ -498,7 +493,8 @@ func egressSVCIPTRulesForEndpoints(svc *kapi.Service, v4Eps, v6Eps []string) []i

comment, _ := cache.MetaNamespaceKeyFunc(svc)
for _, lb := range svc.Status.LoadBalancer.Ingress {
lbProto := getIPTablesProtocol(lb.IP)
lbIPStr := utilnet.ParseIPSloppy(lb.IP).String()
lbProto := getIPTablesProtocol(lbIPStr)
epsForProto := v4Eps
if lbProto == iptables.ProtocolIPv6 {
epsForProto = v6Eps
Expand All @@ -512,7 +508,7 @@ func egressSVCIPTRulesForEndpoints(svc *kapi.Service, v4Eps, v6Eps []string) []i
"-s", ep,
"-m", "comment", "--comment", comment,
"-j", "SNAT",
"--to-source", lb.IP,
"--to-source", lbIPStr,
},
protocol: lbProto,
})
Expand Down
13 changes: 8 additions & 5 deletions go-controller/pkg/node/gateway_shared_intf.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,15 +201,15 @@ func (npw *nodePortWatcher) updateServiceFlowCache(service *kapi.Service, add, h
// NodePort/Ingress access in the OVS bridge will only ever come from outside of the host
for _, ing := range service.Status.LoadBalancer.Ingress {
if len(ing.IP) > 0 {
err = npw.createLbAndExternalSvcFlows(service, &svcPort, add, hasLocalHostNetworkEp, protocol, actions, ing.IP, "Ingress")
err = npw.createLbAndExternalSvcFlows(service, &svcPort, add, hasLocalHostNetworkEp, protocol, actions, utilnet.ParseIPSloppy(ing.IP).String(), "Ingress")
if err != nil {
klog.Errorf(err.Error())
}
}
}
// flows for externalIPs
for _, externalIP := range service.Spec.ExternalIPs {
err = npw.createLbAndExternalSvcFlows(service, &svcPort, add, hasLocalHostNetworkEp, protocol, actions, externalIP, "External")
err = npw.createLbAndExternalSvcFlows(service, &svcPort, add, hasLocalHostNetworkEp, protocol, actions, utilnet.ParseIPSloppy(externalIP).String(), "External")
if err != nil {
klog.Errorf(err.Error())
}
Expand Down Expand Up @@ -659,8 +659,9 @@ func (npw *nodePortWatcher) SyncServices(services []interface{}) error {

for _, ep := range epSlice.Endpoints {
for _, ip := range ep.Addresses {
if !isHostEndpoint(ip) {
epsToInsert.Insert(ip)
ipStr := utilnet.ParseIPSloppy(ip).String()
if !isHostEndpoint(ipStr) {
epsToInsert.Insert(ipStr)
}
}
}
Expand Down Expand Up @@ -754,7 +755,9 @@ func (npw *nodePortWatcher) DeleteEndpointSlice(epSlice *discovery.EndpointSlice
func getEndpointAddresses(endpointSlice *discovery.EndpointSlice) []string {
endpointsAddress := make([]string, 0)
for _, endpoint := range endpointSlice.Endpoints {
endpointsAddress = append(endpointsAddress, endpoint.Addresses...)
for _, ip := range endpoint.Addresses {
endpointsAddress = append(endpointsAddress, utilnet.ParseIPSloppy(ip).String())
}
}
return endpointsAddress
}
Expand Down
6 changes: 4 additions & 2 deletions go-controller/pkg/node/gateway_shared_intf_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
ktypes "k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"
)

// deletes the local bridge used for DGP and removes the corresponding iface, as well as OVS bridge mappings
Expand Down Expand Up @@ -109,8 +110,9 @@ func updateEgressSVCIptRules(svc *kapi.Service, npw *nodePortWatcher) {

for _, ep := range epSlice.Endpoints {
for _, ip := range ep.Addresses {
if !isHostEndpoint(ip) {
epsToInsert.Insert(ip)
ipStr := utilnet.ParseIPSloppy(ip).String()
if !isHostEndpoint(ipStr) {
epsToInsert.Insert(ipStr)
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion go-controller/pkg/node/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
ktypes "k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"
)

// initLoadBalancerHealthChecker initializes the health check server for
Expand Down Expand Up @@ -152,7 +153,7 @@ func hasLocalHostNetworkEndpoints(epSlices []*discovery.EndpointSlice, nodeAddre
for _, endpoint := range epSlice.Endpoints {
for _, ip := range endpoint.Addresses {
for _, nodeIP := range nodeAddresses {
if nodeIP.String() == ip {
if nodeIP.String() == utilnet.ParseIPSloppy(ip).String() {
return true
}
}
Expand Down
14 changes: 8 additions & 6 deletions go-controller/pkg/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,13 +679,14 @@ func (n *OvnNode) WatchEndpointSlices() error {
for _, port := range oldEndpointSlice.Ports {
for _, endpoint := range oldEndpointSlice.Endpoints {
for _, ip := range endpoint.Addresses {
if doesEPSliceContainEndpoint(newEndpointSlice, ip, *port.Port, *port.Protocol) {
ipStr := utilnet.ParseIPSloppy(ip).String()
if doesEPSliceContainEndpoint(newEndpointSlice, ipStr, *port.Port, *port.Protocol) {
continue
}
if *port.Protocol == kapi.ProtocolUDP { // flush conntrack only for UDP
err := util.DeleteConntrack(ip, *port.Port, *port.Protocol, netlink.ConntrackReplyAnyIP, nil)
err := util.DeleteConntrack(ipStr, *port.Port, *port.Protocol, netlink.ConntrackReplyAnyIP, nil)
if err != nil {
klog.Errorf("Failed to delete conntrack entry for %s: %v", ip, err)
klog.Errorf("Failed to delete conntrack entry for %s: %v", ipStr, err)
}
}
}
Expand All @@ -697,10 +698,11 @@ func (n *OvnNode) WatchEndpointSlices() error {
for _, port := range endpointSlice.Ports {
for _, endpoint := range endpointSlice.Endpoints {
for _, ip := range endpoint.Addresses {
ipStr := utilnet.ParseIPSloppy(ip).String()
if *port.Protocol == kapi.ProtocolUDP { // flush conntrack only for UDP
err := util.DeleteConntrack(ip, *port.Port, *port.Protocol, netlink.ConntrackReplyAnyIP, nil)
err := util.DeleteConntrack(ipStr, *port.Port, *port.Protocol, netlink.ConntrackReplyAnyIP, nil)
if err != nil {
klog.Errorf("Failed to delete conntrack entry for %s: %v", ip, err)
klog.Errorf("Failed to delete conntrack entry for %s: %v", ipStr, err)
}
}
}
Expand Down Expand Up @@ -855,7 +857,7 @@ func doesEPSliceContainEndpoint(epSlice *discovery.EndpointSlice,
for _, port := range epSlice.Ports {
for _, endpoint := range epSlice.Endpoints {
for _, ip := range endpoint.Addresses {
if ip == epIP && *port.Port == epPort && *port.Protocol == protocol {
if utilnet.ParseIPSloppy(ip).String() == epIP && *port.Port == epPort && *port.Protocol == protocol {
return true
}
}
Expand Down
2 changes: 1 addition & 1 deletion go-controller/pkg/node/port_claim.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func handleService(svc *kapi.Service, handler handler) []error {
}
for _, externalIP := range svc.Spec.ExternalIPs {
klog.V(5).Infof("Handle ExternalIPs service %s external IP %s port %d", svc.Name, externalIP, svcPort.Port)
if err := handlePort(getDescription(svcPort.Name, svc, false), svc, externalIP, svcPort.Port, svcPort.Protocol, handler); err != nil {
if err := handlePort(getDescription(svcPort.Name, svc, false), svc, utilnet.ParseIPSloppy(externalIP).String(), svcPort.Port, svcPort.Protocol, handler); err != nil {
errors = append(errors, err)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"
)

func (c *Controller) onServiceAdd(obj interface{}) {
Expand Down Expand Up @@ -408,8 +409,9 @@ func (c *Controller) allEndpointsFor(svc *corev1.Service) (sets.String, sets.Str

for _, ep := range eps.Endpoints {
for _, ip := range ep.Addresses {
if !services.IsHostEndpoint(ip) {
epsToInsert.Insert(ip)
ipStr := utilnet.ParseIPSloppy(ip).String()
if !services.IsHostEndpoint(ipStr) {
epsToInsert.Insert(ipStr)
}
}
if ep.NodeName != nil {
Expand Down
18 changes: 3 additions & 15 deletions go-controller/pkg/ovn/controller/services/load_balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,9 @@ func buildServiceLBConfigs(service *v1.Service, endpointSlices []*discovery.Endp
perNodeConfigs = append(perNodeConfigs, nodePortLBConfig)
}

// Build up list of vips
vips := append([]string{}, service.Spec.ClusterIPs...)
// Handle old clusters w/o v6 support
if len(vips) == 0 {
vips = []string{service.Spec.ClusterIP}
}
externalVips := []string{}
// ExternalIP
externalVips = append(externalVips, service.Spec.ExternalIPs...)
// LoadBalancer status
for _, ingress := range service.Status.LoadBalancer.Ingress {
if ingress.IP != "" {
externalVips = append(externalVips, ingress.IP)
}
}
// Build up list of vips and externalVips
vips := util.GetClusterIPs(service)
externalVips := util.GetExternalAndLBIPs(service)

// if ETP=Local, then treat ExternalIPs and LoadBalancer IPs specially
// otherwise, they're just cluster IPs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ func Test_buildServiceLBConfigs(t *testing.T) {
service: &v1.Service{
ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: ns},
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeClusterIP,
Type: v1.ServiceTypeLoadBalancer,
ClusterIP: "192.168.1.1",
ClusterIPs: []string{"192.168.1.1", "2002::1"},
Ports: []v1.ServicePort{{
Expand Down
10 changes: 2 additions & 8 deletions go-controller/pkg/ovn/controller/services/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,8 @@ func deleteServiceFromLegacyLBs(nbClient libovsdbclient.Client, service *v1.Serv
vipPortsPerProtocol := map[v1.Protocol]sets.String{}

// Generate list of vip:port by proto
ips := append([]string{}, service.Spec.ClusterIPs...)
if len(ips) == 0 {
ips = append(ips, service.Spec.ClusterIP)
}
ips = append(ips, service.Spec.ExternalIPs...)
for _, ingress := range service.Status.LoadBalancer.Ingress {
ips = append(ips, ingress.IP)
}
ips := util.GetClusterIPs(service)
ips = append(ips, util.GetExternalAndLBIPs(service)...)
for _, svcPort := range service.Spec.Ports {
proto := svcPort.Protocol
ipPorts := make([]string, 0, len(ips))
Expand Down
19 changes: 11 additions & 8 deletions go-controller/pkg/ovn/egressgw.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ func (oc *Controller) addGWRoutesForNamespace(namespace string, egress gatewayIn
}
podIPs := make([]*net.IPNet, 0)
for _, podIP := range pod.Status.PodIPs {
cidr := podIP.IP + GetIPFullMask(podIP.IP)
podIPStr := utilnet.ParseIPSloppy(podIP.IP).String()
cidr := podIPStr + GetIPFullMask(podIPStr)
_, ipNet, err := net.ParseCIDR(cidr)
if err != nil {
return fmt.Errorf("failed to parse CIDR: %s, error: %v", cidr, err)
Expand Down Expand Up @@ -1006,7 +1007,7 @@ func getExGwPodIPs(gatewayPod *kapi.Pod) (sets.String, error) {
}
} else if gatewayPod.Spec.HostNetwork {
for _, podIP := range gatewayPod.Status.PodIPs {
ip := net.ParseIP(podIP.IP)
ip := utilnet.ParseIPSloppy(podIP.IP)
if ip != nil {
foundGws.Insert(ip.String())
}
Expand Down Expand Up @@ -1049,10 +1050,11 @@ func (oc *Controller) buildClusterECMPCacheFromNamespaces(clusterRouteCache map[
continue
}
for _, podIP := range nsPod.Status.PodIPs {
if utilnet.IsIPv6String(gwIP) != utilnet.IsIPv6String(podIP.IP) {
podIPStr := utilnet.ParseIPSloppy(podIP.IP).String()
if utilnet.IsIPv6String(gwIP) != utilnet.IsIPv6String(podIPStr) {
continue
}
if val, ok := clusterRouteCache[podIP.IP]; ok {
if val, ok := clusterRouteCache[podIPStr]; ok {
// add gwIP to cache only if buildClusterECMPCacheFromPods hasn't already added it
gwIPexists := false
for _, existingGwIP := range val {
Expand All @@ -1062,10 +1064,10 @@ func (oc *Controller) buildClusterECMPCacheFromNamespaces(clusterRouteCache map[
}
}
if !gwIPexists {
clusterRouteCache[podIP.IP] = append(clusterRouteCache[podIP.IP], gwIP)
clusterRouteCache[podIPStr] = append(clusterRouteCache[podIPStr], gwIP)
}
} else {
clusterRouteCache[podIP.IP] = []string{gwIP}
clusterRouteCache[podIPStr] = []string{gwIP}
}
}
}
Expand Down Expand Up @@ -1106,10 +1108,11 @@ func (oc *Controller) buildClusterECMPCacheFromPods(clusterRouteCache map[string
continue
}
for _, podIP := range nsPod.Status.PodIPs {
if utilnet.IsIPv6String(gwIP) != utilnet.IsIPv6String(podIP.IP) {
podIPStr := utilnet.ParseIPSloppy(podIP.IP).String()
if utilnet.IsIPv6String(gwIP) != utilnet.IsIPv6String(podIPStr) {
continue
}
clusterRouteCache[podIP.IP] = append(clusterRouteCache[podIP.IP], gwIP)
clusterRouteCache[podIPStr] = append(clusterRouteCache[podIPStr], gwIP)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion go-controller/pkg/ovn/egressip.go
Original file line number Diff line number Diff line change
Expand Up @@ -2371,7 +2371,7 @@ func getNodeInternalAddrs(node *v1.Node) (net.IP, net.IP) {
var v4Addr, v6Addr net.IP
for _, nodeAddr := range node.Status.Addresses {
if nodeAddr.Type == v1.NodeInternalIP {
ip := net.ParseIP(nodeAddr.Address)
ip := utilnet.ParseIPSloppy(nodeAddr.Address)
if !utilnet.IsIPv6(ip) && v4Addr == nil {
v4Addr = ip
} else if utilnet.IsIPv6(ip) && v6Addr == nil {
Expand Down

0 comments on commit fed2993

Please sign in to comment.