Skip to content

Commit

Permalink
feat: let plugins self-register (#1098)
Browse files Browse the repository at this point in the history
# Description

Refactors plugin registration such that Plugins self-register into the
plugin Registry simply by their package being imported instead of being
explicitly registered by external logic. This should look familiar as a
Go plugin pattern as it's fundamentally the same as what's done in
[Telegraf](https://github.com/influxdata/telegraf/blob/543b907cdf46d555afe083de846dc4ed055a4339/plugins/outputs/syslog/syslog.go#L191),
[CoreDNS](https://github.com/coredns/coredns/blob/7429380b1cacebde73084ad3cb6fb676c69bd6b8/core/plugin/zplugin.go)
(Caddy), and others.

- Abuses `init()`s and aliasing to hide anything deeper than
`pkg/plugin` from the rest of Retina, so that Plugins are more
encapsulated.
- Privatize the `Name` const in every Plugin since it no longer needs to
be accessible from outside of their package.
- Remove the `api.Name` type from the Plugin spec since it is redundant.
Creating a string type like that is useful for enums, when you want to
ensure at a data boundary that the string you're ingesting is one of
your allowed values. Plugin names are self-provided consts and not used
like this anyway, so it could only cause confusion. Removing it means we
need to do many less type coercions. This also means we can pass around
the plugin list from the conf (a []string) directly.

## Related Issue

I was investigating making it possible for a Plugin to auto-detect that
it should be enabled (via some auto-enable registry of detectFuncs, or
similar) and wanted to clean this up first so to simplify that
implementation.

---------

Signed-off-by: Evan Baker <[email protected]>
  • Loading branch information
rbtr authored Dec 5, 2024
1 parent 0c1a089 commit 2ba2723
Show file tree
Hide file tree
Showing 40 changed files with 340 additions and 350 deletions.
7 changes: 0 additions & 7 deletions pkg/managers/controllermanager/controllermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/microsoft/retina/pkg/log"
pm "github.com/microsoft/retina/pkg/managers/pluginmanager"
sm "github.com/microsoft/retina/pkg/managers/servermanager"
"github.com/microsoft/retina/pkg/plugin/api"
"github.com/microsoft/retina/pkg/pubsub"
"github.com/microsoft/retina/pkg/telemetry"
"go.uber.org/zap"
Expand Down Expand Up @@ -46,15 +45,9 @@ func NewControllerManager(conf *kcfg.Config, kubeclient kubernetes.Interface, te
factory.WaitForCacheSync(wait.NeverStop)
}

// enabledPlugins := {api.PluginName(conf.EnabledPlugin[])}
enabledPlugins := []api.PluginName{}
for _, pluginName := range conf.EnabledPlugin {
enabledPlugins = append(enabledPlugins, api.PluginName(pluginName))
}
pMgr, err := pm.NewPluginManager(
conf,
tel,
enabledPlugins...,
)
if err != nil {
return nil, err
Expand Down
11 changes: 5 additions & 6 deletions pkg/managers/controllermanager/controllermanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import (
kcfg "github.com/microsoft/retina/pkg/config"
"github.com/microsoft/retina/pkg/log"
pm "github.com/microsoft/retina/pkg/managers/pluginmanager"
"github.com/microsoft/retina/pkg/plugin/api"
"github.com/microsoft/retina/pkg/plugin/api/mock"
plugin "github.com/microsoft/retina/pkg/plugin/mock"
"github.com/microsoft/retina/pkg/telemetry"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -82,23 +81,23 @@ func TestControllerPluginManagerStartFail(t *testing.T) {
log.SetupZapLogger(log.GetDefaultLogOpts())

pluginName := "mockplugin"

cfg := &kcfg.Config{
MetricsInterval: timeInter,
EnablePodLevel: true,
EnabledPlugin: []string{pluginName},
}
mgr, err := pm.NewPluginManager(cfg, telemetry.NewNoopTelemetry(), api.PluginName(pluginName))
mgr, err := pm.NewPluginManager(cfg, telemetry.NewNoopTelemetry())
require.NoError(t, err, "Expected no error, instead got %+v", err)

mockPlugin := mock.NewMockPlugin(ctl)
mockPlugin := plugin.NewMockPlugin(ctl)
mockPlugin.EXPECT().Generate(gomock.Any()).Return(nil).AnyTimes()
mockPlugin.EXPECT().Compile(gomock.Any()).Return(nil).AnyTimes()
mockPlugin.EXPECT().Stop().Return(nil).AnyTimes()
mockPlugin.EXPECT().Init().Return(nil).AnyTimes()
mockPlugin.EXPECT().Name().Return(pluginName).AnyTimes()
mockPlugin.EXPECT().Start(gomock.Any()).Return(errors.New("test error")).AnyTimes()

mgr.SetPlugin(api.PluginName(pluginName), mockPlugin)
mgr.SetPlugin(pluginName, mockPlugin)
cm.pluginManager = mgr

err = cm.Init(context.Background())
Expand Down
7 changes: 1 addition & 6 deletions pkg/managers/pluginmanager/cells_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
v1 "github.com/cilium/cilium/pkg/hubble/api/v1"
"github.com/microsoft/retina/pkg/config"
"github.com/microsoft/retina/pkg/metrics"
"github.com/microsoft/retina/pkg/plugin/api"
"github.com/microsoft/retina/pkg/telemetry"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -44,11 +43,7 @@ func newPluginManager(params pluginManagerParams) (*PluginManager, error) {
// Enable Metrics in retina
metrics.InitializeMetrics()

enabledPlugins := []api.PluginName{}
for _, pluginName := range params.Config.EnabledPlugin {
enabledPlugins = append(enabledPlugins, api.PluginName(pluginName))
}
pluginMgr, err := NewPluginManager(&params.Config, params.Telemetry, enabledPlugins...)
pluginMgr, err := NewPluginManager(&params.Config, params.Telemetry)
if err != nil {
return &PluginManager{}, err
}
Expand Down
49 changes: 20 additions & 29 deletions pkg/managers/pluginmanager/pluginmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ import (
"github.com/microsoft/retina/pkg/log"
"github.com/microsoft/retina/pkg/managers/watchermanager"
"github.com/microsoft/retina/pkg/metrics"
"github.com/microsoft/retina/pkg/plugin/api"
"github.com/microsoft/retina/pkg/plugin"
"github.com/microsoft/retina/pkg/plugin/conntrack"
"github.com/microsoft/retina/pkg/plugin/registry"
"github.com/microsoft/retina/pkg/telemetry"
"github.com/pkg/errors"
"go.uber.org/zap"
Expand All @@ -37,27 +36,19 @@ var (
type PluginManager struct {
cfg *kcfg.Config
l *log.ZapLogger
plugins map[api.PluginName]api.Plugin
plugins map[string]plugin.Plugin
tel telemetry.Telemetry

watcherManager watchermanager.IWatcherManager
}

func init() {
registry.RegisterPlugins()
}

func NewPluginManager(
cfg *kcfg.Config,
tel telemetry.Telemetry,
pluginNames ...api.PluginName,
) (*PluginManager, error) {
func NewPluginManager(cfg *kcfg.Config, tel telemetry.Telemetry) (*PluginManager, error) {
logger := log.Logger().Named("plugin-manager")
mgr := &PluginManager{
cfg: cfg,
l: logger,
tel: tel,
plugins: map[api.PluginName]api.Plugin{},
plugins: map[string]plugin.Plugin{},
}

if mgr.cfg.EnablePodLevel {
Expand All @@ -67,8 +58,8 @@ func NewPluginManager(
mgr.l.Info("plugin manager has pod level disabled")
}

for _, name := range pluginNames {
newPluginFn, ok := registry.PluginHandler[name]
for _, name := range cfg.EnabledPlugin {
newPluginFn, ok := plugin.Get(name)
if !ok {
return nil, fmt.Errorf("plugin %s not found in registry", name)
}
Expand All @@ -80,9 +71,9 @@ func NewPluginManager(

func (p *PluginManager) Stop() {
var wg sync.WaitGroup
for _, plugin := range p.plugins {
for _, pl := range p.plugins {
wg.Add(1)
go func(plugin api.Plugin) {
go func(plugin plugin.Plugin) {
defer wg.Done()
if err := plugin.Stop(); err != nil {
p.l.Error("failed to stop plugin", zap.Error(err))
Expand All @@ -91,32 +82,32 @@ func (p *PluginManager) Stop() {
// even if some plugins fail to stop.
}
p.l.Info("Cleaned up resource for plugin", zap.String("name", plugin.Name()))
}(plugin)
}(pl)
}
wg.Wait()
}

// Reconcile reconciles a particular plugin.
func (p *PluginManager) Reconcile(ctx context.Context, plugin api.Plugin) error {
defer p.tel.StopPerf(p.tel.StartPerf(fmt.Sprintf("reconcile-%s", plugin.Name())))
func (p *PluginManager) Reconcile(ctx context.Context, pl plugin.Plugin) error {
defer p.tel.StopPerf(p.tel.StartPerf("reconcile-" + pl.Name()))
// Regenerate eBPF code and bpf object.
// This maybe no-op for plugins that don't use eBPF.
if err := plugin.Generate(ctx); err != nil {
if err := pl.Generate(ctx); err != nil {
return errors.Wrap(err, "failed to generate plugin")
}
if err := plugin.Compile(ctx); err != nil {
if err := pl.Compile(ctx); err != nil {
return errors.Wrap(err, "failed to compile plugin")
}

// Re-start plugin.
if err := plugin.Stop(); err != nil {
if err := pl.Stop(); err != nil {
return errors.Wrap(err, "failed to stop plugin")
}
if err := plugin.Init(); err != nil {
if err := pl.Init(); err != nil {
return errors.Wrap(err, "failed to init plugin")
}

p.l.Info("Reconciled plugin", zap.String("name", plugin.Name()))
p.l.Info("Reconciled plugin", zap.String("name", pl.Name()))
return nil
}

Expand Down Expand Up @@ -196,22 +187,22 @@ func (p *PluginManager) Start(ctx context.Context) error {
return nil
}

func (p *PluginManager) SetPlugin(name api.PluginName, plugin api.Plugin) {
func (p *PluginManager) SetPlugin(name string, pl plugin.Plugin) {
if p == nil {
return
}

if p.plugins == nil {
p.plugins = map[api.PluginName]api.Plugin{}
p.plugins = map[string]plugin.Plugin{}
}
p.plugins[name] = plugin
p.plugins[name] = pl
}

func (p *PluginManager) SetupChannel(c chan *v1.Event) {
for name, plugin := range p.plugins {
err := plugin.SetupChannel(c)
if err != nil {
p.l.Error("failed to setup channel for plugin", zap.String("plugin name", string(name)), zap.Error(err))
p.l.Error("failed to setup channel for plugin", zap.String("plugin name", name), zap.Error(err))
}
}
}
Loading

0 comments on commit 2ba2723

Please sign in to comment.