Switch grafana server command to use urfave/cli/v2 (#60684)

* switch grafana server to use urfave/cli/v2

* autocomplete support

* lint fix

packaging/autocomplete/bash_autocomplete

@@ -0,0 +1,18 @@
+#! /bin/bash
+
+_cli_bash_autocomplete() {
+  if [[ "${COMP_WORDS[0]}" != "source" ]]; then
+    local cur opts base
+    COMPREPLY=()
+    cur="${COMP_WORDS[COMP_CWORD]}"
+    if [[ "$cur" == "-"* ]]; then
+      opts=$( ${COMP_WORDS[@]:0:$COMP_CWORD} ${cur} --generate-bash-completion )
+    else
+      opts=$( ${COMP_WORDS[@]:0:$COMP_CWORD} --generate-bash-completion )
+    fi
+    COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) )
+    return 0
+  fi
+}
+
+complete -o bashdefault -o default -o nospace -F _cli_bash_autocomplete grafana

packaging/autocomplete/powershell_autocomplete.ps1

@@ -0,0 +1,8 @@
+$name = "grafana"
+Register-ArgumentCompleter -Native -CommandName $name -ScriptBlock {
+     param($commandName, $wordToComplete, $cursorPosition)
+     $other = "$wordToComplete --generate-bash-completion"
+         Invoke-Expression $other | ForEach-Object {
+            [System.Management.Automation.CompletionResult]::new($_, $_, 'ParameterValue', $_)
+         }
+ }

packaging/autocomplete/zsh_autocomplete

@@ -0,0 +1,20 @@
+#compdef grafana
+
+_cli_zsh_autocomplete() {
+  local -a opts
+  local cur
+  cur=${words[-1]}
+  if [[ "$cur" == "-"* ]]; then
+    opts=("${(@f)$(${words[@]:0:#words[@]-1} ${cur} --generate-bash-completion)}")
+  else
+    opts=("${(@f)$(${words[@]:0:#words[@]-1} --generate-bash-completion)}")
+  fi
+
+  if [[ "${opts[1]}" != "" ]]; then
+    _describe 'values' opts
+  else
+    _files
+  fi
+}
+
+compdef _cli_zsh_autocomplete grafana

pkg/cmd/grafana-cli/commands/cli.go

@@ -16,6 +16,23 @@ func CLICommand(version string) *cli.Command {
 		Name:  "cli",
 		Usage: "run the grafana cli",
 		Flags: []cli.Flag{
+			&cli.StringFlag{
+				Name:  "config",
+				Usage: "Path to config file",
+			},
+			&cli.StringFlag{
+				Name:  "homepath",
+				Usage: "Path to Grafana install/home path, defaults to working directory",
+			},
+			&cli.StringFlag{
+				Name:  "configOverrides",
+				Usage: "Configuration options to override defaults as a string. e.g. cfg:default.paths.log=/dev/null",
+			},
+			cli.VersionFlag,
+			&cli.BoolFlag{
+				Name:  "debug, d",
+				Usage: "Enable debug logging",
+			},
 			&cli.StringFlag{
 				Name:    "pluginsDir",
 				Usage:   "Path to the Grafana plugin directory",
@@ -38,23 +55,6 @@ func CLICommand(version string) *cli.Command {
 				Name:  "insecure",
 				Usage: "Skip TLS verification (insecure)",
 			},
-			&cli.BoolFlag{
-				Name:  "debug, d",
-				Usage: "Enable debug logging",
-			},
-			&cli.StringFlag{
-				Name:  "configOverrides",
-				Usage: "Configuration options to override defaults as a string. e.g. cfg:default.paths.log=/dev/null",
-			},
-			&cli.StringFlag{
-				Name:  "homepath",
-				Usage: "Path to Grafana install/home path, defaults to working directory",
-			},
-			&cli.StringFlag{
-				Name:  "config",
-				Usage: "Path to config file",
-			},
-			cli.VersionFlag,
 		},
 		Subcommands: Commands,
 		Before: func(c *cli.Context) error {

pkg/cmd/grafana-server/commands/cli.go

@@ -2,8 +2,6 @@ package commands
 
 import (
 	"context"
-	"errors"
-	"flag"
 	"fmt"
 	"net/http"
 	_ "net/http/pprof"
@@ -26,6 +24,7 @@ import (
 	_ "github.com/grafana/grafana/pkg/services/alerting/conditions"
 	_ "github.com/grafana/grafana/pkg/services/alerting/notifiers"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/urfave/cli/v2"
 )
 
 type ServerOptions struct {
@@ -33,48 +32,99 @@ type ServerOptions struct {
 	Commit      string
 	BuildBranch string
 	BuildStamp  string
-	Args        []string
+	Context     *cli.Context
 }
 
-type exitWithCode struct {
-	reason string
-	code   int
-}
-
-var serverFs = flag.NewFlagSet("server", flag.ContinueOnError)
-
-var clilog = log.New("cli")
-
-func (e exitWithCode) Error() string {
-	return e.reason
+func ServerCommand(version, commit, buildBranch, buildstamp string) *cli.Command {
+	return &cli.Command{
+		Name:  "server",
+		Usage: "run the grafana server",
+		Flags: []cli.Flag{
+			&cli.StringFlag{
+				Name:  "config",
+				Usage: "Path to config file",
+			},
+			&cli.StringFlag{
+				Name:  "homepath",
+				Usage: "Path to Grafana install/home path, defaults to working directory",
+			},
+			&cli.StringFlag{
+				Name:  "pidfile",
+				Usage: "Path to Grafana pid file",
+			},
+			&cli.StringFlag{
+				Name:  "packaging",
+				Value: "unknown",
+				Usage: "describes the way Grafana was installed",
+			},
+			&cli.StringFlag{
+				Name:  "configOverrides",
+				Usage: "Configuration options to override defaults as a string. e.g. cfg:default.paths.log=/dev/null",
+			},
+			cli.VersionFlag,
+			&cli.BoolFlag{
+				Name:  "vv",
+				Usage: "prints current version, all dependencies and exits",
+			},
+			&cli.BoolFlag{
+				Name:  "profile",
+				Value: false,
+				Usage: "Turn on pprof profiling",
+			},
+			&cli.StringFlag{
+				Name:  "profile-addr",
+				Value: "localhost",
+				Usage: "Define custom address for profiling",
+			},
+			&cli.Uint64Flag{
+				Name:  "profile-port",
+				Value: 6060,
+				Usage: "Define custom port for profiling",
+			},
+			&cli.BoolFlag{
+				Name:  "tracing",
+				Value: false,
+				Usage: "Turn on tracing",
+			},
+			&cli.StringFlag{
+				Name:  "tracing-file",
+				Value: "trace.out",
+				Usage: "Define tracing output file",
+			},
+		},
+		Action: func(context *cli.Context) error {
+			return RunServer(ServerOptions{
+				Version:     version,
+				Commit:      commit,
+				BuildBranch: buildBranch,
+				BuildStamp:  buildstamp,
+				Context:     context,
+			})
+		},
+	}
 }
 
-func RunServer(opt ServerOptions) int {
+func RunServer(opt ServerOptions) error {
 	var (
-		configFile = serverFs.String("config", "", "path to config file")
-		homePath   = serverFs.String("homepath", "", "path to grafana install/home path, defaults to working directory")
-		pidFile    = serverFs.String("pidfile", "", "path to pid file")
-		packaging  = serverFs.String("packaging", "unknown", "describes the way Grafana was installed")
-
-		configOverrides = serverFs.String("configOverrides", "", "Configuration options to override defaults as a string. e.g. cfg:default.paths.log=/dev/null")
-
-		v           = serverFs.Bool("v", false, "prints current version and exits")
-		vv          = serverFs.Bool("vv", false, "prints current version, all dependencies and exits")
-		profile     = serverFs.Bool("profile", false, "Turn on pprof profiling")
-		profileAddr = serverFs.String("profile-addr", "localhost", "Define custom address for profiling")
-		profilePort = serverFs.Uint64("profile-port", 6060, "Define custom port for profiling")
-		tracing     = serverFs.Bool("tracing", false, "Turn on tracing")
-		tracingFile = serverFs.String("tracing-file", "trace.out", "Define tracing output file")
+		configFile = opt.Context.String("config")
+		homePath   = opt.Context.String("homepath")
+		pidFile    = opt.Context.String("pidfile")
+		packaging  = opt.Context.String("packaging")
+
+		configOverrides = opt.Context.String("configOverrides")
+
+		v           = opt.Context.Bool("version")
+		vv          = opt.Context.Bool("vv")
+		profile     = opt.Context.Bool("profile")
+		profileAddr = opt.Context.String("profile-addr")
+		profilePort = opt.Context.Uint64("profile-port")
+		tracing     = opt.Context.Bool("tracing")
+		tracingFile = opt.Context.String("tracing-file")
 	)
 
-	if err := serverFs.Parse(opt.Args); err != nil {
-		fmt.Fprintln(os.Stderr, err.Error())
-		return 1
-	}
-
-	if *v || *vv {
+	if v || vv {
 		fmt.Printf("Version %s (commit: %s, branch: %s)\n", opt.Version, opt.Commit, opt.BuildBranch)
-		if *vv {
+		if vv {
 			fmt.Println("Dependencies:")
 			if info, ok := debug.ReadBuildInfo(); ok {
 				for _, dep := range info.Deps {
@@ -82,19 +132,17 @@ func RunServer(opt ServerOptions) int {
 				}
 			}
 		}
-		return 0
+		return nil
 	}
 
-	profileDiagnostics := newProfilingDiagnostics(*profile, *profileAddr, *profilePort)
+	profileDiagnostics := newProfilingDiagnostics(profile, profileAddr, profilePort)
 	if err := profileDiagnostics.overrideWithEnv(); err != nil {
-		fmt.Fprintln(os.Stderr, err.Error())
-		return 1
+		return err
 	}
 
-	traceDiagnostics := newTracingDiagnostics(*tracing, *tracingFile)
+	traceDiagnostics := newTracingDiagnostics(tracing, tracingFile)
 	if err := traceDiagnostics.overrideWithEnv(); err != nil {
-		fmt.Fprintln(os.Stderr, err.Error())
-		return 1
+		return err
 	}
 
 	if profileDiagnostics.enabled {
@@ -112,29 +160,14 @@ func RunServer(opt ServerOptions) int {
 		}()
 	}
 
-	if err := executeServer(*configFile, *homePath, *pidFile, *packaging, *configOverrides, traceDiagnostics, opt); err != nil {
-		code := 1
-		var ewc exitWithCode
-		if errors.As(err, &ewc) {
-			code = ewc.code
-		}
-		if code != 0 {
-			fmt.Fprintf(os.Stderr, "%s\n", err.Error())
-		}
-
-		return code
-	}
-
-	return 0
-}
-
-func executeServer(configFile, homePath, pidFile, packaging, configOverrides string, traceDiagnostics *tracingDiagnostics, opt ServerOptions) error {
 	defer func() {
 		if err := log.Close(); err != nil {
 			fmt.Fprintf(os.Stderr, "Failed to close log: %s\n", err)
 		}
 	}()
 
+	clilog := log.New("cli")
+
 	defer func() {
 		// If we've managed to initialize them, this is the last place
 		// where we're able to log anything that'll end up in Grafana's
@@ -196,7 +229,7 @@ func executeServer(configFile, homePath, pidFile, packaging, configOverrides str
 			Config:   configFile,
 			HomePath: homePath,
 			// tailing arguments have precedence over the options string
-			Args: append(configOptions, serverFs.Args()...),
+			Args: append(configOptions, opt.Context.Args().Slice()...),
 		},
 		server.Options{
 			PidFile:     pidFile,
@@ -207,7 +240,6 @@ func executeServer(configFile, homePath, pidFile, packaging, configOverrides str
 		api.ServerOptions{},
 	)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "Failed to start grafana. error: %s\n", err.Error())
 		return err
 	}
 
@@ -215,15 +247,7 @@ func executeServer(configFile, homePath, pidFile, packaging, configOverrides str
 
 	go listenToSystemSignals(ctx, s)
 
-	if err := s.Run(); err != nil {
-		code := s.ExitCode(err)
-		return exitWithCode{
-			reason: err.Error(),
-			code:   code,
-		}
-	}
-
-	return nil
+	return s.Run()
 }
 
 func validPackaging(packaging string) string {

pkg/cmd/grafana/main.go

@@ -29,23 +29,10 @@ func main() {
 		Version: version,
 		Commands: []*cli.Command{
 			gcli.CLICommand(version),
-			{
-				Name:  "server",
-				Usage: "server <server options>",
-				Action: func(context *cli.Context) error {
-					os.Exit(gsrv.RunServer(gsrv.ServerOptions{
-						Version:     version,
-						Commit:      commit,
-						BuildBranch: buildBranch,
-						BuildStamp:  buildstamp,
-						Args:        context.Args().Slice(),
-					}))
-					return nil
-				},
-				SkipFlagParsing: true,
-			},
+			gsrv.ServerCommand(version, commit, buildBranch, buildstamp),
 		},
-		CommandNotFound: cmdNotFound,
+		CommandNotFound:      cmdNotFound,
+		EnableBashCompletion: true,
 	}
 
 	if err := app.Run(os.Args); err != nil {

pkg/server/server.go

@@ -121,7 +121,10 @@ func (s *Server) init() error {
 	}
 	s.isInitialized = true
 
-	s.writePIDFile()
+	if err := s.writePIDFile(); err != nil {
+		return err
+	}
+
 	if err := metrics.SetEnvironmentInformation(s.cfg.MetricsGrafanaEnvironmentInfo); err != nil {
 		return err
 	}
@@ -203,36 +206,28 @@ func (s *Server) Shutdown(ctx context.Context, reason string) error {
 	return err
 }
 
-// ExitCode returns an exit code for a given error.
-func (s *Server) ExitCode(runError error) int {
-	if runError != nil {
-		s.log.Error("Server shutdown", "error", runError)
-		return 1
-	}
-	return 0
-}
-
 // writePIDFile retrieves the current process ID and writes it to file.
-func (s *Server) writePIDFile() {
+func (s *Server) writePIDFile() error {
 	if s.pidFile == "" {
-		return
+		return nil
 	}
 
 	// Ensure the required directory structure exists.
 	err := os.MkdirAll(filepath.Dir(s.pidFile), 0700)
 	if err != nil {
 		s.log.Error("Failed to verify pid directory", "error", err)
-		os.Exit(1)
+		return fmt.Errorf("failed to verify pid directory: %s", err)
 	}
 
 	// Retrieve the PID and write it to file.
 	pid := strconv.Itoa(os.Getpid())
 	if err := os.WriteFile(s.pidFile, []byte(pid), 0644); err != nil {
 		s.log.Error("Failed to write pidfile", "error", err)
-		os.Exit(1)
+		return fmt.Errorf("failed to write pidfile: %s", err)
 	}
 
 	s.log.Info("Writing PID file", "path", s.pidFile, "pid", pid)
+	return nil
 }
 
 // notifySystemd sends state notifications to systemd.

pkg/server/server_test.go

@@ -61,7 +61,6 @@ func TestServer_Run_Error(t *testing.T) {
 	s := testServer(t, newTestService(nil, false), newTestService(testErr, false))
 	err := s.Run()
 	require.ErrorIs(t, err, testErr)
-	require.NotZero(t, s.ExitCode(err))
 }
 
 func TestServer_Shutdown(t *testing.T) {
@@ -87,7 +86,6 @@ func TestServer_Shutdown(t *testing.T) {
 	}()
 	err := s.Run()
 	require.NoError(t, err)
-	require.Zero(t, s.ExitCode(err))
 
 	err = <-ch
 	require.NoError(t, err)