Repository: submariner-io/lighthouse
Pull request #1114 (2023-02-28 15:15:37)

Automated backport of #1111: Ignore "not ready" addresses for EndpointSlices
Backport of #1111 on release-0.14.

#1111: Ignore "not ready" addresses for EndpointSlices

For details on the backport process, see the [backport requests](https://submariner.io/development/backports/) page.

submariner-bot: 🤖 Created branch: z_pr1114/tpantelis/automated-backport-of-#1111-upstream-release-0.14 
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

Patch:
diff --git a/coredns/endpointslice/map.go b/coredns/endpointslice/map.go
index 0d877bf..61922eb 100644
--- a/coredns/endpointslice/map.go
+++ b/coredns/endpointslice/map.go
@@ -186,6 +186,12 @@ func (m *Map) Put(es *discovery.EndpointSlice) {
 	}
 
 	for _, endpoint := range es.Endpoints {
+		// Skip if not ready. Note: we're treating nil as ready to be on the safe side as the EndpointConditions doc
+		// states "In most cases consumers should interpret this unknown state (ie nil) as ready".
+		if endpoint.Conditions.Ready != nil && !*endpoint.Conditions.Ready {
+			continue
+		}
+
 		var records []serviceimport.DNSRecord
 
 		addresses := endpoint.Addresses
diff --git a/pkg/agent/controller/endpoint.go b/pkg/agent/controller/endpoint.go
index a8cae2d..cfa2b69 100644
--- a/pkg/agent/controller/endpoint.go
+++ b/pkg/agent/controller/endpoint.go
@@ -176,7 +176,7 @@ func (e *EndpointController) endpointSliceFromEndpoints(endpoints *corev1.Endpoi
 			})
 		}
 
-		if allAddressesIPv6(append(subset.Addresses, subset.NotReadyAddresses...)) {
+		if allAddressesIPv6(subset.Addresses) {
 			endpointSlice.AddressType = discovery.AddressTypeIPv6
 		}
 
@@ -186,14 +186,6 @@ func (e *EndpointController) endpointSliceFromEndpoints(endpoints *corev1.Endpoi
 		}
 
 		endpointSlice.Endpoints = append(endpointSlice.Endpoints, newEndpoints...)
-
-		newEndpoints, retry = e.getEndpointsFromAddresses(subset.NotReadyAddresses, endpointSlice.AddressType, false)
-		if retry {
-			// TODO: We may not want unready endpoints at all
-			return nil, true
-		}
-
-		endpointSlice.Endpoints = append(endpointSlice.Endpoints, newEndpoints...)
 	}
 
 	if op == syncer.Create {


Test patch:
diff --git a/coredns/endpointslice/map_test.go b/coredns/endpointslice/map_test.go
index 94991f4..d08a8c4 100644
--- a/coredns/endpointslice/map_test.go
+++ b/coredns/endpointslice/map_test.go
@@ -29,6 +29,7 @@ import (
 	discovery "k8s.io/api/discovery/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	fakeKubeClient "k8s.io/client-go/kubernetes/fake"
+	"k8s.io/utils/pointer"
 )
 
 var _ = Describe("EndpointSlice Map", func() {
@@ -161,6 +162,12 @@ func newEndpointSlice(namespace, name, clusterID string, endpointIPs []string) *
 			{
 				Addresses: endpointIPs,
 			},
+			{
+				Addresses: []string{"1.2.3.4"},
+				Conditions: discovery.EndpointConditions{
+					Ready: pointer.Bool(false),
+				},
+			},
 		},
 	}
 }
diff --git a/pkg/agent/controller/controller_suite_test.go b/pkg/agent/controller/controller_suite_test.go
index d1f6190..8005f48 100644
--- a/pkg/agent/controller/controller_suite_test.go
+++ b/pkg/agent/controller/controller_suite_test.go
@@ -68,7 +68,6 @@ var (
 	nodeName = "my-node"
 	hostName = "my-host"
 	ready    = true
-	notReady = false
 )
 
 func init() {
@@ -493,11 +492,10 @@ func awaitAndVerifyEndpointSlice(endpointSliceClient dynamic.ResourceInterface,
 	if addresses == nil {
 		addresses = []string{
 			endpoints.Subsets[0].Addresses[0].IP, endpoints.Subsets[0].Addresses[1].IP,
-			endpoints.Subsets[0].NotReadyAddresses[0].IP,
 		}
 	}
 
-	Expect(endpointSlice.Endpoints).To(HaveLen(3))
+	Expect(endpointSlice.Endpoints).To(HaveLen(2))
 	Expect(endpointSlice.Endpoints[0]).To(Equal(discovery.Endpoint{
 		Addresses:  []string{addresses[0]},
 		Conditions: discovery.EndpointConditions{Ready: &ready},
@@ -509,11 +507,6 @@ func awaitAndVerifyEndpointSlice(endpointSliceClient dynamic.ResourceInterface,
 		Conditions: discovery.EndpointConditions{Ready: &ready},
 		NodeName:   &nodeName,
 	}))
-	Expect(endpointSlice.Endpoints[2]).To(Equal(discovery.Endpoint{
-		Addresses:  []string{addresses[2]},
-		Hostname:   &endpoints.Subsets[0].NotReadyAddresses[0].TargetRef.Name,
-		Conditions: discovery.EndpointConditions{Ready: &notReady},
-	}))
 
 	Expect(endpointSlice.Ports).To(HaveLen(1))
 
diff --git a/pkg/agent/controller/headless_service_test.go b/pkg/agent/controller/headless_service_test.go
index 649e01a..f9e8c66 100644
--- a/pkg/agent/controller/headless_service_test.go
+++ b/pkg/agent/controller/headless_service_test.go
@@ -74,7 +74,7 @@ var _ = Describe("Headless service syncing", func() {
 			t.endpoints.Subsets[0].Addresses = append(t.endpoints.Subsets[0].Addresses, corev1.EndpointAddress{IP: "192.168.5.3"})
 			t.updateEndpoints()
 			t.awaitUpdatedServiceImport("")
-			t.awaitUpdatedEndpointSlice(append(t.endpointIPs(), "10.253.6.1"))
+			t.awaitUpdatedEndpointSlice(t.endpointIPs())
 		})
 	})