plugins: improve external plugin development UX and mimic Nomad. (has…

…hicorp#82) The UX for developing external plugins was OK, but required some knowledge of the go-plugin library in order to correctly setup and serve the plugin. This change aims to lower the barrier to entry on writing plugins by making the required code simpler and also mimicking the Nomad plugin interface. PluginInfo has been moved into the base package as part of this work. This seems a better logic location for the definition as it is the response object of a defined interface function inside this package. Co-Authored-By: Chris Baker <[email protected]>
lcheruka · Apr 9, 2020 · dda0102 · dda0102
1 parent 9a9551e
commit dda0102
Show file tree

Hide file tree

Showing 15 changed files with 109 additions and 71 deletions.
diff --git a/plugins/apm/apm.go b/plugins/apm/apm.go
@@ -4,12 +4,12 @@ import (
 	"net/rpc"
 
 	plugin "github.com/hashicorp/go-plugin"
-	"github.com/hashicorp/nomad-autoscaler/plugins"
+	"github.com/hashicorp/nomad-autoscaler/plugins/base"
 )
 
 type APM interface {
 	Query(q string) (float64, error)
-	PluginInfo() (*plugins.PluginInfo, error)
+	PluginInfo() (*base.PluginInfo, error)
 	SetConfig(config map[string]string) error
 }
 
@@ -36,8 +36,8 @@ func (r *RPC) Query(q string) (float64, error) {
 	return resp, nil
 }
 
-func (r *RPC) PluginInfo() (*plugins.PluginInfo, error) {
-	var resp plugins.PluginInfo
+func (r *RPC) PluginInfo() (*base.PluginInfo, error) {
+	var resp base.PluginInfo
 	err := r.client.Call("Plugin.PluginInfo", new(interface{}), &resp)
 	if err != nil {
 		return &resp, err
@@ -65,7 +65,7 @@ func (s *RPCServer) Query(q string, resp *float64) error {
 	return nil
 }
 
-func (s *RPCServer) PluginInfo(_ interface{}, r *plugins.PluginInfo) error {
+func (s *RPCServer) PluginInfo(_ interface{}, r *base.PluginInfo) error {
 	resp, err := s.Impl.PluginInfo()
 	if resp != nil {
 		*r = *resp

diff --git a/plugins/base/base.go b/plugins/base/base.go
@@ -1,7 +1,5 @@
 package base
 
-import "github.com/hashicorp/nomad-autoscaler/plugins"
-
 // Plugin is the common interface that all Autoscaler plugins should implement.
 // It defines basic functionality which helps the Autoscaler core deal with
 // plugins in a common manner.
@@ -11,10 +9,18 @@ type Plugin interface {
 	// the plugin's setup as well as lifecycle. Any error generated during the
 	// plugin's internal process to create and return this information should be
 	// sent back to the caller so it can be presented.
-	PluginInfo() (*plugins.PluginInfo, error)
+	PluginInfo() (*PluginInfo, error)
 
 	// SetConfig is used by the Autoscaler core to set plugin-specific
 	// configuration on the remote target. If this call fails, the plugin
 	// should be considered in a terminal state.
 	SetConfig(config map[string]string) error
 }
+
+// PluginInfo is the information used by plugins to identify themselves and
+// contains critical information about their configuration. It is used within
+// the base plugin PluginInfo response RPC call.
+type PluginInfo struct {
+	Name       string
+	PluginType string
+}
diff --git a/plugins/builtin/apm/nomad/main.go b/plugins/builtin/apm/nomad/main.go
@@ -1,17 +1,16 @@
 package main
 
 import (
-	"github.com/hashicorp/go-plugin"
+	hclog "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad-autoscaler/plugins"
-	"github.com/hashicorp/nomad-autoscaler/plugins/apm"
 	nomadapm "github.com/hashicorp/nomad-autoscaler/plugins/builtin/apm/nomad/plugin"
 )
 
 func main() {
-	plugin.Serve(&plugin.ServeConfig{
-		HandshakeConfig: plugins.Handshake,
-		Plugins: map[string]plugin.Plugin{
-			plugins.PluginTypeAPM: &apm.Plugin{Impl: &nomadapm.APMPlugin{}},
-		},
-	})
+	plugins.Serve(factory)
+}
+
+// factory returns a new instance of the Nomad APM plugin.
+func factory(log hclog.Logger) interface{} {
+	return nomadapm.NewNomadPlugin(log)
 }
diff --git a/plugins/builtin/apm/nomad/plugin/plugin.go b/plugins/builtin/apm/nomad/plugin/plugin.go
@@ -7,6 +7,7 @@ import (
 	nomadHelper "github.com/hashicorp/nomad-autoscaler/helper/nomad"
 	"github.com/hashicorp/nomad-autoscaler/plugins"
 	"github.com/hashicorp/nomad-autoscaler/plugins/apm"
+	"github.com/hashicorp/nomad-autoscaler/plugins/base"
 	"github.com/hashicorp/nomad/api"
 )
 
@@ -25,7 +26,7 @@ var (
 		Factory: func(l hclog.Logger) interface{} { return NewNomadPlugin(l) },
 	}
 
-	pluginInfo = &plugins.PluginInfo{
+	pluginInfo = &base.PluginInfo{
 		Name:       pluginName,
 		PluginType: plugins.PluginTypeAPM,
 	}
@@ -55,6 +56,6 @@ func (a *APMPlugin) SetConfig(config map[string]string) error {
 	return nil
 }
 
-func (a *APMPlugin) PluginInfo() (*plugins.PluginInfo, error) {
+func (a *APMPlugin) PluginInfo() (*base.PluginInfo, error) {
 	return pluginInfo, nil
 }
diff --git a/plugins/builtin/apm/prometheus/main.go b/plugins/builtin/apm/prometheus/main.go
@@ -1,17 +1,16 @@
 package main
 
 import (
-	plugin "github.com/hashicorp/go-plugin"
+	hclog "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad-autoscaler/plugins"
-	"github.com/hashicorp/nomad-autoscaler/plugins/apm"
 	prometheus "github.com/hashicorp/nomad-autoscaler/plugins/builtin/apm/prometheus/plugin"
 )
 
 func main() {
-	plugin.Serve(&plugin.ServeConfig{
-		HandshakeConfig: plugins.Handshake,
-		Plugins: map[string]plugin.Plugin{
-			plugins.PluginTypeAPM: &apm.Plugin{Impl: &prometheus.APMPlugin{}},
-		},
-	})
+	plugins.Serve(factory)
+}
+
+// factory returns a new instance of the Prometheus APM plugin.
+func factory(log hclog.Logger) interface{} {
+	return prometheus.NewPrometheusPlugin(log)
 }
diff --git a/plugins/builtin/apm/prometheus/plugin/plugin.go b/plugins/builtin/apm/prometheus/plugin/plugin.go
@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad-autoscaler/plugins"
 	"github.com/hashicorp/nomad-autoscaler/plugins/apm"
+	"github.com/hashicorp/nomad-autoscaler/plugins/base"
 	"github.com/prometheus/client_golang/api"
 	v1 "github.com/prometheus/client_golang/api/prometheus/v1"
 	"github.com/prometheus/common/model"
@@ -28,7 +29,7 @@ var (
 		Factory: func(l hclog.Logger) interface{} { return NewPrometheusPlugin(l) },
 	}
 
-	pluginInfo = &plugins.PluginInfo{
+	pluginInfo = &base.PluginInfo{
 		Name:       pluginName,
 		PluginType: plugins.PluginTypeAPM,
 	}
@@ -66,7 +67,7 @@ func (a *APMPlugin) SetConfig(config map[string]string) error {
 	return nil
 }
 
-func (a *APMPlugin) PluginInfo() (*plugins.PluginInfo, error) {
+func (a *APMPlugin) PluginInfo() (*base.PluginInfo, error) {
 	return pluginInfo, nil
 }
 

diff --git a/plugins/builtin/strategy/target-value/main.go b/plugins/builtin/strategy/target-value/main.go
@@ -1,17 +1,16 @@
 package main
 
 import (
-	"github.com/hashicorp/go-plugin"
+	hclog "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad-autoscaler/plugins"
 	targetvalue "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/target-value/plugin"
-	"github.com/hashicorp/nomad-autoscaler/plugins/strategy"
 )
 
 func main() {
-	plugin.Serve(&plugin.ServeConfig{
-		HandshakeConfig: plugins.Handshake,
-		Plugins: map[string]plugin.Plugin{
-			plugins.PluginTypeStrategy: &strategy.Plugin{Impl: &targetvalue.StrategyPlugin{}},
-		},
-	})
+	plugins.Serve(factory)
+}
+
+// factory returns a new instance of the TargetValue Strategy plugin.
+func factory(log hclog.Logger) interface{} {
+	return targetvalue.NewTargetValuePlugin(log)
 }
diff --git a/plugins/builtin/strategy/target-value/plugin/plugin.go b/plugins/builtin/strategy/target-value/plugin/plugin.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad-autoscaler/plugins"
+	"github.com/hashicorp/nomad-autoscaler/plugins/base"
 	"github.com/hashicorp/nomad-autoscaler/plugins/strategy"
 )
 
@@ -26,7 +27,7 @@ var (
 		Factory: func(l hclog.Logger) interface{} { return NewTargetValuePlugin(l) },
 	}
 
-	pluginInfo = &plugins.PluginInfo{
+	pluginInfo = &base.PluginInfo{
 		Name:       pluginName,
 		PluginType: plugins.PluginTypeStrategy,
 	}
@@ -57,7 +58,7 @@ func (s *StrategyPlugin) SetConfig(config map[string]string) error {
 }
 
 // PluginInfo satisfies the PluginInfo function on the base.Plugin interface.
-func (s *StrategyPlugin) PluginInfo() (*plugins.PluginInfo, error) {
+func (s *StrategyPlugin) PluginInfo() (*base.PluginInfo, error) {
 	return pluginInfo, nil
 }
 

diff --git a/plugins/builtin/strategy/target-value/plugin/plugin_test.go b/plugins/builtin/strategy/target-value/plugin/plugin_test.go
@@ -5,7 +5,7 @@ import (
 	"testing"
 
 	hclog "github.com/hashicorp/go-hclog"
-	"github.com/hashicorp/nomad-autoscaler/plugins"
+	"github.com/hashicorp/nomad-autoscaler/plugins/base"
 	"github.com/hashicorp/nomad-autoscaler/plugins/strategy"
 	"github.com/stretchr/testify/assert"
 )
@@ -20,7 +20,7 @@ func TestStrategyPlugin_SetConfig(t *testing.T) {
 
 func TestStrategyPlugin_PluginInfo(t *testing.T) {
 	s := &StrategyPlugin{}
-	expectedOutput := &plugins.PluginInfo{Name: "target-value", PluginType: "strategy"}
+	expectedOutput := &base.PluginInfo{Name: "target-value", PluginType: "strategy"}
 	actualOutput, err := s.PluginInfo()
 	assert.Nil(t, err)
 	assert.Equal(t, expectedOutput, actualOutput)

diff --git a/plugins/builtin/target/nomad/main.go b/plugins/builtin/target/nomad/main.go
@@ -1,17 +1,16 @@
 package main
 
 import (
-	"github.com/hashicorp/go-plugin"
+	hclog "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad-autoscaler/plugins"
 	nomadtarget "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/nomad/plugin"
-	"github.com/hashicorp/nomad-autoscaler/plugins/target"
 )
 
 func main() {
-	plugin.Serve(&plugin.ServeConfig{
-		HandshakeConfig: plugins.Handshake,
-		Plugins: map[string]plugin.Plugin{
-			plugins.PluginTypeTarget: &target.Plugin{Impl: &nomadtarget.TargetPlugin{}},
-		},
-	})
+	plugins.Serve(factory)
+}
+
+// factory returns a new instance of the Nomad Target plugin.
+func factory(log hclog.Logger) interface{} {
+	return nomadtarget.NewNomadPlugin(log)
 }
diff --git a/plugins/builtin/target/nomad/plugin/plugin.go b/plugins/builtin/target/nomad/plugin/plugin.go
@@ -6,6 +6,7 @@ import (
 	"github.com/hashicorp/go-hclog"
 	nomadHelper "github.com/hashicorp/nomad-autoscaler/helper/nomad"
 	"github.com/hashicorp/nomad-autoscaler/plugins"
+	"github.com/hashicorp/nomad-autoscaler/plugins/base"
 	"github.com/hashicorp/nomad-autoscaler/plugins/strategy"
 	"github.com/hashicorp/nomad/api"
 )
@@ -25,7 +26,7 @@ var (
 		Factory: func(l hclog.Logger) interface{} { return NewNomadPlugin(l) },
 	}
 
-	pluginInfo = &plugins.PluginInfo{
+	pluginInfo = &base.PluginInfo{
 		Name:       pluginName,
 		PluginType: plugins.PluginTypeTarget,
 	}
@@ -55,7 +56,7 @@ func (t *TargetPlugin) SetConfig(config map[string]string) error {
 	return nil
 }
 
-func (t *TargetPlugin) PluginInfo() (*plugins.PluginInfo, error) {
+func (t *TargetPlugin) PluginInfo() (*base.PluginInfo, error) {
 	return pluginInfo, nil
 }
 

diff --git a/plugins/manager/manager.go b/plugins/manager/manager.go
@@ -41,7 +41,7 @@ type PluginManager struct {
 // plugin. Not all details are required depending on whether the plugin is
 // internal or external; these are noted on the params.
 type pluginInfo struct {
-	baseInfo *plugins.PluginInfo
+	baseInfo *base.PluginInfo
 	config   map[string]string
 
 	// args and exePath are required to execute the external plugin command.
@@ -132,7 +132,7 @@ func (pm *PluginManager) dispensePlugins() error {
 
 		var (
 			inst PluginInstance
-			info *plugins.PluginInfo
+			info *base.PluginInfo
 			err  error
 		)
 		if pInfo.factory != nil {
@@ -174,7 +174,7 @@ func (pm *PluginManager) dispensePlugins() error {
 }
 
 // launchInternalPlugin is used to dispense internal plugins.
-func (pm *PluginManager) launchInternalPlugin(id plugins.PluginID, info *pluginInfo) (PluginInstance, *plugins.PluginInfo, error) {
+func (pm *PluginManager) launchInternalPlugin(id plugins.PluginID, info *pluginInfo) (PluginInstance, *base.PluginInfo, error) {
 
 	raw := info.factory(pm.logger.ResetNamed("internal_plugin"))
 
@@ -188,7 +188,7 @@ func (pm *PluginManager) launchInternalPlugin(id plugins.PluginID, info *pluginI
 // launchExternalPlugin is used to dispense external plugins. These plugins
 // are therefore run as separate binaries and require more setup than internal
 // ones.
-func (pm *PluginManager) launchExternalPlugin(id plugins.PluginID, info *pluginInfo) (PluginInstance, *plugins.PluginInfo, error) {
+func (pm *PluginManager) launchExternalPlugin(id plugins.PluginID, info *pluginInfo) (PluginInstance, *base.PluginInfo, error) {
 
 	// Create a new client for the external plugin. This includes items such as
 	// the command to execute and also the logger to use. The loggers name is
@@ -223,7 +223,7 @@ func (pm *PluginManager) launchExternalPlugin(id plugins.PluginID, info *pluginI
 	return &externalPluginInstance{instance: raw, client: client}, pInfo, nil
 }
 
-func (pm *PluginManager) pluginLaunchCheck(id plugins.PluginID, raw interface{}) (*plugins.PluginInfo, error) {
+func (pm *PluginManager) pluginLaunchCheck(id plugins.PluginID, raw interface{}) (*base.PluginInfo, error) {
 
 	// Check that the plugin implements the base plugin interface. As these are
 	// external plugins we need to check this safely, otherwise an incorrect

diff --git a/plugins/plugin.go b/plugins/plugin.go
@@ -5,6 +5,9 @@ import (
 
 	hclog "github.com/hashicorp/go-hclog"
 	plugin "github.com/hashicorp/go-plugin"
+	"github.com/hashicorp/nomad-autoscaler/plugins/apm"
+	"github.com/hashicorp/nomad-autoscaler/plugins/strategy"
+	"github.com/hashicorp/nomad-autoscaler/plugins/target"
 )
 
 const (
@@ -69,10 +72,39 @@ func (p PluginID) String() string {
 	return fmt.Sprintf("%q (%v)", p.Name, p.PluginType)
 }
 
-// PluginInfo is the information used by plugins to identify themselves and
-// contains critical information about their configuration. It is used within
-// the base plugin PluginInfo response RPC call.
-type PluginInfo struct {
-	Name       string
-	PluginType string
+// Serve is used to serve a Nomad Autoscaler Plugin.
+func Serve(f PluginFactory) {
+	logger := hclog.New(&hclog.LoggerOptions{
+		Level:      hclog.Trace,
+		JSONFormat: true,
+	})
+
+	// Generate the plugin.
+	p := f(logger)
+	if p == nil {
+		logger.Error("plugin factory returned nil")
+		return
+	}
+
+	// Build the base plugin configuration which is independent of the plugin
+	// type.
+	pCfg := plugin.ServeConfig{
+		HandshakeConfig: Handshake,
+		Logger:          logger,
+	}
+
+	switch pType := p.(type) {
+	case apm.APM:
+		pCfg.Plugins = map[string]plugin.Plugin{PluginTypeAPM: &apm.Plugin{Impl: p.(apm.APM)}}
+	case target.Target:
+		pCfg.Plugins = map[string]plugin.Plugin{PluginTypeTarget: &target.Plugin{Impl: p.(target.Target)}}
+	case strategy.Strategy:
+		pCfg.Plugins = map[string]plugin.Plugin{PluginTypeStrategy: &strategy.Plugin{Impl: p.(strategy.Strategy)}}
+	default:
+		logger.Error("unsupported plugin type %q", pType)
+		return
+	}
+
+	// Serve the plugin; lovely jubbly.
+	plugin.Serve(&pCfg)
 }