Refactoring PluginManager to be a self registering service (#11755)

* refator: refactored PluginManager to be a self registering service, a lot more work needed to fully make plugin manager use instance variables and not so many globals

pkg/cmd/grafana-server/server.go

@@ -17,6 +17,7 @@ import (
 	"github.com/grafana/grafana/pkg/middleware"
 	"github.com/grafana/grafana/pkg/registry"
 	"github.com/grafana/grafana/pkg/services/dashboards"
+	"github.com/grafana/grafana/pkg/services/notifications"
 	"github.com/grafana/grafana/pkg/services/provisioning"
 
 	"golang.org/x/sync/errgroup"
@@ -25,8 +26,6 @@ import (
 	"github.com/grafana/grafana/pkg/log"
 	"github.com/grafana/grafana/pkg/login"
 	"github.com/grafana/grafana/pkg/metrics"
-	"github.com/grafana/grafana/pkg/plugins"
-	"github.com/grafana/grafana/pkg/services/notifications"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 
@@ -34,6 +33,7 @@ import (
 	"github.com/grafana/grafana/pkg/tracing"
 
 	_ "github.com/grafana/grafana/pkg/extensions"
+	_ "github.com/grafana/grafana/pkg/plugins"
 	_ "github.com/grafana/grafana/pkg/services/alerting"
 	_ "github.com/grafana/grafana/pkg/services/cleanup"
 	_ "github.com/grafana/grafana/pkg/services/search"
@@ -73,12 +73,6 @@ func (g *GrafanaServerImpl) Start() error {
 	login.Init()
 	social.NewOAuthService()
 
-	pluginManager, err := plugins.NewPluginManager(g.context)
-	if err != nil {
-		return fmt.Errorf("Failed to start plugins. error: %v", err)
-	}
-	g.childRoutines.Go(func() error { return pluginManager.Run(g.context) })
-
 	if err := provisioning.Init(g.context, setting.HomePath, setting.Cfg); err != nil {
 		return fmt.Errorf("Failed to provision Grafana from config. error: %v", err)
 	}

pkg/plugins/dashboard_importer_test.go

@@ -1,7 +1,6 @@
 package plugins
 
 import (
-	"context"
 	"io/ioutil"
 	"testing"
 
@@ -91,10 +90,11 @@ func pluginScenario(desc string, t *testing.T, fn func()) {
 		setting.Cfg = ini.Empty()
 		sec, _ := setting.Cfg.NewSection("plugin.test-app")
 		sec.NewKey("path", "../../tests/test-app")
-		err := initPlugins(context.Background())
+
+		pm := &PluginManager{}
+		err := pm.Init()
 
 		So(err, ShouldBeNil)
-
 		Convey(desc, fn)
 	})
 }

pkg/plugins/dashboards_test.go

@@ -1,7 +1,6 @@
 package plugins
 
 import (
-	"context"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/bus"
@@ -18,7 +17,9 @@ func TestPluginDashboards(t *testing.T) {
 		setting.Cfg = ini.Empty()
 		sec, _ := setting.Cfg.NewSection("plugin.test-app")
 		sec.NewKey("path", "../../tests/test-app")
-		err := initPlugins(context.Background())
+
+		pm := &PluginManager{}
+		err := pm.Init()
 
 		So(err, ShouldBeNil)
 

pkg/plugins/dashboards_updater.go

@@ -1,8 +1,6 @@
 package plugins
 
 import (
-	"time"
-
 	"github.com/grafana/grafana/pkg/bus"
 	m "github.com/grafana/grafana/pkg/models"
 )
@@ -11,10 +9,8 @@ func init() {
 	bus.AddEventListener(handlePluginStateChanged)
 }
 
-func updateAppDashboards() {
+func (pm *PluginManager) updateAppDashboards() {
-	time.Sleep(time.Second * 5)
+	pm.log.Debug("Looking for App Dashboard Updates")
-
-	plog.Debug("Looking for App Dashboard Updates")
 
 	query := m.GetPluginSettingsQuery{OrgId: 0}
 

pkg/plugins/datasource_plugin.go

@@ -76,7 +76,7 @@ func composeBinaryName(executable, os, arch string) string {
 	return fmt.Sprintf("%s_%s_%s%s", executable, os, strings.ToLower(arch), extension)
 }
 
-func (p *DataSourcePlugin) initBackendPlugin(ctx context.Context, log log.Logger) error {
+func (p *DataSourcePlugin) startBackendPlugin(ctx context.Context, log log.Logger) error {
 	p.log = log.New("plugin-id", p.Id)
 
 	err := p.spawnSubProcess()

pkg/plugins/plugins.go

@@ -11,8 +11,10 @@ import (
 	"path/filepath"
 	"reflect"
 	"strings"
+	"time"
 
 	"github.com/grafana/grafana/pkg/log"
+	"github.com/grafana/grafana/pkg/registry"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 )
@@ -39,30 +41,12 @@ type PluginManager struct {
 	log log.Logger
 }
 
-func NewPluginManager(ctx context.Context) (*PluginManager, error) {
+func init() {
-	err := initPlugins(ctx)
+	registry.RegisterService(&PluginManager{})
-
-	if err != nil {
-		return nil, err
-	}
-
-	return &PluginManager{
-		log: log.New("plugins"),
-	}, nil
 }
 
-func (p *PluginManager) Run(ctx context.Context) error {
+func (pm *PluginManager) Init() error {
-	<-ctx.Done()
+	pm.log = log.New("plugins")
-
-	for _, p := range DataSources {
-		p.Kill()
-	}
-
-	p.log.Info("Stopped Plugins", "reason", ctx.Err())
-	return ctx.Err()
-}
-
-func initPlugins(ctx context.Context) error {
 	plog = log.New("plugins")
 
 	DataSources = map[string]*DataSourcePlugin{}
@@ -76,7 +60,7 @@ func initPlugins(ctx context.Context) error {
 		"app":        AppPlugin{},
 	}
 
-	plog.Info("Starting plugin search")
+	pm.log.Info("Starting plugin search")
 	scan(path.Join(setting.StaticRootPath, "app/plugins"))
 
 	// check if plugins dir exists
@@ -99,13 +83,6 @@ func initPlugins(ctx context.Context) error {
 	}
 
 	for _, ds := range DataSources {
-		if ds.Backend {
-			err := ds.initBackendPlugin(ctx, plog)
-			if err != nil {
-				plog.Error("Failed to init plugin.", "error", err, "plugin", ds.Id)
-			}
-		}
-
 		ds.initFrontendPlugin()
 	}
 
@@ -113,8 +90,40 @@ func initPlugins(ctx context.Context) error {
 		app.initApp()
 	}
 
-	go StartPluginUpdateChecker()
+	return nil
-	go updateAppDashboards()
+}
+
+func (pm *PluginManager) startBackendPlugins(ctx context.Context) error {
+	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)
+			}
+		}
+	}
+
+	return nil
+}
+
+func (pm *PluginManager) Run(ctx context.Context) error {
+	pm.startBackendPlugins(ctx)
+	pm.updateAppDashboards()
+	pm.checkForUpdates()
+
+	ticker := time.NewTicker(time.Minute * 10)
+	for {
+		select {
+		case <-ticker.C:
+			pm.checkForUpdates()
+		case <-ctx.Done():
+			break
+		}
+	}
+
+	// kil backend plugins
+	for _, p := range DataSources {
+		p.Kill()
+	}
 
 	return nil
 }

pkg/plugins/plugins_test.go

@@ -1,7 +1,6 @@
 package plugins
 
 import (
-	"context"
 	"path/filepath"
 	"testing"
 
@@ -15,7 +14,9 @@ func TestPluginScans(t *testing.T) {
 	Convey("When scanning for plugins", t, func() {
 		setting.StaticRootPath, _ = filepath.Abs("../../public/")
 		setting.Cfg = ini.Empty()
-		err := initPlugins(context.Background())
+
+		pm := &PluginManager{}
+		err := pm.Init()
 
 		So(err, ShouldBeNil)
 		So(len(DataSources), ShouldBeGreaterThan, 1)
@@ -30,7 +31,9 @@ func TestPluginScans(t *testing.T) {
 		setting.Cfg = ini.Empty()
 		sec, _ := setting.Cfg.NewSection("plugin.nginx-app")
 		sec.NewKey("path", "../../tests/test-app")
-		err := initPlugins(context.Background())
+
+		pm := &PluginManager{}
+		err := pm.Init()
 
 		So(err, ShouldBeNil)
 		So(len(Apps), ShouldBeGreaterThan, 0)

pkg/plugins/update_checker.go

@@ -26,23 +26,6 @@ type GithubLatest struct {
 	Testing string `json:"testing"`
 }
 
-func StartPluginUpdateChecker() {
-	if !setting.CheckForUpdates {
-		return
-	}
-
-	// do one check directly
-	go checkForUpdates()
-
-	ticker := time.NewTicker(time.Minute * 10)
-	for {
-		select {
-		case <-ticker.C:
-			checkForUpdates()
-		}
-	}
-}
-
 func getAllExternalPluginSlugs() string {
 	var result []string
 	for _, plug := range Plugins {
@@ -56,8 +39,12 @@ func getAllExternalPluginSlugs() string {
 	return strings.Join(result, ",")
 }
 
-func checkForUpdates() {
+func (pm *PluginManager) checkForUpdates() {
-	log.Trace("Checking for updates")
+	if !setting.CheckForUpdates {
+		return
+	}
+
+	pm.log.Debug("Checking for updates")
 
 	pluginSlugs := getAllExternalPluginSlugs()
 	resp, err := httpClient.Get("https://grafana.com/api/plugins/versioncheck?slugIn=" + pluginSlugs + "&grafanaVersion=" + setting.BuildVersion)