Switched from eps.ready to eps.serving

It is necessary because `.ready` flag is set to `False` immediately during shutdown of the pod, while pod is still alive. And `.serving` flag is `True` while pod is healthy (even while shutting down).

So this change unlocks the ability to implement graceful shutdown for pods.

Sample scenario:

1. Pod is healthy and running, then it receives a shutdown signal (eg: pod is just deleted, or the node is drained)
2. Pod handles the kill signal and starts gracefully shutting down. At this state `.ready = False, .serving = True`
3. With new implementation - because `.serving == True` the pod's IP is still announced, which allows traffic for already established connections to freely flow to it
4. Then the pod completes its graceful shutdown ceremony and quits. At this point endpoint is removed from the endpointslice, and the IP is removed from announces.

Fixed metallb#2074

Signed-off-by: Ivan Kurnosov <zerkms@zerkms.com>

internal/k8s/epslices/endpoint_slices.go

@@ -26,13 +26,16 @@ const (
 
 const SlicesServiceIndexName = "ServiceName"
 
-// IsConditionReady tells if the conditions represent a ready state, interpreting
-// nil ready as ready.
-func IsConditionReady(conditions discovery.EndpointConditions) bool {
-	if conditions.Ready == nil {
-		return true
+// 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 {
+		if conditions.Ready == nil {
+			return true
+		}
+		return *conditions.Ready
 	}
-	return *conditions.Ready
+	return *conditions.Serving
 }
 
 func ServiceKeyForSlice(endpointSlice *discovery.EndpointSlice) (types.NamespacedName, error) {

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
 					}
 				}

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{

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

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),
 									},
 								},
 							},