Move nil EndpointSlice conditions in isReady (#4136)

Move nil EndpointSlice conditions handling into
`EndpointSliceEndpoints.isReady()`.


EndpointSlice endpoints with missing conditions is already treated as
ready. So instead of having this check outside isReady moved it within.

This makes the `isReady` function handle all cases and avoids requiring
future callers to duplicate the nil-condition check.

## Tests

```bash
go test ./dataclients/kubernetes -run TestEndpointSliceEndpointIsReady
go test ./dataclients/kubernetes
```

Suggested label: refactor or bugfix (actually not a bug)


Fixes #4137

Signed-off-by: shrikant23codes <jiturishrikant@gmail.com>

Author: shrikant23codes

dataclients/kubernetes/clusterclient.go

@@ -571,12 +571,6 @@ func collectReadyEndpoints(endpointSlices *endpointSliceList) map[definitions.Re
 					// we should delete it, because of eventual consistency
 					// it is actually terminating
 					delete(resEps, address)
-				} else if ep.Conditions == nil {
-					// if conditions are nil then we need to treat is as ready
-					resEps[address] = &skipperEndpoint{
-						Address: address,
-						Zone:    ep.Zone,
-					}
 				} else if ep.isReady() {
 					resEps[address] = &skipperEndpoint{
 						Address: address,

dataclients/kubernetes/endpointslices.go

@@ -157,5 +157,5 @@ func (ep *EndpointSliceEndpoints) isReady() bool {
 	}
 	// defaults to ready, see also https://github.com/kubernetes/kubernetes/blob/91aca10d5984313c1c5858979d4946ff9446615f/pkg/proxy/endpointslicecache.go#L137C39-L139
 	// we ignore serving because of https://github.com/zalando/skipper/issues/2684
-	return ep.Conditions.Ready == nil || *ep.Conditions.Ready
+	return ep.Conditions == nil || ep.Conditions.Ready == nil || *ep.Conditions.Ready
 }

dataclients/kubernetes/endpointslices_test.go

@@ -89,3 +89,47 @@ func TestAddresses(t *testing.T) {
 		},
 	}).addresses())
 }
+
+func TestEndpointSliceEndpointIsReady(t *testing.T) {
+	ready := true
+	notReady := false
+	terminating := true
+
+	for _, tt := range []struct {
+		name       string
+		conditions *endpointsliceCondition
+		want       bool
+	}{
+		{
+			name: "nil conditions default to ready",
+			want: true,
+		},
+		{
+			name:       "nil ready condition defaults to ready",
+			conditions: &endpointsliceCondition{},
+			want:       true,
+		},
+		{
+			name:       "ready true",
+			conditions: &endpointsliceCondition{Ready: &ready},
+			want:       true,
+		},
+		{
+			name:       "ready false",
+			conditions: &endpointsliceCondition{Ready: &notReady},
+			want:       false,
+		},
+		{
+			name:       "terminating overrides ready",
+			conditions: &endpointsliceCondition{Terminating: &terminating},
+			want:       false,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			ep := &EndpointSliceEndpoints{Conditions: tt.conditions}
+			if got := ep.isReady(); got != tt.want {
+				t.Fatalf("isReady() = %v, want = %v", got, tt.want)
+			}
+		})
+	}
+}