Repository: submariner-io/lighthouse
Pull request #1111 (2023-02-27 15:00:10)

Ignore "not ready" addresses for EndpointSlices
Fixes https://github.com/submariner-io/lighthouse/issues/1090



submariner-bot: 🤖 Created branch: z_pr1111/tpantelis/not_ready_addrs 
🚀 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 fb62001..950979e 100644
--- a/coredns/endpointslice/map.go
+++ b/coredns/endpointslice/map.go
@@ -178,6 +178,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 8fc26b5..6f5e755 100644
--- a/pkg/agent/controller/endpoint.go
+++ b/pkg/agent/controller/endpoint.go
@@ -162,7 +162,7 @@ func (e *EndpointController) endpointSliceFromEndpoints(endpoints *corev1.Endpoi
 			})
 		}
 
-		if allAddressesIPv6(append(subset.Addresses, subset.NotReadyAddresses...)) {
+		if allAddressesIPv6(subset.Addresses) {
 			endpointSlice.AddressType = discovery.AddressTypeIPv6
 		}
 
@@ -172,14 +172,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 f975c42..3cabb3a 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"
 	mcsv1a1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1"
 )
 
@@ -162,6 +163,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 2ae8f9f..ffee3c5 100644
--- a/pkg/agent/controller/controller_suite_test.go
+++ b/pkg/agent/controller/controller_suite_test.go
@@ -66,7 +66,6 @@ var (
 	nodeName = "my-node"
 	hostName = "my-host"
 	ready    = true
-	notReady = false
 )
 
 func init() {
@@ -485,11 +484,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},
@@ -501,11 +499,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 3938a48..245d74a 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())
 		})
 	})