Switched from eps.ready to eps.serving

Fixes metallb#2074
zerkms · Sep 20, 2023 · 910281b · 910281b
1 parent ec253a4
commit 910281b
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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),
 									},
 								},
 							},