diff --git a/internal/controller/bindings/boundendpoint_poller.go b/internal/controller/bindings/boundendpoint_poller.go index 3a733c9f..b98f679f 100644 --- a/internal/controller/bindings/boundendpoint_poller.go +++ b/internal/controller/bindings/boundendpoint_poller.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "reflect" - "sync" "time" "github.com/go-logr/logr" @@ -65,11 +64,8 @@ type BoundEndpointPoller struct { // If draining, polling is skipped to prevent creating new resources. DrainState DrainState - // portAllocatorMu protects concurrent access to portAllocator. - // The polling goroutine reassigns it while reconcile action goroutines call methods on it. - portAllocatorMu sync.Mutex - - // portAllocator manages the unique port allocations + // portAllocator manages the unique port allocations. + // portBitmap is internally thread-safe. portAllocator *portBitmap // Channel to stop the API polling goroutine @@ -249,9 +245,7 @@ func (r *BoundEndpointPoller) reconcileBoundEndpointsFromAPI(ctx context.Context } // reassign port allocations - r.portAllocatorMu.Lock() - r.portAllocator = currentPortAllocations - r.portAllocatorMu.Unlock() + r.portAllocator.Replace(currentPortAllocations) toCreate, toUpdate, toDelete := r.filterBoundEndpointActions(ctx, existingBoundEndpoints, desiredBoundEndpoints) @@ -384,9 +378,7 @@ func (r *BoundEndpointPoller) createBinding(ctx context.Context, desired binding name := hashURL(desired.Spec.GetEndpointURL()) // allocate a port - r.portAllocatorMu.Lock() port, err := r.portAllocator.SetAny() - r.portAllocatorMu.Unlock() if err != nil { r.Log.Error(err, "Failed to allocate port for BoundEndpoint", "name", name, "url", desired.Spec.GetEndpointURL()) return err @@ -548,11 +540,9 @@ func (r *BoundEndpointPoller) deleteBinding(ctx context.Context, boundEndpoint b log.Info("Deleted BoundEndpoint", "name", boundEndpoint.Name, "url", boundEndpoint.Spec.GetEndpointURL()) // unset the port allocation - r.portAllocatorMu.Lock() if err := r.portAllocator.Unset(boundEndpoint.Spec.Port); err != nil { log.Error(err, "Failed to unset port allocation", "port", boundEndpoint.Spec.Port, "name", boundEndpoint.Name) } - r.portAllocatorMu.Unlock() return nil } diff --git a/internal/controller/bindings/port_allocator.go b/internal/controller/bindings/port_allocator.go index ebbd2abc..f355c049 100644 --- a/internal/controller/bindings/port_allocator.go +++ b/internal/controller/bindings/port_allocator.go @@ -76,3 +76,11 @@ func (pb *portBitmap) NumFree() uint64 { defer pb.mu.Unlock() return pb.ports.Unselected() } + +// Replace atomically swaps the internal bitmap state with that of another portBitmap. +// The other portBitmap should not be used concurrently during this call. +func (pb *portBitmap) Replace(other *portBitmap) { + pb.mu.Lock() + defer pb.mu.Unlock() + pb.ports = other.ports +}