diff --git a/internal/k8s/epslices/endpoint_slices.go b/internal/k8s/epslices/endpoint_slices.go index 872c697754e..33497a351a8 100644 --- a/internal/k8s/epslices/endpoint_slices.go +++ b/internal/k8s/epslices/endpoint_slices.go @@ -35,6 +35,15 @@ func IsConditionReady(conditions discovery.EndpointConditions) bool { return *conditions.Ready } +// IsConditionServing tells if the conditions represent a serving state, deferring +// to ready state if serving == nil. +func IsConditionServing(conditions discovery.EndpointConditions) bool { + if conditions.Serving == nil { + return IsConditionReady(conditions) + } + return *conditions.Serving +} + func ServiceKeyForSlice(endpointSlice *discovery.EndpointSlice) (types.NamespacedName, error) { if endpointSlice == nil { return types.NamespacedName{}, fmt.Errorf("nil EndpointSlice") diff --git a/speaker/bgp_controller.go b/speaker/bgp_controller.go index d6829bd7bf9..5d54b787e99 100644 --- a/speaker/bgp_controller.go +++ b/speaker/bgp_controller.go @@ -137,13 +137,13 @@ func hasHealthyEndpoint(eps epslices.EpsOrSlices, filterNode func(*string) bool) continue } for _, addr := range ep.Addresses { - if _, ok := ready[addr]; !ok && epslices.IsConditionReady(ep.Conditions) { + if _, ok := ready[addr]; !ok && epslices.IsConditionServing(ep.Conditions) { // Only set true if nothing else has expressed an // opinion. This means that false will take precedence // if there's any unready ports for a given endpoint. ready[addr] = true } - if !epslices.IsConditionReady(ep.Conditions) { + if !epslices.IsConditionServing(ep.Conditions) { ready[addr] = false } } diff --git a/speaker/bgp_controller_test.go b/speaker/bgp_controller_test.go index af324184abd..6db53b7fb7a 100644 --- a/speaker/bgp_controller_test.go +++ b/speaker/bgp_controller_test.go @@ -1505,6 +1505,44 @@ func TestBGPSpeakerEPSlices(t *testing.T) { }, }, + { + desc: "Endpoint list contains serving but not ready endpoints", + balancer: "test1", + svc: &v1.Service{ + Spec: v1.ServiceSpec{ + Type: "LoadBalancer", + ExternalTrafficPolicy: "Cluster", + }, + Status: statusAssigned("10.20.30.1"), + }, + eps: epslices.EpsOrSlices{ + SlicesVal: []discovery.EndpointSlice{ + { + Endpoints: []discovery.Endpoint{ + { + Addresses: []string{ + "2.3.4.5", + }, + NodeName: stringPtr("iris"), + Conditions: discovery.EndpointConditions{ + Ready: pointer.BoolPtr(false), + Serving: pointer.BoolPtr(true), + }, + }, + }, + }, + }, + Type: epslices.Slices, + }, + wantAds: map[string][]*bgp.Advertisement{ + "1.2.3.4:0": { + { + Prefix: ipnet("10.20.30.1/32"), + }, + }, + }, + }, + { desc: "Multiple advertisement config", config: &config.Config{ diff --git a/speaker/layer2_controller.go b/speaker/layer2_controller.go index ab61fba4bb5..d4155cf5931 100644 --- a/speaker/layer2_controller.go +++ b/speaker/layer2_controller.go @@ -66,7 +66,7 @@ func usableNodes(eps epslices.EpsOrSlices, speakers map[string]bool) []string { case epslices.Slices: for _, slice := range eps.SlicesVal { for _, ep := range slice.Endpoints { - if !epslices.IsConditionReady(ep.Conditions) { + if !epslices.IsConditionServing(ep.Conditions) { continue } if ep.NodeName == nil { @@ -209,7 +209,7 @@ func activeEndpointExists(eps epslices.EpsOrSlices) bool { case epslices.Slices: for _, slice := range eps.SlicesVal { for _, ep := range slice.Endpoints { - if !epslices.IsConditionReady(ep.Conditions) { + if !epslices.IsConditionServing(ep.Conditions) { continue } return true diff --git a/speaker/layer2_controller_test.go b/speaker/layer2_controller_test.go index 4c8815de02c..7f2fbef3c5e 100644 --- a/speaker/layer2_controller_test.go +++ b/speaker/layer2_controller_test.go @@ -151,6 +151,11 @@ func TestUsableNodes(t *testing.T) { } func TestUsableNodesEPSlices(t *testing.T) { + b := &fakeBGP{ + t: t, + } + newBGP = b.NewSessionManager + c, err := newController(controllerConfig{ MyNode: "iris1", Logger: log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr)), @@ -302,6 +307,40 @@ func TestUsableNodesEPSlices(t *testing.T) { usableSpeakers: map[string]bool{"iris1": true, "iris2": true}, cExpectedResult: []string{"iris1"}, }, + { + desc: "Two endpoints, different hosts, not ready but serving", + eps: epslices.EpsOrSlices{ + SlicesVal: []discovery.EndpointSlice{ + { + Endpoints: []discovery.Endpoint{ + { + Addresses: []string{ + "2.3.4.5", + }, + NodeName: stringPtr("iris1"), + Conditions: discovery.EndpointConditions{ + Ready: pointer.BoolPtr(false), + Serving: pointer.BoolPtr(true), + }, + }, + { + Addresses: []string{ + "2.3.4.15", + }, + NodeName: stringPtr("iris2"), + Conditions: discovery.EndpointConditions{ + Ready: pointer.BoolPtr(false), + Serving: pointer.BoolPtr(true), + }, + }, + }, + }, + }, + Type: epslices.Slices, + }, + usableSpeakers: map[string]bool{"iris1": true, "iris2": true}, + cExpectedResult: []string{"iris1", "iris2"}, + }, } for _, test := range tests { @@ -1367,7 +1406,8 @@ func TestShouldAnnounceEPSlices(t *testing.T) { }, NodeName: stringPtr("iris2"), Conditions: discovery.EndpointConditions{ - Ready: pointer.BoolPtr(true), + Ready: pointer.BoolPtr(false), + Serving: pointer.BoolPtr(true), }, }, },