Skip to content

Commit

Permalink
fix(nrc): multiple services with the same VIP
Browse files Browse the repository at this point in the history
Properly consider the readiness of all services in the case where
multiple services share the same VIP. Don't withdraw a VIP just because
one service is not ready.
  • Loading branch information
aauren committed Dec 11, 2020
1 parent 46e903a commit f8aed0c
Showing 1 changed file with 19 additions and 4 deletions.
23 changes: 19 additions & 4 deletions pkg/controllers/routing/ecmp_vip.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@ func (nrc *NetworkRoutingController) handleServiceUpdate(svc *v1core.Service) {
return
}

toAdvertise, toWithdraw, err := nrc.getVIPsForService(svc, true)
toAdvertise, toWithdraw, err := nrc.getActiveVIPs()
if err != nil {
glog.Errorf("error getting routes for service: %s, err: %s", svc.Name, err)
glog.Errorf("error getting routes for services: %s", err)
return
}

// update export policies so that new VIP's gets addedd to clusteripprefixsit and vip gets advertised to peers
// update export policies so that new VIP's gets added to clusteripprefixset and vip gets advertised to peers
err = nrc.AddPolicies()
if err != nil {
glog.Errorf("Error adding BGP policies: %s", err.Error())
Expand Down Expand Up @@ -369,7 +369,22 @@ func (nrc *NetworkRoutingController) getVIPs(onlyActiveEndpoints bool) ([]string
}
}

return toAdvertiseList, toWithdrawList, nil
// We need to account for the niche case where multiple services may have the same VIP, in this case, one service
// might be ready while the other service is not. We still want to advertise the VIP as long as there is at least
// one active endpoint on the node or we might introduce a service disruption.
finalToWithdrawList := make([]string, 0)
OUTER:
for _, withdrawVIP := range toWithdrawList {
for _, advertiseVIP := range toAdvertiseList {
if withdrawVIP == advertiseVIP {
// if there is a VIP that is set to both be advertised and withdrawn, don't add it to the final withdraw list
continue OUTER
}
}
finalToWithdrawList = append(finalToWithdrawList, withdrawVIP)
}

return toAdvertiseList, finalToWithdrawList, nil
}

func (nrc *NetworkRoutingController) shouldAdvertiseService(svc *v1core.Service, annotation string, defaultValue bool) bool {
Expand Down

0 comments on commit f8aed0c

Please sign in to comment.