Use plugin.Name instead of string in all plugin confiugrations (#362)

David Chung · web-flow · commit a7d5dabaa774 · 2017-01-13T23:13:03.000-08:00
Signed-off-by: David Chung &lt;david.chung@docker.com&gt;
diff --git a/cmd/cli/flavor.go b/cmd/cli/flavor.go
@@ -9,6 +9,7 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/infrakit/pkg/discovery"
+	"github.com/docker/infrakit/pkg/plugin"
 	"github.com/docker/infrakit/pkg/plugin/group/types"
 	flavor_plugin "github.com/docker/infrakit/pkg/rpc/flavor"
 	"github.com/docker/infrakit/pkg/spi/flavor"
@@ -28,7 +29,7 @@ func flavorPluginCommand(plugins func() discovery.Plugins) *cobra.Command {
 
 	cmd.PersistentPreRunE = func(c *cobra.Command, args []string) error {
 
-		endpoint, err := plugins().Find(*name)
+		endpoint, err := plugins().Find(plugin.Name(*name))
 		if err != nil {
 			return err
 		}
diff --git a/cmd/cli/group.go b/cmd/cli/group.go
@@ -10,6 +10,7 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/infrakit/pkg/discovery"
+	"github.com/docker/infrakit/pkg/plugin"
 	group_plugin "github.com/docker/infrakit/pkg/rpc/group"
 	"github.com/docker/infrakit/pkg/spi/group"
 	"github.com/spf13/cobra"
@@ -31,7 +32,7 @@ func groupPluginCommand(plugins func() discovery.Plugins) *cobra.Command {
 	name := cmd.PersistentFlags().String("name", DefaultGroupPluginName, "Name of plugin")
 	cmd.PersistentPreRunE = func(c *cobra.Command, args []string) error {
 
-		endpoint, err := plugins().Find(*name)
+		endpoint, err := plugins().Find(plugin.Name(*name))
 		if err != nil {
 			return err
 		}
diff --git a/cmd/cli/instance.go b/cmd/cli/instance.go
@@ -27,7 +27,7 @@ func instancePluginCommand(plugins func() discovery.Plugins) *cobra.Command {
 	name := cmd.PersistentFlags().String("name", "", "Name of plugin")
 	cmd.PersistentPreRunE = func(c *cobra.Command, args []string) error {
 
-		endpoint, err := plugins().Find(*name)
+		endpoint, err := plugins().Find(plugin.Name(*name))
 		if err != nil {
 			return err
 		}
diff --git a/cmd/group/main.go b/cmd/group/main.go
@@ -36,15 +36,15 @@ func main() {
 			return err
 		}
 
-		instancePluginLookup := func(n string) (instance.Plugin, error) {
+		instancePluginLookup := func(n plugin.Name) (instance.Plugin, error) {
 			endpoint, err := plugins.Find(n)
 			if err != nil {
 				return nil, err
 			}
 			return instance_client.NewClient(plugin.Name(n), endpoint.Address), nil
 		}
 
-		flavorPluginLookup := func(n string) (flavor.Plugin, error) {
+		flavorPluginLookup := func(n plugin.Name) (flavor.Plugin, error) {
 			endpoint, err := plugins.Find(n)
 			if err != nil {
 				return nil, err
diff --git a/pkg/cli/info.go b/pkg/cli/info.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/docker/infrakit/pkg/discovery"
+	"github.com/docker/infrakit/pkg/plugin"
 	"github.com/docker/infrakit/pkg/rpc/client"
 	"github.com/spf13/cobra"
 )
@@ -17,7 +18,7 @@ func InfoCommand(plugins func() discovery.Plugins) *cobra.Command {
 	}
 	name := cmd.PersistentFlags().String("name", "", "Name of plugin")
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
-		endpoint, err := plugins().Find(*name)
+		endpoint, err := plugins().Find(plugin.Name(*name))
 		if err != nil {
 			return err
 		}
diff --git a/pkg/discovery/dir.go b/pkg/discovery/dir.go
@@ -17,8 +17,8 @@ type dirPluginDiscovery struct {
 }
 
 // Find returns a plugin by name
-func (r *dirPluginDiscovery) Find(name string) (*plugin.Endpoint, error) {
-	lookup, _ := plugin.Name(name).GetLookupAndType()
+func (r *dirPluginDiscovery) Find(name plugin.Name) (*plugin.Endpoint, error) {
+	lookup, _ := name.GetLookupAndType()
 	plugins, err := r.List()
 	if err != nil {
 		return nil, err
diff --git a/pkg/discovery/dir_test.go b/pkg/discovery/dir_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/docker/infrakit/pkg/plugin"
 	rpc "github.com/docker/infrakit/pkg/rpc/instance"
 	"github.com/docker/infrakit/pkg/rpc/server"
 	"github.com/stretchr/testify/require"
@@ -44,33 +45,33 @@ func TestDirDiscovery(t *testing.T) {
 	discover, err := newDirPluginDiscovery(dir)
 	require.NoError(t, err)
 
-	p, err := discover.Find(name1)
+	p, err := discover.Find(plugin.Name(name1))
 	require.NoError(t, err)
 	require.NotNil(t, p)
 
-	p, err = discover.Find(name2)
+	p, err = discover.Find(plugin.Name(name2))
 	require.NoError(t, err)
 	require.NotNil(t, p)
 
 	// Now we stop the servers
 	server1.Stop()
 	blockWhileFileExists(path1)
 
-	p, err = discover.Find(name1)
+	p, err = discover.Find(plugin.Name(name1))
 	require.Error(t, err)
 
-	p, err = discover.Find(name2)
+	p, err = discover.Find(plugin.Name(name2))
 	require.NoError(t, err)
 	require.NotNil(t, p)
 
 	server2.Stop()
 
 	blockWhileFileExists(path2)
 
-	p, err = discover.Find(name1)
+	p, err = discover.Find(plugin.Name(name1))
 	require.Error(t, err)
 
-	p, err = discover.Find(name2)
+	p, err = discover.Find(plugin.Name(name2))
 	require.Error(t, err)
 
 	list, err := discover.List()
diff --git a/pkg/discovery/discovery.go b/pkg/discovery/discovery.go
@@ -12,7 +12,7 @@ import (
 // Plugins provides access to plugin discovery.
 type Plugins interface {
 	// Find looks up the plugin by name.  The name can be of the form $lookup[/$subtype].  See GetLookupAndType().
-	Find(name string) (*plugin.Endpoint, error)
+	Find(name plugin.Name) (*plugin.Endpoint, error)
 	List() (map[string]*plugin.Endpoint, error)
 }
 
diff --git a/pkg/example/flavor/combo/flavor_test.go b/pkg/example/flavor/combo/flavor_test.go
@@ -3,14 +3,16 @@ package main
 import (
 	"encoding/json"
 	"errors"
+	"testing"
+
 	mock_flavor "github.com/docker/infrakit/pkg/mock/spi/flavor"
+	"github.com/docker/infrakit/pkg/plugin"
 	"github.com/docker/infrakit/pkg/plugin/group"
 	"github.com/docker/infrakit/pkg/plugin/group/types"
 	"github.com/docker/infrakit/pkg/spi/flavor"
 	"github.com/docker/infrakit/pkg/spi/instance"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
-	"testing"
 )
 
 func jsonPtr(v string) *json.RawMessage {
@@ -32,8 +34,8 @@ var inst = instance.Spec{
 }
 
 func pluginLookup(plugins map[string]flavor.Plugin) group.FlavorPluginLookup {
-	return func(key string) (flavor.Plugin, error) {
-		plugin, has := plugins[key]
+	return func(key plugin.Name) (flavor.Plugin, error) {
+		plugin, has := plugins[key.String()]
 		if has {
 			return plugin, nil
 		}
diff --git a/pkg/example/flavor/combo/main.go b/pkg/example/flavor/combo/main.go
@@ -6,6 +6,7 @@ import (
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/infrakit/pkg/cli"
 	"github.com/docker/infrakit/pkg/discovery"
+	"github.com/docker/infrakit/pkg/plugin"
 	flavor_rpc "github.com/docker/infrakit/pkg/rpc/flavor"
 	"github.com/docker/infrakit/pkg/spi/flavor"
 	"github.com/spf13/cobra"
@@ -27,7 +28,7 @@ func main() {
 			os.Exit(1)
 		}
 
-		flavorPluginLookup := func(n string) (flavor.Plugin, error) {
+		flavorPluginLookup := func(n plugin.Name) (flavor.Plugin, error) {
 			endpoint, err := plugins.Find(n)
 			if err != nil {
 				return nil, err
diff --git a/pkg/manager/group_plugin_impl.go b/pkg/manager/group_plugin_impl.go
@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 
 	log "github.com/Sirupsen/logrus"
+	"github.com/docker/infrakit/pkg/plugin"
 	rpc "github.com/docker/infrakit/pkg/rpc/group"
 	"github.com/docker/infrakit/pkg/spi/group"
 )
@@ -17,7 +18,7 @@ func (m *manager) proxyForGroupPlugin(name string) (group.Plugin, error) {
 	// A late-binding proxy so that we don't have a problem with having to
 	// start up the manager as the last of all the plugins.
 	return NewProxy(func() (group.Plugin, error) {
-		endpoint, err := m.plugins.Find(name)
+		endpoint, err := m.plugins.Find(plugin.Name(name))
 		if err != nil {
 			return nil, err
 		}
diff --git a/pkg/plugin/group/group.go b/pkg/plugin/group/group.go
@@ -3,13 +3,15 @@ package group
 import (
 	"errors"
 	"fmt"
+	"sync"
+	"time"
+
 	log "github.com/Sirupsen/logrus"
+	plugin_base "github.com/docker/infrakit/pkg/plugin"
 	"github.com/docker/infrakit/pkg/plugin/group/types"
 	"github.com/docker/infrakit/pkg/spi/flavor"
 	"github.com/docker/infrakit/pkg/spi/group"
 	"github.com/docker/infrakit/pkg/spi/instance"
-	"sync"
-	"time"
 )
 
 const (
@@ -18,10 +20,10 @@ const (
 )
 
 // InstancePluginLookup helps with looking up an instance plugin by name
-type InstancePluginLookup func(string) (instance.Plugin, error)
+type InstancePluginLookup func(plugin_base.Name) (instance.Plugin, error)
 
 // FlavorPluginLookup helps with looking up a flavor plugin by name
-type FlavorPluginLookup func(string) (flavor.Plugin, error)
+type FlavorPluginLookup func(plugin_base.Name) (flavor.Plugin, error)
 
 // NewGroupPlugin creates a new group plugin.
 func NewGroupPlugin(
diff --git a/pkg/plugin/group/integration_test.go b/pkg/plugin/group/integration_test.go
@@ -3,15 +3,17 @@ package group
 import (
 	"encoding/json"
 	"fmt"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	plugin_base "github.com/docker/infrakit/pkg/plugin"
 	"github.com/docker/infrakit/pkg/plugin/group/types"
 	"github.com/docker/infrakit/pkg/spi/flavor"
 	"github.com/docker/infrakit/pkg/spi/group"
 	"github.com/docker/infrakit/pkg/spi/instance"
 	"github.com/stretchr/testify/require"
-	"strings"
-	"sync"
-	"testing"
-	"time"
 )
 
 const (
@@ -33,7 +35,7 @@ var (
 	leaderIDs = []instance.LogicalID{"192.168.0.4", "192.168.0.5", "192.168.0.6"}
 )
 
-func flavorPluginLookup(_ string) (flavor.Plugin, error) {
+func flavorPluginLookup(_ plugin_base.Name) (flavor.Plugin, error) {
 	return &testFlavor{}, nil
 }
 
@@ -86,8 +88,8 @@ func leaderProperties(logicalIDs []instance.LogicalID, data string) *json.RawMes
 }
 
 func pluginLookup(pluginName string, plugin instance.Plugin) InstancePluginLookup {
-	return func(key string) (instance.Plugin, error) {
-		if key == pluginName {
+	return func(key plugin_base.Name) (instance.Plugin, error) {
+		if key.String() == pluginName {
 			return plugin, nil
 		}
 		return nil, nil
@@ -185,7 +187,7 @@ func TestRollingUpdate(t *testing.T) {
 			return flavor.Unhealthy, nil
 		},
 	}
-	flavorLookup := func(_ string) (flavor.Plugin, error) {
+	flavorLookup := func(_ plugin_base.Name) (flavor.Plugin, error) {
 		return &flavorPlugin, nil
 	}
 
@@ -492,7 +494,7 @@ func TestFreeGroupWhileConverging(t *testing.T) {
 			return flavor.Unknown, nil
 		},
 	}
-	flavorLookup := func(_ string) (flavor.Plugin, error) {
+	flavorLookup := func(_ plugin_base.Name) (flavor.Plugin, error) {
 		return &flavorPlugin, nil
 	}
 
@@ -533,7 +535,7 @@ func TestUpdateFailsWhenInstanceIsUnhealthy(t *testing.T) {
 			return flavor.Healthy, nil
 		},
 	}
-	flavorLookup := func(_ string) (flavor.Plugin, error) {
+	flavorLookup := func(_ plugin_base.Name) (flavor.Plugin, error) {
 		return &flavorPlugin, nil
 	}
 
diff --git a/pkg/plugin/group/types/types.go b/pkg/plugin/group/types/types.go
@@ -5,6 +5,8 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
+
+	"github.com/docker/infrakit/pkg/plugin"
 	"github.com/docker/infrakit/pkg/spi/group"
 	"github.com/docker/infrakit/pkg/spi/instance"
 )
@@ -24,13 +26,13 @@ type AllocationMethod struct {
 
 // InstancePlugin is the structure that describes an instance plugin.
 type InstancePlugin struct {
-	Plugin     string
+	Plugin     plugin.Name
 	Properties *json.RawMessage // this will be the Spec of the plugin
 }
 
 // FlavorPlugin describes the flavor configuration
 type FlavorPlugin struct {
-	Plugin     string
+	Plugin     plugin.Name
 	Properties *json.RawMessage // this will be the Spec of the plugin
 }
 
diff --git a/pkg/plugin/name.go b/pkg/plugin/name.go
@@ -1,6 +1,7 @@
 package plugin
 
 import (
+	"fmt"
 	"strings"
 )
 
@@ -23,3 +24,25 @@ func (r Name) GetLookupAndType() (string, string) {
 func (r Name) String() string {
 	return string(r)
 }
+
+// MarshalJSON implements the JSON marshaler interface
+func (r *Name) MarshalJSON() ([]byte, error) {
+	if r == nil {
+		return nil, nil
+	}
+	return []byte(r.String()), nil
+}
+
+// UnmarshalJSON implements the JSON unmarshaler interface
+func (r *Name) UnmarshalJSON(data []byte) error {
+	str := string(data)
+	start := strings.Index(str, "\"")
+	last := strings.LastIndex(str, "\"")
+	if start == 0 && last == len(str)-1 {
+		str = str[start+1 : last]
+	} else {
+		return fmt.Errorf("bad-format-for-name:%v", string(data))
+	}
+	*r = Name(str)
+	return nil
+}
diff --git a/pkg/plugin/name_test.go b/pkg/plugin/name_test.go

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ func instancePluginCommand(plugins func() discovery.Plugins) *cobra.Command {`
`27`	`27`	`name := cmd.PersistentFlags().String("name", "", "Name of plugin")`
`28`	`28`	`cmd.PersistentPreRunE = func(c *cobra.Command, args []string) error {`
`29`	`29`
`30`		`- endpoint, err := plugins().Find(*name)`
	`30`	`+ endpoint, err := plugins().Find(plugin.Name(*name))`
`31`	`31`	`if err != nil {`
`32`	`32`	`return err`
`33`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,15 +36,15 @@ func main() {`
`36`	`36`	`return err`
`37`	`37`	`}`
`38`	`38`
`39`		`- instancePluginLookup := func(n string) (instance.Plugin, error) {`
	`39`	`+ instancePluginLookup := func(n plugin.Name) (instance.Plugin, error) {`
`40`	`40`	`endpoint, err := plugins.Find(n)`
`41`	`41`	`if err != nil {`
`42`	`42`	`return nil, err`
`43`	`43`	`}`
`44`	`44`	`return instance_client.NewClient(plugin.Name(n), endpoint.Address), nil`
`45`	`45`	`}`
`46`	`46`
`47`		`- flavorPluginLookup := func(n string) (flavor.Plugin, error) {`
	`47`	`+ flavorPluginLookup := func(n plugin.Name) (flavor.Plugin, error) {`
`48`	`48`	`endpoint, err := plugins.Find(n)`
`49`	`49`	`if err != nil {`
`50`	`50`	`return nil, err`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@ import (`
`5`	`5`	`"fmt"`
`6`	`6`
`7`	`7`	`"github.com/docker/infrakit/pkg/discovery"`
	`8`	`+ "github.com/docker/infrakit/pkg/plugin"`
`8`	`9`	`"github.com/docker/infrakit/pkg/rpc/client"`
`9`	`10`	`"github.com/spf13/cobra"`
`10`	`11`	`)`
`@@ -17,7 +18,7 @@ func InfoCommand(plugins func() discovery.Plugins) *cobra.Command {`
`17`	`18`	`}`
`18`	`19`	`name := cmd.PersistentFlags().String("name", "", "Name of plugin")`
`19`	`20`	`cmd.RunE = func(cmd *cobra.Command, args []string) error {`
`20`		`- endpoint, err := plugins().Find(*name)`
	`21`	`+ endpoint, err := plugins().Find(plugin.Name(*name))`
`21`	`22`	`if err != nil {`
`22`	`23`	`return err`
`23`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ import (`
`12`	`12`	`// Plugins provides access to plugin discovery.`
`13`	`13`	`type Plugins interface {`
`14`	`14`	`// Find looks up the plugin by name. The name can be of the form $lookup[/$subtype]. See GetLookupAndType().`
`15`		`- Find(name string) (*plugin.Endpoint, error)`
	`15`	`+ Find(name plugin.Name) (*plugin.Endpoint, error)`
`16`	`16`	`List() (map[string]*plugin.Endpoint, error)`
`17`	`17`	`}`
`18`	`18`