Skip to content

Commit

Permalink
Improve race condition on parallel IP free.
Browse files Browse the repository at this point in the history
This PR should mitigate a possible race condition where ephemeral IP deletion within one cluster can happen when more than one service binds the same IP address.

The improvement is to put the IP query call behind the mutex acquisition such that the second call for deletion sees the proper IP tags.
  • Loading branch information
Gerrit91 committed Jul 30, 2024
1 parent 92e4c6c commit 7e31700
Showing 1 changed file with 12 additions and 8 deletions.
20 changes: 12 additions & 8 deletions pkg/controllers/loadbalancer/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,37 +206,41 @@ func (l *LoadBalancerController) UpdateLoadBalancer(ctx context.Context, cluster
func (l *LoadBalancerController) EnsureLoadBalancerDeleted(ctx context.Context, clusterName string, service *v1.Service) error {
klog.Infof("EnsureLoadBalancerDeleted: clusterName %q, namespace %q, serviceName %q, serviceStatus: %v", clusterName, service.Namespace, service.Name, service.Status)

s := *service
serviceTag := tags.BuildClusterServiceFQNTag(l.clusterID, s.GetNamespace(), s.GetName())
serviceTag := tags.BuildClusterServiceFQNTag(l.clusterID, service.GetNamespace(), service.GetName())

l.ipUpdateMutex.Lock()
defer l.ipUpdateMutex.Unlock()

ips, err := l.MetalService.FindProjectIPsWithTag(ctx, l.projectID, serviceTag)
if err != nil {
return err
}

l.ipUpdateMutex.Lock()
defer l.ipUpdateMutex.Unlock()

for _, ip := range ips {
ip := ip
err := retrygo.Do(
func() error {
newTags, last := l.removeServiceTag(*ip, serviceTag)
iu := &models.V1IPUpdateRequest{

newIP, err := l.MetalService.UpdateIP(ctx, &models.V1IPUpdateRequest{
Ipaddress: ip.Ipaddress,
Tags: newTags,
}
newIP, err := l.MetalService.UpdateIP(ctx, iu)
})
if err != nil {
return fmt.Errorf("could not update ip with new tags: %w", err)
}

klog.Infof("updated ip: %q", pointer.SafeDeref(newIP.Ipaddress))

if *ip.Type == models.V1IPBaseTypeEphemeral && last {
klog.Infof("freeing unused ephemeral ip: %s, tags: %s", *ip.Ipaddress, newTags)

err := l.MetalService.FreeIP(ctx, *ip.Ipaddress)
if err != nil {
return fmt.Errorf("unable to delete ip %s: %w", *ip.Ipaddress, err)
}
}

return nil
},
)
Expand Down

0 comments on commit 7e31700

Please sign in to comment.