Skip to content

Commit

Permalink
lb_config: Reduce number of required LB backend templates for NodePort.
Browse files Browse the repository at this point in the history 
In general, template OVN load balancers used for implementing NodePort
services might have completely different sets of backends on different
nodes.

In particular however, there are quite a few cases in which node port
services end up using the same backends on all nodes, e.g., for services
that don't have ETP/ITP=local and that select only OVN networked pods as
backends.

Detect such situations and avoid using templated backends.  Use instead a
shared list of backend addresses.  This reduces NB DB size of the
Chassis_Template_Var table, from O(N * S) to O(N), where N is the
number of nodes and S is the number of node port services that match the
criteria defined above.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
  • Loading branch information
@dceara
dceara committed Apr 5, 2023
1 parent 3a05d50 commit 35e0f44
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 60 deletions.
133 changes: 102 additions & 31 deletions go-controller/pkg/ovn/controller/services/lb_config.go
View file
Expand Up @@ -39,8 +39,9 @@ type lbConfig struct {
hasNodePort bool
}

func (c *lbConfig) makeNodeSwitchTargetIPs(node *nodeInfo, epIPs []string) []string {
targetIPs := epIPs
func (c *lbConfig) makeNodeSwitchTargetIPs(node *nodeInfo, epIPs []string) (targetIPs []string, changed bool) {
targetIPs = epIPs
changed = false

if c.externalTrafficLocal {
// for ExternalTrafficPolicy=Local, remove non-local endpoints from the router/switch targets
Expand All @@ -53,11 +54,17 @@ func (c *lbConfig) makeNodeSwitchTargetIPs(node *nodeInfo, epIPs []string) []str
targetIPs = util.FilterIPsSlice(targetIPs, node.nodeSubnets(), true)
}

return targetIPs
// We potentially only removed stuff from the original slice, so just
// comparing lenghts is enough.
if len(targetIPs) != len(epIPs) {
changed = true
}
return
}

func (c *lbConfig) makeNodeRouterTargetIPs(node *nodeInfo, epIPs []string, hostMasqueradeIP string) []string {
targetIPs := epIPs
func (c *lbConfig) makeNodeRouterTargetIPs(node *nodeInfo, epIPs []string, hostMasqueradeIP string) (targetIPs []string, changed bool) {
targetIPs = epIPs
changed = false

if c.externalTrafficLocal {
// for ExternalTrafficPolicy=Local, remove non-local endpoints from the router/switch targets
Expand All @@ -67,9 +74,13 @@ func (c *lbConfig) makeNodeRouterTargetIPs(node *nodeInfo, epIPs []string, hostM

// any targets local to the node need to have a special
// harpin IP added, but only for the router LB
targetIPs = util.UpdateIPsSlice(targetIPs, node.nodeIPsStr(), []string{hostMasqueradeIP})
targetIPs, updated := util.UpdateIPsSlice(targetIPs, node.nodeIPsStr(), []string{hostMasqueradeIP})

return targetIPs
// We either only removed stuff from the original slice, or updated some IPs.
if len(targetIPs) != len(epIPs) || updated {
changed = true
}
return
}

// just used for consistent ordering
Expand Down Expand Up @@ -347,12 +358,33 @@ func buildTemplateLBs(service *v1.Service, configs []lbConfig, nodes []nodeInfo,

for range config.vips {
klog.V(5).Infof(" buildTemplateLBs() service %s/%s adding rules", service.Namespace, service.Name)

// If all targets have exactly the same IPs on all nodes there's
// no need to use a template, just use the same list of explicit
// targets on all nodes.
switchV4TargetNeedsTemplate := false
switchV6TargetNeedsTemplate := false
routerV4TargetNeedsTemplate := false
routerV6TargetNeedsTemplate := false

for _, node := range nodes {
switchV4targetips := config.makeNodeSwitchTargetIPs(&node, config.eps.V4IPs)
switchV6targetips := config.makeNodeSwitchTargetIPs(&node, config.eps.V6IPs)
switchV4targetips, changed := config.makeNodeSwitchTargetIPs(&node, config.eps.V4IPs)
if !switchV4TargetNeedsTemplate && changed {
switchV4TargetNeedsTemplate = true
}
switchV6targetips, changed := config.makeNodeSwitchTargetIPs(&node, config.eps.V6IPs)
if !switchV6TargetNeedsTemplate && changed {
switchV6TargetNeedsTemplate = true
}

routerV4targetips := config.makeNodeRouterTargetIPs(&node, config.eps.V4IPs, types.V4HostMasqueradeIP)
routerV6targetips := config.makeNodeRouterTargetIPs(&node, config.eps.V6IPs, types.V6HostMasqueradeIP)
routerV4targetips, changed := config.makeNodeRouterTargetIPs(&node, config.eps.V4IPs, types.V4HostMasqueradeIP)
if !routerV4TargetNeedsTemplate && changed {
routerV4TargetNeedsTemplate = true
}
routerV6targetips, changed := config.makeNodeRouterTargetIPs(&node, config.eps.V6IPs, types.V6HostMasqueradeIP)
if !routerV6TargetNeedsTemplate && changed {
routerV6TargetNeedsTemplate = true
}

switchV4TemplateTarget.Value[node.chassisID] = addrsToString(
joinHostsPort(switchV4targetips, config.eps.Port))
Expand All @@ -365,23 +397,62 @@ func buildTemplateLBs(service *v1.Service, configs []lbConfig, nodes []nodeInfo,
joinHostsPort(routerV6targetips, config.eps.Port))
}

switchV4Rules = append(switchV4Rules, LBRule{
Source: Addr{Template: nodeIPv4Template, Port: config.inport},
Targets: []Addr{{Template: switchV4TemplateTarget}},
})
switchV6Rules = append(switchV6Rules, LBRule{
Source: Addr{Template: nodeIPv6Template, Port: config.inport},
Targets: []Addr{{Template: switchV6TemplateTarget}},
})
sharedV4Targets := []Addr{}
sharedV6Targets := []Addr{}
if !switchV4TargetNeedsTemplate || !routerV4TargetNeedsTemplate {
sharedV4Targets = joinHostsPort(config.eps.V4IPs, config.eps.Port)
}
if !switchV6TargetNeedsTemplate || !routerV6TargetNeedsTemplate {
sharedV6Targets = joinHostsPort(config.eps.V6IPs, config.eps.Port)
}

routerV4Rules = append(routerV4Rules, LBRule{
Source: Addr{Template: nodeIPv4Template, Port: config.inport},
Targets: []Addr{{Template: routerV4TemplateTarget}},
})
routerV6Rules = append(routerV6Rules, LBRule{
Source: Addr{Template: nodeIPv6Template, Port: config.inport},
Targets: []Addr{{Template: routerV6TemplateTarget}},
})
if switchV4TargetNeedsTemplate {
switchV4Rules = append(switchV4Rules, LBRule{
Source: Addr{Template: nodeIPv4Template, Port: config.inport},
Targets: []Addr{{Template: switchV4TemplateTarget}},
})
} else {
switchV4Rules = append(switchV4Rules, LBRule{
Source: Addr{Template: nodeIPv4Template, Port: config.inport},
Targets: sharedV4Targets,
})
}

if switchV6TargetNeedsTemplate {
switchV6Rules = append(switchV6Rules, LBRule{
Source: Addr{Template: nodeIPv6Template, Port: config.inport},
Targets: []Addr{{Template: switchV6TemplateTarget}},
})
} else {
switchV6Rules = append(switchV6Rules, LBRule{
Source: Addr{Template: nodeIPv6Template, Port: config.inport},
Targets: sharedV6Targets,
})
}

if routerV4TargetNeedsTemplate {
routerV4Rules = append(routerV4Rules, LBRule{
Source: Addr{Template: nodeIPv4Template, Port: config.inport},
Targets: []Addr{{Template: routerV4TemplateTarget}},
})
} else {
routerV4Rules = append(routerV4Rules, LBRule{
Source: Addr{Template: nodeIPv4Template, Port: config.inport},
Targets: sharedV4Targets,
})
}

if routerV6TargetNeedsTemplate {
routerV6Rules = append(routerV6Rules, LBRule{
Source: Addr{Template: nodeIPv6Template, Port: config.inport},
Targets: []Addr{{Template: routerV6TemplateTarget}},
})
} else {
routerV6Rules = append(routerV6Rules, LBRule{
Source: Addr{Template: nodeIPv6Template, Port: config.inport},
Targets: sharedV6Targets,
})
}
}
}

Expand Down Expand Up @@ -485,11 +556,11 @@ func buildPerNodeLBs(service *v1.Service, configs []lbConfig, nodes []nodeInfo)
switchRules := make([]LBRule, 0, len(configs))

for _, config := range configs {
switchV4targetips := config.makeNodeSwitchTargetIPs(&node, config.eps.V4IPs)
switchV6targetips := config.makeNodeSwitchTargetIPs(&node, config.eps.V6IPs)
switchV4targetips, _ := config.makeNodeSwitchTargetIPs(&node, config.eps.V4IPs)
switchV6targetips, _ := config.makeNodeSwitchTargetIPs(&node, config.eps.V6IPs)

routerV4targetips := config.makeNodeRouterTargetIPs(&node, config.eps.V4IPs, types.V4HostMasqueradeIP)
routerV6targetips := config.makeNodeRouterTargetIPs(&node, config.eps.V6IPs, types.V6HostMasqueradeIP)
routerV4targetips, _ := config.makeNodeRouterTargetIPs(&node, config.eps.V4IPs, types.V4HostMasqueradeIP)
routerV6targetips, _ := config.makeNodeRouterTargetIPs(&node, config.eps.V6IPs, types.V6HostMasqueradeIP)

routerV4targets := joinHostsPort(routerV4targetips, config.eps.Port)
routerV6targets := joinHostsPort(routerV6targetips, config.eps.Port)
Expand Down
61 changes: 35 additions & 26 deletions go-controller/pkg/ovn/controller/services/services_controller_test.go
View file
Expand Up @@ -331,17 +331,16 @@ func TestSyncServices(t *testing.T) {
},
ExternalIDs: serviceExternalIDs(namespacedServiceName(ns, serviceName)),
},
nodeSwitchTemplateLoadBalancer(nodePort, serviceName, ns),
nodeRouterTemplateLoadBalancer(nodePort, serviceName, ns),
nodeMergedTemplateLoadBalancer(nodePort, serviceName, ns, outport, nodeAEndpointIP, nodeBEndpointIP),
nodeLogicalSwitch(nodeA, initialLsGroups),
nodeLogicalSwitch(nodeB, initialLsGroups),
nodeLogicalRouter(nodeA, initialLrGroups),
nodeLogicalRouter(nodeB, initialLrGroups),
lbGroup(types.ClusterLBGroupName, loadBalancerClusterWideTCPServiceName(ns, serviceName)),
lbGroup(types.ClusterSwitchLBGroupName, nodeSwitchTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
lbGroup(types.ClusterRouterLBGroupName, nodeRouterTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
nodeTemplate(firstNode, nodePort, serviceName, ns, outport, nodeAEndpointIP, nodeBEndpointIP),
nodeTemplate(secondNode, nodePort, serviceName, ns, outport, nodeAEndpointIP, nodeBEndpointIP),
lbGroup(types.ClusterSwitchLBGroupName, nodeMergedTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
lbGroup(types.ClusterRouterLBGroupName, nodeMergedTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
nodeIPTemplate(firstNode),
nodeIPTemplate(secondNode),
},
},
{
Expand Down Expand Up @@ -414,17 +413,16 @@ func TestSyncServices(t *testing.T) {
},
ExternalIDs: serviceExternalIDs(namespacedServiceName(ns, serviceName)),
},
nodeSwitchTemplateLoadBalancer(nodePort, serviceName, ns),
nodeRouterTemplateLoadBalancer(nodePort, serviceName, ns),
nodeMergedTemplateLoadBalancer(nodePort, serviceName, ns, outport, nodeAEndpointIP, nodeBEndpointIP),
nodeLogicalSwitch(nodeA, initialLsGroups),
nodeLogicalSwitch(nodeB, initialLsGroups),
nodeLogicalRouter(nodeA, initialLrGroups),
nodeLogicalRouter(nodeB, initialLrGroups),
lbGroup(types.ClusterLBGroupName, loadBalancerClusterWideTCPServiceName(ns, serviceName)),
lbGroup(types.ClusterSwitchLBGroupName, nodeSwitchTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
lbGroup(types.ClusterRouterLBGroupName, nodeRouterTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
nodeTemplate(firstNode, nodePort, serviceName, ns, outport, nodeAEndpointIP, nodeBEndpointIP),
nodeTemplate(secondNode, nodePort, serviceName, ns, outport, nodeAEndpointIP, nodeBEndpointIP),
lbGroup(types.ClusterSwitchLBGroupName, nodeMergedTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
lbGroup(types.ClusterRouterLBGroupName, nodeMergedTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
nodeIPTemplate(firstNode),
nodeIPTemplate(secondNode),
},
nodeToDelete: nodeConfig(nodeA, nodeAHostIP),
dbStateAfterDeleting: []libovsdbtest.TestData{
Expand All @@ -438,19 +436,16 @@ func TestSyncServices(t *testing.T) {
},
ExternalIDs: serviceExternalIDs(namespacedServiceName(ns, serviceName)),
},
nodeSwitchTemplateLoadBalancer(nodePort, serviceName, ns),
nodeRouterTemplateLoadBalancer(nodePort, serviceName, ns),
nodeMergedTemplateLoadBalancer(nodePort, serviceName, ns, outport, nodeAEndpointIP, nodeBEndpointIP),
nodeLogicalSwitch(nodeA, initialLsGroups),
nodeLogicalSwitch(nodeB, initialLsGroups),
nodeLogicalRouter(nodeA, initialLrGroups),
nodeLogicalRouter(nodeB, initialLrGroups),
lbGroup(types.ClusterLBGroupName, loadBalancerClusterWideTCPServiceName(ns, serviceName)),
lbGroup(types.ClusterSwitchLBGroupName, nodeSwitchTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
lbGroup(types.ClusterRouterLBGroupName, nodeRouterTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
// node-A chassis template var is cleaned up by the DefaultNetworkController()
// deleteNodeEvent handler. And that doesn't get run in this test.
nodeTemplate(firstNode, nodePort, serviceName, ns, outport, nodeAEndpointIP, nodeBEndpointIP),
nodeTemplate(secondNode, nodePort, serviceName, ns, outport, nodeAEndpointIP, nodeBEndpointIP),
lbGroup(types.ClusterSwitchLBGroupName, nodeMergedTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
lbGroup(types.ClusterRouterLBGroupName, nodeMergedTemplateLoadBalancerName(ns, serviceName, v1.IPv4Protocol)),
nodeIPTemplate(firstNode),
nodeIPTemplate(secondNode),
},
},
}
Expand Down Expand Up @@ -568,6 +563,14 @@ func nodeRouterTemplateLoadBalancerName(serviceNamespace string, serviceName str
addressFamily)
}

func nodeMergedTemplateLoadBalancerName(serviceNamespace string, serviceName string, addressFamily v1.IPFamily) string {
return fmt.Sprintf(
"Service_%s/%s_TCP_node_switch_template_%s_merged",
serviceNamespace,
serviceName,
addressFamily)
}

func servicesOptions() map[string]string {
return map[string]string{
"event": "false",
Expand Down Expand Up @@ -637,12 +640,18 @@ func nodeIPTemplate(node *nodeInfo) *nbdb.ChassisTemplateVar {
}
}

func nodeTemplate(node *nodeInfo, nodePort int32, serviceName string, serviceNamespace string, outputPort int32, endpointIPs ...string) *nbdb.ChassisTemplateVar {
template := nodeIPTemplate(node)
template.Variables[makeLBNodeIPTemplateName(v1.IPv4Protocol)] = node.nodeIPs[0].String()
template.Variables[makeTarget(serviceName, serviceNamespace, "TCP", nodePort, "node_switch_template", v1.IPv4Protocol)] = computeEndpoints(outputPort, endpointIPs...)
template.Variables[makeTarget(serviceName, serviceNamespace, "TCP", nodePort, "node_router_template", v1.IPv4Protocol)] = computeEndpoints(outputPort, endpointIPs...)
return template
func nodeMergedTemplateLoadBalancer(nodePort int32, serviceName string, serviceNamespace string, outputPort int32, endpointIPs ...string) *nbdb.LoadBalancer {
nodeTemplateIP := makeTemplate(makeLBNodeIPTemplateName(v1.IPv4Protocol))
return &nbdb.LoadBalancer{
UUID: nodeMergedTemplateLoadBalancerName(serviceNamespace, serviceName, v1.IPv4Protocol),
Name: nodeMergedTemplateLoadBalancerName(serviceNamespace, serviceName, v1.IPv4Protocol),
Options: templateServicesOptions(),
Protocol: &nbdb.LoadBalancerProtocolTCP,
Vips: map[string]string{
endpoint(refTemplate(nodeTemplateIP.Name), nodePort): computeEndpoints(outputPort, endpointIPs...),
},
ExternalIDs: serviceExternalIDs(namespacedServiceName(serviceNamespace, serviceName)),
}
}

func refTemplate(template string) string {
Expand Down
7 changes: 5 additions & 2 deletions go-controller/pkg/util/util.go
View file
Expand Up @@ -168,21 +168,24 @@ func HashForOVN(s string) string {
}

// UpdateIPsSlice will search for values of oldIPs in the slice "s" and update it with newIPs values of same IP family
func UpdateIPsSlice(s, oldIPs, newIPs []string) []string {
func UpdateIPsSlice(s, oldIPs, newIPs []string) ([]string, bool) {
n := make([]string, len(s))
copy(n, s)
updated := false
for i, entry := range s {
for _, oldIP := range oldIPs {
if entry == oldIP {
for _, newIP := range newIPs {
if utilnet.IsIPv6(net.ParseIP(oldIP)) {
if utilnet.IsIPv6(net.ParseIP(newIP)) {
n[i] = newIP
updated = true
break
}
} else {
if !utilnet.IsIPv6(net.ParseIP(newIP)) {
n[i] = newIP
updated = true
break
}
}
Expand All @@ -191,7 +194,7 @@ func UpdateIPsSlice(s, oldIPs, newIPs []string) []string {
}
}
}
return n
return n, updated
}

// FilterIPsSlice will filter a list of IPs by a list of CIDRs. By default,
Expand Down
13 changes: 12 additions & 1 deletion go-controller/pkg/util/util_unit_test.go
View file
Expand Up @@ -110,57 +110,68 @@ func TestUpdateIPsSlice(t *testing.T) {
name string
s, oldIPs, newIPs []string
want []string
changed bool
}{
{
"Tests no matching IPs to remove",
[]string{"192.168.1.1", "10.0.0.1", "127.0.0.2"},
[]string{"1.1.1.1"},
[]string{"9.9.9.9", "fe99::1"},
[]string{"192.168.1.1", "10.0.0.1", "127.0.0.2"},
false,
},
{
"Tests some matching IPs to replace",
[]string{"192.168.1.1", "10.0.0.1", "127.0.0.2"},
[]string{"10.0.0.1"},
[]string{"9.9.9.9", "fe99::1"},
[]string{"192.168.1.1", "9.9.9.9", "127.0.0.2"},
true,
},
{
"Tests matching IPv6 to replace",
[]string{"fed9::5", "ab13::1e15", "3dfd::99ac"},
[]string{"3dfd::99ac"},
[]string{"9.9.9.9", "fe99::1"},
[]string{"fed9::5", "ab13::1e15", "fe99::1"},
true,
},
{
"Tests match but nothing to replace with",
[]string{"fed9::5", "ab13::1e15", "3dfd::99ac"},
[]string{"3dfd::99ac"},
[]string{"9.9.9.9"},
[]string{"fed9::5", "ab13::1e15", "3dfd::99ac"},
false,
},
{
"Tests with no newIPs",
[]string{"fed9::5", "ab13::1e15", "3dfd::99ac"},
[]string{"3dfd::99ac"},
[]string{},
[]string{"fed9::5", "ab13::1e15", "3dfd::99ac"},
false,
},
{
"Tests with no newIPs or oldIPs",
[]string{"fed9::5", "ab13::1e15", "3dfd::99ac"},
[]string{},
[]string{},
[]string{"fed9::5", "ab13::1e15", "3dfd::99ac"},
false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ans := UpdateIPsSlice(tt.s, tt.oldIPs, tt.newIPs)
ans, changed := UpdateIPsSlice(tt.s, tt.oldIPs, tt.newIPs)
if !reflect.DeepEqual(ans, tt.want) {
t.Errorf("got %v, want %v", ans, tt.want)
}

if tt.changed != changed {
t.Errorf("got changed %t, want %t", changed, tt.changed)
}
})
}
}
Expand Down

0 comments on commit 35e0f44

Please sign in to comment.