pkg/cmd: Check errors (#19700)

* pkg/cmd: Check errors
* pkg/cmd: Make sure server waits on services, even in case of error
* pkg/cmd: Inform of error to show help
* pkg/cmd: Only warn on failure to send systemd notification
* pkg/cmd: Don't log errors stemming from context cancelation
* pkg/cmd: Don't fail if unable to write to systemd

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

@@ -22,11 +22,14 @@ func runDbCommand(command func(commandLine utils.CommandLine, sqlStore *sqlstore
 		cfg := setting.NewCfg()
 
 		configOptions := strings.Split(cmd.GlobalString("configOverrides"), " ")
-		cfg.Load(&setting.CommandLineArgs{
+		if err := cfg.Load(&setting.CommandLineArgs{
 			Config:   cmd.ConfigFile(),
 			HomePath: cmd.HomePath(),
 			Args:     append(configOptions, cmd.Args()...), // tailing arguments have precedence over the options string
-		})
+		}); err != nil {
+			logger.Errorf("\n%s: Failed to load configuration", color.RedString("Error"))
+			os.Exit(1)
+		}
 
 		if debug {
 			cfg.LogConfigSources()
@@ -35,13 +38,19 @@ func runDbCommand(command func(commandLine utils.CommandLine, sqlStore *sqlstore
 		engine := &sqlstore.SqlStore{}
 		engine.Cfg = cfg
 		engine.Bus = bus.GetBus()
-		engine.Init()
+		if err := engine.Init(); err != nil {
+			logger.Errorf("\n%s: Failed to initialize SQL engine", color.RedString("Error"))
+			os.Exit(1)
+		}
 
 		if err := command(cmd, engine); err != nil {
 			logger.Errorf("\n%s: ", color.RedString("Error"))
 			logger.Errorf("%s\n\n", err)
 
-			cmd.ShowHelp()
+			if err := cmd.ShowHelp(); err != nil {
+				logger.Errorf("\n%s: Failed to show help: %s %s\n\n", color.RedString("Error"),
+					color.RedString("✗"), err)
+			}
 			os.Exit(1)
 		}
 
@@ -57,7 +66,10 @@ func runPluginCommand(command func(commandLine utils.CommandLine) error) func(co
 			logger.Errorf("\n%s: ", color.RedString("Error"))
 			logger.Errorf("%s %s\n\n", color.RedString("✗"), err)
 
-			cmd.ShowHelp()
+			if err := cmd.ShowHelp(); err != nil {
+				logger.Errorf("\n%s: Failed to show help: %s %s\n\n", color.RedString("Error"),
+					color.RedString("✗"), err)
+			}
 			os.Exit(1)
 		}
 

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

@@ -80,8 +80,9 @@ func (fcli *FakeCommandLine) FlagNames() []string {
 	return flagNames
 }
 
-func (fcli *FakeCommandLine) ShowHelp() {
+func (fcli *FakeCommandLine) ShowHelp() error {
 	fcli.HelpShown = true
+	return nil
 }
 
 func (fcli *FakeCommandLine) Application() *cli.App {

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

@@ -121,7 +121,10 @@ func InstallPlugin(pluginName, version string, c utils.CommandLine) error {
 
 	res, _ := s.ReadPlugin(pluginFolder, pluginName)
 	for _, v := range res.Dependencies.Plugins {
-		InstallPlugin(v.Id, "", c)
+		if err := InstallPlugin(v.Id, "", c); err != nil {
+			return errutil.Wrapf(err, "Failed to install plugin '%s'", v.Id)
+		}
+
 		logger.Infof("Installed dependency: %v ✔\n", v.Id)
 	}
 

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

@@ -5,6 +5,7 @@ import (
 	"github.com/grafana/grafana/pkg/cmd/grafana-cli/logger"
 	s "github.com/grafana/grafana/pkg/cmd/grafana-cli/services"
 	"github.com/grafana/grafana/pkg/cmd/grafana-cli/utils"
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 func upgradeCommand(c utils.CommandLine) error {
@@ -24,7 +25,10 @@ func upgradeCommand(c utils.CommandLine) error {
 	}
 
 	if shouldUpgrade(localPlugin.Info.Version, &plugin) {
-		s.RemoveInstalledPlugin(pluginsDir, pluginName)
+		if err := s.RemoveInstalledPlugin(pluginsDir, pluginName); err != nil {
+			return errutil.Wrapf(err, "Failed to remove plugin '%s'", pluginName)
+		}
+
 		return InstallPlugin(pluginName, "", c)
 	}
 

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

@@ -80,7 +80,9 @@ func ReadPlugin(pluginDir, pluginName string) (m.InstalledPlugin, error) {
 	}
 
 	res := m.InstalledPlugin{}
-	json.Unmarshal(data, &res)
+	if err := json.Unmarshal(data, &res); err != nil {
+		return res, err
+	}
 
 	if res.Info.Version == "" {
 		res.Info.Version = "0.0.0"

pkg/cmd/grafana-cli/utils/command_line.go

@@ -7,7 +7,7 @@ import (
 )
 
 type CommandLine interface {
-	ShowHelp()
+	ShowHelp() error
 	ShowVersion()
 	Application() *cli.App
 	Args() cli.Args
@@ -35,8 +35,8 @@ type ContextCommandLine struct {
 	*cli.Context
 }
 
-func (c *ContextCommandLine) ShowHelp() {
+func (c *ContextCommandLine) ShowHelp() error {
-	cli.ShowCommandHelp(c.Context, c.Command.Name)
+	return cli.ShowCommandHelp(c.Context, c.Command.Name)
 }
 
 func (c *ContextCommandLine) ShowVersion() {

pkg/cmd/grafana-server/server.go

@@ -39,6 +39,8 @@ import (
 	_ "github.com/grafana/grafana/pkg/services/search"
 	_ "github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/util/errutil"
+	"golang.org/x/xerrors"
 )
 
 // NewServer returns a new instance of Server.
@@ -79,7 +81,7 @@ type Server struct {
 
 // Run initializes and starts services. This will block until all services have
 // exited. To initiate shutdown, call the Shutdown method in another goroutine.
-func (s *Server) Run() error {
+func (s *Server) Run() (err error) {
 	s.loadConfiguration()
 	s.writePIDFile()
 
@@ -88,8 +90,8 @@ func (s *Server) Run() error {
 
 	services := registry.GetServices()
 
-	if err := s.buildServiceGraph(services); err != nil {
+	if err = s.buildServiceGraph(services); err != nil {
-		return err
+		return
 	}
 
 	// Initialize services.
@@ -107,18 +109,17 @@ func (s *Server) Run() error {
 
 	// Start background services.
 	for _, svc := range services {
-		// Variable is needed for accessing loop variable in function callback
-		descriptor := svc
-
 		service, ok := svc.Instance.(registry.BackgroundService)
 		if !ok {
 			continue
 		}
 
-		if registry.IsDisabled(descriptor.Instance) {
+		if registry.IsDisabled(svc.Instance) {
 			continue
 		}
 
+		// Variable is needed for accessing loop variable in callback
+		descriptor := svc
 		s.childRoutines.Go(func() error {
 			// Don't start new services when server is shutting down.
 			if s.shutdownInProgress {
@@ -134,19 +135,28 @@ func (s *Server) Run() error {
 				}
 			}
 
+			// Mark that we are in shutdown mode
+			// So more services are not started
 			s.shutdownInProgress = true
-
 			return nil
 		})
 	}
 
-	notifySystemd("READY=1")
+	defer func() {
+		s.log.Debug("Waiting on services...")
+		if waitErr := s.childRoutines.Wait(); waitErr != nil && !xerrors.Is(waitErr, context.Canceled) {
+			s.log.Error("A service failed", "err", waitErr)
+			if err == nil {
+				err = waitErr
+			}
+		}
+	}()
 
-	return s.childRoutines.Wait()
+	s.notifySystemd("READY=1")
+
+	return
 }
 
-// Shutdown initiates a shutdown of the services, and waits for all services to
-// exit.
 func (s *Server) Shutdown(reason string) {
 	s.log.Info("Shutdown started", "reason", reason)
 	s.shutdownReason = reason
@@ -156,7 +166,9 @@ func (s *Server) Shutdown(reason string) {
 	s.shutdownFn()
 
 	// wait for child routines
-	s.childRoutines.Wait()
+	if err := s.childRoutines.Wait(); err != nil && !xerrors.Is(err, context.Canceled) {
+		s.log.Error("Failed waiting for services to shutdown", "err", err)
+	}
 }
 
 // ExitCode returns an exit code for a given error.
@@ -216,13 +228,13 @@ func (s *Server) buildServiceGraph(services []*registry.Descriptor) error {
 	// Provide services and their dependencies to the graph.
 	for _, obj := range objs {
 		if err := serviceGraph.Provide(&inject.Object{Value: obj}); err != nil {
-			return fmt.Errorf("Failed to provide object to the graph: %v", err)
+			return errutil.Wrapf(err, "Failed to provide object to the graph")
 		}
 	}
 
 	// Resolve services and their dependencies.
 	if err := serviceGraph.Populate(); err != nil {
-		return fmt.Errorf("Failed to populate service dependency: %v", err)
+		return errutil.Wrapf(err, "Failed to populate service dependency")
 	}
 
 	return nil
@@ -252,25 +264,27 @@ func (s *Server) loadConfiguration() {
 }
 
 // notifySystemd sends state notifications to systemd.
-func notifySystemd(state string) error {
+func (s *Server) notifySystemd(state string) {
 	notifySocket := os.Getenv("NOTIFY_SOCKET")
-
 	if notifySocket == "" {
-		return fmt.Errorf("NOTIFY_SOCKET environment variable empty or unset")
+		s.log.Debug(
+			"NOTIFY_SOCKET environment variable empty or unset, can't send systemd notification")
+		return
 	}
 
 	socketAddr := &net.UnixAddr{
 		Name: notifySocket,
 		Net:  "unixgram",
 	}
-
 	conn, err := net.DialUnix(socketAddr.Net, nil, socketAddr)
 	if err != nil {
-		return err
+		s.log.Warn("Failed to connect to systemd", "err", err, "socket", notifySocket)
+		return
 	}
 	defer conn.Close()
 
 	_, err = conn.Write([]byte(state))
-
+	if err != nil {
-	return err
+		s.log.Warn("Failed to write notification to systemd", "err", err)
+	}
 }