From 0b1913af6f05864f1416fffd02aeaf4c3eb595e9 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Fri, 11 Oct 2019 21:02:15 +0200 Subject: [PATCH] pkg/plugins: Check errors (#19715) * pkg/plugins: Check errors --- pkg/plugins/dashboard_importer_test.go | 7 +-- pkg/plugins/dashboards_test.go | 7 ++- pkg/plugins/datasource_plugin.go | 36 ++++++++----- pkg/plugins/plugins.go | 75 ++++++++++++++++++-------- pkg/plugins/plugins_test.go | 11 ++-- 5 files changed, 90 insertions(+), 46 deletions(-) diff --git a/pkg/plugins/dashboard_importer_test.go b/pkg/plugins/dashboard_importer_test.go index ca8dfcd515c..9a554d4298f 100644 --- a/pkg/plugins/dashboard_importer_test.go +++ b/pkg/plugins/dashboard_importer_test.go @@ -89,12 +89,13 @@ func pluginScenario(desc string, t *testing.T, fn func()) { Convey("Given a plugin", t, func() { setting.Raw = ini.Empty() sec, _ := setting.Raw.NewSection("plugin.test-app") - sec.NewKey("path", "testdata/test-app") + _, err := sec.NewKey("path", "testdata/test-app") + So(err, ShouldBeNil) pm := &PluginManager{} - err := pm.Init() - + err = pm.Init() So(err, ShouldBeNil) + Convey(desc, fn) }) } diff --git a/pkg/plugins/dashboards_test.go b/pkg/plugins/dashboards_test.go index 6fc6ace0e00..f1042b8f401 100644 --- a/pkg/plugins/dashboards_test.go +++ b/pkg/plugins/dashboards_test.go @@ -12,15 +12,14 @@ import ( ) func TestPluginDashboards(t *testing.T) { - Convey("When asking plugin dashboard info", t, func() { setting.Raw = ini.Empty() sec, _ := setting.Raw.NewSection("plugin.test-app") - sec.NewKey("path", "testdata/test-app") + _, err := sec.NewKey("path", "testdata/test-app") + So(err, ShouldBeNil) pm := &PluginManager{} - err := pm.Init() - + err = pm.Init() So(err, ShouldBeNil) bus.AddHandler("test", func(query *m.GetDashboardQuery) error { diff --git a/pkg/plugins/datasource_plugin.go b/pkg/plugins/datasource_plugin.go index 1379daf5a6d..1da105860ad 100644 --- a/pkg/plugins/datasource_plugin.go +++ b/pkg/plugins/datasource_plugin.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/datasource/wrapper" "github.com/grafana/grafana/pkg/tsdb" plugin "github.com/hashicorp/go-plugin" + "golang.org/x/xerrors" ) // DataSourcePlugin contains all metadata about a datasource plugin @@ -60,12 +61,17 @@ var handshakeConfig = plugin.HandshakeConfig{ func (p *DataSourcePlugin) startBackendPlugin(ctx context.Context, log log.Logger) error { p.log = log.New("plugin-id", p.Id) - err := p.spawnSubProcess() - if err == nil { - go p.restartKilledProcess(ctx) + if err := p.spawnSubProcess(); err != nil { + return err } - return err + go func() { + if err := p.restartKilledProcess(ctx); err != nil { + p.log.Error("Attempting to restart killed process failed", "err", err) + } + }() + + return nil } func (p *DataSourcePlugin) spawnSubProcess() error { @@ -105,15 +111,21 @@ func (p *DataSourcePlugin) restartKilledProcess(ctx context.Context) error { for { select { case <-ctx.Done(): - return ctx.Err() - case <-ticker.C: - if p.client.Exited() { - err := p.spawnSubProcess() - p.log.Debug("Spawning new sub process", "name", p.Name, "id", p.Id) - if err != nil { - p.log.Error("Failed to spawn subprocess") - } + if err := ctx.Err(); err != nil && !xerrors.Is(err, context.Canceled) { + return err } + return nil + case <-ticker.C: + if !p.client.Exited() { + continue + } + + if err := p.spawnSubProcess(); err != nil { + p.log.Error("Failed to restart plugin", "err", err) + continue + } + + p.log.Debug("Plugin process restarted") } } } diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index dcef2f0f76e..8da779d4efe 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -17,6 +17,8 @@ import ( "github.com/grafana/grafana/pkg/registry" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" + "github.com/grafana/grafana/pkg/util/errutil" + "golang.org/x/xerrors" ) var ( @@ -63,7 +65,10 @@ func (pm *PluginManager) Init() error { } pm.log.Info("Starting plugin search") - scan(path.Join(setting.StaticRootPath, "app/plugins")) + plugDir := path.Join(setting.StaticRootPath, "app/plugins") + if err := pm.scan(plugDir); err != nil { + return errutil.Wrapf(err, "Failed to scan main plugin directory '%s'", plugDir) + } // check if plugins dir exists if _, err := os.Stat(setting.PluginsPath); os.IsNotExist(err) { @@ -71,14 +76,22 @@ func (pm *PluginManager) Init() error { plog.Error("Failed to create plugin dir", "dir", setting.PluginsPath, "error", err) } else { plog.Info("Plugin dir created", "dir", setting.PluginsPath) - scan(setting.PluginsPath) + if err := pm.scan(setting.PluginsPath); err != nil { + return errutil.Wrapf(err, "Failed to scan configured plugin directory '%s'", + setting.PluginsPath) + } } } else { - scan(setting.PluginsPath) + if err := pm.scan(setting.PluginsPath); err != nil { + return errutil.Wrapf(err, "Failed to scan configured plugin directory '%s'", + setting.PluginsPath) + } } // check plugin paths defined in config - checkPluginPaths() + if err := pm.checkPluginPaths(); err != nil { + return err + } for _, panel := range Panels { panel.initFrontendPlugin() @@ -95,16 +108,16 @@ func (pm *PluginManager) Init() error { return nil } -func (pm *PluginManager) startBackendPlugins(ctx context.Context) error { +func (pm *PluginManager) startBackendPlugins(ctx context.Context) { for _, ds := range DataSources { - if ds.Backend { - if err := ds.startBackendPlugin(ctx, plog); err != nil { - pm.log.Error("Failed to init plugin.", "error", err, "plugin", ds.Id) - } + if !ds.Backend { + continue + } + + if err := ds.startBackendPlugin(ctx, plog); err != nil { + pm.log.Error("Failed to init plugin.", "error", err, "plugin", ds.Id) } } - - return nil } func (pm *PluginManager) Run(ctx context.Context) error { @@ -132,32 +145,49 @@ func (pm *PluginManager) Run(ctx context.Context) error { return ctx.Err() } -func checkPluginPaths() error { +func (pm *PluginManager) checkPluginPaths() error { for _, section := range setting.Raw.Sections() { - if strings.HasPrefix(section.Name(), "plugin.") { - path := section.Key("path").String() - if path != "" { - scan(path) - } + if !strings.HasPrefix(section.Name(), "plugin.") { + continue + } + + path := section.Key("path").String() + if path == "" { + continue + } + + if err := pm.scan(path); err != nil { + return errutil.Wrapf(err, "Failed to scan directory configured for plugin '%s': '%s'", + section.Name(), path) } } + return nil } -func scan(pluginDir string) error { +// scan a directory for plugins. +func (pm *PluginManager) scan(pluginDir string) error { scanner := &PluginScanner{ pluginPath: pluginDir, } if err := util.Walk(pluginDir, true, true, scanner.walker); err != nil { + if xerrors.Is(err, os.ErrNotExist) { + pm.log.Debug("Couldn't scan dir '%s' since it doesn't exist") + return nil + } + if xerrors.Is(err, os.ErrPermission) { + pm.log.Debug("Couldn't scan dir '%s' due to lack of permissions") + return nil + } if pluginDir != "data/plugins" { - log.Warn("Could not scan dir \"%v\" error: %s", pluginDir, err) + pm.log.Warn("Could not scan dir", "pluginDir", pluginDir, "err", err) } return err } if len(scanner.errors) > 0 { - return errors.New("Some plugins failed to load") + return errutil.Wrapf(scanner.errors[0], "Some plugins failed to load") } return nil @@ -167,7 +197,6 @@ func (scanner *PluginScanner) walker(currentPath string, f os.FileInfo, err erro // We scan all the subfolders for plugin.json (with some exceptions) so that we also load embedded plugins, for // example https://github.com/raintank/worldping-app/tree/master/dist/grafana-worldmap-panel worldmap panel plugin // is embedded in worldping app. - if err != nil { return err } @@ -227,7 +256,9 @@ func (scanner *PluginScanner) loadPluginJson(pluginJsonFilePath string) error { } } - reader.Seek(0, 0) + if _, err := reader.Seek(0, 0); err != nil { + return err + } return loader.Load(jsonParser, currentDir) } diff --git a/pkg/plugins/plugins_test.go b/pkg/plugins/plugins_test.go index b5d7a752fe5..00c1c42b182 100644 --- a/pkg/plugins/plugins_test.go +++ b/pkg/plugins/plugins_test.go @@ -29,15 +29,16 @@ func TestPluginScans(t *testing.T) { Convey("When reading app plugin definition", t, func() { setting.Raw = ini.Empty() - sec, _ := setting.Raw.NewSection("plugin.nginx-app") - sec.NewKey("path", "testdata/test-app") + sec, err := setting.Raw.NewSection("plugin.nginx-app") + So(err, ShouldBeNil) + _, err = sec.NewKey("path", "testdata/test-app") + So(err, ShouldBeNil) pm := &PluginManager{} - err := pm.Init() - + err = pm.Init() So(err, ShouldBeNil) - So(len(Apps), ShouldBeGreaterThan, 0) + So(len(Apps), ShouldBeGreaterThan, 0) So(Apps["test-app"].Info.Logos.Large, ShouldEqual, "public/plugins/test-app/img/logo_large.png") So(Apps["test-app"].Info.Screenshots[1].Path, ShouldEqual, "public/plugins/test-app/img/screenshot2.png") })