From 60686f55ff3e70f00fa22a45c5ec4ca04c975581 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Wed, 5 Feb 2025 16:37:18 +0700 Subject: [PATCH] cmd/cli: set ProxyLogger correctly for interactive commands The ProxyLogger must only be set after mainLog is fully initialized. However, it's being set before the final initialization of mainlog, causing it still refers to stale old pointer. To fix this, introduce a new function to discard ProxyLogger explicitly, and use this function to init logging for all interactive commands. --- cmd/cli/cli.go | 4 +--- cmd/cli/commands.go | 8 ++++---- cmd/cli/main.go | 10 ++++++++++ 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go index f59d4bb..49adca3 100644 --- a/cmd/cli/cli.go +++ b/cmd/cli/cli.go @@ -267,8 +267,6 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { // Log config do not have thing to validate, so it's safe to init log here, // so it's able to log information in processCDFlags. logWriters := initLogging() - // TODO: find a better way. - ctrld.ProxyLogger.Store(mainLog.Load()) // Initializing internal logging after global logging. p.initInternalLogging(logWriters) @@ -1023,7 +1021,7 @@ func uninstall(p *prog, s service.Service) { {s.Stop, false, "Stop"}, {s.Uninstall, true, "Uninstall"}, } - initLogging() + initInteractiveLogging() if doTasks(tasks) { if err := p.router.ConfigureService(svcConfig); err != nil { mainLog.Load().Fatal().Err(err).Msg("could not configure service") diff --git a/cmd/cli/commands.go b/cmd/cli/commands.go index 3dae547..43ec485 100644 --- a/cmd/cli/commands.go +++ b/cmd/cli/commands.go @@ -336,7 +336,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c mainLog.Load().Fatal().Msgf("failed to unmarshal config: %v", err) } - initLogging() + initInteractiveLogging() tasks := []task{ {s.Stop, false, "Stop"}, resetDnsTask(p, s, isCtrldInstalled, currentIface), @@ -399,7 +399,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c mainLog.Load().Fatal().Msgf("failed to unmarshal config: %v", err) } - initLogging() + initInteractiveLogging() if nextdns != "" { removeNextDNSFromArgs(sc) @@ -588,7 +588,7 @@ func initStopCmd() *cobra.Command { p.requiredMultiNICsConfig = ir.All } - initLogging() + initInteractiveLogging() status, err := s.Status() if errors.Is(err, service.ErrNotInstalled) { @@ -695,7 +695,7 @@ func initRestartCmd() *cobra.Command { p.requiredMultiNICsConfig = ir.All } - initLogging() + initInteractiveLogging() if cdMode { doValidateCdRemoteConfig(cdUID) diff --git a/cmd/cli/main.go b/cmd/cli/main.go index 7041318..819797a 100644 --- a/cmd/cli/main.go +++ b/cmd/cli/main.go @@ -106,6 +106,14 @@ func initLogging() []io.Writer { return initLoggingWithBackup(true) } +// initInteractiveLogging is like initLogging, but the ProxyLogger is discarded +// to be used for all interactive commands. +func initInteractiveLogging() { + initLogging() + l := zerolog.New(io.Discard) + ctrld.ProxyLogger.Store(&l) +} + // initLoggingWithBackup initializes log setup base on current config. // If doBackup is true, backup old log file with ".1" suffix. // @@ -143,6 +151,8 @@ func initLoggingWithBackup(doBackup bool) []io.Writer { multi := zerolog.MultiLevelWriter(writers...) l := mainLog.Load().Output(multi).With().Logger() mainLog.Store(&l) + // TODO: find a better way. + ctrld.ProxyLogger.Store(&l) zerolog.SetGlobalLevel(zerolog.NoticeLevel) logLevel := cfg.Service.LogLevel