Codecov Report

Attention: Patch coverage is 42.30769% with 15 lines in your changes missing coverage. Please review.

Project coverage is 66.89%. Comparing base (18207c1) to head (acdfb45).

@@            Coverage Diff             @@
##             main    #5066      +/-   ##
==========================================
+ Coverage   66.79%   66.89%   +0.09%     
==========================================
  Files         211      211              
  Lines       32928    32932       +4     
==========================================
+ Hits        21995    22029      +34     
+ Misses       9601     9580      -21     
+ Partials     1332     1323       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zhaohuabing can you help explain why can't we use a waitgroup anymore ?

@zhaohuabing can you help explain why can't we use a waitgroup anymore ?

A WaitGroup blocks the goroutines until WaitGroup.Done() is called. The blocked goroutines can't be terminated after reloading the envoy-gateway-config ConfigMap, result in goroutine leaks.

Using a channel allows the blocked goroutines select on both the Elected and Context.Done channels, so they can be terminated after envoy-gateway-config ConfigMap is reloaded by cancelling the Context.

thanks for the explaination @zhaohuabing , can we achieve the same by

• calling cfg.Elected.Done() when shutting down

  gateway/internal/cmd/server.go

  Line 228 in 8b89dad

  cfg.Logger.Info("runners are shutting down")

  , unblocking the go routines
• instantiating a new cfg object in the hook

  gateway/internal/cmd/server.go

  Line 59 in 8b89dad

  cfg.Logger.Info("Setup runners")

cc @zirain

thanks for the explaination @zhaohuabing , can we achieve the same by

• calling cfg.Elected.Done() when shutting down

  gateway/internal/cmd/server.go

  Line 228 in 8b89dad

  cfg.Logger.Info("runners are shutting down")

  
  , unblocking the go routines

There will be two issues in this approach:

• We don't know if cfg.Elected.Done() has been called or not, and calling it repeately will result in panic.
• Unblocking it will result in the gorotines to continue execution when they shouldn't.

• instantiating a new cfg object in the hook

  gateway/internal/cmd/server.go

  Line 59 in 8b89dad

  cfg.Logger.Info("Setup runners")

cc @zirain

Goroutine leaks.

Channel is a common approach for blocking multiple goroutines while they wait for a signal. This allows these goroutines to exit gracefully when the associated context is canceled. The k8s controller manager also uses a similar approach to broadcast the Elected status.