Skip to content

Commit

Permalink
UPSTREAM: <carry>: Prefer local endpoint for cluster DNS service
Browse files Browse the repository at this point in the history 
This commit fixes bug 1919737.

https://bugzilla.redhat.com/show_bug.cgi?id=1919737

* pkg/proxy/iptables/proxier.go (syncProxyRules): Prefer a local endpoint
for the cluster DNS service.
  • Loading branch information
@Miciah @bertinatto
Miciah authored and bertinatto committed Dec 11, 2024
1 parent 9827eb6 commit 832f154
Show file tree
Hide file tree
Showing 2 changed files with 182 additions and 0 deletions.
15 changes: 15 additions & 0 deletions pkg/proxy/iptables/proxier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,21 @@ func (proxier *Proxier) syncProxyRules() {
allEndpoints := proxier.endpointsMap[svcName]
clusterEndpoints, localEndpoints, allLocallyReachableEndpoints, hasEndpoints := proxy.CategorizeEndpoints(allEndpoints, svcInfo, proxier.nodeLabels)

// Prefer local endpoint for the DNS service.
// Fixes <https://bugzilla.redhat.com/show_bug.cgi?id=1919737>.
// TODO: Delete this once node-level topology is
// implemented and the DNS operator is updated to use it.
if svcPortNameString == "openshift-dns/dns-default:dns" || svcPortNameString == "openshift-dns/dns-default:dns-tcp" {
for _, ep := range clusterEndpoints {
if ep.IsLocal() {
klog.V(4).Infof("Found a local endpoint %q for service %q; preferring the local endpoint and ignoring %d other endpoints", ep.String(), svcPortNameString, len(clusterEndpoints)-1)
clusterEndpoints = []proxy.Endpoint{ep}
allLocallyReachableEndpoints = clusterEndpoints
break
}
}
}

// clusterPolicyChain contains the endpoints used with "Cluster" traffic policy
clusterPolicyChain := svcInfo.clusterPolicyChainName
usesClusterPolicyChain := len(clusterEndpoints) > 0 && svcInfo.UsesClusterEndpoints()
Expand Down
167 changes: 167 additions & 0 deletions pkg/proxy/iptables/proxier_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2081,6 +2081,173 @@ func TestClusterIPGeneral(t *testing.T) {
})
}

func TestOpenShiftDNSHackTCP(t *testing.T) {
ipt := iptablestest.NewFake()
fp := NewFakeProxier(ipt)
svcIP := "172.30.0.10"
svcPort := 53
podPort := 5353
svcPortName := proxy.ServicePortName{
NamespacedName: makeNSN("openshift-dns", "dns-default"),
Port: "dns-tcp",
Protocol: v1.ProtocolTCP,
}

makeServiceMap(fp,
makeTestService(svcPortName.Namespace, svcPortName.Name, func(svc *v1.Service) {
svc.Spec.ClusterIP = svcIP
svc.Spec.Ports = []v1.ServicePort{{
Name: svcPortName.Port,
Port: int32(svcPort),
Protocol: svcPortName.Protocol,
}}
}),
)

populateEndpointSlices(fp,
makeTestEndpointSlice(svcPortName.Namespace, svcPortName.Name, 1, func(eps *discovery.EndpointSlice) {
eps.AddressType = discovery.AddressTypeIPv4
eps.Endpoints = []discovery.Endpoint{{
// This endpoint is ignored because it's remote
Addresses: []string{"10.180.0.2"},
NodeName: ptr.To("node2"),
}, {
Addresses: []string{"10.180.0.1"},
NodeName: ptr.To(testHostname),
}}
eps.Ports = []discovery.EndpointPort{{
Name: ptr.To(svcPortName.Port),
Port: ptr.To[int32](int32(podPort)),
Protocol: &svcPortName.Protocol,
}}
}),
)

fp.syncProxyRules()

runPacketFlowTests(t, getLine(), ipt, testNodeIPs, []packetFlowTest{
{
name: "TCP DNS only goes to local endpoint",
sourceIP: "10.0.0.2",
destIP: "172.30.0.10",
destPort: 53,
output: "10.180.0.1:5353",
},
})
}

func TestOpenShiftDNSHackUDP(t *testing.T) {
ipt := iptablestest.NewFake()
fp := NewFakeProxier(ipt)
svcIP := "172.30.0.10"
svcPort := 53
podPort := 5353
svcPortName := proxy.ServicePortName{
NamespacedName: makeNSN("openshift-dns", "dns-default"),
Port: "dns",
Protocol: v1.ProtocolUDP,
}

makeServiceMap(fp,
makeTestService(svcPortName.Namespace, svcPortName.Name, func(svc *v1.Service) {
svc.Spec.ClusterIP = svcIP
svc.Spec.Ports = []v1.ServicePort{{
Name: svcPortName.Port,
Port: int32(svcPort),
Protocol: svcPortName.Protocol,
}}
}),
)

populateEndpointSlices(fp,
makeTestEndpointSlice(svcPortName.Namespace, svcPortName.Name, 1, func(eps *discovery.EndpointSlice) {
eps.AddressType = discovery.AddressTypeIPv4
eps.Endpoints = []discovery.Endpoint{{
// This endpoint is ignored because it's remote
Addresses: []string{"10.180.0.2"},
NodeName: ptr.To("node2"),
}, {
Addresses: []string{"10.180.0.1"},
NodeName: ptr.To(testHostname),
}}
eps.Ports = []discovery.EndpointPort{{
Name: ptr.To(svcPortName.Port),
Port: ptr.To[int32](int32(podPort)),
Protocol: &svcPortName.Protocol,
}}
}),
)

fp.syncProxyRules()

runPacketFlowTests(t, getLine(), ipt, testNodeIPs, []packetFlowTest{
{
name: "UDP DNS only goes to local endpoint",
sourceIP: "10.0.0.2",
protocol: v1.ProtocolUDP,
destIP: "172.30.0.10",
destPort: 53,
output: "10.180.0.1:5353",
},
})
}

func TestOpenShiftDNSHackFallback(t *testing.T) {
ipt := iptablestest.NewFake()
fp := NewFakeProxier(ipt)
svcIP := "172.30.0.10"
svcPort := 53
podPort := 5353
svcPortName := proxy.ServicePortName{
NamespacedName: makeNSN("openshift-dns", "dns-default"),
Port: "dns",
Protocol: v1.ProtocolUDP,
}

makeServiceMap(fp,
makeTestService(svcPortName.Namespace, svcPortName.Name, func(svc *v1.Service) {
svc.Spec.ClusterIP = svcIP
svc.Spec.Ports = []v1.ServicePort{{
Name: svcPortName.Port,
Port: int32(svcPort),
Protocol: svcPortName.Protocol,
}}
}),
)

populateEndpointSlices(fp,
makeTestEndpointSlice(svcPortName.Namespace, svcPortName.Name, 1, func(eps *discovery.EndpointSlice) {
eps.AddressType = discovery.AddressTypeIPv4
// Both endpoints are used because neither is local
eps.Endpoints = []discovery.Endpoint{{
Addresses: []string{"10.180.1.2"},
NodeName: ptr.To("node2"),
}, {
Addresses: []string{"10.180.2.3"},
NodeName: ptr.To("node3"),
}}
eps.Ports = []discovery.EndpointPort{{
Name: ptr.To(svcPortName.Port),
Port: ptr.To[int32](int32(podPort)),
Protocol: &svcPortName.Protocol,
}}
}),
)

fp.syncProxyRules()

runPacketFlowTests(t, getLine(), ipt, testNodeIPs, []packetFlowTest{
{
name: "DNS goes to all endpoints when none are local",
sourceIP: "10.0.0.2",
protocol: v1.ProtocolUDP,
destIP: "172.30.0.10",
destPort: 53,
output: "10.180.1.2:5353, 10.180.2.3:5353",
},
})
}

func TestLoadBalancer(t *testing.T) {
ipt := iptablestest.NewFake()
fp := NewFakeProxier(ipt)
Expand Down

0 comments on commit 832f154

Please sign in to comment.