Merge pull request #380 from sctb512/fix-toml-file-not-work

Fix the problem of snapshotter's toml configuration file not working

cmd/containerd-nydus-grpc/pkg/command/flags.go

@@ -8,18 +8,9 @@
 package command
 
 import (
-	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli/v2"
 )
 
-const (
-	defaultAddress           = "/run/containerd-nydus/containerd-nydus-grpc.sock"
-	defaultLogLevel          = logrus.InfoLevel
-	defaultRootDir           = "/var/lib/containerd-nydus"
-	defaultDaemonMode string = "multiple"
-	defaultFsDriver   string = "fusedev"
-)
-
 type Args struct {
 	Address               string
 	LogLevel              string
@@ -48,13 +39,11 @@ func buildFlags(args *Args) []cli.Flag {
 		},
 		&cli.StringFlag{
 			Name:        "root",
-			Value:       defaultRootDir,
 			Usage:       "the directory storing snapshotter working states",
 			Destination: &args.RootDir,
 		},
 		&cli.StringFlag{
 			Name:        "address",
-			Value:       defaultAddress,
 			Usage:       "gRPC socket path",
 			Destination: &args.Address,
 		},
@@ -71,19 +60,16 @@ func buildFlags(args *Args) []cli.Flag {
 		},
 		&cli.StringFlag{
 			Name:        "daemon-mode",
-			Value:       defaultDaemonMode,
 			Usage:       "spawning nydusd daemon mode, legal values include \"multiple\", \"shared\" or \"none\"",
 			Destination: &args.DaemonMode,
 		},
 		&cli.StringFlag{
 			Name:        "fs-driver",
-			Value:       defaultFsDriver,
 			Usage:       "fulfill image service based on what fs driver, possible values include \"fusedev\", \"fscache\"",
 			Destination: &args.FsDriver,
 		},
 		&cli.StringFlag{
 			Name:        "log-level",
-			Value:       defaultLogLevel.String(),
 			Usage:       "logging level, possible values \"trace\", \"debug\", \"info\", \"warn\", \"error\"",
 			Destination: &args.LogLevel,
 		},

cmd/containerd-nydus-grpc/pkg/command/flags_test.go

@@ -20,7 +20,7 @@ func TestNewFlags(t *testing.T) {
 		err := i.Apply(set)
 		assert.Nil(t, err)
 	}
-	err := set.Parse([]string{"--config-path", "/etc/testconfig", "--root", "/root"})
+	err := set.Parse([]string{"--config-path", "/etc/testconfig", "--root", "/root", "--log-level", "info"})
 	assert.Nil(t, err)
 	assert.Equal(t, flags.Args.NydusdConfigPath, "/etc/testconfig")
 	assert.Equal(t, flags.Args.LogLevel, "info")

config/config.go

@@ -108,7 +108,7 @@ type DaemonConfig struct {
 }
 
 type LoggingConfig struct {
-	LogToStdout         bool   `toml:"log_to_stdout"`
+	LogToStdout         bool
 	LogLevel            string `toml:"level"`
 	LogDir              string `toml:"dir"`
 	RotateLogMaxSize    int    `toml:"log_rotation_max_size"`
@@ -255,32 +255,44 @@ func ValidateConfig(c *SnapshotterConfig) error {
 // Always let options from CLI override those from configuration file.
 func ParseParameters(args *command.Args, cfg *SnapshotterConfig) error {
 	// --- essential configuration
-	cfg.Address = args.Address
-	cfg.Root = args.RootDir
+	if args.Address != "" {
+		cfg.Address = args.Address
+	}
+	if args.RootDir != "" {
+		cfg.Root = args.RootDir
+	}
 
 	// Give --shared-daemon higher priority
-	cfg.DaemonMode = args.DaemonMode
+	if args.DaemonMode != "" {
+		cfg.DaemonMode = args.DaemonMode
+	}
 
 	// --- image processor configuration
 	// empty
 
 	// --- daemon configuration
 	daemonConfig := &cfg.DaemonConfig
-	daemonConfig.NydusdConfigPath = args.NydusdConfigPath
+	if args.NydusdConfigPath != "" {
+		daemonConfig.NydusdConfigPath = args.NydusdConfigPath
+	}
 	if args.NydusdPath != "" {
 		daemonConfig.NydusdPath = args.NydusdPath
 	}
 	if args.NydusImagePath != "" {
 		daemonConfig.NydusImagePath = args.NydusImagePath
 	}
-	daemonConfig.FsDriver = args.FsDriver
+	if args.FsDriver != "" {
+		daemonConfig.FsDriver = args.FsDriver
+	}
 
 	// --- cache manager configuration
 	// empty
 
 	// --- logging configuration
 	logConfig := &cfg.LoggingConfig
-	logConfig.LogLevel = args.LogLevel
+	if args.LogLevel != "" {
+		logConfig.LogLevel = args.LogLevel
+	}
 	logConfig.LogToStdout = args.LogToStdout
 
 	// --- remote storage configuration

config/config_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/containerd/nydus-snapshotter/cmd/containerd-nydus-grpc/pkg/command"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -81,6 +82,20 @@ func TestLoadSnapshotterTOMLConfig(t *testing.T) {
 
 	A.EqualValues(cfg, &exampleConfig)
 
+	var args = command.Args{}
+	args.RootDir = "/var/lib/containerd/nydus"
+	exampleConfig.Root = "/var/lib/containerd/nydus"
+	err = ParseParameters(&args, cfg)
+	A.NoError(err)
+	A.EqualValues(cfg, &exampleConfig)
+
+	A.EqualValues(cfg.LoggingConfig.LogToStdout, false)
+
+	args.LogToStdout = true
+	err = ParseParameters(&args, cfg)
+	A.NoError(err)
+	A.EqualValues(cfg.LoggingConfig.LogToStdout, true)
+
 	err = ProcessConfigurations(cfg)
 	A.NoError(err)
 

misc/snapshotter/config.toml

@@ -35,8 +35,6 @@ nydusd_config = "/etc/nydus/nydusd-config.fusedev.json"
 threads_number = 4
 
 [log]
-# Print logs to stdout rather than logging files
-log_to_stdout = false
 # Snapshotter's log level
 level = "info"
 log_rotation_compress = true

tests/e2e/k8s/snapshotter-cri.yaml

@@ -158,7 +158,6 @@ data:
     log_rotation_max_backups = 5
     # In unit MB(megabytes)
     log_rotation_max_size = 1
-    log_to_stdout = false
 
     [remote]
     convert_vpc_registry = false

tests/e2e/k8s/snapshotter-kubeconf.yaml

@@ -181,7 +181,6 @@ data:
     log_rotation_max_backups = 5
     # In unit MB(megabytes)
     log_rotation_max_size = 1
-    log_to_stdout = false
 
     [remote]
     convert_vpc_registry = false