[CLI Config Parity] migrate log-file legacy flag/config to new config

cmd/flags.go

@@ -485,11 +485,6 @@ func resolvePathForTheFlagInContext(flagKey string, c *cli.Context) (err error)
 // GCSFUSE_PARENT_PROCESS_DIR. Child process is spawned when --foreground flag
 // is disabled.
 func resolvePathForTheFlagsInContext(c *cli.Context) (err error) {
-	err = resolvePathForTheFlagInContext("log-file", c)
-	if err != nil {
-		return fmt.Errorf("resolving for log-file: %w", err)
-	}
-
 	err = resolvePathForTheFlagInContext("config-file", c)
 	if err != nil {
 		return fmt.Errorf("resolving for config-file: %w", err)
@@ -500,11 +495,6 @@ func resolvePathForTheFlagsInContext(c *cli.Context) (err error) {
 
 // resolveConfigFilePaths resolves the config file paths specified in the config file.
 func resolveConfigFilePaths(mountConfig *config.MountConfig) (err error) {
-	mountConfig.LogConfig.FilePath, err = resolveFilePath(mountConfig.LogConfig.FilePath, "logging: file")
-	if err != nil {
-		return
-	}
-
 	// Resolve cache-dir path
 	resolvedPath, err := resolveFilePath(string(mountConfig.CacheDir), "cache-dir")
 	mountConfig.CacheDir = resolvedPath

cmd/flags_test.go

@@ -274,23 +274,18 @@ func (t *FlagsTest) TestResolvePathForTheFlagInContext() {
 	currentWorkingDir, err := os.Getwd()
 	assert.Equal(t.T(), nil, err)
 	app.Action = func(appCtx *cli.Context) {
-		err = resolvePathForTheFlagInContext("log-file", appCtx)
-		assert.Equal(t.T(), nil, err)
 		err = resolvePathForTheFlagInContext("key-file", appCtx)
 		assert.Equal(t.T(), nil, err)
 		err = resolvePathForTheFlagInContext("config-file", appCtx)
 		assert.Equal(t.T(), nil, err)
 
-		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
-			appCtx.String("log-file"))
 		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
 			appCtx.String("key-file"))
 		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "config.yaml"),
 			appCtx.String("config-file"))
 	}
 	// Simulate argv.
-	fullArgs := []string{"some_app", "--log-file=test.txt",
-		"--key-file=test.txt", "--config-file=config.yaml"}
+	fullArgs := []string{"some_app", "--key-file=test.txt", "--config-file=config.yaml"}
 
 	err = app.Run(fullArgs)
 
@@ -303,13 +298,12 @@ func (t *FlagsTest) TestResolvePathForTheFlagsInContext() {
 	assert.Equal(t.T(), nil, err)
 	app.Action = func(appCtx *cli.Context) {
 		resolvePathForTheFlagsInContext(appCtx)
-		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
-			appCtx.String("log-file"))
+
 		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "config.yaml"),
 			appCtx.String("config-file"))
 	}
 	// Simulate argv.
-	fullArgs := []string{"some_app", "--log-file=test.txt", "--config-file=config.yaml"}
+	fullArgs := []string{"some_app", "--config-file=config.yaml"}
 
 	err = app.Run(fullArgs)
 
@@ -417,17 +411,13 @@ func (t *FlagsTest) TestValidateFlagsForUnsupportedExperimentalMetadataPrefetchO
 
 func (t *FlagsTest) Test_resolveConfigFilePaths() {
 	mountConfig := &config.MountConfig{}
-	mountConfig.LogConfig = config.LogConfig{
-		FilePath: "~/test.txt",
-	}
 	mountConfig.CacheDir = "~/cache-dir"
 
 	err := resolveConfigFilePaths(mountConfig)
 
 	assert.Equal(t.T(), nil, err)
 	homeDir, err := os.UserHomeDir()
 	assert.Equal(t.T(), nil, err)
-	assert.Equal(t.T(), filepath.Join(homeDir, "test.txt"), mountConfig.LogConfig.FilePath)
 	assert.EqualValues(t.T(), filepath.Join(homeDir, "cache-dir"), mountConfig.CacheDir)
 }
 
@@ -437,7 +427,6 @@ func (t *FlagsTest) Test_resolveConfigFilePaths_WithoutSettingPaths() {
 	err := resolveConfigFilePaths(mountConfig)
 
 	assert.Equal(t.T(), nil, err)
-	assert.Equal(t.T(), "", mountConfig.LogConfig.FilePath)
 	assert.EqualValues(t.T(), "", mountConfig.CacheDir)
 }
 

cmd/legacy_main.go

@@ -258,7 +258,7 @@ func runCLIApp(c *cli.Context) (err error) {
 		return fmt.Errorf("error resolving flags and configs: %w", err)
 	}
 
-	config.OverrideWithLoggingFlags(mountConfig, flags.LogFile, flags.LogFormat,
+	config.OverrideWithLoggingFlags(mountConfig, flags.LogFormat,
 		flags.DebugFuse, flags.DebugGCS, flags.DebugMutex)
 	config.OverrideWithIgnoreInterruptsFlag(c, mountConfig, flags.IgnoreInterrupts)
 	config.OverrideWithAnonymousAccessFlag(c, mountConfig, flags.AnonymousAccess)
@@ -276,7 +276,7 @@ func runCLIApp(c *cli.Context) (err error) {
 	}
 
 	if flags.Foreground {
-		err = logger.InitLogFile(mountConfig.LogConfig)
+		err = logger.InitLogFile(mountConfig.LogConfig, newConfig.Logging)
 		if err != nil {
 			return fmt.Errorf("init log file: %w", err)
 		}
@@ -296,7 +296,7 @@ func runCLIApp(c *cli.Context) (err error) {
 	// Do not log these in stdout in case of daemonized run
 	// if these are already being logged into a log-file, otherwise
 	// there will be duplicate logs for these in both places (stdout and log-file).
-	if flags.Foreground || mountConfig.LogConfig.FilePath == "" {
+	if flags.Foreground || newConfig.Logging.FilePath == "" {
 		flagsStringified, err := util.Stringify(*flags)
 		if err != nil {
 			logger.Warnf("failed to stringify cli flags: %v", err)

cmd/legacy_param_mapper_test.go

@@ -394,3 +394,51 @@ func TestPopulateConfigFromLegacyFlags_KeyFileResolution(t *testing.T) {
 		})
 	}
 }
+
+func TestPopulateConfigFromLegacyFlags_LogFileResolution(t *testing.T) {
+	currentWorkingDir, err := os.Getwd()
+	require.Nil(t, err)
+	var logFileTests = []struct {
+		testName        string
+		givenLogFile    string
+		expectedLogFile cfg.ResolvedPath
+	}{
+		{
+			testName:        "absolute path",
+			givenLogFile:    "/tmp/log-file.json",
+			expectedLogFile: "/tmp/log-file.json",
+		},
+		{
+			testName:        "relative path",
+			givenLogFile:    "~/Documents/log-file.json",
+			expectedLogFile: cfg.ResolvedPath(path.Join(os.Getenv("HOME"), "Documents/log-file.json")),
+		},
+		{
+			testName:        "current working directory",
+			givenLogFile:    "log-file.json",
+			expectedLogFile: cfg.ResolvedPath(path.Join(currentWorkingDir, "log-file.json")),
+		},
+		{
+			testName:        "empty path",
+			givenLogFile:    "",
+			expectedLogFile: "",
+		},
+	}
+
+	for _, tc := range logFileTests {
+		t.Run(tc.testName, func(t *testing.T) {
+			mockCLICtx := &mockCLIContext{}
+			legacyFlagStorage := &flagStorage{
+				ClientProtocol: mountpkg.HTTP2,
+				LogFile:        tc.givenLogFile,
+			}
+			legacyMountCfg := &config.MountConfig{}
+
+			resolvedConfig, err := PopulateNewConfigFromLegacyFlagsAndConfig(mockCLICtx, legacyFlagStorage, legacyMountCfg)
+
+			if assert.Nil(t, err) {
+				assert.Equal(t, tc.expectedLogFile, resolvedConfig.Logging.FilePath)
+			}
+		})
+	}
+}

internal/config/config_util.go

@@ -34,12 +34,8 @@ const (
 
 // OverrideWithLoggingFlags overwrites the configs with the flag values if the
 // config values are empty.
-func OverrideWithLoggingFlags(mountConfig *MountConfig, logFile string, logFormat string,
+func OverrideWithLoggingFlags(mountConfig *MountConfig, logFormat string,
 	debugFuse bool, debugGCS bool, debugMutex bool) {
-	// If log file is not set in config file, override it with flag value.
-	if mountConfig.LogConfig.FilePath == "" {
-		mountConfig.LogConfig.FilePath = logFile
-	}
 	// If log format is not set in config file, override it with flag value.
 	if mountConfig.LogConfig.Format == "" {
 		mountConfig.LogConfig.Format = logFormat

internal/config/config_util_test.go

@@ -59,17 +59,15 @@ func (t *ConfigTest) TestOverrideLoggingFlags_WithNonEmptyLogConfigs() {
 	mountConfig := &MountConfig{}
 	mountConfig.LogConfig = LogConfig{
 		Severity: ERROR,
-		FilePath: "/tmp/hello.txt",
 		Format:   "text",
 	}
 	mountConfig.WriteConfig = WriteConfig{
 		CreateEmptyFile: true,
 	}
 
-	OverrideWithLoggingFlags(mountConfig, f.LogFile, f.LogFormat, f.DebugFuse, f.DebugGCS, f.DebugMutex)
+	OverrideWithLoggingFlags(mountConfig, f.LogFormat, f.DebugFuse, f.DebugGCS, f.DebugMutex)
 
 	assert.Equal(t.T(), "text", mountConfig.LogConfig.Format)
-	assert.Equal(t.T(), "/tmp/hello.txt", mountConfig.LogConfig.FilePath)
 	assert.Equal(t.T(), TRACE, mountConfig.LogConfig.Severity)
 }
 
@@ -81,17 +79,15 @@ func (t *ConfigTest) TestOverrideLoggingFlags_WithEmptyLogConfigs() {
 	mountConfig := &MountConfig{}
 	mountConfig.LogConfig = LogConfig{
 		Severity: INFO,
-		FilePath: "",
 		Format:   "",
 	}
 	mountConfig.WriteConfig = WriteConfig{
 		CreateEmptyFile: true,
 	}
 
-	OverrideWithLoggingFlags(mountConfig, f.LogFile, f.LogFormat, f.DebugFuse, f.DebugGCS, f.DebugMutex)
+	OverrideWithLoggingFlags(mountConfig, f.LogFormat, f.DebugFuse, f.DebugGCS, f.DebugMutex)
 
 	assert.Equal(t.T(), "json", mountConfig.LogConfig.Format)
-	assert.Equal(t.T(), "a.txt", mountConfig.LogConfig.FilePath)
 	assert.Equal(t.T(), INFO, mountConfig.LogConfig.Severity)
 }
 

internal/logger/logger.go

@@ -23,6 +23,7 @@ import (
 	"os"
 	"runtime/debug"
 
+	"github.com/googlecloudplatform/gcsfuse/v2/cfg"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
 	"gopkg.in/natefinch/lumberjack.v2"
 )
@@ -51,14 +52,14 @@ var (
 // config.
 // Here, background true means, this InitLogFile has been called for the
 // background daemon.
-func InitLogFile(logConfig config.LogConfig) error {
+func InitLogFile(legacyLogConfig config.LogConfig, newLogConfig cfg.LoggingConfig) error {
 	var f *os.File
 	var sysWriter *syslog.Writer
 	var fileWriter *lumberjack.Logger
 	var err error
-	if logConfig.FilePath != "" {
+	if newLogConfig.FilePath != "" {
 		f, err = os.OpenFile(
-			logConfig.FilePath,
+			string(newLogConfig.FilePath),
 			os.O_WRONLY|os.O_CREATE|os.O_APPEND,
 			0644,
 		)
@@ -67,9 +68,9 @@ func InitLogFile(logConfig config.LogConfig) error {
 		}
 		fileWriter = &lumberjack.Logger{
 			Filename:   f.Name(),
-			MaxSize:    logConfig.LogRotateConfig.MaxFileSizeMB,
-			MaxBackups: logConfig.LogRotateConfig.BackupFileCount,
-			Compress:   logConfig.LogRotateConfig.Compress,
+			MaxSize:    legacyLogConfig.LogRotateConfig.MaxFileSizeMB,
+			MaxBackups: legacyLogConfig.LogRotateConfig.BackupFileCount,
+			Compress:   legacyLogConfig.LogRotateConfig.Compress,
 		}
 	} else {
 		if _, ok := os.LookupEnv(GCSFuseInBackgroundMode); ok {
@@ -90,11 +91,11 @@ func InitLogFile(logConfig config.LogConfig) error {
 		file:            f,
 		sysWriter:       sysWriter,
 		fileWriter:      fileWriter,
-		format:          logConfig.Format,
-		level:           logConfig.Severity,
-		logRotateConfig: logConfig.LogRotateConfig,
+		format:          legacyLogConfig.Format,
+		level:           legacyLogConfig.Severity,
+		logRotateConfig: legacyLogConfig.LogRotateConfig,
 	}
-	defaultLogger = defaultLoggerFactory.newLogger(logConfig.Severity)
+	defaultLogger = defaultLoggerFactory.newLogger(legacyLogConfig.Severity)
 
 	return nil
 }

internal/logger/logger_test.go

@@ -21,6 +21,7 @@ import (
 	"regexp"
 	"testing"
 
+	"github.com/googlecloudplatform/gcsfuse/v2/cfg"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
@@ -274,18 +275,18 @@ func (t *LoggerTest) TestInitLogFile() {
 	filePath += "/log.txt"
 	fileSize := 100
 	backupFileCount := 2
-	logConfig := config.LogConfig{
+	legacyLogConfig := config.LogConfig{
 		Severity: config.DEBUG,
 		Format:   format,
-		FilePath: filePath,
 		LogRotateConfig: config.LogRotateConfig{
 			MaxFileSizeMB:   fileSize,
 			BackupFileCount: backupFileCount,
 			Compress:        true,
 		},
 	}
+	newLogConfig := cfg.LoggingConfig{FilePath: cfg.ResolvedPath(filePath)}
 
-	err := InitLogFile(logConfig)
+	err := InitLogFile(legacyLogConfig, newLogConfig)
 
 	assert.NoError(t.T(), err)
 	assert.Equal(t.T(), filePath, defaultLoggerFactory.file.Name())

tools/integration_tests/log_rotation/log_rotation_test.go

@@ -94,6 +94,7 @@ func TestMain(m *testing.M) {
 	// Set up directory for logs.
 	logDirPath = setup.SetUpLogDirForTestDirTests(logDirName)
 	logFilePath = path.Join(logDirPath, logFileName)
+	setup.SetLogFile(logFilePath)
 
 	// Set up config files.
 	// TODO: add tests for backupLogFileCount = 0.