From e282b6c6b3010b4f9e3390d4df82aa4633836436 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Tue, 25 Oct 2022 21:47:28 -0400 Subject: [PATCH] pkg/controller/endpointslice: remove all references to the EndpointSliceTerminatingCondition feature gate Signed-off-by: Andrew Sy Kim --- .../endpointslice_controller_test.go | 242 +-------------- pkg/controller/endpointslice/reconciler.go | 5 +- .../endpointslice/reconciler_test.go | 162 +++++----- pkg/controller/endpointslice/utils.go | 13 +- pkg/controller/endpointslice/utils_test.go | 289 ++++++++---------- .../util/endpoint/controller_utils.go | 9 +- .../util/endpoint/controller_utils_test.go | 2 +- 7 files changed, 247 insertions(+), 475 deletions(-) diff --git a/pkg/controller/endpointslice/endpointslice_controller_test.go b/pkg/controller/endpointslice/endpointslice_controller_test.go index 589fc642d7b..aaf933e685b 100644 --- a/pkg/controller/endpointslice/endpointslice_controller_test.go +++ b/pkg/controller/endpointslice/endpointslice_controller_test.go @@ -35,17 +35,14 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" - featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/endpointslice/topologycache" endpointutil "k8s.io/kubernetes/pkg/controller/util/endpoint" endpointsliceutil "k8s.io/kubernetes/pkg/controller/util/endpointslice" - "k8s.io/kubernetes/pkg/features" utilpointer "k8s.io/utils/pointer" ) @@ -418,12 +415,11 @@ func TestSyncService(t *testing.T) { deletionTimestamp := metav1.Now() testcases := []struct { - name string - service *v1.Service - pods []*v1.Pod - expectedEndpointPorts []discovery.EndpointPort - expectedEndpoints []discovery.Endpoint - terminatingGateEnabled bool + name string + service *v1.Service + pods []*v1.Pod + expectedEndpointPorts []discovery.EndpointPort + expectedEndpoints []discovery.Endpoint }{ { name: "pods with multiple IPs and Service with ipFamilies=ipv4", @@ -522,7 +518,9 @@ func TestSyncService(t *testing.T) { expectedEndpoints: []discovery.Endpoint{ { Conditions: discovery.EndpointConditions{ - Ready: utilpointer.BoolPtr(true), + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), }, Addresses: []string{"10.0.0.1"}, TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod0"}, @@ -530,7 +528,9 @@ func TestSyncService(t *testing.T) { }, { Conditions: discovery.EndpointConditions{ - Ready: utilpointer.BoolPtr(true), + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), }, Addresses: []string{"10.0.0.2"}, TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod1"}, @@ -635,7 +635,9 @@ func TestSyncService(t *testing.T) { expectedEndpoints: []discovery.Endpoint{ { Conditions: discovery.EndpointConditions{ - Ready: utilpointer.BoolPtr(true), + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), }, Addresses: []string{"fd08::5678:0000:0000:9abc:def0"}, TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod1"}, @@ -644,7 +646,7 @@ func TestSyncService(t *testing.T) { }, }, { - name: "Terminating pods with EndpointSliceTerminatingCondition enabled", + name: "Terminating pods", service: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "foobar", @@ -757,114 +759,9 @@ func TestSyncService(t *testing.T) { NodeName: utilpointer.StringPtr("node-1"), }, }, - terminatingGateEnabled: true, }, { - name: "Terminating pods with EndpointSliceTerminatingCondition disabled", - service: &v1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foobar", - Namespace: "default", - CreationTimestamp: creationTimestamp, - }, - Spec: v1.ServiceSpec{ - Ports: []v1.ServicePort{ - {Name: "tcp-example", TargetPort: intstr.FromInt(80), Protocol: v1.ProtocolTCP}, - {Name: "udp-example", TargetPort: intstr.FromInt(161), Protocol: v1.ProtocolUDP}, - {Name: "sctp-example", TargetPort: intstr.FromInt(3456), Protocol: v1.ProtocolSCTP}, - }, - Selector: map[string]string{"foo": "bar"}, - IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, - }, - }, - pods: []*v1.Pod{ - { - // one ready pod for comparison - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "pod0", - Labels: map[string]string{"foo": "bar"}, - DeletionTimestamp: nil, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{ - Name: "container-1", - }}, - NodeName: "node-1", - }, - Status: v1.PodStatus{ - PodIP: "10.0.0.1", - PodIPs: []v1.PodIP{{ - IP: "10.0.0.1", - }}, - Conditions: []v1.PodCondition{ - { - Type: v1.PodReady, - Status: v1.ConditionTrue, - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "pod1", - Labels: map[string]string{"foo": "bar"}, - DeletionTimestamp: &deletionTimestamp, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{ - Name: "container-1", - }}, - NodeName: "node-1", - }, - Status: v1.PodStatus{ - PodIP: "10.0.0.2", - PodIPs: []v1.PodIP{ - { - IP: "10.0.0.2", - }, - }, - Conditions: []v1.PodCondition{ - { - Type: v1.PodReady, - Status: v1.ConditionTrue, - }, - }, - }, - }, - }, - expectedEndpointPorts: []discovery.EndpointPort{ - { - Name: utilpointer.StringPtr("sctp-example"), - Protocol: protoPtr(v1.ProtocolSCTP), - Port: utilpointer.Int32Ptr(int32(3456)), - }, - { - Name: utilpointer.StringPtr("udp-example"), - Protocol: protoPtr(v1.ProtocolUDP), - Port: utilpointer.Int32Ptr(int32(161)), - }, - { - Name: utilpointer.StringPtr("tcp-example"), - Protocol: protoPtr(v1.ProtocolTCP), - Port: utilpointer.Int32Ptr(int32(80)), - }, - }, - expectedEndpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Ready: utilpointer.BoolPtr(true), - }, - Addresses: []string{"10.0.0.1"}, - TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod0"}, - NodeName: utilpointer.StringPtr("node-1"), - }, - }, - terminatingGateEnabled: false, - }, - { - name: "Not ready terminating pods with EndpointSliceTerminatingCondition enabled", + name: "Not ready terminating pods", service: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "foobar", @@ -977,118 +874,11 @@ func TestSyncService(t *testing.T) { NodeName: utilpointer.StringPtr("node-1"), }, }, - terminatingGateEnabled: true, - }, - { - name: "Not ready terminating pods with EndpointSliceTerminatingCondition disabled", - service: &v1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foobar", - Namespace: "default", - CreationTimestamp: creationTimestamp, - }, - Spec: v1.ServiceSpec{ - Ports: []v1.ServicePort{ - {Name: "tcp-example", TargetPort: intstr.FromInt(80), Protocol: v1.ProtocolTCP}, - {Name: "udp-example", TargetPort: intstr.FromInt(161), Protocol: v1.ProtocolUDP}, - {Name: "sctp-example", TargetPort: intstr.FromInt(3456), Protocol: v1.ProtocolSCTP}, - }, - Selector: map[string]string{"foo": "bar"}, - IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, - }, - }, - pods: []*v1.Pod{ - { - // one ready pod for comparison - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "pod0", - Labels: map[string]string{"foo": "bar"}, - DeletionTimestamp: nil, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{ - Name: "container-1", - }}, - NodeName: "node-1", - }, - Status: v1.PodStatus{ - PodIP: "10.0.0.1", - PodIPs: []v1.PodIP{{ - IP: "10.0.0.1", - }}, - Conditions: []v1.PodCondition{ - { - Type: v1.PodReady, - Status: v1.ConditionTrue, - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "pod1", - Labels: map[string]string{"foo": "bar"}, - DeletionTimestamp: &deletionTimestamp, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{{ - Name: "container-1", - }}, - NodeName: "node-1", - }, - Status: v1.PodStatus{ - PodIP: "10.0.0.2", - PodIPs: []v1.PodIP{ - { - IP: "10.0.0.2", - }, - }, - Conditions: []v1.PodCondition{ - { - Type: v1.PodReady, - Status: v1.ConditionFalse, - }, - }, - }, - }, - }, - expectedEndpointPorts: []discovery.EndpointPort{ - { - Name: utilpointer.StringPtr("sctp-example"), - Protocol: protoPtr(v1.ProtocolSCTP), - Port: utilpointer.Int32Ptr(int32(3456)), - }, - { - Name: utilpointer.StringPtr("udp-example"), - Protocol: protoPtr(v1.ProtocolUDP), - Port: utilpointer.Int32Ptr(int32(161)), - }, - { - Name: utilpointer.StringPtr("tcp-example"), - Protocol: protoPtr(v1.ProtocolTCP), - Port: utilpointer.Int32Ptr(int32(80)), - }, - }, - expectedEndpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Ready: utilpointer.BoolPtr(true), - }, - Addresses: []string{"10.0.0.1"}, - TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod0"}, - NodeName: utilpointer.StringPtr("node-1"), - }, - }, - terminatingGateEnabled: false, }, } for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EndpointSliceTerminatingCondition, testcase.terminatingGateEnabled)() - client, esController := newController([]string{"node-1"}, time.Duration(0)) for _, pod := range testcase.pods { diff --git a/pkg/controller/endpointslice/reconciler.go b/pkg/controller/endpointslice/reconciler.go index 160fca2bf3c..f97d3ccf43d 100644 --- a/pkg/controller/endpointslice/reconciler.go +++ b/pkg/controller/endpointslice/reconciler.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" - utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" @@ -38,7 +37,6 @@ import ( "k8s.io/kubernetes/pkg/controller/endpointslice/topologycache" endpointutil "k8s.io/kubernetes/pkg/controller/util/endpoint" endpointsliceutil "k8s.io/kubernetes/pkg/controller/util/endpointslice" - "k8s.io/kubernetes/pkg/features" ) // reconciler is responsible for transforming current EndpointSlice state into @@ -154,8 +152,7 @@ func (r *reconciler) reconcileByAddressType(service *corev1.Service, pods []*cor desiredEndpointsByPortMap := map[endpointutil.PortMapKey]endpointsliceutil.EndpointSet{} for _, pod := range pods { - includeTerminating := service.Spec.PublishNotReadyAddresses || utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceTerminatingCondition) - if !endpointutil.ShouldPodBeInEndpoints(pod, includeTerminating) { + if !endpointutil.ShouldPodBeInEndpoints(pod, true) { continue } diff --git a/pkg/controller/endpointslice/reconciler_test.go b/pkg/controller/endpointslice/reconciler_test.go index ff5f99abe2e..c2db2fe18f9 100644 --- a/pkg/controller/endpointslice/reconciler_test.go +++ b/pkg/controller/endpointslice/reconciler_test.go @@ -32,18 +32,15 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" corelisters "k8s.io/client-go/listers/core/v1" k8stesting "k8s.io/client-go/testing" - featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-base/metrics/testutil" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/endpointslice/metrics" "k8s.io/kubernetes/pkg/controller/endpointslice/topologycache" endpointsliceutil "k8s.io/kubernetes/pkg/controller/util/endpointslice" - "k8s.io/kubernetes/pkg/features" utilpointer "k8s.io/utils/pointer" ) @@ -119,17 +116,20 @@ func TestReconcile1Pod(t *testing.T) { expectedEndpoint discovery.Endpoint expectedLabels map[string]string expectedEndpointPerSlice map[discovery.AddressType][]discovery.Endpoint - terminatingGateEnabled bool }{ "no-family-service": { service: noFamilyService, expectedEndpointPerSlice: map[discovery.AddressType][]discovery.Endpoint{ discovery.AddressTypeIPv4: { { - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1.2.3.4"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -144,29 +144,6 @@ func TestReconcile1Pod(t *testing.T) { }, }, "ipv4": { - service: svcv4, - expectedEndpointPerSlice: map[discovery.AddressType][]discovery.Endpoint{ - discovery.AddressTypeIPv4: { - { - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), - TargetRef: &corev1.ObjectReference{ - Kind: "Pod", - Namespace: namespace, - Name: "pod1", - }, - }, - }, - }, - expectedLabels: map[string]string{ - discovery.LabelManagedBy: controllerName, - discovery.LabelServiceName: "foo", - corev1.IsHeadlessService: "", - }, - }, - "ipv4-with-terminating-gate-enabled": { service: svcv4, expectedEndpointPerSlice: map[discovery.AddressType][]discovery.Endpoint{ discovery.AddressTypeIPv4: { @@ -192,17 +169,20 @@ func TestReconcile1Pod(t *testing.T) { discovery.LabelServiceName: "foo", corev1.IsHeadlessService: "", }, - terminatingGateEnabled: true, }, "ipv4-clusterip": { service: svcv4ClusterIP, expectedEndpointPerSlice: map[discovery.AddressType][]discovery.Endpoint{ discovery.AddressTypeIPv4: { { - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1.2.3.4"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -213,10 +193,14 @@ func TestReconcile1Pod(t *testing.T) { }, expectedAddressType: discovery.AddressTypeIPv4, expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1.2.3.4"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -233,10 +217,14 @@ func TestReconcile1Pod(t *testing.T) { expectedEndpointPerSlice: map[discovery.AddressType][]discovery.Endpoint{ discovery.AddressTypeIPv4: { { - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1.2.3.4"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -247,10 +235,14 @@ func TestReconcile1Pod(t *testing.T) { }, expectedAddressType: discovery.AddressTypeIPv4, expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1.2.3.4"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -269,10 +261,14 @@ func TestReconcile1Pod(t *testing.T) { expectedEndpointPerSlice: map[discovery.AddressType][]discovery.Endpoint{ discovery.AddressTypeIPv4: { { - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1.2.3.4"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -283,10 +279,14 @@ func TestReconcile1Pod(t *testing.T) { }, expectedAddressType: discovery.AddressTypeIPv4, expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1.2.3.4"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -305,10 +305,14 @@ func TestReconcile1Pod(t *testing.T) { expectedEndpointPerSlice: map[discovery.AddressType][]discovery.Endpoint{ discovery.AddressTypeIPv6: { { - Addresses: []string{"1234::5678:0000:0000:9abc:def0"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1234::5678:0000:0000:9abc:def0"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -329,10 +333,14 @@ func TestReconcile1Pod(t *testing.T) { expectedEndpointPerSlice: map[discovery.AddressType][]discovery.Endpoint{ discovery.AddressTypeIPv6: { { - Addresses: []string{"1234::5678:0000:0000:9abc:def0"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1234::5678:0000:0000:9abc:def0"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -352,10 +360,14 @@ func TestReconcile1Pod(t *testing.T) { expectedEndpointPerSlice: map[discovery.AddressType][]discovery.Endpoint{ discovery.AddressTypeIPv6: { { - Addresses: []string{"1234::5678:0000:0000:9abc:def0"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1234::5678:0000:0000:9abc:def0"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -365,10 +377,14 @@ func TestReconcile1Pod(t *testing.T) { }, discovery.AddressTypeIPv4: { { - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), + Addresses: []string{"1.2.3.4"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), TargetRef: &corev1.ObjectReference{ Kind: "Pod", Namespace: namespace, @@ -386,8 +402,6 @@ func TestReconcile1Pod(t *testing.T) { for name, testCase := range testCases { t.Run(name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EndpointSliceTerminatingCondition, testCase.terminatingGateEnabled)() - client := newClientset() setupMetrics() triggerTime := time.Now().UTC() diff --git a/pkg/controller/endpointslice/utils.go b/pkg/controller/endpointslice/utils.go index 742427a0289..448ae9fb347 100644 --- a/pkg/controller/endpointslice/utils.go +++ b/pkg/controller/endpointslice/utils.go @@ -20,14 +20,13 @@ import ( "fmt" "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" podutil "k8s.io/kubernetes/pkg/api/v1/pod" @@ -35,7 +34,6 @@ import ( "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/apis/discovery/validation" endpointutil "k8s.io/kubernetes/pkg/controller/util/endpoint" - "k8s.io/kubernetes/pkg/features" utilnet "k8s.io/utils/net" ) @@ -49,7 +47,9 @@ func podToEndpoint(pod *v1.Pod, node *v1.Node, service *v1.Service, addressType ep := discovery.Endpoint{ Addresses: getEndpointAddresses(pod.Status, service, addressType), Conditions: discovery.EndpointConditions{ - Ready: &ready, + Ready: &ready, + Serving: &serving, + Terminating: &terminating, }, TargetRef: &v1.ObjectReference{ Kind: "Pod", @@ -59,11 +59,6 @@ func podToEndpoint(pod *v1.Pod, node *v1.Node, service *v1.Service, addressType }, } - if utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceTerminatingCondition) { - ep.Conditions.Serving = &serving - ep.Conditions.Terminating = &terminating - } - if pod.Spec.NodeName != "" { ep.NodeName = &pod.Spec.NodeName } diff --git a/pkg/controller/endpointslice/utils_test.go b/pkg/controller/endpointslice/utils_test.go index 22c70964891..ee96ec9b077 100644 --- a/pkg/controller/endpointslice/utils_test.go +++ b/pkg/controller/endpointslice/utils_test.go @@ -31,11 +31,8 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/rand" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/features" utilpointer "k8s.io/utils/pointer" ) @@ -251,131 +248,11 @@ func TestPodToEndpoint(t *testing.T) { svc *v1.Service expectedEndpoint discovery.Endpoint publishNotReadyAddresses bool - terminatingGateEnabled bool }{ { name: "Ready pod", pod: readyPod, svc: &svc, - expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.5"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - NodeName: utilpointer.StringPtr("node-1"), - TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - }, - }, - }, - { - name: "Ready pod + publishNotReadyAddresses", - pod: readyPod, - svc: &svcPublishNotReady, - expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.5"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - NodeName: utilpointer.StringPtr("node-1"), - TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - }, - }, - }, - { - name: "Unready pod", - pod: unreadyPod, - svc: &svc, - expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.5"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(false)}, - NodeName: utilpointer.StringPtr("node-1"), - TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - }, - }, - }, - { - name: "Unready pod + publishNotReadyAddresses", - pod: unreadyPod, - svc: &svcPublishNotReady, - expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.5"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - NodeName: utilpointer.StringPtr("node-1"), - TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - }, - }, - }, - { - name: "Ready pod + node labels", - pod: readyPod, - node: node1, - svc: &svc, - expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.5"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), - TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - }, - }, - }, - { - name: "Multi IP Ready pod + node labels", - pod: multiIPPod, - node: node1, - svc: &svc, - expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.4"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), - TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - }, - }, - }, - { - name: "Ready pod + hostname", - pod: readyPodHostname, - node: node1, - svc: &svc, - expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.5"}, - Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, - Hostname: &readyPodHostname.Spec.Hostname, - Zone: utilpointer.StringPtr("us-central1-a"), - NodeName: utilpointer.StringPtr("node-1"), - TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPodHostname.Name, - UID: readyPodHostname.UID, - }, - }, - }, - { - name: "Ready pod, terminating gate enabled", - pod: readyPod, - svc: &svc, expectedEndpoint: discovery.Endpoint{ Addresses: []string{"1.2.3.5"}, Conditions: discovery.EndpointConditions{ @@ -391,16 +268,17 @@ func TestPodToEndpoint(t *testing.T) { UID: readyPod.UID, }, }, - terminatingGateEnabled: true, }, { - name: "Ready terminating pod, terminating gate disabled", - pod: readyTerminatingPod, - svc: &svc, + name: "Ready pod + publishNotReadyAddresses", + pod: readyPod, + svc: &svcPublishNotReady, expectedEndpoint: discovery.Endpoint{ Addresses: []string{"1.2.3.5"}, Conditions: discovery.EndpointConditions{ - Ready: utilpointer.BoolPtr(false), + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), }, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ @@ -410,10 +288,136 @@ func TestPodToEndpoint(t *testing.T) { UID: readyPod.UID, }, }, - terminatingGateEnabled: false, }, { - name: "Ready terminating pod, terminating gate enabled", + name: "Unready pod", + pod: unreadyPod, + svc: &svc, + expectedEndpoint: discovery.Endpoint{ + Addresses: []string{"1.2.3.5"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(false), + Serving: utilpointer.BoolPtr(false), + Terminating: utilpointer.BoolPtr(false), + }, + NodeName: utilpointer.StringPtr("node-1"), + TargetRef: &v1.ObjectReference{ + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, + }, + }, + }, + { + name: "Unready pod + publishNotReadyAddresses", + pod: unreadyPod, + svc: &svcPublishNotReady, + expectedEndpoint: discovery.Endpoint{ + Addresses: []string{"1.2.3.5"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(false), + Terminating: utilpointer.BoolPtr(false), + }, + NodeName: utilpointer.StringPtr("node-1"), + TargetRef: &v1.ObjectReference{ + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, + }, + }, + }, + { + name: "Ready pod + node labels", + pod: readyPod, + node: node1, + svc: &svc, + expectedEndpoint: discovery.Endpoint{ + Addresses: []string{"1.2.3.5"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), + TargetRef: &v1.ObjectReference{ + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, + }, + }, + }, + { + name: "Multi IP Ready pod + node labels", + pod: multiIPPod, + node: node1, + svc: &svc, + expectedEndpoint: discovery.Endpoint{ + Addresses: []string{"1.2.3.4"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), + TargetRef: &v1.ObjectReference{ + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, + }, + }, + }, + { + name: "Ready pod + hostname", + pod: readyPodHostname, + node: node1, + svc: &svc, + expectedEndpoint: discovery.Endpoint{ + Addresses: []string{"1.2.3.5"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + Hostname: &readyPodHostname.Spec.Hostname, + Zone: utilpointer.StringPtr("us-central1-a"), + NodeName: utilpointer.StringPtr("node-1"), + TargetRef: &v1.ObjectReference{ + Kind: "Pod", + Namespace: ns, + Name: readyPodHostname.Name, + UID: readyPodHostname.UID, + }, + }, + }, + { + name: "Ready pod", + pod: readyPod, + svc: &svc, + expectedEndpoint: discovery.Endpoint{ + Addresses: []string{"1.2.3.5"}, + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + Serving: utilpointer.BoolPtr(true), + Terminating: utilpointer.BoolPtr(false), + }, + NodeName: utilpointer.StringPtr("node-1"), + TargetRef: &v1.ObjectReference{ + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, + }, + }, + }, + { + name: "Ready terminating pod", pod: readyTerminatingPod, svc: &svc, expectedEndpoint: discovery.Endpoint{ @@ -431,29 +435,9 @@ func TestPodToEndpoint(t *testing.T) { UID: readyPod.UID, }, }, - terminatingGateEnabled: true, }, { - name: "Not ready terminating pod, terminating gate disabled", - pod: unreadyTerminatingPod, - svc: &svc, - expectedEndpoint: discovery.Endpoint{ - Addresses: []string{"1.2.3.5"}, - Conditions: discovery.EndpointConditions{ - Ready: utilpointer.BoolPtr(false), - }, - NodeName: utilpointer.StringPtr("node-1"), - TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - }, - }, - terminatingGateEnabled: false, - }, - { - name: "Not ready terminating pod, terminating gate enabled", + name: "Not ready terminating pod", pod: unreadyTerminatingPod, svc: &svc, expectedEndpoint: discovery.Endpoint{ @@ -471,14 +455,11 @@ func TestPodToEndpoint(t *testing.T) { UID: readyPod.UID, }, }, - terminatingGateEnabled: true, }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EndpointSliceTerminatingCondition, testCase.terminatingGateEnabled)() - endpoint := podToEndpoint(testCase.pod, testCase.node, testCase.svc, discovery.AddressTypeIPv4) if !reflect.DeepEqual(testCase.expectedEndpoint, endpoint) { t.Errorf("Expected endpoint: %+v, got: %+v", testCase.expectedEndpoint, endpoint) diff --git a/pkg/controller/util/endpoint/controller_utils.go b/pkg/controller/util/endpoint/controller_utils.go index addaccc9d68..24410bf8fa0 100644 --- a/pkg/controller/util/endpoint/controller_utils.go +++ b/pkg/controller/util/endpoint/controller_utils.go @@ -306,16 +306,11 @@ func EndpointsEqualBeyondHash(ep1, ep2 *discovery.Endpoint) bool { return false } - // Serving and Terminating will only be set when the EndpointSliceTerminatingCondition feature is on. - // Ignore their difference if the expected or actual value is nil, which means the feature enablement is changed. - // Otherwise all EndpointSlices in the system would be updated on the first controller-manager restart even without - // actual changes, leading to delay in processing legitimate updates. - // Its value will be set to the expected one when there is an actual change triggering update of this EndpointSlice. - if ep1.Conditions.Serving != nil && ep2.Conditions.Serving != nil && *ep1.Conditions.Serving != *ep2.Conditions.Serving { + if boolPtrChanged(ep1.Conditions.Serving, ep2.Conditions.Serving) { return false } - if ep1.Conditions.Terminating != nil && ep2.Conditions.Terminating != nil && *ep1.Conditions.Terminating != *ep2.Conditions.Terminating { + if boolPtrChanged(ep1.Conditions.Terminating, ep2.Conditions.Terminating) { return false } diff --git a/pkg/controller/util/endpoint/controller_utils_test.go b/pkg/controller/util/endpoint/controller_utils_test.go index e9656b0a8dd..c06adb20c93 100644 --- a/pkg/controller/util/endpoint/controller_utils_test.go +++ b/pkg/controller/util/endpoint/controller_utils_test.go @@ -776,7 +776,7 @@ func TestEndpointsEqualBeyondHash(t *testing.T) { Zone: utilpointer.StringPtr("zone-1"), NodeName: utilpointer.StringPtr("node-1"), }, - expected: true, + expected: false, }, { name: "Serving condition changed from false to true",