Skip to content

Commit 919ba2d

Browse files
authored
refactor: replace glog in flags.go (#6530)
1 parent a1d2389 commit 919ba2d

File tree

6 files changed

+127
-83
lines changed

6 files changed

+127
-83
lines changed

cmd/nginx-ingress/flags.go

Lines changed: 82 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
11
package main
22

33
import (
4+
"context"
45
"flag"
56
"fmt"
67
"net"
78
"os"
89
"regexp"
910
"strings"
1011

11-
"github.com/golang/glog"
1212
api_v1 "k8s.io/api/core/v1"
1313
"k8s.io/apimachinery/pkg/labels"
1414
"k8s.io/apimachinery/pkg/util/validation"
15+
16+
nlog "github.com/nginxinc/kubernetes-ingress/internal/logger"
17+
"github.com/nginxinc/kubernetes-ingress/internal/logger/levels"
1518
)
1619

1720
const (
@@ -229,86 +232,95 @@ func parseFlags() {
229232
}
230233
}
231234

232-
func initValidate() {
235+
func initValidate(ctx context.Context) {
236+
l := nlog.LoggerFromContext(ctx)
233237
logFormatValidationError := validateLogFormat(*logFormat)
234238
if logFormatValidationError != nil {
235-
glog.Warningf("Invalid log format: %s. Valid options are: glog, text, json. Falling back to default: %s", *logFormat, logFormatDefault)
239+
l.Warn(fmt.Sprintf("Invalid log format: %s. Valid options are: glog, text, json. Falling back to default: %s", *logFormat, logFormatDefault))
236240
}
237241

238242
logLevelValidationError := validateLogLevel(*logLevel)
239243
if logLevelValidationError != nil {
240-
glog.Warningf("Invalid log level: %s. Valid options are: trace, debug, info, warning, error, fatal. Falling back to default: %s", *logLevel, logLevelDefault)
244+
l.Warn(fmt.Sprintf("Invalid log level: %s. Valid options are: trace, debug, info, warning, error, fatal. Falling back to default: %s", *logLevel, logLevelDefault))
241245
}
242246

243247
if *enableLatencyMetrics && !*enablePrometheusMetrics {
244-
glog.Warning("enable-latency-metrics flag requires enable-prometheus-metrics, latency metrics will not be collected")
248+
l.Warn("enable-latency-metrics flag requires enable-prometheus-metrics, latency metrics will not be collected")
245249
*enableLatencyMetrics = false
246250
}
247251

248252
if *enableServiceInsight && !*nginxPlus {
249-
glog.Warning("enable-service-insight flag support is for NGINX Plus, service insight endpoint will not be exposed")
253+
l.Warn("enable-service-insight flag support is for NGINX Plus, service insight endpoint will not be exposed")
250254
*enableServiceInsight = false
251255
}
252256

253257
if *enableDynamicWeightChangesReload && !*nginxPlus {
254-
glog.Warning("weight-changes-dynamic-reload flag support is for NGINX Plus, Dynamic Weight Changes will not be enabled")
258+
l.Warn("weight-changes-dynamic-reload flag support is for NGINX Plus, Dynamic Weight Changes will not be enabled")
255259
*enableDynamicWeightChangesReload = false
256260
}
257261

258-
mustValidateInitialChecks()
259-
mustValidateWatchedNamespaces()
260-
mustValidateFlags()
262+
mustValidateInitialChecks(ctx)
263+
mustValidateWatchedNamespaces(ctx)
264+
mustValidateFlags(ctx)
261265
}
262266

263-
func mustValidateInitialChecks() {
267+
func mustValidateInitialChecks(ctx context.Context) {
268+
l := nlog.LoggerFromContext(ctx)
264269
err := flag.Lookup("logtostderr").Value.Set("true")
265270
if err != nil {
266-
glog.Fatalf("Error setting logtostderr to true: %v", err)
271+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Error setting logtostderr to true: %v", err))
272+
os.Exit(1)
267273
}
268274

269275
err = flag.Lookup("include_year").Value.Set("true")
270276
if err != nil {
271-
glog.Fatalf("Error setting include_year flag: %v", err)
277+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Error setting include_year flag: %v", err))
278+
os.Exit(1)
272279
}
273280

274281
if startupCheckFn != nil {
275282
err := startupCheckFn()
276283
if err != nil {
277-
glog.Fatalf("Failed startup check: %v", err)
284+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Failed startup check: %v", err))
285+
os.Exit(1)
278286
}
279-
glog.Info("AWS startup check passed")
287+
l.Info("AWS startup check passed")
280288
}
281289

282-
glog.Infof("Starting with flags: %+q", os.Args[1:])
290+
l.Info(fmt.Sprintf("Starting with flags: %+q", os.Args[1:]))
283291

284292
unparsed := flag.Args()
285293
if len(unparsed) > 0 {
286-
glog.Warningf("Ignoring unhandled arguments: %+q", unparsed)
294+
l.Warn(fmt.Sprintf("Ignoring unhandled arguments: %+q", unparsed))
287295
}
288296
}
289297

290298
// mustValidateWatchedNamespaces calls internally os.Exit if it can't validate namespaces.
291-
func mustValidateWatchedNamespaces() {
299+
func mustValidateWatchedNamespaces(ctx context.Context) {
300+
l := nlog.LoggerFromContext(ctx)
292301
if *watchNamespace != "" && *watchNamespaceLabel != "" {
293-
glog.Fatal("watch-namespace and -watch-namespace-label are mutually exclusive")
302+
l.Log(ctx, levels.LevelFatal, "watch-namespace and -watch-namespace-label are mutually exclusive")
303+
os.Exit(1)
294304
}
295305

296306
watchNamespaces = strings.Split(*watchNamespace, ",")
297307

298308
if *watchNamespace != "" {
299-
glog.Infof("Namespaces watched: %v", watchNamespaces)
309+
l.Info(fmt.Sprintf("Namespaces watched: %v", watchNamespaces))
300310
namespacesNameValidationError := validateNamespaceNames(watchNamespaces)
301311
if namespacesNameValidationError != nil {
302-
glog.Fatalf("Invalid values for namespaces: %v", namespacesNameValidationError)
312+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid values for namespaces: %v", namespacesNameValidationError))
313+
os.Exit(1)
303314
}
304315
}
305316

306317
if len(*watchSecretNamespace) > 0 {
307318
watchSecretNamespaces = strings.Split(*watchSecretNamespace, ",")
308-
glog.Infof("Namespaces watched for secrets: %v", watchSecretNamespaces)
319+
l.Debug(fmt.Sprintf("Namespaces watched for secrets: %v", watchSecretNamespaces))
309320
namespacesNameValidationError := validateNamespaceNames(watchSecretNamespaces)
310321
if namespacesNameValidationError != nil {
311-
glog.Fatalf("Invalid values for secret namespaces: %v", namespacesNameValidationError)
322+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid values for secret namespaces: %v", namespacesNameValidationError))
323+
os.Exit(1)
312324
}
313325
} else {
314326
// empty => default to watched namespaces
@@ -319,107 +331,131 @@ func mustValidateWatchedNamespaces() {
319331
var err error
320332
_, err = labels.Parse(*watchNamespaceLabel)
321333
if err != nil {
322-
glog.Fatalf("Unable to parse label %v for watch namespace label: %v", *watchNamespaceLabel, err)
334+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Unable to parse label %v for watch namespace label: %v", *watchNamespaceLabel, err))
335+
os.Exit(1)
323336
}
324337
}
325338
}
326339

327340
// mustValidateFlags checks the values for various flags
328341
// and calls os.Exit if any of the flags is invalid.
329-
func mustValidateFlags() {
342+
// nolint:gocyclo
343+
func mustValidateFlags(ctx context.Context) {
344+
l := nlog.LoggerFromContext(ctx)
330345
healthStatusURIValidationError := validateLocation(*healthStatusURI)
331346
if healthStatusURIValidationError != nil {
332-
glog.Fatalf("Invalid value for health-status-uri: %v", healthStatusURIValidationError)
347+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid value for health-status-uri: %v", healthStatusURIValidationError))
348+
os.Exit(1)
333349
}
334350

335351
statusLockNameValidationError := validateResourceName(*leaderElectionLockName)
336352
if statusLockNameValidationError != nil {
337-
glog.Fatalf("Invalid value for leader-election-lock-name: %v", statusLockNameValidationError)
353+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid value for leader-election-lock-name: %v", statusLockNameValidationError))
354+
os.Exit(1)
338355
}
339356

340357
statusPortValidationError := validatePort(*nginxStatusPort)
341358
if statusPortValidationError != nil {
342-
glog.Fatalf("Invalid value for nginx-status-port: %v", statusPortValidationError)
359+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid value for nginx-status-port: %v", statusPortValidationError))
360+
os.Exit(1)
343361
}
344362

345363
metricsPortValidationError := validatePort(*prometheusMetricsListenPort)
346364
if metricsPortValidationError != nil {
347-
glog.Fatalf("Invalid value for prometheus-metrics-listen-port: %v", metricsPortValidationError)
365+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid value for prometheus-metrics-listen-port: %v", metricsPortValidationError))
366+
os.Exit(1)
348367
}
349368

350369
readyStatusPortValidationError := validatePort(*readyStatusPort)
351370
if readyStatusPortValidationError != nil {
352-
glog.Fatalf("Invalid value for ready-status-port: %v", readyStatusPortValidationError)
371+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid value for ready-status-port: %v", readyStatusPortValidationError))
372+
os.Exit(1)
353373
}
354374

355375
healthProbePortValidationError := validatePort(*serviceInsightListenPort)
356376
if healthProbePortValidationError != nil {
357-
glog.Fatalf("Invalid value for service-insight-listen-port: %v", metricsPortValidationError)
377+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid value for service-insight-listen-port: %v", metricsPortValidationError))
378+
os.Exit(1)
358379
}
359380

360381
var err error
361382
allowedCIDRs, err = parseNginxStatusAllowCIDRs(*nginxStatusAllowCIDRs)
362383
if err != nil {
363-
glog.Fatalf(`Invalid value for nginx-status-allow-cidrs: %v`, err)
384+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid value for nginx-status-allow-cidrs: %v", err))
385+
os.Exit(1)
364386
}
365387

366388
if *appProtectLogLevel != appProtectLogLevelDefault && *appProtect && *nginxPlus {
367389
appProtectlogLevelValidationError := validateLogLevel(*appProtectLogLevel)
368390
if appProtectlogLevelValidationError != nil {
369-
glog.Fatalf("Invalid value for app-protect-log-level: %v", *appProtectLogLevel)
391+
l.Log(ctx, levels.LevelFatal, fmt.Sprintf("Invalid value for app-protect-log-level: %v", *appProtectLogLevel))
392+
os.Exit(1)
370393
}
371394
}
372395

373396
if *enableTLSPassthrough && !*enableCustomResources {
374-
glog.Fatal("enable-tls-passthrough flag requires -enable-custom-resources")
397+
l.Log(ctx, levels.LevelFatal, "enable-tls-passthrough flag requires -enable-custom-resources")
398+
os.Exit(1)
375399
}
376400

377401
if *appProtect && !*nginxPlus {
378-
glog.Fatal("NGINX App Protect support is for NGINX Plus only")
402+
l.Log(ctx, levels.LevelFatal, "NGINX App Protect support is for NGINX Plus only")
403+
os.Exit(1)
379404
}
380405

381406
if *appProtectLogLevel != appProtectLogLevelDefault && !*appProtect && !*nginxPlus {
382-
glog.Fatal("app-protect-log-level support is for NGINX Plus only and App Protect is enable")
407+
l.Log(ctx, levels.LevelFatal, "app-protect-log-level support is for NGINX Plus only and App Protect is enable")
408+
os.Exit(1)
383409
}
384410

385411
if *appProtectDos && !*nginxPlus {
386-
glog.Fatal("NGINX App Protect Dos support is for NGINX Plus only")
412+
l.Log(ctx, levels.LevelFatal, "NGINX App Protect Dos support is for NGINX Plus only")
413+
os.Exit(1)
387414
}
388415

389416
if *appProtectDosDebug && !*appProtectDos && !*nginxPlus {
390-
glog.Fatal("NGINX App Protect Dos debug support is for NGINX Plus only and App Protect Dos is enable")
417+
l.Log(ctx, levels.LevelFatal, "NGINX App Protect Dos debug support is for NGINX Plus only and App Protect Dos is enable")
418+
os.Exit(1)
391419
}
392420

393421
if *appProtectDosMaxDaemons != 0 && !*appProtectDos && !*nginxPlus {
394-
glog.Fatal("NGINX App Protect Dos max daemons support is for NGINX Plus only and App Protect Dos is enable")
422+
l.Log(ctx, levels.LevelFatal, "NGINX App Protect Dos max daemons support is for NGINX Plus only and App Protect Dos is enable")
423+
os.Exit(1)
395424
}
396425

397426
if *appProtectDosMaxWorkers != 0 && !*appProtectDos && !*nginxPlus {
398-
glog.Fatal("NGINX App Protect Dos max workers support is for NGINX Plus and App Protect Dos is enable")
427+
l.Log(ctx, levels.LevelFatal, "NGINX App Protect Dos max workers support is for NGINX Plus and App Protect Dos is enable")
428+
os.Exit(1)
399429
}
400430

401431
if *appProtectDosMemory != 0 && !*appProtectDos && !*nginxPlus {
402-
glog.Fatal("NGINX App Protect Dos memory support is for NGINX Plus and App Protect Dos is enable")
432+
l.Log(ctx, levels.LevelFatal, "NGINX App Protect Dos memory support is for NGINX Plus and App Protect Dos is enable")
433+
os.Exit(1)
403434
}
404435

405436
if *enableInternalRoutes && *spireAgentAddress == "" {
406-
glog.Fatal("enable-internal-routes flag requires spire-agent-address")
437+
l.Log(ctx, levels.LevelFatal, "enable-internal-routes flag requires spire-agent-address")
438+
os.Exit(1)
407439
}
408440

409441
if *enableCertManager && !*enableCustomResources {
410-
glog.Fatal("enable-cert-manager flag requires -enable-custom-resources")
442+
l.Log(ctx, levels.LevelFatal, "enable-cert-manager flag requires -enable-custom-resources")
443+
os.Exit(1)
411444
}
412445

413446
if *enableExternalDNS && !*enableCustomResources {
414-
glog.Fatal("enable-external-dns flag requires -enable-custom-resources")
447+
l.Log(ctx, levels.LevelFatal, "enable-external-dns flag requires -enable-custom-resources")
448+
os.Exit(1)
415449
}
416450

417451
if *ingressLink != "" && *externalService != "" {
418-
glog.Fatal("ingresslink and external-service cannot both be set")
452+
l.Log(ctx, levels.LevelFatal, "ingresslink and external-service cannot both be set")
453+
os.Exit(1)
419454
}
420455

421456
if *agent && !*appProtect {
422-
glog.Fatal("NGINX Agent is used to enable the Security Monitoring dashboard and requires NGINX App Protect to be enabled")
457+
l.Log(ctx, levels.LevelFatal, "NGINX Agent is used to enable the Security Monitoring dashboard and requires NGINX App Protect to be enabled")
458+
os.Exit(1)
423459
}
424460
}
425461

cmd/nginx-ingress/main.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,20 @@ import (
4646

4747
nic_logger "github.com/nginxinc/kubernetes-ingress/internal/logger"
4848
nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"
49+
"github.com/nginxinc/kubernetes-ingress/internal/logger/levels"
4950
)
5051

5152
// Injected during build
5253
var (
5354
version string
5455
telemetryEndpoint string
5556
logLevels = map[string]slog.Level{
56-
"trace": nic_glog.LevelTrace,
57-
"debug": nic_glog.LevelDebug,
58-
"info": nic_glog.LevelInfo,
59-
"warning": nic_glog.LevelWarning,
60-
"error": nic_glog.LevelError,
61-
"fatal": nic_glog.LevelFatal,
57+
"trace": levels.LevelTrace,
58+
"debug": levels.LevelDebug,
59+
"info": levels.LevelInfo,
60+
"warning": levels.LevelWarning,
61+
"error": levels.LevelError,
62+
"fatal": levels.LevelFatal,
6263
}
6364
)
6465

@@ -79,7 +80,7 @@ func main() {
7980
ctx := initLogger(*logFormat, logLevels[*logLevel], os.Stdout)
8081
_ = nic_logger.LoggerFromContext(ctx)
8182

82-
initValidate()
83+
initValidate(ctx)
8384
parsedFlags := os.Args[1:]
8485

8586
buildOS := os.Getenv("BUILD_OS")

cmd/nginx-ingress/main_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"testing"
77

88
nic_logger "github.com/nginxinc/kubernetes-ingress/internal/logger"
9-
nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"
9+
"github.com/nginxinc/kubernetes-ingress/internal/logger/levels"
1010
)
1111

1212
func TestLogFormats(t *testing.T) {
@@ -35,9 +35,9 @@ func TestLogFormats(t *testing.T) {
3535
for _, tc := range testCases {
3636
t.Run(tc.name, func(t *testing.T) {
3737
var buf bytes.Buffer
38-
ctx := initLogger(tc.format, nic_glog.LevelInfo, &buf)
38+
ctx := initLogger(tc.format, levels.LevelInfo, &buf)
3939
l := nic_logger.LoggerFromContext(ctx)
40-
l.Log(ctx, nic_glog.LevelInfo, "test")
40+
l.Log(ctx, levels.LevelInfo, "test")
4141
got := buf.String()
4242
re := regexp.MustCompile(tc.wantre)
4343
if !re.MatchString(got) {

internal/logger/glog/handler.go

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,8 @@ import (
1111
"strconv"
1212
"strings"
1313
"sync"
14-
)
1514

16-
const (
17-
// LevelTrace - Trace Level Logging same as glog.V(3)
18-
LevelTrace = slog.Level(-8)
19-
// LevelDebug - Debug Level Logging same as glog.V(2)
20-
LevelDebug = slog.LevelDebug
21-
// LevelInfo - Info Level Logging same as glog.Info()
22-
LevelInfo = slog.LevelInfo
23-
// LevelWarning - Warn Level Logging same as glog.Warning()
24-
LevelWarning = slog.LevelWarn
25-
// LevelError - Error Level Logging same as glog.Error()
26-
LevelError = slog.LevelError
27-
// LevelFatal - Fatal Level Logging same as glog.Fatal()
28-
LevelFatal = slog.Level(12)
15+
"github.com/nginxinc/kubernetes-ingress/internal/logger/levels"
2916
)
3017

3118
// Handler holds all the parameters for the handler
@@ -80,17 +67,17 @@ func (h *Handler) Handle(_ context.Context, r slog.Record) error {
8067
buf := make([]byte, 0, 1024)
8168
// LogLevel
8269
switch r.Level {
83-
case LevelTrace:
70+
case levels.LevelTrace:
8471
buf = append(buf, "I"...)
85-
case LevelDebug:
72+
case levels.LevelDebug:
8673
buf = append(buf, "I"...)
87-
case LevelInfo:
74+
case levels.LevelInfo:
8875
buf = append(buf, "I"...)
89-
case LevelWarning:
76+
case levels.LevelWarning:
9077
buf = append(buf, "W"...)
91-
case LevelError:
78+
case levels.LevelError:
9279
buf = append(buf, "E"...)
93-
case LevelFatal:
80+
case levels.LevelFatal:
9481
buf = append(buf, "F"...)
9582
}
9683

0 commit comments

Comments
 (0)