Plugins: Add termination stage to plugin loader pipeline (#72822)
* add termination stage * uid -> pluginID (for now) * also fix fakes * add simple test * Fix logger name Co-authored-by: Giuseppe Guerra <giuseppe.guerra@grafana.com> * inline stop func call Co-authored-by: Giuseppe Guerra <giuseppe.guerra@grafana.com> --------- Co-authored-by: Giuseppe Guerra <giuseppe.guerra@grafana.com>
This commit is contained in:
Will Browne
co-authored by
Giuseppe Guerra
Giuseppe Guerra
parent
7bc6d32eb9
commit
60b4a0b2a4
@@ -14,6 +14,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/bootstrap"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/discovery"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/initialization"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/termination"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/process"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/registry"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/signature"
|
||||
@@ -27,6 +28,7 @@ type Loader struct {
|
||||
discovery discovery.Discoverer
|
||||
bootstrap bootstrap.Bootstrapper
|
||||
initializer initialization.Initializer
|
||||
termination termination.Terminator
|
||||
|
||||
processManager process.Service
|
||||
pluginRegistry registry.Service
|
||||
@@ -45,15 +47,17 @@ type Loader struct {
|
||||
func ProvideService(cfg *config.Cfg, authorizer plugins.PluginLoaderAuthorizer,
|
||||
pluginRegistry registry.Service, roleRegistry plugins.RoleRegistry, assetPath *assetpath.Service,
|
||||
angularInspector angularinspector.Inspector, externalServiceRegistry oauth.ExternalServiceRegistry,
|
||||
discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, initializer initialization.Initializer) *Loader {
|
||||
discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, initializer initialization.Initializer,
|
||||
termination termination.Terminator) *Loader {
|
||||
return New(cfg, authorizer, pluginRegistry, process.NewManager(pluginRegistry), roleRegistry, assetPath,
|
||||
angularInspector, externalServiceRegistry, discovery, bootstrap, initializer)
|
||||
angularInspector, externalServiceRegistry, discovery, bootstrap, initializer, termination)
|
||||
}
|
||||
|
||||
func New(cfg *config.Cfg, authorizer plugins.PluginLoaderAuthorizer, pluginRegistry registry.Service,
|
||||
processManager process.Service, roleRegistry plugins.RoleRegistry, assetPath *assetpath.Service,
|
||||
angularInspector angularinspector.Inspector, externalServiceRegistry oauth.ExternalServiceRegistry,
|
||||
discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, initializer initialization.Initializer) *Loader {
|
||||
discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, initializer initialization.Initializer,
|
||||
termination termination.Terminator) *Loader {
|
||||
return &Loader{
|
||||
pluginRegistry: pluginRegistry,
|
||||
signatureValidator: signature.NewValidator(authorizer),
|
||||
@@ -68,6 +72,7 @@ func New(cfg *config.Cfg, authorizer plugins.PluginLoaderAuthorizer, pluginRegis
|
||||
discovery: discovery,
|
||||
bootstrap: bootstrap,
|
||||
initializer: initializer,
|
||||
termination: termination,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -178,40 +183,7 @@ func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins
|
||||
}
|
||||
|
||||
func (l *Loader) Unload(ctx context.Context, pluginID string) error {
|
||||
plugin, exists := l.pluginRegistry.Plugin(ctx, pluginID)
|
||||
if !exists {
|
||||
return plugins.ErrPluginNotInstalled
|
||||
}
|
||||
|
||||
if plugin.IsCorePlugin() || plugin.IsBundledPlugin() {
|
||||
return plugins.ErrUninstallCorePlugin
|
||||
}
|
||||
|
||||
if err := l.unload(ctx, plugin); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (l *Loader) unload(ctx context.Context, p *plugins.Plugin) error {
|
||||
l.log.Debug("Stopping plugin process", "pluginId", p.ID)
|
||||
|
||||
if err := l.processManager.Stop(ctx, p.ID); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := l.pluginRegistry.Remove(ctx, p.ID); err != nil {
|
||||
return err
|
||||
}
|
||||
l.log.Debug("Plugin unregistered", "pluginId", p.ID)
|
||||
|
||||
if remover, ok := p.FS.(plugins.FSRemover); ok {
|
||||
if err := remover.Remove(); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
return l.termination.Terminate(ctx, pluginID)
|
||||
}
|
||||
|
||||
func (l *Loader) PluginErrors() []*plugins.Error {
|
||||
|
||||
@@ -2,6 +2,7 @@ package loader
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"testing"
|
||||
@@ -18,6 +19,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/bootstrap"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/discovery"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/initialization"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/termination"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/signature"
|
||||
"github.com/grafana/grafana/pkg/plugins/manager/sources"
|
||||
"github.com/grafana/grafana/pkg/plugins/pluginscdn"
|
||||
@@ -435,11 +437,15 @@ func TestLoader_Load(t *testing.T) {
|
||||
angularInspector, err := angularinspector.NewStaticInspector()
|
||||
require.NoError(t, err)
|
||||
|
||||
terminationStage, err := termination.New(tt.cfg, termination.Opts{})
|
||||
require.NoError(t, err)
|
||||
|
||||
l := New(tt.cfg, signature.NewUnsignedAuthorizer(tt.cfg), fakes.NewFakePluginRegistry(),
|
||||
fakes.NewFakeProcessManager(), fakes.NewFakeRoleRegistry(),
|
||||
assetpath.ProvideService(pluginscdn.ProvideService(tt.cfg)), angularInspector, &fakes.FakeOauthService{},
|
||||
discovery.New(tt.cfg, discovery.Opts{}), bootstrap.New(tt.cfg, bootstrap.Opts{}),
|
||||
initialization.New(tt.cfg, initialization.Opts{}))
|
||||
initialization.New(tt.cfg, initialization.Opts{}),
|
||||
terminationStage)
|
||||
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got, err := l.Load(context.Background(), sources.NewLocalSource(tt.class, tt.pluginPaths))
|
||||
@@ -506,7 +512,7 @@ func TestLoader_Load(t *testing.T) {
|
||||
steps = append(steps, "initialize")
|
||||
return ps, nil
|
||||
},
|
||||
})
|
||||
}, &fakes.FakeTerminator{})
|
||||
|
||||
got, err := l.Load(context.Background(), src)
|
||||
require.NoError(t, err)
|
||||
@@ -516,6 +522,41 @@ func TestLoader_Load(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestLoader_Unload(t *testing.T) {
|
||||
t.Run("Termination stage error is returned from Unload", func(t *testing.T) {
|
||||
pluginID := "grafana-test-panel"
|
||||
cfg := &config.Cfg{}
|
||||
angularInspector, err := angularinspector.NewStaticInspector()
|
||||
require.NoError(t, err)
|
||||
|
||||
tcs := []struct {
|
||||
expectedErr error
|
||||
}{
|
||||
{
|
||||
expectedErr: errors.New("plugin not found"),
|
||||
},
|
||||
{
|
||||
expectedErr: nil,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tcs {
|
||||
l := New(cfg, signature.NewUnsignedAuthorizer(cfg), fakes.NewFakePluginRegistry(), fakes.NewFakeProcessManager(),
|
||||
fakes.NewFakeRoleRegistry(), assetpath.ProvideService(pluginscdn.ProvideService(cfg)), angularInspector,
|
||||
&fakes.FakeOauthService{}, &fakes.FakeDiscoverer{}, &fakes.FakeBootstrapper{}, &fakes.FakeInitializer{},
|
||||
&fakes.FakeTerminator{
|
||||
TerminateFunc: func(ctx context.Context, pID string) error {
|
||||
require.Equal(t, pluginID, pID)
|
||||
return tc.expectedErr
|
||||
},
|
||||
})
|
||||
|
||||
err = l.Unload(context.Background(), pluginID)
|
||||
require.ErrorIs(t, err, tc.expectedErr)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func mustNewStaticFSForTests(t *testing.T, dir string) plugins.FS {
|
||||
sfs, err := plugins.NewStaticFS(plugins.NewLocalFS(dir))
|
||||
require.NoError(t, err)
|
||||
|
||||
Reference in New Issue
Block a user