From 911cb4a3b95da4c2e471a834032b4970a7b337f5 Mon Sep 17 00:00:00 2001 From: jason yang Date: Mon, 15 May 2023 10:34:32 +0000 Subject: [PATCH] squash commits Signed-off-by: jason yang --- CHANGELOG.md | 1 + internal/pkg/warewulfd/daemon.go | 8 +- internal/pkg/wwlog/wwlog.go | 98 +++++------ internal/pkg/wwlog/wwlog_test.go | 284 +++++++++++++++++++++++++++++++ 4 files changed, 334 insertions(+), 57 deletions(-) create mode 100644 internal/pkg/wwlog/wwlog_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d1522e07..00a9a795 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added experimental dnsmasq support. - Check for formal correct IP and MAC addresses for command line options and when reading in the configurations +- Write log messages to stderr rather than stdout. #768 ## [4.4.0] 2023-01-18 diff --git a/internal/pkg/warewulfd/daemon.go b/internal/pkg/warewulfd/daemon.go index bfa16f9d..6833b2e9 100644 --- a/internal/pkg/warewulfd/daemon.go +++ b/internal/pkg/warewulfd/daemon.go @@ -9,9 +9,9 @@ import ( "syscall" "time" + warewulfconf "github.com/hpcng/warewulf/internal/pkg/config" "github.com/hpcng/warewulf/internal/pkg/util" "github.com/hpcng/warewulf/internal/pkg/version" - warewulfconf "github.com/hpcng/warewulf/internal/pkg/config" "github.com/hpcng/warewulf/internal/pkg/wwlog" "github.com/pkg/errors" ) @@ -73,7 +73,7 @@ func DaemonInitLogging() error { } wwlog.SetLogFormatter(wwlog.DefaultFormatter) - wwlog.SetLogWriters(logwriter, logwriter) + wwlog.SetLogWriter(logwriter) } @@ -107,12 +107,12 @@ func DaemonStart() error { os.Setenv("WAREWULFD_LOGLEVEL", strconv.Itoa(logLevel)) } - f, err := os.OpenFile(WAREWULFD_LOGFILE, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) + f, err := os.OpenFile(WAREWULFD_LOGFILE, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0o644) if err != nil { return err } - p, err := os.OpenFile(WAREWULFD_PIDFILE, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) + p, err := os.OpenFile(WAREWULFD_PIDFILE, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o644) if err != nil { return err } diff --git a/internal/pkg/wwlog/wwlog.go b/internal/pkg/wwlog/wwlog.go index 7879c728..98f67f2a 100644 --- a/internal/pkg/wwlog/wwlog.go +++ b/internal/pkg/wwlog/wwlog.go @@ -2,29 +2,29 @@ package wwlog import ( "fmt" - "os" "io" + "os" + "reflect" + "runtime" + "sort" "strings" "time" - "runtime" - "reflect" - "sort" ) type LogRecord struct { Level int - Err error - Msg string - Args []interface{} - Pc uintptr - File string - Line int - Time time.Time + Err error + Msg string + Args []interface{} + Pc uintptr + File string + Line int + Time time.Time } /* - Format a log message from a record - Only called if rec.level >= logLevel +Format a log message from a record +Only called if rec.level >= logLevel */ type LogFormatter func(logLevel int, rec *LogRecord) string @@ -47,19 +47,20 @@ var ( DEBUG = SetLevelName(10, "DEBUG") ) -var levelNums = []int{0} -var levelNames = []string{"NOTSET"} -var logLevel = INFO -var logOut io.Writer = os.Stdout -var logErr io.Writer = os.Stderr -var logFormatter LogFormatter = DefaultFormatter +var ( + levelNums = []int{0} + levelNames = []string{"NOTSET"} + logLevel = INFO + logErr io.Writer = os.Stderr + logFormatter LogFormatter = DefaultFormatter +) func LevelNameEff(level int) (int, int, string) { n := len(levelNums) idx := sort.SearchInts(levelNums, level) if idx >= n { - idx = n-1 + idx = n - 1 } eff_level := levelNums[idx] @@ -69,7 +70,6 @@ func LevelNameEff(level int) (int, int, string) { } func LevelName(level int) string { - _, _, name := LevelNameEff(level) return name } @@ -80,8 +80,7 @@ func SetLevelName(level int, name string) int { if idx < n && levelNums[idx] == level { levelNames[idx] = name - - }else{ + } else { levelNums = append(levelNums, level) levelNames = append(levelNames, name) @@ -101,7 +100,7 @@ func SetLevelName(level int, name string) int { func DefaultFormatter(logLevel int, rec *LogRecord) string { message := fmt.Sprintf(rec.Msg, rec.Args...) - if ( !strings.HasSuffix(message, "\n") ) { + if !strings.HasSuffix(message, "\n") { // ensure written messages are separated by at least one newline message += "\n" } @@ -109,10 +108,9 @@ func DefaultFormatter(logLevel int, rec *LogRecord) string { if rec.Err != nil { if logLevel < VERBOSE { // when debugging errors, add file and line number, and any stack trace - message += fmt.Sprintf("%s:%d\n%+v\n", rec.File, rec.Line, rec.Err ) - - }else{ - message += fmt.Sprintf("%v\n", rec.Err ) + message += fmt.Sprintf("%s:%d\n%+v\n", rec.File, rec.Line, rec.Err) + } else { + message += fmt.Sprintf("%v\n", rec.Err) } } @@ -133,7 +131,6 @@ func DefaultFormatter(logLevel int, rec *LogRecord) string { return fmt.Sprintf("%-11s: %s", name, message) } - func EnabledForLevel(level int) bool { return level >= logLevel } @@ -153,17 +150,15 @@ func GetLogLevel() int { } /* -Set the log output writers -By default they are set to os.Stdout and os.Stderr +Set the log output writer +By default they are set to output writer */ -func SetLogWriters(out io.Writer, err io.Writer) { - logOut = out +func SetLogWriter(err io.Writer) { logErr = err - Debug("Set log writers") } -func GetLogWriters() (io.Writer, io.Writer) { - return logOut, logErr +func GetLogWriter() io.Writer { + return logErr } /* @@ -180,10 +175,9 @@ func GetLogFormatter() LogFormatter { } /* - Internal method to create a log record +Internal method to create a log record */ func LogCaller(level int, skip int, err error, message string, a ...interface{}) { - if EnabledForLevel(level) { pc, file, line, ok := runtime.Caller(skip + 1) if !ok { @@ -191,22 +185,19 @@ func LogCaller(level int, skip int, err error, message string, a ...interface{}) } rec := LogRecord{ - Level : level, - Err : err, - Msg : message, - Args : a, - Pc : pc, - File : file, - Line : line, - Time : time.Now() } + Level: level, + Err: err, + Msg: message, + Args: a, + Pc: pc, + File: file, + Line: line, + Time: time.Now(), + } message = logFormatter(logLevel, &rec) - if level >= ERROR { - fmt.Fprint(logErr, message) - } else { - fmt.Fprint(logOut, message) - } + fmt.Fprint(logErr, message) } } @@ -218,8 +209,9 @@ func Printf(level int, message string, a ...interface{}) { LogCaller(level, 1, nil, message, a...) } -/******************************************************************************* - Named log level functions +/* +****************************************************************************** +Named log level functions */ func Log(level int, message string, a ...interface{}) { LogCaller(level, 1, nil, message, a...) diff --git a/internal/pkg/wwlog/wwlog_test.go b/internal/pkg/wwlog/wwlog_test.go new file mode 100644 index 00000000..72d61899 --- /dev/null +++ b/internal/pkg/wwlog/wwlog_test.go @@ -0,0 +1,284 @@ +package wwlog + +import ( + "errors" + "io" + "os" + "strings" + "testing" +) + +type ( + levelTypeFunc func(int, string, ...interface{}) + msgTypeFunc func(string, ...interface{}) + errTypeFunc func(error, string, ...interface{}) + levelErrTypeFunc func(int, error, string, ...interface{}) +) + +func Test_Log(t *testing.T) { + t.Logf("Running Test_Log test") + SetLogLevel(DEBUG) + + tests := []struct { + name string + msgTypeFunc msgTypeFunc + levelTypeFunc levelTypeFunc + errTypeFunc errTypeFunc + levelErrTypeFunc levelErrTypeFunc + level int + err error + message string + args interface{} + expect string + exactMatch bool + }{ + { + name: "Log DEBUG", + levelTypeFunc: Log, + level: DEBUG, + message: "Log", + expect: "DEBUG : Log\n", + exactMatch: true, + }, + { + name: "Log ERROR", + levelTypeFunc: Log, + level: ERROR, + message: "Log", + expect: "ERROR : Log\n", + exactMatch: true, + }, + { + name: "LogExc", + levelErrTypeFunc: LogExc, + level: INFO, + err: errors.New("error"), + message: "Log", + expect: "INFO : Log\nerror\n", + }, + { + name: "Debug", + msgTypeFunc: Debug, + message: "Debug", + expect: "DEBUG : Debug\n", + exactMatch: true, + }, + { + name: "DebugExc", + errTypeFunc: DebugExc, + message: "Debug", + err: errors.New("error"), + expect: "DEBUG : Debug\nerror\n", + }, + { + name: "SecDebug", + msgTypeFunc: SecDebug, + message: "Debug", + expect: "SECDEBUG : Debug\n", + exactMatch: true, + }, + { + name: "Verbose", + msgTypeFunc: Verbose, + message: "Verbose", + expect: "VERBOSE: Verbose\n", + exactMatch: true, + }, + { + name: "VerboseExc", + errTypeFunc: VerboseExc, + message: "Verbose", + err: errors.New("error"), + expect: "VERBOSE: Verbose\nerror\n", + }, + { + name: "SecVerbose", + msgTypeFunc: SecVerbose, + message: "Verbose", + expect: "SECVERBOSE : Verbose\n", + exactMatch: true, + }, + { + name: "Info", + msgTypeFunc: Info, + message: "Info", + expect: "INFO : Info\n", + exactMatch: true, + }, + { + name: "InfoExc", + errTypeFunc: InfoExc, + message: "Info", + err: errors.New("error"), + expect: "INFO : Info\nerror\n", + }, + { + name: "SecInfo", + msgTypeFunc: SecInfo, + message: "Info", + expect: "SECINFO: Info\n", + exactMatch: true, + }, + { + name: "Serv", + msgTypeFunc: Serv, + message: "Serv", + expect: "SERV : Serv\n", + exactMatch: true, + }, + { + name: "Recv", + msgTypeFunc: Recv, + message: "Recv", + expect: "RECV : Recv\n", + exactMatch: true, + }, + { + name: "Send", + msgTypeFunc: Send, + message: "Send", + expect: "SEND : Send\n", + exactMatch: true, + }, + { + name: "Warn", + msgTypeFunc: Warn, + message: "Warn", + expect: "WARN : Warn\n", + exactMatch: true, + }, + { + name: "WarnExc", + errTypeFunc: WarnExc, + message: "Warn", + err: errors.New("error"), + expect: "WARN : Warn\nerror\n", + }, + { + name: "SecWarn", + msgTypeFunc: SecWarn, + message: "Warn", + expect: "SECWARN: Warn\n", + exactMatch: true, + }, + { + name: "Error", + msgTypeFunc: Error, + message: "Error", + expect: "ERROR : Error\n", + exactMatch: true, + }, + { + name: "ErrorExc", + errTypeFunc: ErrorExc, + message: "Error", + err: errors.New("error"), + expect: "ERROR : Error\nerror\n", + }, + { + name: "SecError", + msgTypeFunc: SecError, + message: "Error", + expect: "SECERROR : Error\n", + exactMatch: true, + }, + { + name: "Denied", + msgTypeFunc: Denied, + message: "Denied", + expect: "DENIED : Denied\n", + exactMatch: true, + }, + { + name: "Critical", + msgTypeFunc: Critical, + message: "Critical", + expect: "CRITICAL : Critical\n", + exactMatch: true, + }, + { + name: "CriticalExc", + errTypeFunc: CriticalExc, + message: "Critical", + err: errors.New("error"), + expect: "CRITICAL : Critical\nerror\n", + }, + { + name: "SecCritical", + msgTypeFunc: SecCritical, + message: "Critical", + expect: "SECCRITICAL: Critical\n", + exactMatch: true, + }, + } + + for _, tt := range tests { + oldErr := os.Stderr + + r, w, err := os.Pipe() + if err != nil { + t.Errorf("Could not create stderr pipe, err:%v", err) + t.FailNow() + } + os.Stderr = w + // make sure os.Stderr is always reset + defer func() { + os.Stderr = oldErr + }() + + SetLogWriter(os.Stderr) + + if tt.msgTypeFunc != nil { + if tt.args != nil { + tt.msgTypeFunc(tt.message, tt.args) + } else { + tt.msgTypeFunc(tt.message) + } + } else if tt.levelTypeFunc != nil { + if tt.args != nil { + tt.levelTypeFunc(tt.level, tt.message, tt.args) + } else { + tt.levelTypeFunc(tt.level, tt.message) + } + } else if tt.errTypeFunc != nil { + if tt.args != nil { + tt.errTypeFunc(tt.err, tt.message, tt.args) + } else { + tt.errTypeFunc(tt.err, tt.message) + } + } else if tt.levelErrTypeFunc != nil { + if tt.args != nil { + tt.levelErrTypeFunc(tt.level, tt.err, tt.message, tt.args) + } else { + tt.levelErrTypeFunc(tt.level, tt.err, tt.message) + } + } else { + os.Stderr = oldErr + t.Errorf("One of `msgTypeFunc`, `levelTypeFunc` and `errTypeFunc` should be set") + t.FailNow() + } + + outCh := make(chan string, 1) + go func() { + out, _ := io.ReadAll(r) + outCh <- string(out) + }() + + w.Close() + os.Stderr = oldErr + out := <-outCh + if tt.exactMatch { + if out != tt.expect { + t.Errorf("Test: %s failed with unexpected output out: `%s`, expect: `%s`", tt.name, out, tt.expect) + t.FailNow() + } + } else { + for _, line := range strings.Split(tt.expect, "\n") { + if !strings.Contains(out, line) { + t.Errorf("Test: %s should contain output expect: `%s`, out:`%s`", tt.name, line, out) + t.FailNow() + } + } + } + } +}