Merge pull request #298 from nais/logrotate-fixes

Author: tronghn

cmd/device-agent/main.go

@@ -61,8 +61,8 @@ func main() {
 	programContext, programCancel := context.WithCancel(context.Background())
 	handleSignals(programCancel)
 
-	logDir := filepath.Join(cfg.ConfigDir, "logs")
-	logger.SetupLogger(cfg.LogLevel, logDir, "agent")
+	logDir := filepath.Join(cfg.ConfigDir, logger.LogDir)
+	logger.SetupLogger(cfg.LogLevel, logDir, logger.Agent)
 
 	cfg.PopulateAgentConfiguration()
 

cmd/helper/main.go

@@ -8,15 +8,16 @@ import (
 	"syscall"
 	"time"
 
+	log "github.com/sirupsen/logrus"
+	flag "github.com/spf13/pflag"
+	"google.golang.org/grpc"
+
 	"github.com/nais/device/pkg/helper"
 	"github.com/nais/device/pkg/helper/config"
 	"github.com/nais/device/pkg/logger"
 	"github.com/nais/device/pkg/pb"
 	"github.com/nais/device/pkg/unixsocket"
 	"github.com/nais/device/pkg/version"
-	log "github.com/sirupsen/logrus"
-	flag "github.com/spf13/pflag"
-	"google.golang.org/grpc"
 )
 
 var cfg = helper.Config{}
@@ -38,7 +39,7 @@ func main() {
 
 	osConfigurator := helper.New(cfg)
 
-	logger.SetupLogger(cfg.LogLevel, config.LogDir, "helper")
+	logger.SetupLogger(cfg.LogLevel, config.LogDir, logger.Helper)
 
 	log.Infof("naisdevice-helper %s starting up", version.Version)
 	log.Infof("configuration: %+v", cfg)

cmd/systray/main.go

@@ -10,13 +10,14 @@ import (
 	"syscall"
 	"time"
 
+	log "github.com/sirupsen/logrus"
+	flag "github.com/spf13/pflag"
+
 	"github.com/nais/device/pkg/config"
 	"github.com/nais/device/pkg/logger"
 	"github.com/nais/device/pkg/notify"
 	"github.com/nais/device/pkg/systray"
 	"github.com/nais/device/pkg/version"
-	log "github.com/sirupsen/logrus"
-	flag "github.com/spf13/pflag"
 )
 
 func handleSignals(cancel context.CancelFunc) {
@@ -61,8 +62,8 @@ func main() {
 	flag.StringVar(&cfg.GrpcAddress, "grpc-address", cfg.GrpcAddress, "path to device-agent unix socket")
 	flag.Parse()
 
-	logDir := filepath.Join(cfg.ConfigDir, "logs")
-	logger.SetupLogger(cfg.LogLevel, logDir, "systray")
+	logDir := filepath.Join(cfg.ConfigDir, logger.LogDir)
+	logger.SetupLogger(cfg.LogLevel, logDir, logger.Systray)
 
 	conn, err := net.Dial("unix", cfg.GrpcAddress)
 	if err != nil {

pkg/device-agent/eventloop.go

@@ -317,7 +317,7 @@ func (das *DeviceAgentServer) EventLoop(ctx context.Context) {
 						case codes.OK:
 							attempt = 0
 						case codes.Unavailable:
-							log.Warnf("Synchronize config: not connected to API server!")
+							log.Warnf("Synchronize config: not connected to API server: %v", err)
 							time.Sleep(apiServerRetryInterval)
 						case codes.Unauthenticated:
 							log.Errorf("Logging in: %s", err)

pkg/logger/agent_2020-12-31.log

@@ -10,9 +10,17 @@ import (
 	easy "github.com/t-tomalak/logrus-easy-formatter"
 )
 
-const logfileMaxAge = time.Hour * 24 * 7
+const (
+	Agent   = "agent"
+	Helper  = "helper"
+	Systray = "systray"
 
-func SetupLogger(level, logDir, name string) {
+	LogDir = "logs"
+
+	logfileMaxAge = time.Hour * 24 * 7
+)
+
+func SetupLogger(level, logDir, prefix string) {
 	err := os.MkdirAll(logDir, 0o755)
 	if err != nil {
 		log.Fatalf("Creating log dir: %v", err)
@@ -24,9 +32,9 @@ func SetupLogger(level, logDir, name string) {
 	}
 
 	// clean up old log file without date
-	_ = os.Remove(filepath.Join(logDir, name+".log"))
+	_ = os.Remove(filepath.Join(logDir, prefix+".log"))
 
-	filename := createLogFileName(name, time.Now())
+	filename := createLogFileName(prefix, time.Now())
 	logFilePath := filepath.Join(logDir, filename)
 
 	logFile, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o664)

pkg/logger/logger.go

@@ -107,3 +107,71 @@ func TestRotate(t *testing.T) {
 	}
 
 }
+
+func TestLatestFilename(t *testing.T) {
+	now, err := time.Parse(time.DateOnly, "2021-01-05")
+	assert.NoError(t, err)
+	existingFiles := []string{
+		createLogFileName("agent", now),
+		createLogFileName("agent", now.Add(-time.Hour*24*3)),
+		createLogFileName("systray", now),
+		createLogFileName("systray", now.Add(-time.Hour*24*3)),
+		createLogFileName("helper", now),
+		createLogFileName("helper", now.Add(-time.Hour*24*4)),
+	}
+
+	tests := []struct {
+		name   string
+		prefix string
+		files  []string
+		want   string
+	}{
+		{
+			name:   "latest agent file name",
+			prefix: "agent",
+			files:  existingFiles,
+			want:   createLogFileName("agent", now),
+		},
+		{
+			name:   "latest helper file name",
+			prefix: "helper",
+			files:  existingFiles,
+			want:   createLogFileName("helper", now),
+		},
+		{
+			name:   "latest systray file name",
+			prefix: "systray",
+			files:  existingFiles,
+			want:   createLogFileName("systray", now),
+		},
+		{
+			name:   "current date if no matching files",
+			prefix: "systray",
+			files: []string{
+				createLogFileName("agent", now),
+				createLogFileName("agent", now.Add(-time.Hour*24*3)),
+				createLogFileName("helper", now),
+				createLogFileName("helper", now.Add(-time.Hour*24*4)),
+			},
+			want: createLogFileName("systray", time.Now()),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			testDir := t.TempDir()
+			assert.NoError(t, err)
+
+			err := os.Chdir(testDir)
+			assert.NoError(t, err)
+
+			for _, fileName := range tt.files {
+				_, err = os.Create(fileName)
+				assert.NoError(t, err)
+			}
+
+			actual := LatestFilename(testDir, tt.prefix)
+			assert.Equal(t, tt.want, actual)
+		})
+	}
+}

pkg/logger/logger_test.go

@@ -60,3 +60,41 @@ func filterOldFilesByDate(files []os.DirEntry, treshold time.Time) ([]os.DirEntr
 func createLogFileName(prefix string, t time.Time) string {
 	return fmt.Sprintf("%s_%s.log", prefix, t.Format(time.DateOnly))
 }
+
+func LatestFilepath(logDirPath, prefix string) string {
+	return filepath.Join(logDirPath, LatestFilename(logDirPath, prefix))
+}
+
+func LatestFilename(logDirPath, prefix string) string {
+	logFiles, err := os.ReadDir(logDirPath)
+	if err != nil {
+		log.Errorf("open log dir: %v", err)
+	}
+
+	newestFilename := createLogFileName(prefix, time.Now())
+	newestDate := time.Time{}
+
+	filenameFormat := regexp.MustCompile(fmt.Sprintf(`^(%s)_(\d{4}-\d{2}-\d{2})\.log$`, prefix))
+
+	for _, logFile := range logFiles {
+		matches := filenameFormat.FindAllStringSubmatch(logFile.Name(), -1)
+		if len(matches) != 1 || len(matches[0]) != 3 {
+			log.Debug("ignoring file: ", logFile)
+			continue
+		}
+
+		date := matches[0][2]
+		logDate, err := time.Parse(time.DateOnly, date)
+		if err != nil {
+			log.Errorf("inferring latest log file: unable to parse date: %q, err: %v", date, err)
+			continue
+		}
+
+		if logDate.After(newestDate) {
+			newestDate = logDate
+			newestFilename = logFile.Name()
+		}
+	}
+
+	return newestFilename
+}

pkg/logger/rotate.go

@@ -11,6 +11,8 @@ import (
 	"time"
 
 	"github.com/nais/device/pkg/helper"
+	helperconfig "github.com/nais/device/pkg/helper/config"
+	"github.com/nais/device/pkg/logger"
 
 	"github.com/nais/device/assets"
 	"github.com/nais/device/pkg/device-agent/open"
@@ -421,23 +423,24 @@ func (gui *Gui) handleGuiEvent(guiEvent GuiEvent) {
 		}
 
 	case HelperLogClicked:
-		err := open.Open(filepath.Join(gui.Config.ConfigDir, "logs", "helper.log"))
+		err := open.Open(logger.LatestFilepath(helperconfig.LogDir, logger.Helper))
 		if err != nil {
-			log.Warn("opening device agent helper log: %w", err)
+			log.Warnf("opening device agent helper log: %v", err)
 		}
 
 	case DeviceLogClicked:
-		err := open.Open(filepath.Join(gui.Config.ConfigDir, "logs", "agent.log"))
+		logDirPath := filepath.Join(gui.Config.ConfigDir, logger.LogDir)
+		err := open.Open(logger.LatestFilepath(logDirPath, logger.Agent))
 		if err != nil {
-			log.Warn("opening device agent log: %w", err)
+			log.Warnf("opening device agent log: %v", err)
 		}
 
 	case ZipLogsClicked:
-		logDir := filepath.Join(gui.Config.ConfigDir, "logs")
+		userLogDirPath := filepath.Join(gui.Config.ConfigDir, logger.LogDir)
 		logFiles := [3]string{
-			filepath.Join(logDir, "helper.log"),
-			filepath.Join(logDir, "agent.log"),
-			filepath.Join(logDir, "systray.log"),
+			logger.LatestFilepath(userLogDirPath, logger.Agent),
+			logger.LatestFilepath(helperconfig.LogDir, logger.Helper),
+			logger.LatestFilepath(userLogDirPath, logger.Systray),
 		}
 		zipLocation, err := helper.ZipLogFiles(logFiles[:])
 		if err != nil {
@@ -449,15 +452,16 @@ func (gui *Gui) handleGuiEvent(guiEvent GuiEvent) {
 		}
 
 	case LogClicked:
-		err := open.Open(filepath.Join(gui.Config.ConfigDir, "logs", "systray.log"))
+		logDirPath := filepath.Join(gui.Config.ConfigDir, logger.LogDir)
+		err := open.Open(logger.LatestFilepath(logDirPath, logger.Systray))
 		if err != nil {
-			log.Warn("opening device agent log: %w", err)
+			log.Warnf("opening systray log: %v", err)
 		}
 
 	case AcceptableUseClicked:
 		err := open.Open("https://naisdevice-approval.nais.io/")
 		if err != nil {
-			log.Warn("opening device agent log: %w", err)
+			log.Warnf("opening naisdevice approval page: %v", err)
 		}
 	case QuitClicked:
 		_, err := gui.DeviceAgentClient.Logout(context.Background(), &pb.LogoutRequest{})