Skip to content

Commit

Permalink
Merge pull request #3328 from zeeke/ocpbugs-4485
Browse files Browse the repository at this point in the history 
Serve NodePort services on secondary IP addresses
  • Loading branch information
@trozet
trozet committed Apr 21, 2023
2 parents b37a99e + 8bd9b3e commit 7bc2e16
Show file tree
Hide file tree
Showing 10 changed files with 234 additions and 70 deletions.
4 changes: 3 additions & 1 deletion go-controller/pkg/node/gateway_iptables.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,9 @@ func getGatewayIPTRules(service *kapi.Service, localEndpoints []string, svcHasLo
if svcTypeIsETPLocal && !svcHasLocalHostNetEndPnt {
// case1 (see function description for details)
// A DNAT rule to masqueradeIP is added that takes priority over DNAT to clusterIP.
rules = append(rules, getNodePortIPTRules(svcPort, clusterIP, svcPort.NodePort, svcHasLocalHostNetEndPnt, svcTypeIsETPLocal)...)
if config.Gateway.Mode == config.GatewayModeLocal {
rules = append(rules, getNodePortIPTRules(svcPort, clusterIP, svcPort.NodePort, svcHasLocalHostNetEndPnt, svcTypeIsETPLocal)...)
}
// add a skip SNAT rule to OVN-KUBE-SNAT-MGMTPORT to preserve sourceIP for etp=local traffic.
rules = append(rules, getNodePortETPLocalIPTRules(svcPort, clusterIP)...)
}
Expand Down
11 changes: 3 additions & 8 deletions go-controller/pkg/node/gateway_localnet_linux_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,6 @@ var _ = Describe("Node Operations", func() {
"OVN-KUBE-ETP": []string{
fmt.Sprintf("-p %s -d %s --dport %v -j DNAT --to-destination %s:%v", service.Spec.Ports[0].Protocol, service.Status.LoadBalancer.Ingress[0].IP, service.Spec.Ports[0].Port, types.V4HostETPLocalMasqueradeIP, service.Spec.Ports[0].NodePort),
fmt.Sprintf("-p %s -d %s --dport %v -j DNAT --to-destination %s:%v", service.Spec.Ports[0].Protocol, externalIP, service.Spec.Ports[0].Port, types.V4HostETPLocalMasqueradeIP, service.Spec.Ports[0].NodePort),
fmt.Sprintf("-p %s -m addrtype --dst-type LOCAL --dport %v -j DNAT --to-destination %s:%v", service.Spec.Ports[0].Protocol, service.Spec.Ports[0].NodePort, types.V4HostETPLocalMasqueradeIP, service.Spec.Ports[0].NodePort),
},
"OVN-KUBE-ITP": []string{},
"OVN-KUBE-EGRESS-SVC": []string{"-m mark --mark 0x3f0 -m comment --comment Do not SNAT to SVC VIP -j RETURN"},
Expand Down Expand Up @@ -2116,9 +2115,7 @@ var _ = Describe("Node Operations", func() {
"OVN-KUBE-SNAT-MGMTPORT": []string{
fmt.Sprintf("-p TCP --dport %v -j RETURN", service.Spec.Ports[0].NodePort),
},
"OVN-KUBE-ETP": []string{
fmt.Sprintf("-p %s -m addrtype --dst-type LOCAL --dport %v -j DNAT --to-destination %s:%v", service.Spec.Ports[0].Protocol, service.Spec.Ports[0].NodePort, types.V4HostETPLocalMasqueradeIP, service.Spec.Ports[0].NodePort),
},
"OVN-KUBE-ETP": []string{},
"OVN-KUBE-ITP": []string{},
"OVN-KUBE-EGRESS-SVC": []string{"-m mark --mark 0x3f0 -m comment --comment Do not SNAT to SVC VIP -j RETURN"},
},
Expand Down Expand Up @@ -2405,10 +2402,8 @@ var _ = Describe("Node Operations", func() {
"OVN-KUBE-SNAT-MGMTPORT": []string{
fmt.Sprintf("-p TCP --dport %v -j RETURN", service.Spec.Ports[0].NodePort),
},
"OVN-KUBE-ITP": []string{},
"OVN-KUBE-ETP": []string{
fmt.Sprintf("-p %s -m addrtype --dst-type LOCAL --dport %v -j DNAT --to-destination %s:%v", service.Spec.Ports[0].Protocol, service.Spec.Ports[0].NodePort, types.V4HostETPLocalMasqueradeIP, service.Spec.Ports[0].NodePort),
},
"OVN-KUBE-ITP": []string{},
"OVN-KUBE-ETP": []string{},
"OVN-KUBE-EGRESS-SVC": []string{"-m mark --mark 0x3f0 -m comment --comment Do not SNAT to SVC VIP -j RETURN"},
},
"filter": {},
Expand Down
8 changes: 4 additions & 4 deletions go-controller/pkg/ovn/controller/services/lb_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ 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, updated := util.UpdateIPsSlice(targetIPs, node.nodeIPsStr(), []string{hostMasqueradeIP})
targetIPs, updated := util.UpdateIPsSlice(targetIPs, node.l3gatewayAddressesStr(), []string{hostMasqueradeIP})

// We either only removed stuff from the original slice, or updated some IPs.
if len(targetIPs) != len(epIPs) || updated {
Expand Down Expand Up @@ -108,8 +108,8 @@ var protos = []v1.Protocol{
// - services with InternalTrafficPolicy=Local
//
// Template LBs will be created for
// - services with NodePort set but *without* ExternalTrafficPolicy=Local or
// affinity timeout set.
// - services with NodePort set but *without* ExternalTrafficPolicy=Local or
// affinity timeout set.
func buildServiceLBConfigs(service *v1.Service, endpointSlices []*discovery.EndpointSlice, useLBGroup, useTemplates bool) (perNodeConfigs, templateConfigs, clusterConfigs []lbConfig) {
needsAffinityTimeout := hasSessionAffinityTimeOut(service)

Expand Down Expand Up @@ -573,7 +573,7 @@ func buildPerNodeLBs(service *v1.Service, configs []lbConfig, nodes []nodeInfo)
vips := make([]string, 0, len(config.vips))
for _, vip := range config.vips {
if vip == placeholderNodeIPs {
vips = append(vips, node.nodeIPsStr()...)
vips = append(vips, node.hostAddressesStr()...)
} else {
vips = append(vips, vip)
}
Expand Down
100 changes: 82 additions & 18 deletions go-controller/pkg/ovn/controller/services/lb_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -750,16 +750,18 @@ func Test_buildClusterLBs(t *testing.T) {

defaultNodes := []nodeInfo{
{
name: "node-a",
nodeIPs: []net.IP{net.ParseIP("10.0.0.1")},
gatewayRouterName: "gr-node-a",
switchName: "switch-node-a",
name: "node-a",
l3gatewayAddresses: []net.IP{net.ParseIP("10.0.0.1")},
hostAddresses: []net.IP{net.ParseIP("10.0.0.1")},
gatewayRouterName: "gr-node-a",
switchName: "switch-node-a",
},
{
name: "node-b",
nodeIPs: []net.IP{net.ParseIP("10.0.0.2")},
gatewayRouterName: "gr-node-b",
switchName: "switch-node-b",
name: "node-b",
l3gatewayAddresses: []net.IP{net.ParseIP("10.0.0.2")},
hostAddresses: []net.IP{net.ParseIP("10.0.0.2")},
gatewayRouterName: "gr-node-b",
switchName: "switch-node-b",
},
}

Expand Down Expand Up @@ -978,18 +980,20 @@ func Test_buildPerNodeLBs(t *testing.T) {

defaultNodes := []nodeInfo{
{
name: "node-a",
nodeIPs: []net.IP{net.ParseIP("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-a",
l3gatewayAddresses: []net.IP{net.ParseIP("10.0.0.1")},
hostAddresses: []net.IP{net.ParseIP("10.0.0.1"), net.ParseIP("10.0.0.111")},
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: []net.IP{net.ParseIP("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)}},
name: "node-b",
l3gatewayAddresses: []net.IP{net.ParseIP("10.0.0.2")},
hostAddresses: []net.IP{net.ParseIP("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)}},
},
}

Expand Down Expand Up @@ -1079,6 +1083,10 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 80},
Targets: []Addr{{IP: "10.128.0.2", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 80},
Targets: []Addr{{IP: "10.128.0.2", Port: 8080}},
},
},
Opts: defaultOpts,
},
Expand Down Expand Up @@ -1109,6 +1117,10 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 80},
Targets: []Addr{{IP: "10.128.0.2", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 80},
Targets: []Addr{{IP: "10.128.0.2", Port: 8080}},
},
},
Opts: defaultOpts,
},
Expand Down Expand Up @@ -1166,6 +1178,10 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 80},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 80},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}},
},
},
Opts: defaultOpts,
},
Expand All @@ -1183,6 +1199,10 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 80},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 80},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
},
Opts: defaultOpts,
},
Expand Down Expand Up @@ -1220,6 +1240,10 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 80},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 80},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}},
},
},
Opts: defaultOpts,
},
Expand All @@ -1237,6 +1261,10 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 80},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 80},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
},
Opts: defaultOpts,
},
Expand Down Expand Up @@ -1315,6 +1343,10 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 80},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 80},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}},
},
},
},
{
Expand All @@ -1335,6 +1367,14 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 80},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
{
Source: Addr{IP: "169.254.169.3", Port: 80},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 80},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
},
Opts: defaultOpts,
},
Expand Down Expand Up @@ -1733,6 +1773,10 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 34345},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}}, // special skip_snat=true LB for ETP=local; used in SGW mode
},
{
Source: Addr{IP: "10.0.0.111", Port: 34345},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}},
},
},
},
{
Expand All @@ -1751,6 +1795,14 @@ func Test_buildPerNodeLBs(t *testing.T) {
},
{
Source: Addr{IP: "10.0.0.1", Port: 34345},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
{
Source: Addr{IP: "169.254.169.3", Port: 34345},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 34345},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}}, // don't filter out eps for nodePorts on switches when ETP=local
},
},
Expand Down Expand Up @@ -1820,6 +1872,10 @@ func Test_buildPerNodeLBs(t *testing.T) {
Source: Addr{IP: "10.0.0.1", Port: 34345},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}}, // special skip_snat=true LB for ETP=local; used in SGW mode
},
{
Source: Addr{IP: "10.0.0.111", Port: 34345},
Targets: []Addr{{IP: "169.254.169.2", Port: 8080}},
},
},
},
{
Expand All @@ -1838,6 +1894,14 @@ func Test_buildPerNodeLBs(t *testing.T) {
},
{
Source: Addr{IP: "10.0.0.1", Port: 34345},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
{
Source: Addr{IP: "169.254.169.3", Port: 34345},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}},
},
{
Source: Addr{IP: "10.0.0.111", Port: 34345},
Targets: []Addr{{IP: "10.0.0.1", Port: 8080}}, // don't filter out eps for nodePorts on switches when ETP=local
},
},
Expand Down

0 comments on commit 7bc2e16

Please sign in to comment.