Skip to content

Commit

Permalink
bgpv2: filter terminating backends from endpoint selection
Browse files Browse the repository at this point in the history
Filtering out backends which are terminating when creating local
endpoint state. This will result in quicker route withdrawal if local
backends go into terminating state, without waiting for graceful
shutdown period of the pods.

Signed-off-by: harsimran pabla <hpabla@isovalent.com>
  • Loading branch information
harsimran-pabla authored and julianwiedmann committed May 17, 2024
1 parent ffa783f commit 9e03698
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/bgpv1/manager/reconcilerv2/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ endpointsLoop:
}

for _, be := range eps.Backends {
if be.NodeName == localNodeName {
if !be.Terminating && be.NodeName == localNodeName {
// At least one endpoint is available on this node. We
// can add service to the local services set.
ls.Insert(svcKey)
Expand Down
54 changes: 54 additions & 0 deletions pkg/bgpv1/manager/reconcilerv2/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,30 @@ var (
},
}

eps1LocalTerminating = &k8s.Endpoints{
ObjectMeta: slim_metav1.ObjectMeta{
Name: "svc-1",
Namespace: "non-default",
},
EndpointSliceID: k8s.EndpointSliceID{
ServiceID: k8s.ServiceID{
Name: redSvcKey.Name,
Namespace: redSvcKey.Namespace,
},
EndpointSliceName: "svc-1",
},
Backends: map[cmtypes.AddrCluster]*k8s.Backend{
cmtypes.MustParseAddrCluster("10.0.0.1"): {
NodeName: "node1",
Terminating: true,
},
cmtypes.MustParseAddrCluster("2001:db8:1000::1"): {
NodeName: "node1",
Terminating: true,
},
},
}

eps1Remote = &k8s.Endpoints{
ObjectMeta: slim_metav1.ObjectMeta{
Name: "svc-1",
Expand Down Expand Up @@ -682,6 +706,36 @@ func Test_ServiceLBReconciler(t *testing.T) {
},
},
},
{
name: "Service (LB) with advertisement(LB) - matching labels (eTP=local, backends are terminating)",
peerConfig: []*v2alpha1.CiliumBGPPeerConfig{redPeerConfig},
services: []*slim_corev1.Service{redLBSvcWithETP(slim_corev1.ServiceExternalTrafficPolicyLocal)},
lbIPPools: []*v2alpha1.CiliumLoadBalancerIPPool{redLBPool},
endpoints: []*k8s.Endpoints{eps1LocalTerminating},
advertisements: []*v2alpha1.CiliumBGPAdvertisement{
redSvcAdvertWithAdvertisements(lbSvcAdvertWithSelector(redSvcSelector)),
},
expectedMetadata: ServiceReconcilerMetadata{
ServicePaths: ResourceAFPathsMap{},
ServiceRoutePolicies: ResourceRoutePolicyMap{},
ServiceAdvertisements: PeerAdvertisements{
"red-peer-65001": PeerFamilyAdvertisements{
{Afi: "ipv4", Safi: "unicast"}: []v2alpha1.BGPAdvertisement{
lbSvcAdvertWithSelector(redSvcSelector),
},
{Afi: "ipv6", Safi: "unicast"}: []v2alpha1.BGPAdvertisement{
lbSvcAdvertWithSelector(redSvcSelector),
},
},
},
LBPoolRoutePolicies: ResourceRoutePolicyMap{ // route policies will exists even if there are no local eps
redLBPoolKey: {
redPeer65001v4LBRPName: redPeer65001v4LBRP,
redPeer65001v6LBRPName: redPeer65001v6LBRP,
},
},
},
},
}

for _, tt := range tests {
Expand Down

0 comments on commit 9e03698

Please sign in to comment.