Skip to content

Commit cc77f07

Browse files
Miciahbertinatto
Miciah
authored and
bertinatto
committed
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 c78d8e4 commit cc77f07

File tree

2 files changed

+182
-0
lines changed

2 files changed

+182
-0
lines changed

‎pkg/proxy/iptables/proxier.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,6 +937,21 @@ func (proxier *Proxier) syncProxyRules() (retryError error) {
937937
allEndpoints := proxier.endpointsMap[svcName]
938938
clusterEndpoints, localEndpoints, allLocallyReachableEndpoints, hasEndpoints := proxy.CategorizeEndpoints(allEndpoints, svcInfo, proxier.nodeName, proxier.topologyLabels)
939939

940+
// Prefer local endpoint for the DNS service.
941+
// Fixes <https://bugzilla.redhat.com/show_bug.cgi?id=1919737>.
942+
// TODO: Delete this once node-level topology is
943+
// implemented and the DNS operator is updated to use it.
944+
if svcPortNameString == "openshift-dns/dns-default:dns" || svcPortNameString == "openshift-dns/dns-default:dns-tcp" {
945+
for _, ep := range clusterEndpoints {
946+
if ep.IsLocal() {
947+
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)
948+
clusterEndpoints = []proxy.Endpoint{ep}
949+
allLocallyReachableEndpoints = clusterEndpoints
950+
break
951+
}
952+
}
953+
}
954+
940955
// clusterPolicyChain contains the endpoints used with "Cluster" traffic policy
941956
clusterPolicyChain := svcInfo.clusterPolicyChainName
942957
usesClusterPolicyChain := len(clusterEndpoints) > 0 && svcInfo.UsesClusterEndpoints()

‎pkg/proxy/iptables/proxier_test.go‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2082,6 +2082,173 @@ func TestClusterIPGeneral(t *testing.T) {
20822082
})
20832083
}
20842084

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

0 commit comments

Comments
 (0)