[breaking] Split daemon mode configs from core configs (#1622)

* Split daemon mode configs from core configs

* Enhanced documentation

Co-authored-by: per1234 <[email protected]>

* Apply suggestions from code review

Co-authored-by: per1234 <[email protected]>

Co-authored-by: per1234 <[email protected]>

Author: silvanocerza

cli/cli.go

@@ -17,7 +17,6 @@ package cli
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"strings"
 
@@ -37,7 +36,6 @@ import (
 	"github.com/arduino/arduino-cli/cli/lib"
 	"github.com/arduino/arduino-cli/cli/monitor"
 	"github.com/arduino/arduino-cli/cli/outdated"
-	"github.com/arduino/arduino-cli/cli/output"
 	"github.com/arduino/arduino-cli/cli/sketch"
 	"github.com/arduino/arduino-cli/cli/update"
 	"github.com/arduino/arduino-cli/cli/updater"
@@ -47,17 +45,14 @@ import (
 	"github.com/arduino/arduino-cli/configuration"
 	"github.com/arduino/arduino-cli/i18n"
 	"github.com/arduino/arduino-cli/inventory"
-	"github.com/fatih/color"
-	"github.com/mattn/go-colorable"
-	"github.com/rifflock/lfshook"
+	"github.com/arduino/arduino-cli/logging"
+	"github.com/arduino/arduino-cli/output"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	semver "go.bug.st/relaxed-semver"
 )
 
 var (
-	verbose            bool
-	outputFormat       string
 	configFile         string
 	updaterMessageChan chan *semver.Version = make(chan *semver.Version)
 )
@@ -104,57 +99,34 @@ func createCliCommandTree(cmd *cobra.Command) {
 	cmd.AddCommand(burnbootloader.NewCommand())
 	cmd.AddCommand(version.NewCommand())
 
-	cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, tr("Print the logs on the standard output."))
 	validLogLevels := []string{"trace", "debug", "info", "warn", "error", "fatal", "panic"}
-	cmd.PersistentFlags().String("log-level", "", tr("Messages with this level and above will be logged. Valid levels are: %s", strings.Join(validLogLevels, ", ")))
+	validLogFormats := []string{"text", "json"}
+	cmd.PersistentFlags().String("log-level", "info", tr("Messages with this level and above will be logged. Valid levels are: %s", strings.Join(validLogLevels, ", ")))
+	cmd.PersistentFlags().String("log-file", "", tr("Path to the file where logs will be written."))
+	cmd.PersistentFlags().String("log-format", "text", tr("The output format for the logs, can be: %s", strings.Join(validLogFormats, ", ")))
 	cmd.RegisterFlagCompletionFunc("log-level", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		return validLogLevels, cobra.ShellCompDirectiveDefault
 	})
-	cmd.PersistentFlags().String("log-file", "", tr("Path to the file where logs will be written."))
-	validLogFormats := []string{"text", "json"}
-	cmd.PersistentFlags().String("log-format", "", tr("The output format for the logs, can be: %s", strings.Join(validLogFormats, ", ")))
 	cmd.RegisterFlagCompletionFunc("log-format", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		return validLogFormats, cobra.ShellCompDirectiveDefault
 	})
 	validOutputFormats := []string{"text", "json", "jsonmini", "yaml"}
-	cmd.PersistentFlags().StringVar(&outputFormat, "format", "text", tr("The output format for the logs, can be: %s", strings.Join(validOutputFormats, ", ")))
+	cmd.PersistentFlags().BoolP("verbose", "v", false, tr("Print the logs on the standard output."))
+	cmd.PersistentFlags().String("format", "text", tr("The output format for the logs, can be: %s", strings.Join(validOutputFormats, ", ")))
+	cmd.PersistentFlags().Bool("no-color", false, "Disable colored output.")
 	cmd.RegisterFlagCompletionFunc("format", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		return validOutputFormats, cobra.ShellCompDirectiveDefault
 	})
 	cmd.PersistentFlags().StringVar(&configFile, "config-file", "", tr("The custom config file (if not specified the default will be used)."))
 	cmd.PersistentFlags().StringSlice("additional-urls", []string{}, tr("Comma-separated list of additional URLs for the Boards Manager."))
-	cmd.PersistentFlags().Bool("no-color", false, "Disable colored output.")
 	configuration.BindFlags(cmd, configuration.Settings)
 }
 
-// convert the string passed to the `--log-level` option to the corresponding
-// logrus formal level.
-func toLogLevel(s string) (t logrus.Level, found bool) {
-	t, found = map[string]logrus.Level{
-		"trace": logrus.TraceLevel,
-		"debug": logrus.DebugLevel,
-		"info":  logrus.InfoLevel,
-		"warn":  logrus.WarnLevel,
-		"error": logrus.ErrorLevel,
-		"fatal": logrus.FatalLevel,
-		"panic": logrus.PanicLevel,
-	}[s]
-
-	return
-}
-
-func parseFormatString(arg string) (feedback.OutputFormat, bool) {
-	f, found := map[string]feedback.OutputFormat{
-		"json":     feedback.JSON,
-		"jsonmini": feedback.JSONMini,
-		"text":     feedback.Text,
-		"yaml":     feedback.YAML,
-	}[strings.ToLower(arg)]
-
-	return f, found
-}
-
 func preRun(cmd *cobra.Command, args []string) {
+	if cmd.Name() == "daemon" {
+		return
+	}
+
 	configFile := configuration.Settings.ConfigFileUsed()
 
 	// initialize inventory
@@ -164,12 +136,13 @@ func preRun(cmd *cobra.Command, args []string) {
 		os.Exit(errorcodes.ErrBadArgument)
 	}
 
-	// https://no-color.org/
-	color.NoColor = configuration.Settings.GetBool("output.no_color") || os.Getenv("NO_COLOR") != ""
-
-	// Set default feedback output to colorable
-	feedback.SetOut(colorable.NewColorableStdout())
-	feedback.SetErr(colorable.NewColorableStderr())
+	outputFormat, err := cmd.Flags().GetString("format")
+	if err != nil {
+		feedback.Errorf(tr("Error getting flag value: %s", err))
+		os.Exit(errorcodes.ErrBadCall)
+	}
+	noColor := configuration.Settings.GetBool("output.no_color") || os.Getenv("NO_COLOR") != ""
+	output.Setup(outputFormat, noColor)
 
 	updaterMessageChan = make(chan *semver.Version)
 	go func() {
@@ -185,70 +158,19 @@ func preRun(cmd *cobra.Command, args []string) {
 		updaterMessageChan <- updater.CheckForUpdate(currentVersion)
 	}()
 
-	//
-	// Prepare logging
-	//
-
-	// decide whether we should log to stdout
-	if verbose {
-		// if we print on stdout, do it in full colors
-		logrus.SetOutput(colorable.NewColorableStdout())
-		logrus.SetFormatter(&logrus.TextFormatter{
-			ForceColors:   true,
-			DisableColors: color.NoColor,
-		})
-	} else {
-		logrus.SetOutput(ioutil.Discard)
-	}
-
-	// set the Logger format
-	logFormat := strings.ToLower(configuration.Settings.GetString("logging.format"))
-	if logFormat == "json" {
-		logrus.SetFormatter(&logrus.JSONFormatter{})
-	}
-
-	// should we log to file?
-	logFile := configuration.Settings.GetString("logging.file")
-	if logFile != "" {
-		file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
-		if err != nil {
-			fmt.Println(tr("Unable to open file for logging: %s", logFile))
-			os.Exit(errorcodes.ErrBadCall)
-		}
-
-		// we use a hook so we don't get color codes in the log file
-		if logFormat == "json" {
-			logrus.AddHook(lfshook.NewHook(file, &logrus.JSONFormatter{}))
-		} else {
-			logrus.AddHook(lfshook.NewHook(file, &logrus.TextFormatter{}))
-		}
-	}
-
-	// configure logging filter
-	if lvl, found := toLogLevel(configuration.Settings.GetString("logging.level")); !found {
-		feedback.Errorf(tr("Invalid option for --log-level: %s"), configuration.Settings.GetString("logging.level"))
-		os.Exit(errorcodes.ErrBadArgument)
-	} else {
-		logrus.SetLevel(lvl)
-	}
-
-	//
-	// Prepare the Feedback system
-	//
-
-	// normalize the format strings
-	outputFormat = strings.ToLower(outputFormat)
-	// configure the output package
-	output.OutputFormat = outputFormat
-	// check the right output format was passed
-	format, found := parseFormatString(outputFormat)
-	if !found {
-		feedback.Errorf(tr("Invalid output format: %s"), outputFormat)
+	// Setups logging if necessary
+	verbose, err := cmd.Flags().GetBool("verbose")
+	if err != nil {
+		feedback.Errorf(tr("Error getting flag value: %s", err))
 		os.Exit(errorcodes.ErrBadCall)
 	}
-
-	// use the output format to configure the Feedback
-	feedback.SetFormat(format)
+	logging.Setup(
+		verbose,
+		noColor,
+		configuration.Settings.GetString("logging.level"),
+		configuration.Settings.GetString("logging.file"),
+		configuration.Settings.GetString("logging.format"),
+	)
 
 	//
 	// Print some status info and check command is consistent

cli/config/validate.go

@@ -26,7 +26,6 @@ import (
 
 var validMap = map[string]reflect.Kind{
 	"board_manager.additional_urls": reflect.Slice,
-	"daemon.port":                   reflect.String,
 	"directories.data":              reflect.String,
 	"directories.downloads":         reflect.String,
 	"directories.user":              reflect.String,

cli/daemon/daemon.go

@@ -31,7 +31,8 @@ import (
 	"github.com/arduino/arduino-cli/commands/daemon"
 	"github.com/arduino/arduino-cli/configuration"
 	"github.com/arduino/arduino-cli/i18n"
-	"github.com/arduino/arduino-cli/metrics"
+	"github.com/arduino/arduino-cli/logging"
+	"github.com/arduino/arduino-cli/output"
 	srv_commands "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	srv_debug "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/debug/v1"
 	srv_monitor "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/monitor/v1"
@@ -44,64 +45,89 @@ import (
 
 var (
 	tr           = i18n.Tr
-	daemonize    bool
-	debug        bool
 	debugFilters []string
+
+	daemonConfigFile string
 )
 
 // NewCommand created a new `daemon` command
 func NewCommand() *cobra.Command {
 	daemonCommand := &cobra.Command{
 		Use:     "daemon",
-		Short:   tr("Run as a daemon on port: %s", configuration.Settings.GetString("daemon.port")),
-		Long:    tr("Running as a daemon the initialization of cores and libraries is done only once."),
+		Short:   tr("Run as a daemon on specified IP and port"),
+		Long:    tr("Running as a daemon multiple different client can use the same Arduino CLI process with different settings."),
 		Example: "  " + os.Args[0] + " daemon",
 		Args:    cobra.NoArgs,
 		Run:     runDaemonCommand,
 	}
-	daemonCommand.PersistentFlags().String("port", "", tr("The TCP port the daemon will listen to"))
-	configuration.Settings.BindPFlag("daemon.port", daemonCommand.PersistentFlags().Lookup("port"))
-	daemonCommand.Flags().BoolVar(&daemonize, "daemonize", false, tr("Do not terminate daemon process if the parent process dies"))
-	daemonCommand.Flags().BoolVar(&debug, "debug", false, tr("Enable debug logging of gRPC calls"))
-	daemonCommand.Flags().StringSliceVar(&debugFilters, "debug-filter", []string{}, tr("Display only the provided gRPC calls"))
+	daemonCommand.Flags().String("ip", "127.0.0.1", tr("The IP the daemon will listen to"))
+	daemonCommand.Flags().String("port", "50051", tr("The TCP port the daemon will listen to"))
+	daemonCommand.Flags().Bool("daemonize", false, tr("Run daemon process in background"))
+	daemonCommand.Flags().Bool("debug", false, tr("Enable debug logging of gRPC calls"))
+	daemonCommand.Flags().StringSlice("debug-filter", []string{}, tr("Display only the provided gRPC calls when debug is enabled"))
+	daemonCommand.Flags().Bool("metrics-enabled", false, tr("Enable local metrics collection"))
+	daemonCommand.Flags().String("metrics-address", ":9090", tr("Metrics local address"))
+	// Metrics for the time being are ignored and unused, might as well hide this setting
+	// from the user since they would do nothing.
+	daemonCommand.Flags().MarkHidden("metrics-enabled")
+	daemonCommand.Flags().MarkHidden("metrics-address")
+
+	daemonCommand.Flags().StringVar(&daemonConfigFile, "config-file", "", tr("The daemon config file (if not specified default values will be used)."))
 	return daemonCommand
 }
 
 func runDaemonCommand(cmd *cobra.Command, args []string) {
-	logrus.Info("Executing `arduino-cli daemon`")
+	s, err := load(cmd, daemonConfigFile)
+	if err != nil {
+		feedback.Errorf(tr("Error reading daemon config file: %v"), err)
+		os.Exit(errorcodes.ErrGeneric)
+	}
 
-	if configuration.Settings.GetBool("metrics.enabled") {
-		metrics.Activate("daemon")
-		stats.Incr("daemon", stats.T("success", "true"))
-		defer stats.Flush()
+	noColor := s.NoColor || os.Getenv("NO_COLOR") != ""
+	output.Setup(s.OutputFormat, noColor)
+
+	if daemonConfigFile != "" {
+		// Tell the user which config file we're using only after output setup
+		feedback.Printf(tr("Using daemon config file %s", daemonConfigFile))
 	}
-	port := configuration.Settings.GetString("daemon.port")
+
+	logging.Setup(
+		s.Verbose,
+		noColor,
+		s.LogLevel,
+		s.LogFile,
+		s.LogFormat,
+	)
+
+	logrus.Info("Executing `arduino-cli daemon`")
+
 	gRPCOptions := []grpc.ServerOption{}
-	if debug {
+	if s.Debug {
+		debugFilters = s.DebugFilter
 		gRPCOptions = append(gRPCOptions,
 			grpc.UnaryInterceptor(unaryLoggerInterceptor),
 			grpc.StreamInterceptor(streamLoggerInterceptor),
 		)
 	}
-	s := grpc.NewServer(gRPCOptions...)
+	server := grpc.NewServer(gRPCOptions...)
 	// Set specific user-agent for the daemon
 	configuration.Settings.Set("network.user_agent_ext", "daemon")
 
 	// register the commands service
-	srv_commands.RegisterArduinoCoreServiceServer(s, &daemon.ArduinoCoreServerImpl{
+	srv_commands.RegisterArduinoCoreServiceServer(server, &daemon.ArduinoCoreServerImpl{
 		VersionString: globals.VersionInfo.VersionString,
 	})
 
 	// Register the monitors service
-	srv_monitor.RegisterMonitorServiceServer(s, &daemon.MonitorService{})
+	srv_monitor.RegisterMonitorServiceServer(server, &daemon.MonitorService{})
 
 	// Register the settings service
-	srv_settings.RegisterSettingsServiceServer(s, &daemon.SettingsService{})
+	srv_settings.RegisterSettingsServiceServer(server, &daemon.SettingsService{})
 
 	// Register the debug session service
-	srv_debug.RegisterDebugServiceServer(s, &daemon.DebugService{})
+	srv_debug.RegisterDebugServiceServer(server, &daemon.DebugService{})
 
-	if !daemonize {
+	if !s.Daemonize {
 		// When parent process ends terminate also the daemon
 		go func() {
 			// Stdin is closed when the controlling parent process ends
@@ -112,51 +138,50 @@ func runDaemonCommand(cmd *cobra.Command, args []string) {
 		}()
 	}
 
-	ip := "127.0.0.1"
-	lis, err := net.Listen("tcp", fmt.Sprintf("%s:%s", ip, port))
+	lis, err := net.Listen("tcp", fmt.Sprintf("%s:%s", s.IP, s.Port))
 	if err != nil {
 		// Invalid port, such as "Foo"
 		var dnsError *net.DNSError
 		if errors.As(err, &dnsError) {
-			feedback.Errorf(tr("Failed to listen on TCP port: %[1]s. %[2]s is unknown name."), port, dnsError.Name)
+			feedback.Errorf(tr("Failed to listen on TCP port: %[1]s. %[2]s is unknown name."), s.Port, dnsError.Name)
 			os.Exit(errorcodes.ErrCoreConfig)
 		}
 		// Invalid port number, such as -1
 		var addrError *net.AddrError
 		if errors.As(err, &addrError) {
-			feedback.Errorf(tr("Failed to listen on TCP port: %[1]s. %[2]s is an invalid port."), port, addrError.Addr)
+			feedback.Errorf(tr("Failed to listen on TCP port: %[1]s. %[2]s is an invalid port."), s.Port, addrError.Addr)
 			os.Exit(errorcodes.ErrCoreConfig)
 		}
 		// Port is already in use
 		var syscallErr *os.SyscallError
 		if errors.As(err, &syscallErr) && errors.Is(syscallErr.Err, syscall.EADDRINUSE) {
-			feedback.Errorf(tr("Failed to listen on TCP port: %s. Address already in use."), port)
+			feedback.Errorf(tr("Failed to listen on TCP port: %s. Address already in use."), s.Port)
 			os.Exit(errorcodes.ErrNetwork)
 		}
-		feedback.Errorf(tr("Failed to listen on TCP port: %[1]s. Unexpected error: %[2]v"), port, err)
+		feedback.Errorf(tr("Failed to listen on TCP port: %[1]s. Unexpected error: %[2]v"), s.Port, err)
 		os.Exit(errorcodes.ErrGeneric)
 	}
 
 	// We need to parse the port used only if the user let
 	// us choose it randomly, in all other cases we already
 	// know which is used.
-	if port == "0" {
+	if s.Port == "0" {
 		address := lis.Addr()
 		split := strings.Split(address.String(), ":")
 
 		if len(split) == 0 {
 			feedback.Error(tr("Failed choosing port, address: %s", address))
 		}
 
-		port = split[len(split)-1]
+		s.Port = split[len(split)-1]
 	}
 
 	feedback.PrintResult(daemonResult{
-		IP:   ip,
-		Port: port,
+		IP:   s.IP,
+		Port: s.Port,
 	})
 
-	if err := s.Serve(lis); err != nil {
+	if err := server.Serve(lis); err != nil {
 		logrus.Fatalf("Failed to serve: %v", err)
 	}
 }