Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit f01cafa

Browse files
Miciahbertinatto
Miciah
authored and
bertinatto
committedNov 28, 2024
UPSTREAM: <carry>: Prefer local endpoint for cluster DNS service
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.
1 parent 0e44530 commit f01cafa

File tree

2 files changed

+182
-0
lines changed

2 files changed

+182
-0
lines changed
 

‎pkg/proxy/iptables/proxier.go

+15
Original file line numberDiff line numberDiff line change
@@ -1000,6 +1000,21 @@ func (proxier *Proxier) syncProxyRules() {
10001000
allEndpoints := proxier.endpointsMap[svcName]
10011001
clusterEndpoints, localEndpoints, allLocallyReachableEndpoints, hasEndpoints := proxy.CategorizeEndpoints(allEndpoints, svcInfo, proxier.nodeLabels)
10021002

1003+
// Prefer local endpoint for the DNS service.
1004+
// Fixes <https://bugzilla.redhat.com/show_bug.cgi?id=1919737>.
1005+
// TODO: Delete this once node-level topology is
1006+
// implemented and the DNS operator is updated to use it.
1007+
if svcPortNameString == "openshift-dns/dns-default:dns" || svcPortNameString == "openshift-dns/dns-default:dns-tcp" {
1008+
for _, ep := range clusterEndpoints {
1009+
if ep.IsLocal() {
1010+
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)
1011+
clusterEndpoints = []proxy.Endpoint{ep}
1012+
allLocallyReachableEndpoints = clusterEndpoints
1013+
break
1014+
}
1015+
}
1016+
}
1017+
10031018
// clusterPolicyChain contains the endpoints used with "Cluster" traffic policy
10041019
clusterPolicyChain := svcInfo.clusterPolicyChainName
10051020
usesClusterPolicyChain := len(clusterEndpoints) > 0 && svcInfo.UsesClusterEndpoints()

‎pkg/proxy/iptables/proxier_test.go

+167
Original file line numberDiff line numberDiff line change
@@ -2081,6 +2081,173 @@ func TestClusterIPGeneral(t *testing.T) {
20812081
})
20822082
}
20832083

2084+
func TestOpenShiftDNSHackTCP(t *testing.T) {
2085+
ipt := iptablestest.NewFake()
2086+
fp := NewFakeProxier(ipt)
2087+
svcIP := "172.30.0.10"
2088+
svcPort := 53
2089+
podPort := 5353
2090+
svcPortName := proxy.ServicePortName{
2091+
NamespacedName: makeNSN("openshift-dns", "dns-default"),
2092+
Port: "dns-tcp",
2093+
Protocol: v1.ProtocolTCP,
2094+
}
2095+
2096+
makeServiceMap(fp,
2097+
makeTestService(svcPortName.Namespace, svcPortName.Name, func(svc *v1.Service) {
2098+
svc.Spec.ClusterIP = svcIP
2099+
svc.Spec.Ports = []v1.ServicePort{{
2100+
Name: svcPortName.Port,
2101+
Port: int32(svcPort),
2102+
Protocol: svcPortName.Protocol,
2103+
}}
2104+
}),
2105+
)
2106+
2107+
populateEndpointSlices(fp,
2108+
makeTestEndpointSlice(svcPortName.Namespace, svcPortName.Name, 1, func(eps *discovery.EndpointSlice) {
2109+
eps.AddressType = discovery.AddressTypeIPv4
2110+
eps.Endpoints = []discovery.Endpoint{{
2111+
// This endpoint is ignored because it's remote
2112+
Addresses: []string{"10.180.0.2"},
2113+
NodeName: ptr.To("node2"),
2114+
}, {
2115+
Addresses: []string{"10.180.0.1"},
2116+
NodeName: ptr.To(testHostname),
2117+
}}
2118+
eps.Ports = []discovery.EndpointPort{{
2119+
Name: ptr.To(svcPortName.Port),
2120+
Port: ptr.To[int32](int32(podPort)),
2121+
Protocol: &svcPortName.Protocol,
2122+
}}
2123+
}),
2124+
)
2125+
2126+
fp.syncProxyRules()
2127+
2128+
runPacketFlowTests(t, getLine(), ipt, testNodeIPs, []packetFlowTest{
2129+
{
2130+
name: "TCP DNS only goes to local endpoint",
2131+
sourceIP: "10.0.0.2",
2132+
destIP: "172.30.0.10",
2133+
destPort: 53,
2134+
output: "10.180.0.1:5353",
2135+
},
2136+
})
2137+
}
2138+
2139+
func TestOpenShiftDNSHackUDP(t *testing.T) {
2140+
ipt := iptablestest.NewFake()
2141+
fp := NewFakeProxier(ipt)
2142+
svcIP := "172.30.0.10"
2143+
svcPort := 53
2144+
podPort := 5353
2145+
svcPortName := proxy.ServicePortName{
2146+
NamespacedName: makeNSN("openshift-dns", "dns-default"),
2147+
Port: "dns",
2148+
Protocol: v1.ProtocolUDP,
2149+
}
2150+
2151+
makeServiceMap(fp,
2152+
makeTestService(svcPortName.Namespace, svcPortName.Name, func(svc *v1.Service) {
2153+
svc.Spec.ClusterIP = svcIP
2154+
svc.Spec.Ports = []v1.ServicePort{{
2155+
Name: svcPortName.Port,
2156+
Port: int32(svcPort),
2157+
Protocol: svcPortName.Protocol,
2158+
}}
2159+
}),
2160+
)
2161+
2162+
populateEndpointSlices(fp,
2163+
makeTestEndpointSlice(svcPortName.Namespace, svcPortName.Name, 1, func(eps *discovery.EndpointSlice) {
2164+
eps.AddressType = discovery.AddressTypeIPv4
2165+
eps.Endpoints = []discovery.Endpoint{{
2166+
// This endpoint is ignored because it's remote
2167+
Addresses: []string{"10.180.0.2"},
2168+
NodeName: ptr.To("node2"),
2169+
}, {
2170+
Addresses: []string{"10.180.0.1"},
2171+
NodeName: ptr.To(testHostname),
2172+
}}
2173+
eps.Ports = []discovery.EndpointPort{{
2174+
Name: ptr.To(svcPortName.Port),
2175+
Port: ptr.To[int32](int32(podPort)),
2176+
Protocol: &svcPortName.Protocol,
2177+
}}
2178+
}),
2179+
)
2180+
2181+
fp.syncProxyRules()
2182+
2183+
runPacketFlowTests(t, getLine(), ipt, testNodeIPs, []packetFlowTest{
2184+
{
2185+
name: "UDP DNS only goes to local endpoint",
2186+
sourceIP: "10.0.0.2",
2187+
protocol: v1.ProtocolUDP,
2188+
destIP: "172.30.0.10",
2189+
destPort: 53,
2190+
output: "10.180.0.1:5353",
2191+
},
2192+
})
2193+
}
2194+
2195+
func TestOpenShiftDNSHackFallback(t *testing.T) {
2196+
ipt := iptablestest.NewFake()
2197+
fp := NewFakeProxier(ipt)
2198+
svcIP := "172.30.0.10"
2199+
svcPort := 53
2200+
podPort := 5353
2201+
svcPortName := proxy.ServicePortName{
2202+
NamespacedName: makeNSN("openshift-dns", "dns-default"),
2203+
Port: "dns",
2204+
Protocol: v1.ProtocolUDP,
2205+
}
2206+
2207+
makeServiceMap(fp,
2208+
makeTestService(svcPortName.Namespace, svcPortName.Name, func(svc *v1.Service) {
2209+
svc.Spec.ClusterIP = svcIP
2210+
svc.Spec.Ports = []v1.ServicePort{{
2211+
Name: svcPortName.Port,
2212+
Port: int32(svcPort),
2213+
Protocol: svcPortName.Protocol,
2214+
}}
2215+
}),
2216+
)
2217+
2218+
populateEndpointSlices(fp,
2219+
makeTestEndpointSlice(svcPortName.Namespace, svcPortName.Name, 1, func(eps *discovery.EndpointSlice) {
2220+
eps.AddressType = discovery.AddressTypeIPv4
2221+
// Both endpoints are used because neither is local
2222+
eps.Endpoints = []discovery.Endpoint{{
2223+
Addresses: []string{"10.180.1.2"},
2224+
NodeName: ptr.To("node2"),
2225+
}, {
2226+
Addresses: []string{"10.180.2.3"},
2227+
NodeName: ptr.To("node3"),
2228+
}}
2229+
eps.Ports = []discovery.EndpointPort{{
2230+
Name: ptr.To(svcPortName.Port),
2231+
Port: ptr.To[int32](int32(podPort)),
2232+
Protocol: &svcPortName.Protocol,
2233+
}}
2234+
}),
2235+
)
2236+
2237+
fp.syncProxyRules()
2238+
2239+
runPacketFlowTests(t, getLine(), ipt, testNodeIPs, []packetFlowTest{
2240+
{
2241+
name: "DNS goes to all endpoints when none are local",
2242+
sourceIP: "10.0.0.2",
2243+
protocol: v1.ProtocolUDP,
2244+
destIP: "172.30.0.10",
2245+
destPort: 53,
2246+
output: "10.180.1.2:5353, 10.180.2.3:5353",
2247+
},
2248+
})
2249+
}
2250+
20842251
func TestLoadBalancer(t *testing.T) {
20852252
ipt := iptablestest.NewFake()
20862253
fp := NewFakeProxier(ipt)

0 commit comments

Comments
 (0)
Please sign in to comment.