fix: eliminate cfg.Logger data race in server()'s reload/shutdown path

Loader.New() aliases the *config.Server passed to it, so the watcher
goroutine's cfg.Logger reassignment on each config reload directly
mutates the same struct instance server()'s top-level select loop
reads from when logging shutdown/hook-error messages. The watcher
writes under cfgMu, but server() read cfg.Logger with no
synchronization at all, producing a genuine data race (confirmed via
the actual CI job log: write at configloader.go:91 vs. read at
server.go:112, and reproduced locally under linux/-race).

Fix: the loader publishes each new logger over a small "latest value"
channel as reloads happen. server() keeps its own local logger
reference and swaps it only upon receiving from that channel, so the
update is synchronized via channel send/receive instead of a shared,
unguarded field.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>

Author: muwaqar-cflt

internal/cmd/server.go

@@ -88,6 +88,10 @@ func server(ctx context.Context, stdout, stderr io.Writer, cfgPath string, hook
 		return err
 	}
 
+	// cfg.Logger is mutated concurrently on reload, so keep our own reference
+	// and update it only via l.LoggerUpdates() below, never read cfg.Logger again.
+	logger := cfg.Logger
+
 	l := loader.New(cfgPath, cfg, hook)
 	if err := l.Start(ctx, stdout); err != nil {
 		return err
@@ -99,17 +103,20 @@ func server(ctx context.Context, stdout, stderr io.Writer, cfgPath string, hook
 
 	for {
 		select {
+		// Pick up the latest logger whenever a config reload replaces it.
+		case newLogger := <-l.LoggerUpdates():
+			logger = newLogger
 		// Exit if the config loader fails to start the runners.
 		// Continuing with failed runners would cause EG to function incorrectly.
 		case err := <-l.Errors():
-			cfg.Logger.Error(err, "failed to start runners")
+			logger.Error(err, "failed to start runners")
 			// Wait for runners to finish before shutting down.
 			// This is to make sure no orphaned runner process is left running in standalone mode.
 			l.Wait()
 			return err
 		// Wait for the context to be done, which usually happens the process receives a SIGTERM or SIGINT.
 		case <-ctx.Done():
-			cfg.Logger.Info("shutting down")
+			logger.Info("shutting down")
 			// Wait for runners to finish before shutting down.
 			// This is to make sure no orphaned runner process is left running in standalone mode.
 			l.Wait()

internal/envoygateway/config/loader/configloader.go

@@ -29,17 +29,21 @@ type Loader struct {
 	hook      HookFunc
 	hookErr   chan error
 
+	// loggerUpdates publishes the logger on each config reload; see LoggerUpdates.
+	loggerUpdates chan logging.Logger
+
 	w filewatcher.FileWatcher
 }
 
 func New(cfgPath string, cfg *config.Server, f HookFunc) *Loader {
 	return &Loader{
-		cfgPath: cfgPath,
-		cfg:     cfg,
-		logger:  cfg.Logger.WithName("config-loader"),
-		hook:    f,
-		hookErr: make(chan error, 1),
-		w:       filewatcher.NewWatcher(),
+		cfgPath:       cfgPath,
+		cfg:           cfg,
+		logger:        cfg.Logger.WithName("config-loader"),
+		hook:          f,
+		hookErr:       make(chan error, 1),
+		loggerUpdates: make(chan logging.Logger, 1),
+		w:             filewatcher.NewWatcher(),
 	}
 }
 
@@ -85,11 +89,13 @@ func (r *Loader) Start(ctx context.Context, logOut io.Writer) error {
 					continue
 				}
 
+				newLogger := logging.NewLogger(logOut, eg.Logging)
 				r.cfgMu.Lock()
 				r.cfg.EnvoyGateway = eg
 				// update cfg logger
-				r.cfg.Logger = logging.NewLogger(logOut, eg.Logging)
+				r.cfg.Logger = newLogger
 				r.cfgMu.Unlock()
+				r.publishLogger(newLogger)
 
 				// cancel last
 				if r.cancel != nil {
@@ -142,6 +148,22 @@ func (r *Loader) Errors() <-chan error {
 	return r.hookErr
 }
 
+// LoggerUpdates returns a channel that receives the latest logger on each config reload.
+func (r *Loader) LoggerUpdates() <-chan logging.Logger {
+	return r.loggerUpdates
+}
+
+func (r *Loader) publishLogger(l logging.Logger) {
+	select {
+	case <-r.loggerUpdates: // drain a stale, unconsumed value, if any
+	default:
+	}
+	select {
+	case r.loggerUpdates <- l:
+	default:
+	}
+}
+
 // Wait returns when success to acquire mutex, which means no hook is running.
 func (r *Loader) Wait() {
 	r.hookMutex.Lock()

release-notes/current/bug_fixes/9581-config-reload-server-logger-data-race.md

@@ -0,0 +1 @@
+Fixed a data race between config-reload and the standalone server's shutdown/error logging by having the config loader publish logger updates over a channel instead of a shared, unguarded field.