Skip to content

Commit

Permalink
plugins: refactor plugin architecture to reduce code repetition. (has…
Browse files Browse the repository at this point in the history 
…hicorp#75)

This work aims to reduce the code repetition in relation to the
Autoscalers plugin, but also make navigating the repo easier with
an eye to the future of versioning plugin interfaces.

The largest addition is the plugin manager which is responsible
for launching, dispensing and providing access to the plugins
from the agent. This manager is not feature complete, but
provides a good basic form of central management to move forward
with. The manager now uses a common base plugin interface which
all plugins should implement. This is not currently embedded in
the plugin interfaces (future) but allows for an easier work
flow. Using the manager now also passes the agent logger to the
plugins, meaning the plugins now log to the level and format set
by the operator.

The default config has been updated to include the nomad-apm and
nomad-target plugins. This means these plugins are loaded by
default without the requirement for the user to specify. It is
not possible to turn these off because of the config merging, but
is something to keep in mind for the future.

The plugin interface declarations have been moved within the
plugin directory to their own sub-dir. From a navigation sense I
believe this makes sense, but also keeps the option of individual
sub go-modules if we wish in the future. The split has also meant
changing the default names of the nomad-local plugin, to be more
descriptive and also remove duplication. This leads to nomad-apm
and nomad-target.

The bultin plugins have been moved within a special directory and
if previously the plugin implemented two interfaces, these have
been split apart. This is due to the problem with implementing a
common base interface across all the plugins which includes a
function like the set function. It is impossible/very hard to
accommodate this setup alongside the base interface. In relation
the Makefile has been updated to reflect both the additional
plugin to build, and also the change in paths to build.

Fully tested with internal and external plugin builds.
  • Loading branch information
@jrasell
jrasell authored Apr 6, 2020
1 parent af60dc2 commit b952153
Show file tree
Hide file tree
Showing 45 changed files with 1,331 additions and 795 deletions.
16 changes: 11 additions & 5 deletions GNUmakefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,29 @@ clean: clean-plugins
@rm -f ./bin/nomad-autoscaler
@echo "==> Done"

bin/plugins/nomad:
bin/plugins/nomad-apm:
@echo "==> Building $@"
@mkdir -p $$(dirname $@)
@cd ./plugins/nomad && go build -o ../../$@
@cd ./plugins/builtin/apm/nomad && go build -o ../../../../$@
@echo "==> Done"

bin/plugins/nomad-target:
@echo "==> Building $@"
@mkdir -p $$(dirname $@)
@cd ./plugins/builtin/target/nomad && go build -o ../../../../$@
@echo "==> Done"

bin/plugins/prometheus:
@echo "==> Building $@"
@mkdir -p $$(dirname $@)
@cd ./plugins/prometheus && go build -o ../../$@
@cd ./plugins/builtin/apm/prometheus && go build -o ../../../../$@
@echo "==> Done"

bin/plugins/target-value:
@echo "==> Building $@"
@mkdir -p $$(dirname $@)
@cd ./plugins/target-value && go build -o ../../$@
@cd ./plugins/builtin/strategy/target-value && go build -o ../../../../$@
@echo "==> Done"

.PHONY: plugins
plugins: bin/plugins/nomad bin/plugins/prometheus bin/plugins/target-value
plugins: bin/plugins/nomad-apm bin/plugins/nomad-target bin/plugins/prometheus bin/plugins/target-value
223 changes: 29 additions & 194 deletions agent/agent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,56 +3,35 @@ package agent
import (
"context"
"fmt"
"os/exec"
"path"
"reflect"
"sync"
"time"

"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-plugin"
"github.com/hashicorp/nomad-autoscaler/agent/config"
apmpkg "github.com/hashicorp/nomad-autoscaler/apm"
nomadHelper "github.com/hashicorp/nomad-autoscaler/helper/nomad"
"github.com/hashicorp/nomad-autoscaler/helper/plugins"
"github.com/hashicorp/nomad-autoscaler/plugins"
apmpkg "github.com/hashicorp/nomad-autoscaler/plugins/apm"
"github.com/hashicorp/nomad-autoscaler/plugins/manager"
strategypkg "github.com/hashicorp/nomad-autoscaler/plugins/strategy"
targetpkg "github.com/hashicorp/nomad-autoscaler/plugins/target"
"github.com/hashicorp/nomad-autoscaler/policystorage"
strategypkg "github.com/hashicorp/nomad-autoscaler/strategy"
targetpkg "github.com/hashicorp/nomad-autoscaler/target"
"github.com/hashicorp/nomad/api"
)

var (
PluginHandshakeConfig = plugin.HandshakeConfig{
ProtocolVersion: 1,
MagicCookieKey: "magic",
MagicCookieValue: "magic",
}
)

type Agent struct {
logger hclog.Logger
config *config.Agent
nomadClient *api.Client
apmPlugins map[string]*Plugin
apmManager *apmpkg.Manager
targetPlugins map[string]*Plugin
targetManager *targetpkg.Manager
strategyPlugins map[string]*Plugin
strategyManager *strategypkg.Manager
logger hclog.Logger
config *config.Agent
nomadClient *api.Client
pluginManager *manager.PluginManager
}

type Plugin struct{}

func NewAgent(c *config.Agent, logger hclog.Logger) *Agent {
return &Agent{
logger: logger,
config: c,
apmPlugins: make(map[string]*Plugin),
apmManager: apmpkg.NewAPMManager(),
targetPlugins: make(map[string]*Plugin),
targetManager: targetpkg.NewTargetManager(),
strategyPlugins: make(map[string]*Plugin),
strategyManager: strategypkg.NewStrategyManager(),
logger: logger,
config: c,
}
}

Expand All @@ -66,8 +45,8 @@ func (a *Agent) Run(ctx context.Context) error {
ps := policystorage.Nomad{Client: a.nomadClient}

// launch plugins
if err := a.loadPlugins(); err != nil {
return fmt.Errorf("failed to load plugins: %v", err)
if err := a.setupPlugins(); err != nil {
return fmt.Errorf("failed to setup plugins: %v", err)
}

// Setup and start the HTTP health server.
Expand Down Expand Up @@ -119,10 +98,7 @@ Loop:
healthServer.stop()

// stop plugins before exiting
a.logger.Info("killing plugins")
a.apmManager.Kill()
a.targetManager.Kill()
a.strategyManager.Kill()
a.pluginManager.KillPlugins()
break Loop
}
}
Expand Down Expand Up @@ -187,147 +163,6 @@ func (a *Agent) generateNomadClient() error {
return nil
}

func (a *Agent) loadPlugins() error {
// load APM plugins
err := a.loadAPMPlugins()
if err != nil {
return err
}

// load target plugins
err = a.loadTargetPlugins()
if err != nil {
return err
}

// load strategy plugins
err = a.loadStrategyPlugins()
if err != nil {
return err
}

return nil
}

func (a *Agent) loadAPMPlugins() error {
// create default local-nomad target
a.config.APMs = append(a.config.APMs, &config.Plugin{
Name: "local-nomad",
Driver: "nomad-apm",
Config: nomadHelper.ConfigToMap(a.config.Nomad),
})

for _, apmConfig := range a.config.APMs {
a.logger.Info("loading APM plugin", "plugin", apmConfig)

if plugins.IsInternal(apmConfig.Driver, a.config.PluginDir) {
plugin := plugins.NewInternalAPM(apmConfig.Driver)
a.apmManager.RegisterInternalPlugin(apmConfig.Name, &plugin)

} else {
pluginConfig := &plugin.ClientConfig{
HandshakeConfig: PluginHandshakeConfig,
Plugins: map[string]plugin.Plugin{
"apm": &apmpkg.Plugin{},
},
Cmd: exec.Command(path.Join(a.config.PluginDir, apmConfig.Driver)),
}
err := a.apmManager.RegisterPlugin(apmConfig.Name, pluginConfig)
if err != nil {
return err
}
}

// configure plugin
apmPlugin, err := a.apmManager.Dispense(apmConfig.Name)
if err != nil {
return err
}
err = (*apmPlugin).SetConfig(apmConfig.Config)
if err != nil {
return err
}
}
return nil
}

func (a *Agent) loadTargetPlugins() error {
// create default local-nomad target
a.config.Targets = append(a.config.Targets, &config.Plugin{
Name: "local-nomad",
Driver: "nomad",
Config: nomadHelper.ConfigToMap(a.config.Nomad),
})

for _, targetConfig := range a.config.Targets {
a.logger.Info("loading Target plugin", "plugin", targetConfig)

if plugins.IsInternal(targetConfig.Driver, a.config.PluginDir) {
plugin := plugins.NewInternalTarget(targetConfig.Driver)
a.targetManager.RegisterInternalPlugin(targetConfig.Name, &plugin)

} else {
pluginConfig := &plugin.ClientConfig{
HandshakeConfig: PluginHandshakeConfig,
Plugins: map[string]plugin.Plugin{
"target": &targetpkg.Plugin{},
},
Cmd: exec.Command(path.Join(a.config.PluginDir, targetConfig.Driver)),
}
err := a.targetManager.RegisterPlugin(targetConfig.Name, pluginConfig)
if err != nil {
return err
}
}

// configure plugin
targetPlugin, err := a.targetManager.Dispense(targetConfig.Name)
if err != nil {
return err
}
err = (*targetPlugin).SetConfig(targetConfig.Config)
if err != nil {
return err
}
}
return nil
}

func (a *Agent) loadStrategyPlugins() error {
for _, strategyConfig := range a.config.Strategies {
a.logger.Info("loading Strategy plugin", "plugin", strategyConfig)

if plugins.IsInternal(strategyConfig.Driver, a.config.PluginDir) {
plugin := plugins.NewInternalStrategy(strategyConfig.Driver)
a.strategyManager.RegisterInternalPlugin(strategyConfig.Name, &plugin)

} else {
pluginConfig := &plugin.ClientConfig{
HandshakeConfig: PluginHandshakeConfig,
Plugins: map[string]plugin.Plugin{
"strategy": &strategypkg.Plugin{},
},
Cmd: exec.Command(path.Join(a.config.PluginDir, strategyConfig.Driver)),
}
err := a.strategyManager.RegisterPlugin(strategyConfig.Name, pluginConfig)
if err != nil {
return err
}
}

// configure plugin
strategyPlugin, err := a.strategyManager.Dispense(strategyConfig.Name)
if err != nil {
return err
}
err = (*strategyPlugin).SetConfig(strategyConfig.Config)
if err != nil {
return err
}
}
return nil
}

func (a *Agent) handlePolicy(p *policystorage.Policy) {
logger := a.logger.With(
"policy_id", p.ID,
Expand All @@ -341,43 +176,43 @@ func (a *Agent) handlePolicy(p *policystorage.Policy) {
return
}

var target targetpkg.Target
var apm apmpkg.APM
var strategy strategypkg.Strategy
var targetInst targetpkg.Target
var apmInst apmpkg.APM
var strategyInst strategypkg.Strategy

// dispense plugins
targetPlugin, err := a.targetManager.Dispense(p.Target.Name)
targetPlugin, err := a.pluginManager.Dispense(p.Target.Name, plugins.PluginTypeTarget)
if err != nil {
logger.Error("target plugin not initialized", "error", err, "plugin", p.Target.Name)
return
}
target = *targetPlugin
targetInst = targetPlugin.Plugin().(targetpkg.Target)

apmPlugin, err := a.apmManager.Dispense(p.Source)
apmPlugin, err := a.pluginManager.Dispense(p.Source, plugins.PluginTypeAPM)
if err != nil {
logger.Error("apm plugin not initialized", "error", err, "plugin", p.Target.Name)
logger.Error("apm plugin not initialized", "error", err, "plugin", p.Source)
return
}
apm = *apmPlugin
apmInst = apmPlugin.Plugin().(apmpkg.APM)

strategyPlugin, err := a.strategyManager.Dispense(p.Strategy.Name)
strategyPlugin, err := a.pluginManager.Dispense(p.Strategy.Name, plugins.PluginTypeStrategy)
if err != nil {
logger.Error("strategy plugin not initialized", "error", err, "plugin", p.Target.Name)
logger.Error("strategy plugin not initialized", "error", err, "plugin", p.Strategy.Name)
return
}
strategy = *strategyPlugin
strategyInst = strategyPlugin.Plugin().(strategypkg.Strategy)

// fetch target count
logger.Info("fetching current count")
currentCount, err := target.Count(p.Target.Config)
currentCount, err := targetInst.Count(p.Target.Config)
if err != nil {
logger.Error("failed to fetch current count", "error", err)
return
}

// query policy's APM
logger.Info("querying APM")
value, err := apm.Query(p.Query)
value, err := apmInst.Query(p.Query)
if err != nil {
logger.Error("failed to query APM", "error", err)
return
Expand All @@ -391,7 +226,7 @@ func (a *Agent) handlePolicy(p *policystorage.Policy) {
Metric: value,
Config: p.Strategy.Config,
}
results, err := strategy.Run(req)
results, err := strategyInst.Run(req)
if err != nil {
logger.Error("failed to calculate strategy", "error", err)
return
Expand Down Expand Up @@ -450,7 +285,7 @@ func (a *Agent) handlePolicy(p *policystorage.Policy) {
"reason", action.Reason, "meta", action.Meta)
}

if err = (*targetPlugin).Scale(action, p.Target.Config); err != nil {
if err = targetInst.Scale(action, p.Target.Config); err != nil {
actionLogger.Error("failed to scale target", "error", err)
continue
}
Expand Down
3 changes: 3 additions & 0 deletions agent/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/hashicorp/hcl/v2/hclsimple"
"github.com/hashicorp/nomad-autoscaler/plugins"
"github.com/mitchellh/copystructure"
)

Expand Down Expand Up @@ -162,6 +163,8 @@ func Default() (*Agent, error) {
Address: defaultNomadAddress,
Region: defaultNomadRegion,
},
APMs: []*Plugin{{Name: plugins.InternalAPMNomad, Driver: plugins.InternalAPMNomad}},
Targets: []*Plugin{{Name: plugins.InternalTargetNomad, Driver: plugins.InternalTargetNomad}},
}, nil
}

Expand Down
12 changes: 12 additions & 0 deletions agent/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ func Test_Default(t *testing.T) {
assert.Equal(t, def.Nomad.Address, "http://127.0.0.1:4646")
assert.Equal(t, "127.0.0.1", def.HTTP.BindAddress)
assert.Equal(t, 8080, def.HTTP.BindPort)
assert.Len(t, def.APMs, 1)
assert.Len(t, def.Targets, 1)
}

func TestAgent_Merge(t *testing.T) {
Expand Down Expand Up @@ -110,6 +112,10 @@ func TestAgent_Merge(t *testing.T) {
SkipVerify: true,
},
APMs: []*Plugin{
{
Name: "nomad-apm",
Driver: "nomad-apm",
},
{
Name: "prometheus",
Driver: "prometheus",
Expand All @@ -121,6 +127,12 @@ func TestAgent_Merge(t *testing.T) {
Driver: "influx-db",
},
},
Targets: []*Plugin{
{
Name: "nomad-target",
Driver: "nomad-target",
},
},
Strategies: []*Plugin{
{
Name: "target-value",
Expand Down
Loading

0 comments on commit b952153

Please sign in to comment.