diff --git a/cmd/main.go b/cmd/main.go index 8e7fc594f..436ef627e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -88,7 +88,7 @@ func init() { os.Exit(1) } - log = logger.ZapCustomLogger(cf.DefaultConfig.Debug, config.LogLevel) + log = logger.ZapCustomLogger(cf.DefaultConfig.Debug, config.LogLevel, config.LogColor) logger.Log = log // Set the controller-runtime logger to prevent the warning about log.SetLogger(...) never being called logf.SetLogger(log.Logger) diff --git a/cmd_clean/main.go b/cmd_clean/main.go index 325098820..e52daf9f9 100644 --- a/cmd_clean/main.go +++ b/cmd_clean/main.go @@ -61,6 +61,7 @@ func main() { flag.StringVar(&envoyHost, "envoyhost", "", "envoy host") flag.IntVar(&envoyPort, "envoyport", 0, "envoy port") flag.IntVar(&config.LogLevel, "log-level", 2, "Use zap-core log system.") + flag.BoolVar(&config.LogColor, "log-color", false, "Enable ANSI color in log output.") flag.Parse() cf = config.NewNSXOpertorConfig() @@ -80,10 +81,10 @@ func main() { ctx, cancel := context.WithTimeout(context.Background(), time.Minute*5) defer cancel() - log = logger.ZapCustomLogger(cf.DefaultConfig.Debug, config.LogLevel) + log = logger.ZapCustomLogger(cf.DefaultConfig.Debug, config.LogLevel, config.LogColor) logger.Log = log logf.SetLogger(log.Logger) - err := clean.Clean(ctx, cf, &log.Logger, cf.DefaultConfig.Debug, config.LogLevel) + err := clean.Clean(ctx, cf, &log.Logger, cf.DefaultConfig.Debug, config.LogLevel, config.LogColor) if err != nil { log.Error(err, "Failed to clean nsx resources") os.Exit(1) diff --git a/pkg/clean/clean.go b/pkg/clean/clean.go index dad93924a..a7287cc5e 100644 --- a/pkg/clean/clean.go +++ b/pkg/clean/clean.go @@ -46,10 +46,10 @@ var Backoff = wait.Backoff{ // GetNSXClientFailed indicate that could not retrieve nsx client to perform cleanup operation // InitCleanupServiceFailed indicate that error happened when trying to initialize cleanup service // CleanupResourceFailed indicate that the cleanup operation failed at some services, the detailed will in the service logs -func Clean(ctx context.Context, cf *config.NSXOperatorConfig, log *logr.Logger, debug bool, logLevel int) error { +func Clean(ctx context.Context, cf *config.NSXOperatorConfig, log *logr.Logger, debug bool, logLevel int, logColor bool) error { // Clean needs to support many instances which each have its own logger if log == nil { - logg := logger.ZapCustomLogger(debug, logLevel).Logger + logg := logger.ZapCustomLogger(debug, logLevel, logColor).Logger log = &logg } diff --git a/pkg/clean/clean_lb_infra_test.go b/pkg/clean/clean_lb_infra_test.go index 9b190b9e9..0122a602e 100644 --- a/pkg/clean/clean_lb_infra_test.go +++ b/pkg/clean/clean_lb_infra_test.go @@ -922,7 +922,7 @@ func convertToResponse(t *testing.T, resources []interface{}, bindingType bindin } func prepareCleaner() *LBInfraCleaner { - log := logger.ZapCustomLogger(false, 0).Logger + log := logger.ZapCustomLogger(false, 0, false).Logger return &LBInfraCleaner{ Service: common.Service{ NSXClient: &nsx.Client{ diff --git a/pkg/clean/clean_test.go b/pkg/clean/clean_test.go index 01d875ce3..0614c1516 100644 --- a/pkg/clean/clean_test.go +++ b/pkg/clean/clean_test.go @@ -41,7 +41,7 @@ func TestClean_ValidationFailed(t *testing.T) { defer patches.Reset() - err := Clean(ctx, cf, &log, debug, logLevel) + err := Clean(ctx, cf, &log, debug, logLevel, false) assert.Error(t, err) assert.Contains(t, err.Error(), "validation failed") } @@ -61,7 +61,7 @@ func TestClean_GetClientFailed(t *testing.T) { return nil }) - err := Clean(ctx, cf, &log, debug, logLevel) + err := Clean(ctx, cf, &log, debug, logLevel, false) assert.Error(t, err) assert.Contains(t, err.Error(), "failed to get nsx client") } @@ -85,7 +85,7 @@ func TestClean_InitError(t *testing.T) { return nil, errors.New("init cleanup service failed") }) - err := Clean(ctx, cf, &log, debug, logLevel) + err := Clean(ctx, cf, &log, debug, logLevel, false) assert.Error(t, err) assert.Contains(t, err.Error(), "init cleanup service failed") } @@ -122,7 +122,7 @@ func TestClean_Cleanup(t *testing.T) { return nil }) - err := Clean(ctx, cf, nil, debug, logLevel) + err := Clean(ctx, cf, nil, debug, logLevel, false) assert.Nil(t, err) assert.True(t, clean.vpcPreCleanupCalled) assert.True(t, clean.vpcChildrenCleanupCalled) diff --git a/pkg/config/config.go b/pkg/config/config.go index 981f9e193..18c19e4f4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -33,6 +33,7 @@ const ( var ( LogLevel int + LogColor bool ProbeAddr, MetricsAddr string WebhookServerPort int configFilePath = "" @@ -165,6 +166,7 @@ func AddFlags() { flag.StringVar(&ProbeAddr, "health-probe-bind-address", ":8384", "The address the probe endpoint binds to.") flag.StringVar(&MetricsAddr, "metrics-bind-address", ":8093", "The address the metrics endpoint binds to.") flag.IntVar(&LogLevel, "log-level", 0, "Use zap-core log system.") + flag.BoolVar(&LogColor, "log-color", false, "Enable ANSI color in log output.") flag.IntVar(&WebhookServerPort, "webhook-server-port", defaultWebhookPort, "Port number to expose the controller webhook server") flag.Parse() } diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 2fcec6be7..f0c729c8d 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -103,15 +103,19 @@ func getLogLevel(cfDebug bool, cfLogLevel int) int { } // ZapCustomLogger creates a CustomLogger with both logr.Logger and zerolog.Logger using the same configuration as ZapLogger -func ZapCustomLogger(cfDebug bool, cfLogLevel int) CustomLogger { +func ZapCustomLogger(cfDebug bool, cfLogLevel int, enableColor bool) CustomLogger { logLevel := getLogLevel(cfDebug, cfLogLevel) // Create the custom console writer with zap-like formatting consoleWriter := zerolog.ConsoleWriter{ Out: os.Stdout, + NoColor: !enableColor, TimeFormat: logTmFmtWithMS, FormatLevel: func(i interface{}) string { levelStr := strings.ToUpper(fmt.Sprintf("%s", i)) + if !enableColor { + return levelStr + } switch levelStr { case "TRACE": return colorWhite + levelStr + colorReset @@ -140,6 +144,9 @@ func ZapCustomLogger(cfDebug bool, cfLogLevel int) CustomLogger { if idx := strings.LastIndex(caller, "/"); idx >= 0 { caller = caller[idx+1:] } + if !enableColor { + return caller + } return colorYellow + caller + colorReset }, FormatFieldName: func(i interface{}) string { @@ -154,7 +161,7 @@ func ZapCustomLogger(cfDebug bool, cfLogLevel int) CustomLogger { // Set the log level based on the calculated level var zeroLogLevel zerolog.Level switch { - case logLevel == 2: + case logLevel >= 2: zeroLogLevel = zerolog.TraceLevel case logLevel == 1: zeroLogLevel = zerolog.DebugLevel diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index cebaaf846..e10401ff4 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -1,8 +1,13 @@ package logger import ( + "bytes" + "fmt" + "strings" "testing" "time" + + "github.com/rs/zerolog" ) func TestZapLoggerLevels(t *testing.T) { @@ -44,7 +49,7 @@ func TestZapLoggerLevels(t *testing.T) { t.Logf("Testing: %s", tc.description) // Create logger with specified configuration - logger := ZapCustomLogger(tc.debug, tc.logLevel).Logger + logger := ZapCustomLogger(tc.debug, tc.logLevel, false).Logger // Test various log levels logger.Info("This is an info message", "test_case", tc.name, "timestamp", time.Now().Format("15:04:05")) @@ -68,7 +73,7 @@ func TestZapLoggerLevels(t *testing.T) { func TestCustomLogger(t *testing.T) { t.Log("Testing CustomLogger with all log levels...") // Test CustomLogger wrapper - customLogger := ZapCustomLogger(true, 2) + customLogger := ZapCustomLogger(true, 2, false) // Test all five log levels customLogger.Trace("This is a trace message", "test_case", "trace_test", "timestamp", time.Now().Format("15:04:05")) @@ -79,3 +84,76 @@ func TestCustomLogger(t *testing.T) { t.Log("CustomLogger test completed - verify all log levels are displayed with proper formatting and colors") } + +func TestGetLogLevel(t *testing.T) { + testCases := []struct { + name string + debug bool + logLevel int + expected int + }{ + {"default", false, 0, 0}, + {"debug_flag_only", true, 0, 2}, + {"log_level_1", false, 1, 1}, + {"log_level_3", false, 3, 3}, + {"debug_with_higher_level", true, 3, 3}, + {"debug_with_lower_level", true, 1, 2}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := getLogLevel(tc.debug, tc.logLevel) + if got != tc.expected { + t.Errorf("getLogLevel(%v, %d) = %d, want %d", tc.debug, tc.logLevel, got, tc.expected) + } + }) + } +} + +func TestAnsiColorControlledByFlag(t *testing.T) { + // Helper: create a console writer via the same logic as ZapCustomLogger, + // but write to a buffer so we can inspect the output. + buildWriter := func(enableColor bool) (*bytes.Buffer, zerolog.Logger) { + buf := &bytes.Buffer{} + cw := zerolog.ConsoleWriter{ + Out: buf, + NoColor: !enableColor, + TimeFormat: logTmFmtWithMS, + FormatLevel: func(i interface{}) string { + levelStr := strings.ToUpper(fmt.Sprintf("%s", i)) + if !enableColor { + return levelStr + } + switch levelStr { + case "INFO": + return colorGreen + levelStr + colorReset + case "WARN": + return colorMagenta + levelStr + colorReset + case "ERROR": + return colorBrightRed + levelStr + colorReset + default: + return levelStr + } + }, + } + l := zerolog.New(cw).Level(zerolog.InfoLevel) + return buf, l + } + + t.Run("color_disabled", func(t *testing.T) { + buf, l := buildWriter(false) + l.Info().Msg("hello") + output := buf.String() + if strings.Contains(output, "\033[") { + t.Errorf("color=false should not contain ANSI codes, got: %q", output) + } + }) + + t.Run("color_enabled", func(t *testing.T) { + buf, l := buildWriter(true) + l.Info().Msg("hello") + output := buf.String() + if !strings.Contains(output, "\033[") { + t.Errorf("color=true should contain ANSI codes, got: %q", output) + } + }) +} diff --git a/pkg/nsx/services/node/node_test.go b/pkg/nsx/services/node/node_test.go index 7d65e34a9..a46e946e3 100644 --- a/pkg/nsx/services/node/node_test.go +++ b/pkg/nsx/services/node/node_test.go @@ -123,7 +123,7 @@ func TestNodeService_GetNodeByName(t *testing.T) { } func TestNodeService_SyncNodeStore(t *testing.T) { - logf.SetLogger(logger.ZapCustomLogger(false, 0).Logger) + logf.SetLogger(logger.ZapCustomLogger(false, 0, false).Logger) service := createMockNodeService() nodeName := "test-node" // Test case: Node not found diff --git a/test/e2e/framework.go b/test/e2e/framework.go index 38e46d0ae..0701fe9df 100644 --- a/test/e2e/framework.go +++ b/test/e2e/framework.go @@ -89,6 +89,7 @@ type TestOptions struct { logsExportOnSuccess bool debugLog bool logLevel int + logColor bool } var testOptions TestOptions diff --git a/test/e2e/main_test.go b/test/e2e/main_test.go index ddae9ad46..a8decf366 100644 --- a/test/e2e/main_test.go +++ b/test/e2e/main_test.go @@ -253,9 +253,10 @@ func testMain(m *testing.M) int { flag.StringVar(&testOptions.vcPassword, "vc-password", "", "The password used by the user when requesting vCenter API session") flag.BoolVar(&testOptions.debugLog, "debug", false, "") flag.IntVar(&testOptions.logLevel, "log-level", 0, "") + flag.BoolVar(&testOptions.logColor, "log-color", false, "Enable ANSI color in log output.") flag.Parse() - log = logger.ZapCustomLogger(testOptions.debugLog, testOptions.logLevel) + log = logger.ZapCustomLogger(testOptions.debugLog, testOptions.logLevel, testOptions.logColor) logger.Log = log // Set the controller-runtime logger to prevent the warning about log.SetLogger(...) never being called logf.SetLogger(log.Logger)