agent: Fixed a bug where syslog error messages marked as notice. (#24820)

The mapping between Nomad log level identifiers and syslog
priorities did not handle the error level string correctly.

.changelog/24820.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+agent: Fixed a bug where Nomad error log messages within syslog showed via the notice priority
+```

command/agent/command.go

@@ -574,20 +574,13 @@ func SetupLoggers(ui cli.Ui, config *Config) (*logutils.LevelFilter, *gatedwrite
 	// Create a log writer, and wrap a logOutput around it
 	writers := []io.Writer{logFilter}
 	logLevel := strings.ToUpper(config.LogLevel)
-	logLevelMap := map[string]gsyslog.Priority{
-		"ERROR": gsyslog.LOG_ERR,
-		"WARN":  gsyslog.LOG_WARNING,
-		"INFO":  gsyslog.LOG_INFO,
-		"DEBUG": gsyslog.LOG_DEBUG,
-		"TRACE": gsyslog.LOG_DEBUG,
-	}
 	if logLevel == "OFF" {
 		config.EnableSyslog = false
 	}
 	// Check if syslog is enabled
 	if config.EnableSyslog {
 		ui.Output(fmt.Sprintf("Config enable_syslog is `true` with log_level=%v", config.LogLevel))
-		l, err := gsyslog.NewLogger(logLevelMap[logLevel], config.SyslogFacility, "nomad")
+		l, err := gsyslog.NewLogger(getSysLogPriority(logLevel), config.SyslogFacility, "nomad")
 		if err != nil {
 			ui.Error(fmt.Sprintf("Syslog setup failed: %v", err))
 			return nil, nil, nil

command/agent/syslog.go

@@ -10,15 +10,26 @@ import (
 	"github.com/hashicorp/logutils"
 )
 
-// levelPriority is used to map a log level to a
-// syslog priority level
+// levelPriority is used to map a log level to a syslog priority level. The
+// level strings should match those described within LevelFilter except for
+// "OFF" which disables syslog.
 var levelPriority = map[string]gsyslog.Priority{
 	"TRACE": gsyslog.LOG_DEBUG,
 	"DEBUG": gsyslog.LOG_INFO,
 	"INFO":  gsyslog.LOG_NOTICE,
 	"WARN":  gsyslog.LOG_WARNING,
-	"ERR":   gsyslog.LOG_ERR,
-	"CRIT":  gsyslog.LOG_CRIT,
+	"ERROR": gsyslog.LOG_ERR,
+}
+
+// getSysLogPriority returns the syslog priority value associated to the passed
+// log level. If the log level does not have a known mapping, the notice
+// priority is returned.
+func getSysLogPriority(level string) gsyslog.Priority {
+	priority, ok := levelPriority[level]
+	if !ok {
+		priority = gsyslog.LOG_NOTICE
+	}
+	return priority
 }
 
 // SyslogWrapper is used to cleanup log messages before
@@ -48,13 +59,7 @@ func (s *SyslogWrapper) Write(p []byte) (int, error) {
 		}
 	}
 
-	// Each log level will be handled by a specific syslog priority
-	priority, ok := levelPriority[level]
-	if !ok {
-		priority = gsyslog.LOG_NOTICE
-	}
-
-	// Attempt the write
-	err := s.l.WriteLevel(priority, afterLevel)
+	// Attempt to write using the converted syslog priority.
+	err := s.l.WriteLevel(getSysLogPriority(level), afterLevel)
 	return len(p), err
 }

command/agent/syslog_test.go

@@ -4,46 +4,86 @@
 package agent
 
 import (
-	"os"
 	"runtime"
 	"testing"
 
 	gsyslog "github.com/hashicorp/go-syslog"
-	"github.com/hashicorp/logutils"
 	"github.com/hashicorp/nomad/ci"
+	"github.com/shoenig/test/must"
 )
 
+func Test_getSysLogPriority(t *testing.T) {
+	ci.Parallel(t)
+
+	if runtime.GOOS == "windows" {
+		t.Skip("Syslog not supported on Windows")
+	}
+
+	testCases := []struct {
+		name                   string
+		inputLogLevel          string
+		expectedSyslogPriority gsyslog.Priority
+	}{
+		{
+			name:                   "trace",
+			inputLogLevel:          "TRACE",
+			expectedSyslogPriority: gsyslog.LOG_DEBUG,
+		},
+		{
+			name:                   "debug",
+			inputLogLevel:          "DEBUG",
+			expectedSyslogPriority: gsyslog.LOG_INFO,
+		},
+		{
+			name:                   "info",
+			inputLogLevel:          "INFO",
+			expectedSyslogPriority: gsyslog.LOG_NOTICE,
+		},
+		{
+			name:                   "warn",
+			inputLogLevel:          "WARN",
+			expectedSyslogPriority: gsyslog.LOG_WARNING,
+		},
+		{
+			name:                   "error",
+			inputLogLevel:          "ERROR",
+			expectedSyslogPriority: gsyslog.LOG_ERR,
+		},
+		{
+			name:                   "unknown",
+			inputLogLevel:          "UNKNOWN",
+			expectedSyslogPriority: gsyslog.LOG_NOTICE,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actualPriority := getSysLogPriority(tc.inputLogLevel)
+			must.Eq(t, tc.expectedSyslogPriority, actualPriority)
+		})
+	}
+}
+
 func TestSyslogFilter(t *testing.T) {
 	ci.Parallel(t)
 	if runtime.GOOS == "windows" {
 		t.Skip("Syslog not supported on Windows")
 	}
-	if os.Getenv("TRAVIS") == "true" {
-		t.Skip("Syslog not supported on travis-ci")
-	}
 
-	l, err := gsyslog.NewLogger(gsyslog.LOG_NOTICE, "LOCAL0", "consul")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
+	l, err := gsyslog.NewLogger(gsyslog.LOG_NOTICE, "LOCAL0", "nomad")
+	must.NoError(t, err)
 
 	filt := LevelFilter()
-	filt.MinLevel = logutils.LogLevel("INFO")
+	filt.MinLevel = "INFO"
 
 	s := &SyslogWrapper{l, filt}
 	n, err := s.Write([]byte("[INFO] test"))
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
-	if n == 0 {
-		t.Fatalf("should have logged")
-	}
+	must.NonZero(t, n)
 
 	n, err = s.Write([]byte("[DEBUG] test"))
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if n != 0 {
-		t.Fatalf("should not have logged")
-	}
+	must.NoError(t, err)
+	must.Zero(t, n)
 }