Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion go-controller/pkg/ovn/controller/services/load_balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,13 @@ func buildServiceLBConfigs(service *v1.Service, endpointSlices []*discovery.Endp
// unless any of the following are true:
// - Any of the endpoints are host-network
// - ETP=local service backed by non-local-host-networked endpoints
// - OCP only HACK: It's an openshift-dns:default-dns service
//
// In that case, we need to create per-node LBs.
if hasHostEndpoints(eps.V4IPs) || hasHostEndpoints(eps.V6IPs) {
if hasHostEndpoints(eps.V4IPs) || hasHostEndpoints(eps.V6IPs) ||
// OCP only hack begin
(service.Namespace == "openshift-dns" && service.Name == "dns-default") {
// OCP only hack end
perNodeConfigs = append(perNodeConfigs, clusterIPConfig)
} else {
clusterConfigs = append(clusterConfigs, clusterIPConfig)
Expand Down Expand Up @@ -311,6 +315,24 @@ func buildPerNodeLBs(service *v1.Service, configs []lbConfig, nodes []nodeInfo)
switchV4Targets := ovnlb.JoinHostsPort(config.eps.V4IPs, config.eps.Port)
switchV6Targets := ovnlb.JoinHostsPort(config.eps.V6IPs, config.eps.Port)

// OCP HACK begin
// TODO: Remove this hack once we add support for ITP:preferLocal and DNS operator starts using it.
if service.Namespace == "openshift-dns" && service.Name == "dns-default" {
// Filter out endpoints that are local to this node.
switchV4targetDNSips := util.FilterIPsSlice(config.eps.V4IPs, node.podSubnets, true)
switchV6targetDNSips := util.FilterIPsSlice(config.eps.V6IPs, node.podSubnets, true)
// If no local endpoints were found add all the endpoints as targets.
if len(switchV4targetDNSips) == 0 {
switchV4targetDNSips = config.eps.V4IPs
}
if len(switchV6targetDNSips) == 0 {
switchV6targetDNSips = config.eps.V6IPs
}
switchV4Targets = ovnlb.JoinHostsPort(switchV4targetDNSips, config.eps.Port)
switchV6Targets = ovnlb.JoinHostsPort(switchV6targetDNSips, config.eps.Port)
}
// OCP HACK end

// Substitute the special vip "node" for the node's physical ips
// This is used for nodeport
vips = make([]string, 0, len(vips))
Expand Down
123 changes: 123 additions & 0 deletions go-controller/pkg/ovn/controller/services/load_balancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1369,3 +1369,126 @@ func Test_buildPerNodeLBs(t *testing.T) {
})
}
}

// OCP hack begin
func Test_buildPerNodeLBs_OCPHackForDNS(t *testing.T) {
oldClusterSubnet := globalconfig.Default.ClusterSubnets
oldGwMode := globalconfig.Gateway.Mode
defer func() {
globalconfig.Gateway.Mode = oldGwMode
globalconfig.Default.ClusterSubnets = oldClusterSubnet
}()
_, cidr4, _ := net.ParseCIDR("10.128.0.0/16")
_, cidr6, _ := net.ParseCIDR("fe00::/64")
globalconfig.Default.ClusterSubnets = []globalconfig.CIDRNetworkEntry{{cidr4, 26}, {cidr6, 26}}

name := "dns-default"
namespace := "openshift-dns"

defaultService := &v1.Service{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeClusterIP,
},
}

defaultNodes := []nodeInfo{
{
name: "node-a",
nodeIPs: []string{"10.0.0.1"},
gatewayRouterName: "gr-node-a",
switchName: "switch-node-a",
podSubnets: []net.IPNet{{IP: net.ParseIP("10.128.0.0"), Mask: net.CIDRMask(24, 32)}},
},
{
name: "node-b",
nodeIPs: []string{"10.0.0.2"},
gatewayRouterName: "gr-node-b",
switchName: "switch-node-b",
podSubnets: []net.IPNet{{IP: net.ParseIP("10.128.1.0"), Mask: net.CIDRMask(24, 32)}},
},
}

defaultExternalIDs := map[string]string{
"k8s.ovn.org/kind": "Service",
"k8s.ovn.org/owner": fmt.Sprintf("%s/%s", namespace, name),
}

//defaultRouters := []string{"gr-node-a", "gr-node-b"}
//defaultSwitches := []string{"switch-node-a", "switch-node-b"}

tc := []struct {
name string
service *v1.Service
configs []lbConfig
expected []ovnlb.LB
}{
{
name: "clusterIP service, standard pods",
service: defaultService,
configs: []lbConfig{
{
vips: []string{"192.168.1.1"},
protocol: v1.ProtocolTCP,
inport: 80,
eps: util.LbEndpoints{
V4IPs: []string{"10.128.0.2", "10.128.1.2"},
Port: 8080,
},
},
},
expected: []ovnlb.LB{
{
Name: "Service_openshift-dns/dns-default_TCP_node_router_node-a_merged",
ExternalIDs: defaultExternalIDs,
Routers: []string{"gr-node-a", "gr-node-b"},
Protocol: "TCP",
Rules: []ovnlb.LBRule{
{
Source: ovnlb.Addr{"192.168.1.1", 80},
Targets: []ovnlb.Addr{{"10.128.0.2", 8080}, {"10.128.1.2", 8080}},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the nice test case. I guess the one caveat here is that if a host -> DNS service it could go to any node rather than it's own node. That's probably OK?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that should be fine, according to the bz, they are worried only about egress traffic from the pods. Maybe if we get yelled at we can do the host->clusterIP->pod then? Because for that I'd need to also hack the vip:ip on the GR-routers which probably isn't that hard to do, just that we'd have twice as many more LBs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, the dns-default clsuterIP svc is mainly used to dns resolution for stuff in overlay. So I think we are ok if we don't cover the host->svc->pod case since that's technically "outside the overlay".

},
},
},
{
Name: "Service_openshift-dns/dns-default_TCP_node_switch_node-a",
ExternalIDs: defaultExternalIDs,
Switches: []string{"switch-node-a"},
Protocol: "TCP",
Rules: []ovnlb.LBRule{
{
Source: ovnlb.Addr{"192.168.1.1", 80},
Targets: []ovnlb.Addr{{"10.128.0.2", 8080}},
},
},
},
{
Name: "Service_openshift-dns/dns-default_TCP_node_switch_node-b",
ExternalIDs: defaultExternalIDs,
Switches: []string{"switch-node-b"},
Protocol: "TCP",
Rules: []ovnlb.LBRule{
{
Source: ovnlb.Addr{"192.168.1.1", 80},
Targets: []ovnlb.Addr{{"10.128.1.2", 8080}},
},
},
},
},
},
}

for i, tt := range tc {
t.Run(fmt.Sprintf("%d_%s", i, tt.name), func(t *testing.T) {

globalconfig.Gateway.Mode = globalconfig.GatewayModeShared
actual := buildPerNodeLBs(tt.service, tt.configs, defaultNodes)
assert.Equal(t, tt.expected, actual, "shared gateway mode not as expected")

globalconfig.Gateway.Mode = globalconfig.GatewayModeLocal
actual = buildPerNodeLBs(tt.service, tt.configs, defaultNodes)
assert.Equal(t, tt.expected, actual, "local gateway mode not as expected")
})
}
}
// OCP hack end