Remove some dead code in the Endpoints controller

There was code to deal with upgrades from pre-dual-stack-aware
apiservers, with a note to "remove when the possibility of upgrading
from a cluster that does not support dual stack is nil".

(This requires fixing the unit tests to fill in
service.Spec.IPFamilies like a modern apiserver would do.)
This commit is contained in:
Dan Winship 2025-02-11 17:17:02 -05:00
parent 9a9f10bc7b
commit 1bd3d34d6f
2 changed files with 99 additions and 137 deletions

View File

@ -214,38 +214,12 @@ func (e *Controller) addPod(obj interface{}) {
func podToEndpointAddressForService(svc *v1.Service, pod *v1.Pod) (*v1.EndpointAddress, error) {
var endpointIP string
ipFamily := v1.IPv4Protocol
if len(svc.Spec.IPFamilies) > 0 {
// controller is connected to an api-server that correctly sets IPFamilies
ipFamily = svc.Spec.IPFamilies[0] // this works for headful and headless
} else {
// controller is connected to an api server that does not correctly
// set IPFamilies (e.g. old api-server during an upgrade)
// TODO (khenidak): remove by when the possibility of upgrading
// from a cluster that does not support dual stack is nil
if len(svc.Spec.ClusterIP) > 0 && svc.Spec.ClusterIP != v1.ClusterIPNone {
// headful service. detect via service clusterIP
if utilnet.IsIPv6String(svc.Spec.ClusterIP) {
ipFamily = v1.IPv6Protocol
}
} else {
// Since this is a headless service we use podIP to identify the family.
// This assumes that status.PodIP is assigned correctly (follows pod cidr and
// pod cidr list order is same as service cidr list order). The expectation is
// this is *most probably* the case.
// if the family was incorrectly identified then this will be corrected once the
// upgrade is completed (controller connects to api-server that correctly defaults services)
if utilnet.IsIPv6String(pod.Status.PodIP) {
ipFamily = v1.IPv6Protocol
}
}
}
wantIPv6 := svc.Spec.IPFamilies[0] == v1.IPv6Protocol
// find an ip that matches the family
for _, podIP := range pod.Status.PodIPs {
if (ipFamily == v1.IPv6Protocol) == utilnet.IsIPv6String(podIP.IP) {
if wantIPv6 == utilnet.IsIPv6String(podIP.IP) {
endpointIP = podIP.IP
break
}

View File

@ -389,6 +389,7 @@ func TestSyncEndpointsWithPodResourceVersionUpdateOnly(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
pod0.ResourceVersion = "3"
@ -474,6 +475,7 @@ func TestSyncEndpointsProtocolTCP(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, TargetPort: intstr.FromInt32(8080), Protocol: "TCP"}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -624,6 +626,7 @@ func TestSyncEndpointsProtocolUDP(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, TargetPort: intstr.FromInt32(8080), Protocol: "UDP"}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -672,6 +675,7 @@ func TestSyncEndpointsProtocolSCTP(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, TargetPort: intstr.FromInt32(8080), Protocol: "SCTP"}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -717,6 +721,7 @@ func TestSyncEndpointsItemsEmptySelectorSelectsAll(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -762,6 +767,7 @@ func TestSyncEndpointsItemsEmptySelectorSelectsAllNotReady(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -807,6 +813,7 @@ func TestSyncEndpointsItemsEmptySelectorSelectsAllMixed(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -855,6 +862,7 @@ func TestSyncEndpointsItemsPreexisting(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -902,6 +910,7 @@ func TestSyncEndpointsItemsPreexistingIdentical(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -928,6 +937,7 @@ func TestSyncEndpointsItems(t *testing.T) {
{Name: "port0", Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)},
{Name: "port1", Port: 88, Protocol: "TCP", TargetPort: intstr.FromInt32(8088)},
},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, "other/foo")
@ -980,6 +990,7 @@ func TestSyncEndpointsItemsWithLabels(t *testing.T) {
{Name: "port0", Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)},
{Name: "port1", Port: 88, Protocol: "TCP", TargetPort: intstr.FromInt32(8088)},
},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -1043,6 +1054,7 @@ func TestSyncEndpointsItemsPreexistingLabelsChange(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -1093,6 +1105,7 @@ func TestWaitsForAllInformersToBeSynced2(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, TargetPort: intstr.FromInt32(8080), Protocol: "TCP"}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
}
endpoints.serviceStore.Add(service)
@ -1144,6 +1157,7 @@ func TestSyncEndpointsHeadlessService(t *testing.T) {
Selector: map[string]string{},
ClusterIP: api.ClusterIPNone,
Ports: []v1.ServicePort{},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
}
originalService := service.DeepCopy()
@ -1198,6 +1212,7 @@ func TestSyncEndpointsItemsExcludeNotReadyPodsWithRestartPolicyNeverAndPhaseFail
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -1242,6 +1257,7 @@ func TestSyncEndpointsItemsExcludeNotReadyPodsWithRestartPolicyNeverAndPhaseSucc
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -1286,6 +1302,7 @@ func TestSyncEndpointsItemsExcludeNotReadyPodsWithRestartPolicyOnFailureAndPhase
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -1319,6 +1336,7 @@ func TestSyncEndpointsHeadlessWithoutPort(t *testing.T) {
Selector: map[string]string{"foo": "bar"},
ClusterIP: "None",
Ports: nil,
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
addPods(endpoints.podStore, ns, 1, 1, 0, ipv4only)
@ -1360,6 +1378,7 @@ func TestPodToEndpointAddressForService(t *testing.T) {
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: "10.0.0.1",
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
},
expectedEndpointFamily: ipv4,
@ -1370,6 +1389,7 @@ func TestPodToEndpointAddressForService(t *testing.T) {
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: "10.0.0.1",
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
},
expectedEndpointFamily: ipv4,
@ -1380,6 +1400,7 @@ func TestPodToEndpointAddressForService(t *testing.T) {
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: "10.0.0.1",
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
},
expectedEndpointFamily: ipv4,
@ -1390,6 +1411,7 @@ func TestPodToEndpointAddressForService(t *testing.T) {
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: v1.ClusterIPNone,
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
},
expectedEndpointFamily: ipv4,
@ -1405,32 +1427,13 @@ func TestPodToEndpointAddressForService(t *testing.T) {
},
expectedEndpointFamily: ipv4,
},
{
name: "v4 legacy headless service, in a dual stack cluster",
ipFamilies: ipv4ipv6,
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: v1.ClusterIPNone,
},
},
expectedEndpointFamily: ipv4,
},
{
name: "v4 legacy headless service, in a dual stack ipv6-primary cluster",
ipFamilies: ipv6ipv4,
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: v1.ClusterIPNone,
},
},
expectedEndpointFamily: ipv6,
},
{
name: "v6 service, in a dual stack cluster",
ipFamilies: ipv4ipv6,
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: "3000::1",
IPFamilies: []v1.IPFamily{v1.IPv6Protocol},
},
},
expectedEndpointFamily: ipv6,
@ -1441,12 +1444,13 @@ func TestPodToEndpointAddressForService(t *testing.T) {
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: v1.ClusterIPNone,
IPFamilies: []v1.IPFamily{v1.IPv6Protocol},
},
},
expectedEndpointFamily: ipv6,
},
{
name: "v6 headless service, in a dual stack cluster (connected to a new api-server)",
name: "v6 headless service, in a dual stack cluster",
ipFamilies: ipv4ipv6,
service: v1.Service{
Spec: v1.ServiceSpec{
@ -1456,39 +1460,13 @@ func TestPodToEndpointAddressForService(t *testing.T) {
},
expectedEndpointFamily: ipv6,
},
{
name: "v6 legacy headless service, in a dual stack cluster (connected to a old api-server)",
ipFamilies: ipv4ipv6,
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: v1.ClusterIPNone, // <- families are not set by api-server
},
},
expectedEndpointFamily: ipv4,
},
// in reality this is a misconfigured cluster
// i.e user is not using dual stack and have PodIP == v4 and ServiceIP==v6
// previously controller could assign wrong ip to endpoint address
// with gate removed. this is no longer the case. this is *not* behavior change
// because previously things would have failed in kube-proxy anyway (due to editing wrong iptables).
{
name: "v6 service, in a v4 only cluster.",
ipFamilies: ipv4only,
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: "3000::1",
},
},
expectError: true,
expectedEndpointFamily: ipv4,
},
// but this will actually give an error
{
name: "v6 service, in a v4 only cluster",
ipFamilies: ipv4only,
service: v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: "3000::1",
IPFamilies: []v1.IPFamily{v1.IPv6Protocol},
},
},
expectError: true,
@ -1568,6 +1546,7 @@ func TestLastTriggerChangeTimeAnnotation(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, TargetPort: intstr.FromInt32(8080), Protocol: "TCP"}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -1623,6 +1602,7 @@ func TestLastTriggerChangeTimeAnnotation_AnnotationOverridden(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, TargetPort: intstr.FromInt32(8080), Protocol: "TCP"}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -1679,6 +1659,7 @@ func TestLastTriggerChangeTimeAnnotation_AnnotationCleared(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{},
Ports: []v1.ServicePort{{Port: 80, TargetPort: intstr.FromInt32(8080), Protocol: "TCP"}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
err := endpoints.syncService(tCtx, ns+"/foo")
@ -1822,6 +1803,7 @@ func TestPodUpdatesBatching(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
@ -1943,6 +1925,7 @@ func TestPodAddsBatching(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
@ -2067,6 +2050,7 @@ func TestPodDeleteBatching(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
@ -2209,6 +2193,7 @@ func TestSyncServiceOverCapacity(t *testing.T) {
Spec: v1.ServiceSpec{
Selector: map[string]string{"foo": "bar"},
Ports: []v1.ServicePort{{Port: 80}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
}
c.serviceStore.Add(svc)
@ -2408,6 +2393,7 @@ func TestMultipleServiceChanges(t *testing.T) {
Selector: map[string]string{"foo": "bar"},
ClusterIP: "None",
Ports: nil,
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
}
@ -2480,6 +2466,7 @@ func TestMultiplePodChanges(t *testing.T) {
Selector: map[string]string{"foo": "bar"},
ClusterIP: "10.0.0.1",
Ports: []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
})
@ -2525,6 +2512,7 @@ func TestSyncServiceAddresses(t *testing.T) {
Type: v1.ServiceTypeClusterIP,
ClusterIP: "1.1.1.1",
Ports: []v1.ServicePort{{Port: 80}},
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
}
}