From 144710c0aa52cea1a9dc95c9d948b60e669d8f7b Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Fri, 25 Nov 2022 12:45:53 -0500 Subject: [PATCH 01/40] start to plugin host impl wip --- ignite/chainconfig/v1/config.go | 4 + ignite/cmd/plugin.go | 2 + ignite/services/plugin/plugin.go | 58 ++++++++++++--- ignite/services/plugin/utils.go | 122 +++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 9 deletions(-) create mode 100644 ignite/services/plugin/utils.go diff --git a/ignite/chainconfig/v1/config.go b/ignite/chainconfig/v1/config.go index 76baa4df21..d0e813863e 100644 --- a/ignite/chainconfig/v1/config.go +++ b/ignite/chainconfig/v1/config.go @@ -46,6 +46,10 @@ type Plugin struct { Path string `yaml:"path"` // With holds arguments passed to the plugin interface With map[string]string `yaml:"with"` + // ShareHost holds context on if a plugin should attach to an already existing rpc server if available + // flagging this true will allow only one plugin server for the given plugin to be created at a time, if another instance if running + // the plugin's RPC client will only attach to it. + ShareHost bool } func (c *Config) SetDefaults() error { diff --git a/ignite/cmd/plugin.go b/ignite/cmd/plugin.go index cd477719c5..413a7c70bc 100644 --- a/ignite/cmd/plugin.go +++ b/ignite/cmd/plugin.go @@ -327,6 +327,8 @@ func printPlugins() { status := "✅ Loaded" if p.Error != nil { status = fmt.Sprintf("❌ Error: %v", p.Error) + entries = append(entries, []string{p.Path, status}) + continue } entries = append(entries, []string{p.Path, status}) } diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index c59242c524..3eac32286a 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -50,6 +50,10 @@ type Plugin struct { binaryName string client *hplugin.Client + + // if a plugin's ShareHost flag is set to true, isHost is used to decern if a + // plugin instance is controlling the rpc server. + isHost bool } // Load loads the plugins found in the chain config. @@ -145,6 +149,11 @@ func (p *Plugin) KillClient() { if p.client != nil { p.client.Kill() } + + if p.isHost { + DeletePluginConf(p.Path) + p.isHost = false + } } func (p *Plugin) isLocal() bool { @@ -194,15 +203,40 @@ func (p *Plugin) load(ctx context.Context) { Output: os.Stderr, Level: hclog.Error, }) - // We're a host! Start by launching the plugin process. - p.client = hplugin.NewClient(&hplugin.ClientConfig{ - HandshakeConfig: handshakeConfig, - Plugins: pluginMap, - Logger: logger, - Cmd: exec.Command(p.binaryPath()), - SyncStderr: os.Stderr, - SyncStdout: os.Stdout, - }) + if p.Plugin.ShareHost && CheckPluginConf(p.Path) { + rconf := hplugin.ReattachConfig{} + err := ReadPluginConfig(p.Path, &rconf) + fmt.Printf("%v\n", rconf) + + if err != nil { + p.Error = err + return + } + + // We're a host! Start by launching the plugin process. + p.client = hplugin.NewClient(&hplugin.ClientConfig{ + HandshakeConfig: handshakeConfig, + Plugins: pluginMap, + Logger: logger, + Reattach: &rconf, + SyncStderr: os.Stderr, + SyncStdout: os.Stdout, + }) + } else { + // We're a host! Start by launching the plugin process. + p.client = hplugin.NewClient(&hplugin.ClientConfig{ + HandshakeConfig: handshakeConfig, + Plugins: pluginMap, + Logger: logger, + Cmd: exec.Command(p.binaryPath()), + SyncStderr: os.Stderr, + SyncStdout: os.Stdout, + }) + + if p.Plugin.ShareHost { + p.isHost = true + } + } // Connect via RPC rpcClient, err := p.client.Client() @@ -221,6 +255,12 @@ func (p *Plugin) load(ctx context.Context) { // We should have an Interface now! This feels like a normal interface // implementation but is in fact over an RPC connection. p.Interface = raw.(Interface) + + if !CheckPluginConf(p.Path) && p.isHost { + fmt.Printf("%v\n", *p.client.ReattachConfig()) + WritePluginConfig(p.Path, *p.client.ReattachConfig()) + } + } // fetch clones the plugin repository at the expected reference. diff --git a/ignite/services/plugin/utils.go b/ignite/services/plugin/utils.go new file mode 100644 index 0000000000..31a2d237bc --- /dev/null +++ b/ignite/services/plugin/utils.go @@ -0,0 +1,122 @@ +package plugin + +import ( + "encoding/gob" + "fmt" + "net" + "path/filepath" + "strings" + + hplugin "github.com/hashicorp/go-plugin" + "github.com/ignite/cli/ignite/chainconfig" + "github.com/ignite/cli/ignite/pkg/cache" +) + +const ( + EXT = ".gob" + cacheFileName = "ignite_cache.db" + cacheNamespace = "plugin.rpc.context" +) + +func init() { + gob.Register(hplugin.ReattachConfig{}) + gob.Register(net.UnixAddr{}) +} + +type ConfigContext struct { + Plugin hplugin.ReattachConfig + Addr net.UnixAddr +} + +func WritePluginConfig(path string, conf hplugin.ReattachConfig) error { + paths := strings.Split(path, "/") + name := paths[len(paths)-1] + + fmt.Println("encoding config") + confCont := ConfigContext{} + + ua, err := net.ResolveUnixAddr(conf.Addr.Network(), conf.Addr.String()) + if err != nil { + return err + } + + confCont.Addr = *ua + conf.Addr = nil + confCont.Plugin = conf + + cache, err := newCache() + + if err != nil { + return err + } + + cache.Put(name, confCont) + + return err +} + +func ReadPluginConfig(path string, ref *hplugin.ReattachConfig) error { + paths := strings.Split(path, "/") + name := paths[len(paths)-1] + cache, err := newCache() + if err != nil { + return err + } + + confCont, err := cache.Get(name) + if err != nil { + return err + } + + *ref = confCont.Plugin + ref.Addr = &confCont.Addr + + return nil +} + +func CheckPluginConf(path string) bool { + paths := strings.Split(path, "/") + name := paths[len(paths)-1] + + cache, err := newCache() + + if err != nil { + return false + } + if _, err := cache.Get(name); err != nil { + return false + } + return true +} + +func DeletePluginConf(path string) error { + paths := strings.Split(path, "/") + name := paths[len(paths)-1] + + cache, err := newCache() + + if err != nil { + return err + } + + if err := cache.Delete(name); err != nil { + return err + } + + return nil +} + +func newCache() (*cache.Cache[ConfigContext], error) { + cacheRootDir, err := chainconfig.ConfigDirPath() + if err != nil { + return nil, err + } + + storage, err := cache.NewStorage(filepath.Join(cacheRootDir, cacheFileName)) + if err != nil { + return nil, err + } + store := cache.New[ConfigContext](storage, cacheNamespace) + + return &store, nil +} From fe20e033e5d63b5f28c18041f0be159ef9c2edfb Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Fri, 25 Nov 2022 12:47:56 -0500 Subject: [PATCH 02/40] comments --- ignite/services/plugin/plugin.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index 3eac32286a..82e6989afe 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -256,6 +256,9 @@ func (p *Plugin) load(ctx context.Context) { // implementation but is in fact over an RPC connection. p.Interface = raw.(Interface) + // write the rpc context to cache if the plugin is declared as host. + // writing it to cache as lost operation within load to assure rpc client's reattach config + // is hydrated. if !CheckPluginConf(p.Path) && p.isHost { fmt.Printf("%v\n", *p.client.ReattachConfig()) WritePluginConfig(p.Path, *p.client.ReattachConfig()) From 07f0e00d413ef27b51af067bd4d5eed98b436de6 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sun, 27 Nov 2022 12:54:30 -0500 Subject: [PATCH 03/40] minor bug updates --- ignite/chainconfig/v1/config.go | 2 +- ignite/services/plugin/plugin.go | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/ignite/chainconfig/v1/config.go b/ignite/chainconfig/v1/config.go index d0e813863e..d9943412eb 100644 --- a/ignite/chainconfig/v1/config.go +++ b/ignite/chainconfig/v1/config.go @@ -49,7 +49,7 @@ type Plugin struct { // ShareHost holds context on if a plugin should attach to an already existing rpc server if available // flagging this true will allow only one plugin server for the given plugin to be created at a time, if another instance if running // the plugin's RPC client will only attach to it. - ShareHost bool + ShareHost bool `yaml:"sharedHost"` } func (c *Config) SetDefaults() error { diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index a7e880e8a9..daf7d5abfd 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -148,11 +148,10 @@ func newPlugin(pluginsDir string, cp chainconfig.Plugin) *Plugin { } func (p *Plugin) KillClient() { - if p.client != nil { - p.client.Kill() - } - if p.isHost { + if p.client != nil { + p.client.Kill() + } DeletePluginConf(p.Path) p.isHost = false } @@ -224,6 +223,7 @@ func (p *Plugin) load(ctx context.Context) { SyncStderr: os.Stderr, SyncStdout: os.Stdout, }) + } else { // We're a host! Start by launching the plugin process. p.client = hplugin.NewClient(&hplugin.ClientConfig{ @@ -235,9 +235,7 @@ func (p *Plugin) load(ctx context.Context) { SyncStdout: os.Stdout, }) - if p.Plugin.ShareHost { - p.isHost = true - } + p.isHost = true } // Connect via RPC From 493df1cebf64b588bd0568dd1ee7ba68ffa8b25d Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Wed, 30 Nov 2022 18:35:34 -0500 Subject: [PATCH 04/40] updates per plugin config migration --- ignite/config/plugins/config.go | 3 ++- ignite/services/plugin/plugin.go | 2 +- ignite/services/plugin/utils.go | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ignite/config/plugins/config.go b/ignite/config/plugins/config.go index 2bdffbd540..d61d997f62 100644 --- a/ignite/config/plugins/config.go +++ b/ignite/config/plugins/config.go @@ -57,7 +57,8 @@ type Plugin struct { // path: github.com/foo/bar/plugin1@v42 Path string `yaml:"path"` // With holds arguments passed to the plugin interface - With map[string]string `yaml:"with"` + With map[string]string `yaml:"with"` + SharedHost bool `yaml:"sharedHost"` } // Clone returns an identical copy of the instance diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index 78c2735109..6c2e6773e8 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -207,7 +207,7 @@ func (p *Plugin) load(ctx context.Context) { Output: os.Stderr, Level: logLevel, }) - if p.Plugin.ShareHost && CheckPluginConf(p.Path) { + if p.Plugin.SharedHost && CheckPluginConf(p.Path) { rconf := hplugin.ReattachConfig{} err := ReadPluginConfig(p.Path, &rconf) fmt.Printf("%v\n", rconf) diff --git a/ignite/services/plugin/utils.go b/ignite/services/plugin/utils.go index 31a2d237bc..cafcd041b2 100644 --- a/ignite/services/plugin/utils.go +++ b/ignite/services/plugin/utils.go @@ -8,7 +8,7 @@ import ( "strings" hplugin "github.com/hashicorp/go-plugin" - "github.com/ignite/cli/ignite/chainconfig" + chainconfig "github.com/ignite/cli/ignite/config" "github.com/ignite/cli/ignite/pkg/cache" ) @@ -107,7 +107,8 @@ func DeletePluginConf(path string) error { } func newCache() (*cache.Cache[ConfigContext], error) { - cacheRootDir, err := chainconfig.ConfigDirPath() + cacheRootDir, err := chainconfig.DirPath() + if err != nil { return nil, err } From a5dfc2339eeb8881460de62da96ea8c7a5e0b780 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sun, 4 Dec 2022 17:52:38 -0500 Subject: [PATCH 05/40] small updates to sharedHost plugin option --- ignite/services/plugin/plugin.go | 8 +++++++- ignite/services/plugin/utils.go | 22 ++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index 6c2e6773e8..93cf579188 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -153,6 +153,9 @@ func (p *Plugin) KillClient() { } DeletePluginConf(p.Path) p.isHost = false + // if the sharedHost config parameter is set to false we shutdown normally. + } else if !p.Plugin.SharedHost { + DeletePluginConf(p.Path) } } @@ -238,7 +241,10 @@ func (p *Plugin) load(ctx context.Context) { SyncStdout: os.Stdout, }) - p.isHost = true + if p.Plugin.SharedHost { + p.isHost = true + } + } // Connect via RPC diff --git a/ignite/services/plugin/utils.go b/ignite/services/plugin/utils.go index cafcd041b2..923189fcef 100644 --- a/ignite/services/plugin/utils.go +++ b/ignite/services/plugin/utils.go @@ -18,6 +18,11 @@ const ( cacheNamespace = "plugin.rpc.context" ) +var ( + storage *cache.Storage + storageCache *cache.Cache[ConfigContext] +) + func init() { gob.Register(hplugin.ReattachConfig{}) gob.Register(net.UnixAddr{}) @@ -35,6 +40,8 @@ func WritePluginConfig(path string, conf hplugin.ReattachConfig) error { fmt.Println("encoding config") confCont := ConfigContext{} + // todo: figure out a better way of resolving the type of network connection is established between plugin server and host + // currently this will always be a unix network socket. but this might not be the case moving forward. ua, err := net.ResolveUnixAddr(conf.Addr.Network(), conf.Addr.String()) if err != nil { return err @@ -112,12 +119,15 @@ func newCache() (*cache.Cache[ConfigContext], error) { if err != nil { return nil, err } - - storage, err := cache.NewStorage(filepath.Join(cacheRootDir, cacheFileName)) - if err != nil { - return nil, err + if storage == nil { + storageTmp, err := cache.NewStorage(filepath.Join(cacheRootDir, cacheFileName)) + if err != nil { + return nil, err + } + storage = &storageTmp + cacheTmp := cache.New[ConfigContext](*storage, cacheNamespace) + storageCache = &cacheTmp } - store := cache.New[ConfigContext](storage, cacheNamespace) - return &store, nil + return storageCache, nil } From d174f8a0052329fe8e3f77dc105418e6017d72f1 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Mon, 5 Dec 2022 16:48:28 -0500 Subject: [PATCH 06/40] update to isolate plugin cache --- ignite/services/plugin/utils.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ignite/services/plugin/utils.go b/ignite/services/plugin/utils.go index 923189fcef..b6e23455ac 100644 --- a/ignite/services/plugin/utils.go +++ b/ignite/services/plugin/utils.go @@ -13,8 +13,7 @@ import ( ) const ( - EXT = ".gob" - cacheFileName = "ignite_cache.db" + cacheFileName = "ignite_plugin_cache.db" cacheNamespace = "plugin.rpc.context" ) @@ -40,7 +39,7 @@ func WritePluginConfig(path string, conf hplugin.ReattachConfig) error { fmt.Println("encoding config") confCont := ConfigContext{} - // todo: figure out a better way of resolving the type of network connection is established between plugin server and host + // TODO: figure out a better way of resolving the type of network connection is established between plugin server and host // currently this will always be a unix network socket. but this might not be the case moving forward. ua, err := net.ResolveUnixAddr(conf.Addr.Network(), conf.Addr.String()) if err != nil { From 3e518b6ba203e0aadc3d8f722fc4cbe5ae64e650 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 6 Dec 2022 09:45:18 -0500 Subject: [PATCH 07/40] rename of utils to cache --- ignite/services/plugin/{utils.go => cache.go} | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) rename ignite/services/plugin/{utils.go => cache.go} (74%) diff --git a/ignite/services/plugin/utils.go b/ignite/services/plugin/cache.go similarity index 74% rename from ignite/services/plugin/utils.go rename to ignite/services/plugin/cache.go index b6e23455ac..773eac96c2 100644 --- a/ignite/services/plugin/utils.go +++ b/ignite/services/plugin/cache.go @@ -4,8 +4,8 @@ import ( "encoding/gob" "fmt" "net" + "path" "path/filepath" - "strings" hplugin "github.com/hashicorp/go-plugin" chainconfig "github.com/ignite/cli/ignite/config" @@ -32,9 +32,12 @@ type ConfigContext struct { Addr net.UnixAddr } -func WritePluginConfig(path string, conf hplugin.ReattachConfig) error { - paths := strings.Split(path, "/") - name := paths[len(paths)-1] +func WritePluginConfig(pluginPath string, conf hplugin.ReattachConfig) error { + name := path.Base(pluginPath) + + if name == "" { + return fmt.Errorf("provided path is invalid: %s", pluginPath) + } fmt.Println("encoding config") confCont := ConfigContext{} @@ -42,6 +45,7 @@ func WritePluginConfig(path string, conf hplugin.ReattachConfig) error { // TODO: figure out a better way of resolving the type of network connection is established between plugin server and host // currently this will always be a unix network socket. but this might not be the case moving forward. ua, err := net.ResolveUnixAddr(conf.Addr.Network(), conf.Addr.String()) + if err != nil { return err } @@ -61,9 +65,13 @@ func WritePluginConfig(path string, conf hplugin.ReattachConfig) error { return err } -func ReadPluginConfig(path string, ref *hplugin.ReattachConfig) error { - paths := strings.Split(path, "/") - name := paths[len(paths)-1] +func ReadPluginConfig(pluginPath string, ref *hplugin.ReattachConfig) error { + name := path.Base(pluginPath) + + if name == "" { + return fmt.Errorf("provided path is invalid: %s", pluginPath) + } + cache, err := newCache() if err != nil { return err @@ -80,9 +88,12 @@ func ReadPluginConfig(path string, ref *hplugin.ReattachConfig) error { return nil } -func CheckPluginConf(path string) bool { - paths := strings.Split(path, "/") - name := paths[len(paths)-1] +func CheckPluginConf(pluginPath string) bool { + name := path.Base(pluginPath) + + if name == "" { + return false + } cache, err := newCache() @@ -95,10 +106,12 @@ func CheckPluginConf(path string) bool { return true } -func DeletePluginConf(path string) error { - paths := strings.Split(path, "/") - name := paths[len(paths)-1] +func DeletePluginConf(pluginPath string) error { + name := path.Base(pluginPath) + if name == "" { + return fmt.Errorf("provided path is invalid: %s", pluginPath) + } cache, err := newCache() if err != nil { From 011fc0612016575f2094f84703333ddccb8dec2c Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 6 Dec 2022 09:45:29 -0500 Subject: [PATCH 08/40] print line removing --- ignite/services/plugin/plugin.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index 93cf579188..831b18aae0 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -213,7 +213,6 @@ func (p *Plugin) load(ctx context.Context) { if p.Plugin.SharedHost && CheckPluginConf(p.Path) { rconf := hplugin.ReattachConfig{} err := ReadPluginConfig(p.Path, &rconf) - fmt.Printf("%v\n", rconf) if err != nil { p.Error = err From 5bea32dad3ca9c507b9b96d861899cf9d4855380 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 6 Dec 2022 09:55:52 -0500 Subject: [PATCH 09/40] tidy --- go.mod | 1 - go.sum | 4 ---- 2 files changed, 5 deletions(-) diff --git a/go.mod b/go.mod index 843e04ca8a..c885118043 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,6 @@ require ( github.com/cosmos/ibc-go/v5 v5.1.0 github.com/emicklei/proto v1.11.0 github.com/emicklei/proto-contrib v0.12.0 - github.com/fatih/color v1.13.0 github.com/ghodss/yaml v1.0.0 github.com/go-git/go-git/v5 v5.4.2 github.com/gobuffalo/genny/v2 v2.1.0 diff --git a/go.sum b/go.sum index fd0e68bb96..91c5681182 100644 --- a/go.sum +++ b/go.sum @@ -1713,10 +1713,6 @@ github.com/syndtr/gocapability v0.0.0-20180916011248-d98352740cb2/go.mod h1:hkRG github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww= github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 h1:epCh84lMvA70Z7CTTCmYQn2CKbY8j86K7/FAIr141uY= github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7/go.mod h1:q4W45IWZaF22tdD+VEXcAWRA037jwmWEB5VWYORlTpc= -github.com/tbruyelle/mdgofmt v0.1.0 h1:sk48Z/BoOccm6DaZI3zqgK+cs2BveF+DgiZqDIPzOh8= -github.com/tbruyelle/mdgofmt v0.1.0/go.mod h1:D3fyKvx4oZq99YeQm5j/gnGmc9w4HogvQMujPVzW+zQ= -github.com/tbruyelle/mdgofmt v0.1.1 h1:ZH/n+jGxWz7ilP9dpeIgSgjPHDRafDdfzPpkILjjjSQ= -github.com/tbruyelle/mdgofmt v0.1.1/go.mod h1:D3fyKvx4oZq99YeQm5j/gnGmc9w4HogvQMujPVzW+zQ= github.com/tbruyelle/mdgofmt v0.1.2 h1:/UZaC5YTKdneMtLhDNe6328DjO2oDFCslmJJ1fqnjCQ= github.com/tbruyelle/mdgofmt v0.1.2/go.mod h1:D3fyKvx4oZq99YeQm5j/gnGmc9w4HogvQMujPVzW+zQ= github.com/tchap/go-patricia v2.2.6+incompatible/go.mod h1:bmLyhP68RS6kStMGxByiQ23RP/odRBOTVjwp2cDyi6I= From f322a8acf2fde17e2a5792ce9106468e2d74ff8b Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 6 Dec 2022 10:03:30 -0500 Subject: [PATCH 10/40] removing print out --- ignite/services/plugin/plugin.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index 9bfef1d0e8..016cd11de4 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -268,7 +268,6 @@ func (p *Plugin) load(ctx context.Context) { // writing it to cache as lost operation within load to assure rpc client's reattach config // is hydrated. if !CheckPluginConf(p.Path) && p.isHost { - fmt.Printf("%v\n", *p.client.ReattachConfig()) WritePluginConfig(p.Path, *p.client.ReattachConfig()) } From cd6e1fc9d45aabee2dbd6a369f74850fc2e06e8d Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sat, 10 Dec 2022 13:21:54 -0500 Subject: [PATCH 11/40] additional comments --- ignite/config/plugins/config.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ignite/config/plugins/config.go b/ignite/config/plugins/config.go index 3744db0e5a..002f95812b 100644 --- a/ignite/config/plugins/config.go +++ b/ignite/config/plugins/config.go @@ -35,8 +35,9 @@ type Plugin struct { // path: github.com/foo/bar/plugin1@v42 Path string `yaml:"path"` // With holds arguments passed to the plugin interface - With map[string]string `yaml:"with"` - SharedHost bool `yaml:"sharedHost"` + With map[string]string `yaml:"with"` + // SharedHost enables sharing a single plugin server across all running instances + SharedHost bool `yaml:"sharedHost"` } // Path return the path of the config file. From 2ce2b446a6d9785036ba1d0684c9794eb7d30472 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sat, 10 Dec 2022 13:56:19 -0500 Subject: [PATCH 12/40] shared host load test --- ignite/services/plugin/plugin_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index 420269b210..ad6434ce2e 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -216,6 +216,16 @@ func TestPluginLoad(t *testing.T) { } }, }, + { + name: "ok: from local sharedhost is on", + buildPlugin: func(t *testing.T) Plugin { + return Plugin{ + Plugin: pluginsconfig.Plugin{SharedHost: true}, + srcPath: scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar"), + binaryName: "bar", + } + }, + }, { name: "ok: from git repo", buildPlugin: func(t *testing.T) Plugin { @@ -340,6 +350,11 @@ func TestPluginLoad(t *testing.T) { require.Regexp(tt.expectedError, p.Error.Error()) return } + + if p.Plugin.SharedHost { + assert.Equal(p.isHost, true) + } + require.NoError(p.Error) require.NotNil(p.Interface) manifest, err := p.Interface.Manifest() From 2244590c697ae802900b303517bd582b25fdfc2a Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sat, 10 Dec 2022 19:44:11 -0500 Subject: [PATCH 13/40] addition of caching tests for plugins --- ignite/services/plugin/cache.go | 1 - ignite/services/plugin/cache_test.go | 36 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 ignite/services/plugin/cache_test.go diff --git a/ignite/services/plugin/cache.go b/ignite/services/plugin/cache.go index 773eac96c2..c9f8cd7c79 100644 --- a/ignite/services/plugin/cache.go +++ b/ignite/services/plugin/cache.go @@ -39,7 +39,6 @@ func WritePluginConfig(pluginPath string, conf hplugin.ReattachConfig) error { return fmt.Errorf("provided path is invalid: %s", pluginPath) } - fmt.Println("encoding config") confCont := ConfigContext{} // TODO: figure out a better way of resolving the type of network connection is established between plugin server and host diff --git a/ignite/services/plugin/cache_test.go b/ignite/services/plugin/cache_test.go new file mode 100644 index 0000000000..670e262fbc --- /dev/null +++ b/ignite/services/plugin/cache_test.go @@ -0,0 +1,36 @@ +package plugin_test + +import ( + "net" + "testing" + + hplugin "github.com/hashicorp/go-plugin" + "github.com/ignite/cli/ignite/services/plugin" + "github.com/stretchr/testify/require" +) + +func TestNewPlugin(t *testing.T) { + const path = "/path/to/awesome/plugin" + unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") + + var rc = hplugin.ReattachConfig{ + Protocol: hplugin.ProtocolNetRPC, + ProtocolVersion: hplugin.CoreProtocolVersion, + Addr: unixFD, + Pid: 24464, + } + + err := plugin.WritePluginConfig(path, rc) + require.NoError(t, err) + + c := hplugin.ReattachConfig{} + err = plugin.ReadPluginConfig(path, &c) + require.NoError(t, err) + + err = plugin.DeletePluginConf(path) + require.NoError(t, err) + + // there should be an error after deleting the config from the cache + err = plugin.ReadPluginConfig(path, &c) + require.Error(t, err) +} From 21e6273fffe9e231d804c602707cb9cee564fa3a Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sat, 10 Dec 2022 19:46:00 -0500 Subject: [PATCH 14/40] fmt --- ignite/services/plugin/cache.go | 6 +----- ignite/services/plugin/cache_test.go | 5 +++-- ignite/services/plugin/plugin.go | 2 -- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/ignite/services/plugin/cache.go b/ignite/services/plugin/cache.go index c9f8cd7c79..66f14587bd 100644 --- a/ignite/services/plugin/cache.go +++ b/ignite/services/plugin/cache.go @@ -8,6 +8,7 @@ import ( "path/filepath" hplugin "github.com/hashicorp/go-plugin" + chainconfig "github.com/ignite/cli/ignite/config" "github.com/ignite/cli/ignite/pkg/cache" ) @@ -44,7 +45,6 @@ func WritePluginConfig(pluginPath string, conf hplugin.ReattachConfig) error { // TODO: figure out a better way of resolving the type of network connection is established between plugin server and host // currently this will always be a unix network socket. but this might not be the case moving forward. ua, err := net.ResolveUnixAddr(conf.Addr.Network(), conf.Addr.String()) - if err != nil { return err } @@ -54,7 +54,6 @@ func WritePluginConfig(pluginPath string, conf hplugin.ReattachConfig) error { confCont.Plugin = conf cache, err := newCache() - if err != nil { return err } @@ -95,7 +94,6 @@ func CheckPluginConf(pluginPath string) bool { } cache, err := newCache() - if err != nil { return false } @@ -112,7 +110,6 @@ func DeletePluginConf(pluginPath string) error { return fmt.Errorf("provided path is invalid: %s", pluginPath) } cache, err := newCache() - if err != nil { return err } @@ -126,7 +123,6 @@ func DeletePluginConf(pluginPath string) error { func newCache() (*cache.Cache[ConfigContext], error) { cacheRootDir, err := chainconfig.DirPath() - if err != nil { return nil, err } diff --git a/ignite/services/plugin/cache_test.go b/ignite/services/plugin/cache_test.go index 670e262fbc..7b9e80eebd 100644 --- a/ignite/services/plugin/cache_test.go +++ b/ignite/services/plugin/cache_test.go @@ -5,15 +5,16 @@ import ( "testing" hplugin "github.com/hashicorp/go-plugin" - "github.com/ignite/cli/ignite/services/plugin" "github.com/stretchr/testify/require" + + "github.com/ignite/cli/ignite/services/plugin" ) func TestNewPlugin(t *testing.T) { const path = "/path/to/awesome/plugin" unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") - var rc = hplugin.ReattachConfig{ + rc := hplugin.ReattachConfig{ Protocol: hplugin.ProtocolNetRPC, ProtocolVersion: hplugin.CoreProtocolVersion, Addr: unixFD, diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index b8b62ed552..07355c1098 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -225,7 +225,6 @@ func (p *Plugin) load(ctx context.Context) { if p.Plugin.SharedHost && CheckPluginConf(p.Path) { rconf := hplugin.ReattachConfig{} err := ReadPluginConfig(p.Path, &rconf) - if err != nil { p.Error = err return @@ -282,7 +281,6 @@ func (p *Plugin) load(ctx context.Context) { if !CheckPluginConf(p.Path) && p.isHost { WritePluginConfig(p.Path, *p.client.ReattachConfig()) } - } // fetch clones the plugin repository at the expected reference. From 15a6eafc2240b54fe9f8ec1121a94ad940833599 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sat, 10 Dec 2022 19:47:15 -0500 Subject: [PATCH 15/40] changelog --- changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.md b/changelog.md index 3e4a54e532..800bc6da6d 100644 --- a/changelog.md +++ b/changelog.md @@ -2,6 +2,7 @@ ### Features +- [#3238](https://github.com/ignite/cli/pull/3238) Add `Sharedhost` plugin option - [#3142](https://github.com/ignite/cli/pull/3142) Add `ignite network request param-change` command. - [#3181](https://github.com/ignite/cli/pull/3181) Addition of `add` `remove` commands for `plugins` - [#3184](https://github.com/ignite/cli/pull/3184) Separate `plugins.yml` config file. From 66bea2bebaf310382e3405d4505d7448200c5f0e Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sun, 11 Dec 2022 12:25:54 -0500 Subject: [PATCH 16/40] additional plugin cache tests --- ignite/services/plugin/cache_test.go | 41 +++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/ignite/services/plugin/cache_test.go b/ignite/services/plugin/cache_test.go index 7b9e80eebd..e39cab2489 100644 --- a/ignite/services/plugin/cache_test.go +++ b/ignite/services/plugin/cache_test.go @@ -10,7 +10,7 @@ import ( "github.com/ignite/cli/ignite/services/plugin" ) -func TestNewPlugin(t *testing.T) { +func TestPluginCacheAdd(t *testing.T) { const path = "/path/to/awesome/plugin" unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") @@ -27,11 +27,50 @@ func TestNewPlugin(t *testing.T) { c := hplugin.ReattachConfig{} err = plugin.ReadPluginConfig(path, &c) require.NoError(t, err) +} + +func TestPluginCacheDelete(t *testing.T) { + const path = "/path/to/awesome/plugin" + unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") + + rc := hplugin.ReattachConfig{ + Protocol: hplugin.ProtocolNetRPC, + ProtocolVersion: hplugin.CoreProtocolVersion, + Addr: unixFD, + Pid: 24464, + } + + err := plugin.WritePluginConfig(path, rc) + require.NoError(t, err) err = plugin.DeletePluginConf(path) require.NoError(t, err) + c := hplugin.ReattachConfig{} // there should be an error after deleting the config from the cache err = plugin.ReadPluginConfig(path, &c) require.Error(t, err) } + +func TestPluginCacheCheck(t *testing.T) { + const path = "/path/to/awesome/plugin" + unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") + + rc := hplugin.ReattachConfig{ + Protocol: hplugin.ProtocolNetRPC, + ProtocolVersion: hplugin.CoreProtocolVersion, + Addr: unixFD, + Pid: 24464, + } + + t.Run("Cache should be hydrated", func(t *testing.T) { + err := plugin.WritePluginConfig(path, rc) + require.NoError(t, err) + require.Equal(t, true, plugin.CheckPluginConf(path)) + }) + + t.Run("Cache should be empty", func(t *testing.T) { + _ = plugin.DeletePluginConf(path) + require.Equal(t, false, plugin.CheckPluginConf(path)) + }) +} From 3b6edabfdd910af9104ec251df6fb0fbc6ab3595 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sun, 11 Dec 2022 13:36:01 -0500 Subject: [PATCH 17/40] update shared host tests --- ignite/services/plugin/cache.go | 2 +- ignite/services/plugin/plugin_test.go | 113 ++++++++++++++++++++++---- 2 files changed, 100 insertions(+), 15 deletions(-) diff --git a/ignite/services/plugin/cache.go b/ignite/services/plugin/cache.go index 66f14587bd..dbeac1eb0c 100644 --- a/ignite/services/plugin/cache.go +++ b/ignite/services/plugin/cache.go @@ -89,7 +89,7 @@ func ReadPluginConfig(pluginPath string, ref *hplugin.ReattachConfig) error { func CheckPluginConf(pluginPath string) bool { name := path.Base(pluginPath) - if name == "" { + if name == "." { return false } diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index ad6434ce2e..a6915ee4ed 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -216,16 +216,6 @@ func TestPluginLoad(t *testing.T) { } }, }, - { - name: "ok: from local sharedhost is on", - buildPlugin: func(t *testing.T) Plugin { - return Plugin{ - Plugin: pluginsconfig.Plugin{SharedHost: true}, - srcPath: scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar"), - binaryName: "bar", - } - }, - }, { name: "ok: from git repo", buildPlugin: func(t *testing.T) Plugin { @@ -351,10 +341,6 @@ func TestPluginLoad(t *testing.T) { return } - if p.Plugin.SharedHost { - assert.Equal(p.isHost, true) - } - require.NoError(p.Error) require.NotNil(p.Interface) manifest, err := p.Interface.Manifest() @@ -368,6 +354,105 @@ func TestPluginLoad(t *testing.T) { } } +func TestPluginLoadSharedHost(t *testing.T) { + // wd, err := os.Getwd() + // require.NoError(t, err) + + // scaffoldPlugin runs Scaffold and updates the go.mod so it uses the + // current ignite/cli sources. + scaffoldPlugin := func(t *testing.T, dir, name string) string { + require := require.New(t) + path, err := Scaffold(dir, name) + require.NoError(err) + // We want the scaffolded plugin to use the current version of ignite/cli, + // for that we need to update the plugin go.mod and add a replace to target + // current ignite/cli + gomod, err := gomodule.ParseAt(path) + require.NoError(err) + // use GOMOD env to get current directory module path + modpath, err := gocmd.Env(gocmd.EnvGOMOD) + require.NoError(err) + modpath = filepath.Dir(modpath) + gomod.AddReplace("github.com/ignite/cli", "", modpath, "") + // Save go.mod + data, err := gomod.Format() + require.NoError(err) + err = os.WriteFile(filepath.Join(path, "go.mod"), data, 0o644) + require.NoError(err) + return path + } + + tests := []struct { + name string + buildPlugin func(t *testing.T) Plugin + expectedError string + instances int + }{ + { + name: "ok: from local sharedhost is on", + buildPlugin: func(t *testing.T) Plugin { + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar") + return Plugin{ + Plugin: pluginsconfig.Plugin{SharedHost: true, Path: path}, + srcPath: path, + binaryName: "bar", + } + }, + instances: 1, + }, + { + name: "ok: from local sharedhost is on", + buildPlugin: func(t *testing.T) Plugin { + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar") + return Plugin{ + Plugin: pluginsconfig.Plugin{SharedHost: true, Path: path}, + srcPath: path, + binaryName: "bar", + } + }, + instances: 2, + }, + { + name: "ok: from local sharedhost is on", + buildPlugin: func(t *testing.T) Plugin { + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar") + return Plugin{ + Plugin: pluginsconfig.Plugin{SharedHost: true, Path: path}, + srcPath: path, + binaryName: "bar", + } + }, + instances: 4, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + // assert := assert.New(t) + var plugins []*Plugin + for i := 0; i < tt.instances; i++ { + p := tt.buildPlugin(t) + p.load(context.Background()) + plugins = append(plugins, &p) + } + + require.Equal(true, CheckPluginConf(plugins[0].Path)) + + for i := len(plugins) - 1; i >= 0; i-- { + plugins[i].KillClient() + if i != 0 { + require.Equal(true, plugins[0].isHost) + rconf := plugins[i].client.ReattachConfig() + require.Equal(rconf.Addr.String(), plugins[i].client.ReattachConfig().Addr.String()) + } else { + require.Equal(false, plugins[0].isHost) + } + } + }) + } +} + func TestPluginClean(t *testing.T) { tests := []struct { name string From 250d15eb26280408119a82f782e0446fadc96aa2 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Mon, 12 Dec 2022 16:15:13 -0500 Subject: [PATCH 18/40] review comments --- ignite/config/plugins/config.go | 12 ++++++++++-- ignite/services/plugin/cache.go | 8 ++++---- ignite/services/plugin/cache_test.go | 18 +++++++++--------- ignite/services/plugin/plugin.go | 16 ++++++++++------ ignite/services/plugin/plugin_test.go | 2 +- 5 files changed, 34 insertions(+), 22 deletions(-) diff --git a/ignite/config/plugins/config.go b/ignite/config/plugins/config.go index 002f95812b..4a3b32ce99 100644 --- a/ignite/config/plugins/config.go +++ b/ignite/config/plugins/config.go @@ -35,9 +35,17 @@ type Plugin struct { // path: github.com/foo/bar/plugin1@v42 Path string `yaml:"path"` // With holds arguments passed to the plugin interface - With map[string]string `yaml:"with"` + With map[string]string `yaml:"with,omitempty"` // SharedHost enables sharing a single plugin server across all running instances - SharedHost bool `yaml:"sharedHost"` + // of a plugin. Useful if a plugin adds or extends long running commands + // + // example: `ignite chain serve` + // + // When enabled, all plugins of the same `Path` loaded from the same configuration will + // attach it's rpc client to a an existing rpc server. + // + // If a plugin instance has no other running plugin servers, it will create one and it will be the host. + SharedHost bool `yaml:"shared_host"` } // Path return the path of the config file. diff --git a/ignite/services/plugin/cache.go b/ignite/services/plugin/cache.go index dbeac1eb0c..23264924f1 100644 --- a/ignite/services/plugin/cache.go +++ b/ignite/services/plugin/cache.go @@ -33,7 +33,7 @@ type ConfigContext struct { Addr net.UnixAddr } -func WritePluginConfig(pluginPath string, conf hplugin.ReattachConfig) error { +func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) error { name := path.Base(pluginPath) if name == "" { @@ -63,7 +63,7 @@ func WritePluginConfig(pluginPath string, conf hplugin.ReattachConfig) error { return err } -func ReadPluginConfig(pluginPath string, ref *hplugin.ReattachConfig) error { +func ReadPluginConfigCache(pluginPath string, ref *hplugin.ReattachConfig) error { name := path.Base(pluginPath) if name == "" { @@ -86,7 +86,7 @@ func ReadPluginConfig(pluginPath string, ref *hplugin.ReattachConfig) error { return nil } -func CheckPluginConf(pluginPath string) bool { +func CheckPluginConfCache(pluginPath string) bool { name := path.Base(pluginPath) if name == "." { @@ -103,7 +103,7 @@ func CheckPluginConf(pluginPath string) bool { return true } -func DeletePluginConf(pluginPath string) error { +func DeletePluginConfCache(pluginPath string) error { name := path.Base(pluginPath) if name == "" { diff --git a/ignite/services/plugin/cache_test.go b/ignite/services/plugin/cache_test.go index e39cab2489..325705b19e 100644 --- a/ignite/services/plugin/cache_test.go +++ b/ignite/services/plugin/cache_test.go @@ -21,11 +21,11 @@ func TestPluginCacheAdd(t *testing.T) { Pid: 24464, } - err := plugin.WritePluginConfig(path, rc) + err := plugin.WritePluginConfigCache(path, rc) require.NoError(t, err) c := hplugin.ReattachConfig{} - err = plugin.ReadPluginConfig(path, &c) + err = plugin.ReadPluginConfigCache(path, &c) require.NoError(t, err) } @@ -40,15 +40,15 @@ func TestPluginCacheDelete(t *testing.T) { Pid: 24464, } - err := plugin.WritePluginConfig(path, rc) + err := plugin.WritePluginConfigCache(path, rc) require.NoError(t, err) - err = plugin.DeletePluginConf(path) + err = plugin.DeletePluginConfCache(path) require.NoError(t, err) c := hplugin.ReattachConfig{} // there should be an error after deleting the config from the cache - err = plugin.ReadPluginConfig(path, &c) + err = plugin.ReadPluginConfigCache(path, &c) require.Error(t, err) } @@ -64,13 +64,13 @@ func TestPluginCacheCheck(t *testing.T) { } t.Run("Cache should be hydrated", func(t *testing.T) { - err := plugin.WritePluginConfig(path, rc) + err := plugin.WritePluginConfigCache(path, rc) require.NoError(t, err) - require.Equal(t, true, plugin.CheckPluginConf(path)) + require.Equal(t, true, plugin.CheckPluginConfCache(path)) }) t.Run("Cache should be empty", func(t *testing.T) { - _ = plugin.DeletePluginConf(path) - require.Equal(t, false, plugin.CheckPluginConf(path)) + _ = plugin.DeletePluginConfCache(path) + require.Equal(t, false, plugin.CheckPluginConfCache(path)) }) } diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index 07355c1098..af1c38efb2 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -163,11 +163,11 @@ func (p *Plugin) KillClient() { if p.client != nil { p.client.Kill() } - DeletePluginConf(p.Path) + DeletePluginConfCache(p.Path) p.isHost = false // if the sharedHost config parameter is set to false we shutdown normally. } else if !p.Plugin.SharedHost { - DeletePluginConf(p.Path) + DeletePluginConfCache(p.Path) } } @@ -222,9 +222,9 @@ func (p *Plugin) load(ctx context.Context) { Output: os.Stderr, Level: logLevel, }) - if p.Plugin.SharedHost && CheckPluginConf(p.Path) { + if p.Plugin.SharedHost && CheckPluginConfCache(p.Path) { rconf := hplugin.ReattachConfig{} - err := ReadPluginConfig(p.Path, &rconf) + err := ReadPluginConfigCache(p.Path, &rconf) if err != nil { p.Error = err return @@ -278,8 +278,12 @@ func (p *Plugin) load(ctx context.Context) { // write the rpc context to cache if the plugin is declared as host. // writing it to cache as lost operation within load to assure rpc client's reattach config // is hydrated. - if !CheckPluginConf(p.Path) && p.isHost { - WritePluginConfig(p.Path, *p.client.ReattachConfig()) + if !CheckPluginConfCache(p.Path) && p.isHost { + err := WritePluginConfigCache(p.Path, *p.client.ReattachConfig()) + if err != nil { + p.Error = err + return + } } } diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index a6915ee4ed..d6ea684218 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -437,7 +437,7 @@ func TestPluginLoadSharedHost(t *testing.T) { plugins = append(plugins, &p) } - require.Equal(true, CheckPluginConf(plugins[0].Path)) + require.Equal(true, CheckPluginConfCache(plugins[0].Path)) for i := len(plugins) - 1; i >= 0; i-- { plugins[i].KillClient() From d20ef29b60294f860393040d25968968ce23183c Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 13 Dec 2022 11:09:42 -0500 Subject: [PATCH 19/40] addition cache test cases --- ignite/services/plugin/cache.go | 11 ++- ignite/services/plugin/cache_test.go | 108 ++++++++++++++++++--------- 2 files changed, 78 insertions(+), 41 deletions(-) diff --git a/ignite/services/plugin/cache.go b/ignite/services/plugin/cache.go index 23264924f1..a319658b23 100644 --- a/ignite/services/plugin/cache.go +++ b/ignite/services/plugin/cache.go @@ -36,12 +36,15 @@ type ConfigContext struct { func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) error { name := path.Base(pluginPath) - if name == "" { + if name == "." { return fmt.Errorf("provided path is invalid: %s", pluginPath) } - confCont := ConfigContext{} + if conf.Addr == nil { + return fmt.Errorf("plugin Address info cannot be empty") + } + confCont := ConfigContext{} // TODO: figure out a better way of resolving the type of network connection is established between plugin server and host // currently this will always be a unix network socket. but this might not be the case moving forward. ua, err := net.ResolveUnixAddr(conf.Addr.Network(), conf.Addr.String()) @@ -66,7 +69,7 @@ func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) erro func ReadPluginConfigCache(pluginPath string, ref *hplugin.ReattachConfig) error { name := path.Base(pluginPath) - if name == "" { + if name == "." { return fmt.Errorf("provided path is invalid: %s", pluginPath) } @@ -106,7 +109,7 @@ func CheckPluginConfCache(pluginPath string) bool { func DeletePluginConfCache(pluginPath string) error { name := path.Base(pluginPath) - if name == "" { + if name == "." { return fmt.Errorf("provided path is invalid: %s", pluginPath) } cache, err := newCache() diff --git a/ignite/services/plugin/cache_test.go b/ignite/services/plugin/cache_test.go index 325705b19e..43bc084d7f 100644 --- a/ignite/services/plugin/cache_test.go +++ b/ignite/services/plugin/cache_test.go @@ -1,4 +1,4 @@ -package plugin_test +package plugin import ( "net" @@ -6,50 +6,84 @@ import ( hplugin "github.com/hashicorp/go-plugin" "github.com/stretchr/testify/require" - - "github.com/ignite/cli/ignite/services/plugin" ) func TestPluginCacheAdd(t *testing.T) { - const path = "/path/to/awesome/plugin" - unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") + t.Run("Should cache plugin config and read from cache", func(t *testing.T) { + const path = "/path/to/awesome/plugin" + unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") + + rc := hplugin.ReattachConfig{ + Protocol: hplugin.ProtocolNetRPC, + ProtocolVersion: hplugin.CoreProtocolVersion, + Addr: unixFD, + Pid: 24464, + } + + err := WritePluginConfigCache(path, rc) + require.NoError(t, err) - rc := hplugin.ReattachConfig{ - Protocol: hplugin.ProtocolNetRPC, - ProtocolVersion: hplugin.CoreProtocolVersion, - Addr: unixFD, - Pid: 24464, - } + c := hplugin.ReattachConfig{} + err = ReadPluginConfigCache(path, &c) + require.NoError(t, err) + }) - err := plugin.WritePluginConfigCache(path, rc) - require.NoError(t, err) + t.Run("Should error writing bad plugin config to cache", func(t *testing.T) { + const path = "/path/to/awesome/plugin" + rc := hplugin.ReattachConfig{ + Protocol: hplugin.ProtocolNetRPC, + ProtocolVersion: hplugin.CoreProtocolVersion, + Addr: nil, + Pid: 24464, + } + + err := WritePluginConfigCache(path, rc) + require.Error(t, err) + }) - c := hplugin.ReattachConfig{} - err = plugin.ReadPluginConfigCache(path, &c) - require.NoError(t, err) + t.Run("Should error with invalid plugin path", func(t *testing.T) { + const path = "" + rc := hplugin.ReattachConfig{ + Protocol: hplugin.ProtocolNetRPC, + ProtocolVersion: hplugin.CoreProtocolVersion, + Addr: nil, + Pid: 24464, + } + + err := WritePluginConfigCache(path, rc) + require.Error(t, err) + }) } func TestPluginCacheDelete(t *testing.T) { - const path = "/path/to/awesome/plugin" - unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") - - rc := hplugin.ReattachConfig{ - Protocol: hplugin.ProtocolNetRPC, - ProtocolVersion: hplugin.CoreProtocolVersion, - Addr: unixFD, - Pid: 24464, - } + t.Run("Delete plugin config after write to cache should remove from cache", func(t *testing.T) { + const path = "/path/to/awesome/plugin" + unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") + + rc := hplugin.ReattachConfig{ + Protocol: hplugin.ProtocolNetRPC, + ProtocolVersion: hplugin.CoreProtocolVersion, + Addr: unixFD, + Pid: 24464, + } + + err := WritePluginConfigCache(path, rc) + require.NoError(t, err) - err := plugin.WritePluginConfigCache(path, rc) - require.NoError(t, err) + err = DeletePluginConfCache(path) + require.NoError(t, err) - err = plugin.DeletePluginConfCache(path) - require.NoError(t, err) + c := hplugin.ReattachConfig{} + // there should be an error after deleting the config from the cache + err = ReadPluginConfigCache(path, &c) + require.Error(t, err) + }) - c := hplugin.ReattachConfig{} - // there should be an error after deleting the config from the cache - err = plugin.ReadPluginConfigCache(path, &c) - require.Error(t, err) + t.Run("Delete plugin config should return error given empty path", func(t *testing.T) { + const path = "" + err := DeletePluginConfCache(path) + require.Error(t, err) + }) } func TestPluginCacheCheck(t *testing.T) { @@ -64,13 +98,13 @@ func TestPluginCacheCheck(t *testing.T) { } t.Run("Cache should be hydrated", func(t *testing.T) { - err := plugin.WritePluginConfigCache(path, rc) + err := WritePluginConfigCache(path, rc) require.NoError(t, err) - require.Equal(t, true, plugin.CheckPluginConfCache(path)) + require.Equal(t, true, CheckPluginConfCache(path)) }) t.Run("Cache should be empty", func(t *testing.T) { - _ = plugin.DeletePluginConfCache(path) - require.Equal(t, false, plugin.CheckPluginConfCache(path)) + _ = DeletePluginConfCache(path) + require.Equal(t, false, CheckPluginConfCache(path)) }) } From c9e7a2f7a2af823abfd19e6a963f8c8255b69712 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 13 Dec 2022 11:16:52 -0500 Subject: [PATCH 20/40] fix typo --- ignite/services/plugin/plugin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index af1c38efb2..b2937a91aa 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -54,7 +54,7 @@ type Plugin struct { client *hplugin.Client - // if a plugin's ShareHost flag is set to true, isHost is used to decern if a + // If a plugin's ShareHost flag is set to true, isHost is used to discern if a // plugin instance is controlling the rpc server. isHost bool } From e7529048ec59acfad49ee598571e3fcf24938928 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 13 Dec 2022 12:06:40 -0500 Subject: [PATCH 21/40] fix comment --- ignite/services/plugin/plugin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index b2937a91aa..26296ff08a 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -230,7 +230,7 @@ func (p *Plugin) load(ctx context.Context) { return } - // We're a host! Start by launching the plugin process. + // We're attaching to an existing server, supply attachment configuration p.client = hplugin.NewClient(&hplugin.ClientConfig{ HandshakeConfig: handshakeConfig, Plugins: pluginMap, From fe8166e92ded65567b3146d9a486dcf25ba34c43 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Wed, 14 Dec 2022 14:53:01 -0500 Subject: [PATCH 22/40] update plugin `KillClient` for shared hosts --- ignite/services/plugin/plugin.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index a463e699c0..ec47daf4e1 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -174,15 +174,17 @@ func RemoveDuplicates(plugins []pluginsconfig.Plugin) (unique []pluginsconfig.Pl // KillClient kills the running plugin client. func (p *Plugin) KillClient() { + if p.Plugin.SharedHost && !p.isHost { + // Don't send kill signal to a shared-host plugin when this process isn't + // the one who initiated it. + return + } + if p.client != nil { + p.client.Kill() + } if p.isHost { - if p.client != nil { - p.client.Kill() - } DeletePluginConfCache(p.Path) p.isHost = false - // if the sharedHost config parameter is set to false we shutdown normally. - } else if !p.Plugin.SharedHost { - DeletePluginConfCache(p.Path) } } From 2e8cd9ba4998db8b61c38b485f70b80fc36d98da Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Sat, 17 Dec 2022 14:30:07 -0500 Subject: [PATCH 23/40] changelog update --- changelog.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/changelog.md b/changelog.md index b642864808..b3c3bcfded 100644 --- a/changelog.md +++ b/changelog.md @@ -23,6 +23,7 @@ ### Changes +- [#3305](https://github.com/ignite/cli/pull/3305) Bump Cosmos SDK version to `v0.46.7`. - [#3068](https://github.com/ignite/cli/pull/3068) Add configs to generated TS code for working with JS projects - [#3071](https://github.com/ignite/cli/pull/3071) Refactor `ignite/templates` package. - [#2892](https://github.com/ignite/cli/pull/2982/) `ignite scaffold vue` and `ignite scaffold react` use v0.4.2 templates @@ -332,7 +333,7 @@ Our new name is **Ignite CLI**! ### Breaking Changes - Deprecated the Starport Modules [tendermint/spm](https://github.com/tendermint/spm) repo and moved the contents to the - Ignite CLI repo [`ignite/pkg/`](https://github.com/ignite/cli/tree/develop/ignite/pkg/) + Ignite CLI repo [`ignite/pkg/`](https://github.com/ignite/cli/tree/main/ignite/pkg/) in [PR 1971](https://github.com/ignite/cli/pull/1971/files) Updates are required if your chain uses these packages: From efd2fb40ec8f11a8cf188aaeab036fe2bf09e329 Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Sun, 18 Dec 2022 11:21:37 +0100 Subject: [PATCH 24/40] fix test by omitting SharedHost from the yml when its false --- ignite/config/plugins/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ignite/config/plugins/config.go b/ignite/config/plugins/config.go index 00ebe6870b..5cb6ef6909 100644 --- a/ignite/config/plugins/config.go +++ b/ignite/config/plugins/config.go @@ -46,7 +46,7 @@ type Plugin struct { // attach it's rpc client to a an existing rpc server. // // If a plugin instance has no other running plugin servers, it will create one and it will be the host. - SharedHost bool `yaml:"shared_host"` + SharedHost bool `yaml:"shared_host,omitempty"` // Global holds whether the plugin is installed globally // (default: $HOME/.ignite/plugins/plugins.yml) or locally for a chain. Global bool `yaml:"-"` From f9d3d25456cb058038f9af545ee096383a9f9eeb Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 20 Dec 2022 09:33:26 -0500 Subject: [PATCH 25/40] update: migration of `sharedHost` flag to plugin manifest --- ignite/config/plugins/config.go | 10 ---------- ignite/services/plugin/interface.go | 10 ++++++++++ ignite/services/plugin/plugin.go | 30 ++++++++++++++++++++--------- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/ignite/config/plugins/config.go b/ignite/config/plugins/config.go index 00ebe6870b..e8fab5168b 100644 --- a/ignite/config/plugins/config.go +++ b/ignite/config/plugins/config.go @@ -37,16 +37,6 @@ type Plugin struct { Path string `yaml:"path"` // With holds arguments passed to the plugin interface With map[string]string `yaml:"with,omitempty"` - // SharedHost enables sharing a single plugin server across all running instances - // of a plugin. Useful if a plugin adds or extends long running commands - // - // example: `ignite chain serve` - // - // When enabled, all plugins of the same `Path` loaded from the same configuration will - // attach it's rpc client to a an existing rpc server. - // - // If a plugin instance has no other running plugin servers, it will create one and it will be the host. - SharedHost bool `yaml:"shared_host"` // Global holds whether the plugin is installed globally // (default: $HOME/.ignite/plugins/plugins.yml) or locally for a chain. Global bool `yaml:"-"` diff --git a/ignite/services/plugin/interface.go b/ignite/services/plugin/interface.go index f5320b6101..f1807259aa 100644 --- a/ignite/services/plugin/interface.go +++ b/ignite/services/plugin/interface.go @@ -59,6 +59,16 @@ type Manifest struct { // Hooks contains the hooks that will be attached to the existing ignite // commands. Hooks []Hook + // SharedHost enables sharing a single plugin server across all running instances + // of a plugin. Useful if a plugin adds or extends long running commands + // + // example: `ignite chain serve` + // + // When enabled, all plugins of the same `Path` loaded from the same configuration will + // attach it's rpc client to a an existing rpc server. + // + // If a plugin instance has no other running plugin servers, it will create one and it will be the host. + SharedHost bool `yaml:"shared_host"` } // Command represents a plugin command. diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index 7b68d08879..1cc86d63bf 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -54,6 +54,8 @@ type Plugin struct { client *hplugin.Client + //manifest holds a cache of the plugin manifest to prevent mant calls over the rpc boundary + manifest Manifest // If a plugin's ShareHost flag is set to true, isHost is used to discern if a // plugin instance is controlling the rpc server. isHost bool @@ -168,14 +170,16 @@ func newPlugin(pluginsDir string, cp pluginsconfig.Plugin, options ...Option) *P // KillClient kills the running plugin client. func (p *Plugin) KillClient() { - if p.Plugin.SharedHost && !p.isHost { + if p.manifest.SharedHost && !p.isHost { // Don't send kill signal to a shared-host plugin when this process isn't // the one who initiated it. return } + if p.client != nil { p.client.Kill() } + if p.isHost { DeletePluginConfCache(p.Path) p.isHost = false @@ -208,6 +212,7 @@ func (p *Plugin) load(ctx context.Context) { return } } + if p.isLocal() { // trigger rebuild for local plugin if binary is outdated if p.outdatedBinary() { @@ -238,7 +243,8 @@ func (p *Plugin) load(ctx context.Context) { Output: os.Stderr, Level: logLevel, }) - if p.Plugin.SharedHost && CheckPluginConfCache(p.Path) { + + if CheckPluginConfCache(p.Path) { rconf := hplugin.ReattachConfig{} err := ReadPluginConfigCache(p.Path, &rconf) if err != nil { @@ -266,14 +272,9 @@ func (p *Plugin) load(ctx context.Context) { SyncStderr: os.Stderr, SyncStdout: os.Stdout, }) - - if p.Plugin.SharedHost { - p.isHost = true - } - } - // Connect via RPC + // :Connect via RPC rpcClient, err := p.client.Client() if err != nil { p.Error = errors.Wrapf(err, "connecting") @@ -291,15 +292,26 @@ func (p *Plugin) load(ctx context.Context) { // implementation but is in fact over an RPC connection. p.Interface = raw.(Interface) + m, err := p.Interface.Manifest() + + if err != nil { + p.Error = errors.Wrapf(err, "manifest load") + } + + p.manifest = m + // write the rpc context to cache if the plugin is declared as host. // writing it to cache as lost operation within load to assure rpc client's reattach config // is hydrated. - if !CheckPluginConfCache(p.Path) && p.isHost { + if m.SharedHost && !CheckPluginConfCache(p.Path) { err := WritePluginConfigCache(p.Path, *p.client.ReattachConfig()) if err != nil { p.Error = err return } + + // set the plugin's rpc server as host so other plugin clients may share + p.isHost = true } } From 651cc3b240fa83f8c2508816470a0aaf686c19be Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 20 Dec 2022 09:33:59 -0500 Subject: [PATCH 26/40] scaffolding and plugin host check changes --- ignite/cmd/plugin.go | 2 +- ignite/services/plugin/scaffold.go | 4 +++- ignite/services/plugin/scaffold_test.go | 2 +- ignite/services/plugin/template/main.go.plush | 1 + 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ignite/cmd/plugin.go b/ignite/cmd/plugin.go index 7ecb3beb02..f50882e090 100644 --- a/ignite/cmd/plugin.go +++ b/ignite/cmd/plugin.go @@ -561,7 +561,7 @@ func NewPluginScaffold() *cobra.Command { return err } moduleName := args[0] - path, err := plugin.Scaffold(wd, moduleName) + path, err := plugin.Scaffold(wd, moduleName, false) if err != nil { return err } diff --git a/ignite/services/plugin/scaffold.go b/ignite/services/plugin/scaffold.go index 8af951dafb..0767c62db7 100644 --- a/ignite/services/plugin/scaffold.go +++ b/ignite/services/plugin/scaffold.go @@ -21,7 +21,7 @@ import ( var fsPluginSource embed.FS // Scaffold generates a plugin structure under dir/path.Base(moduleName). -func Scaffold(dir, moduleName string) (string, error) { +func Scaffold(dir, moduleName string, sharedHost bool) (string, error) { var ( name = filepath.Base(moduleName) finalDir = path.Join(dir, name) @@ -42,6 +42,8 @@ func Scaffold(dir, moduleName string) (string, error) { ctx := plush.NewContext() ctx.Set("ModuleName", moduleName) ctx.Set("Name", name) + ctx.Set("SharedHost", sharedHost) + g.Transformer(xgenny.Transformer(ctx)) r := genny.WetRunner(ctx) err := r.With(g) diff --git a/ignite/services/plugin/scaffold_test.go b/ignite/services/plugin/scaffold_test.go index b7a6668c14..8d7f122df3 100644 --- a/ignite/services/plugin/scaffold_test.go +++ b/ignite/services/plugin/scaffold_test.go @@ -9,7 +9,7 @@ import ( func TestScaffold(t *testing.T) { tmp := t.TempDir() - path, err := Scaffold(tmp, "github.com/foo/bar") + path, err := Scaffold(tmp, "github.com/foo/bar", false) require.NoError(t, err) require.DirExists(t, path) diff --git a/ignite/services/plugin/template/main.go.plush b/ignite/services/plugin/template/main.go.plush index 7f14fddd90..dbdbcd3e63 100644 --- a/ignite/services/plugin/template/main.go.plush +++ b/ignite/services/plugin/template/main.go.plush @@ -45,6 +45,7 @@ func (p) Manifest() (plugin.Manifest, error) { }, // Add hooks here Hooks: []plugin.Hook{}, + SharedHost: <%= SharedHost %>, }, nil } From 0ae8ce22a3179496e9019038d8109400770ca4f8 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 20 Dec 2022 09:34:12 -0500 Subject: [PATCH 27/40] update tests --- ignite/services/plugin/plugin_test.go | 39 ++++++++++++++++----------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index d6ea684218..6ec0b1bf78 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -140,7 +140,7 @@ func TestPluginLoad(t *testing.T) { // current ignite/cli sources. scaffoldPlugin := func(t *testing.T, dir, name string) string { require := require.New(t) - path, err := Scaffold(dir, name) + path, err := Scaffold(dir, name, false) require.NoError(err) // We want the scaffolded plugin to use the current version of ignite/cli, // for that we need to update the plugin go.mod and add a replace to target @@ -210,8 +210,10 @@ func TestPluginLoad(t *testing.T) { { name: "ok: from local", buildPlugin: func(t *testing.T) Plugin { + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar") return Plugin{ - srcPath: scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar"), + Plugin: pluginsconfig.Plugin{Path: path}, + srcPath: path, binaryName: "bar", } }, @@ -223,6 +225,7 @@ func TestPluginLoad(t *testing.T) { cloneDir := t.TempDir() return Plugin{ + Plugin: pluginsconfig.Plugin{Path: path.Join(cloneDir, "remote")}, cloneURL: repoDir, cloneDir: cloneDir, srcPath: path.Join(cloneDir, "remote"), @@ -236,6 +239,7 @@ func TestPluginLoad(t *testing.T) { cloneDir := t.TempDir() return Plugin{ + Plugin: pluginsconfig.Plugin{Path: path.Join(cloneDir, "plugin")}, repoPath: "/xxxx/yyyy", cloneURL: "/xxxx/yyyy", cloneDir: cloneDir, @@ -259,6 +263,7 @@ func TestPluginLoad(t *testing.T) { cloneDir := t.TempDir() return Plugin{ + Plugin: pluginsconfig.Plugin{Path: path.Join(cloneDir, "remote-tag")}, cloneURL: repoDir, reference: "v1", cloneDir: cloneDir, @@ -282,6 +287,7 @@ func TestPluginLoad(t *testing.T) { cloneDir := t.TempDir() return Plugin{ + Plugin: pluginsconfig.Plugin{Path: path.Join(cloneDir, "remote-branch")}, cloneURL: repoDir, reference: "branch1", cloneDir: cloneDir, @@ -300,6 +306,7 @@ func TestPluginLoad(t *testing.T) { cloneDir := t.TempDir() return Plugin{ + Plugin: pluginsconfig.Plugin{Path: path.Join(cloneDir, "remote-branch")}, cloneURL: repoDir, reference: h.Hash().String(), cloneDir: cloneDir, @@ -316,6 +323,7 @@ func TestPluginLoad(t *testing.T) { cloneDir := t.TempDir() return Plugin{ + Plugin: pluginsconfig.Plugin{Path: path.Join(cloneDir, "remote-no-ref")}, cloneURL: repoDir, reference: "doesnt_exists", cloneDir: cloneDir, @@ -362,7 +370,7 @@ func TestPluginLoadSharedHost(t *testing.T) { // current ignite/cli sources. scaffoldPlugin := func(t *testing.T, dir, name string) string { require := require.New(t) - path, err := Scaffold(dir, name) + path, err := Scaffold(dir, name, true) require.NoError(err) // We want the scaffolded plugin to use the current version of ignite/cli, // for that we need to update the plugin go.mod and add a replace to target @@ -389,37 +397,37 @@ func TestPluginLoadSharedHost(t *testing.T) { instances int }{ { - name: "ok: from local sharedhost is on", + name: "ok: from local sharedhost is on 1", buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar") + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-1") return Plugin{ - Plugin: pluginsconfig.Plugin{SharedHost: true, Path: path}, + Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, - binaryName: "bar", + binaryName: "bar-1", } }, instances: 1, }, { - name: "ok: from local sharedhost is on", + name: "ok: from local sharedhost is on 2", buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar") + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-2") return Plugin{ - Plugin: pluginsconfig.Plugin{SharedHost: true, Path: path}, + Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, - binaryName: "bar", + binaryName: "bar-2", } }, instances: 2, }, { - name: "ok: from local sharedhost is on", + name: "ok: from local sharedhost is on 4", buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar") + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4") return Plugin{ - Plugin: pluginsconfig.Plugin{SharedHost: true, Path: path}, + Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, - binaryName: "bar", + binaryName: "bar-4", } }, instances: 4, @@ -429,7 +437,6 @@ func TestPluginLoadSharedHost(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { require := require.New(t) - // assert := assert.New(t) var plugins []*Plugin for i := 0; i < tt.instances; i++ { p := tt.buildPlugin(t) From 09f834bd5c6b947ac5c34b9df82ebb2be2a86a72 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Tue, 20 Dec 2022 09:39:58 -0500 Subject: [PATCH 28/40] format and lint --- ignite/services/plugin/plugin.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index 1cc86d63bf..f90b661fff 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -54,7 +54,7 @@ type Plugin struct { client *hplugin.Client - //manifest holds a cache of the plugin manifest to prevent mant calls over the rpc boundary + // holds a cache of the plugin manifest to prevent mant calls over the rpc boundary manifest Manifest // If a plugin's ShareHost flag is set to true, isHost is used to discern if a // plugin instance is controlling the rpc server. @@ -293,7 +293,6 @@ func (p *Plugin) load(ctx context.Context) { p.Interface = raw.(Interface) m, err := p.Interface.Manifest() - if err != nil { p.Error = errors.Wrapf(err, "manifest load") } From 930580d4d0fca3f29bdcd928eb76d3323e196756 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Wed, 21 Dec 2022 13:58:19 -0500 Subject: [PATCH 29/40] update plugin cache test --- ignite/services/plugin/cache_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ignite/services/plugin/cache_test.go b/ignite/services/plugin/cache_test.go index 43bc084d7f..998e9d9b89 100644 --- a/ignite/services/plugin/cache_test.go +++ b/ignite/services/plugin/cache_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestPluginCacheAdd(t *testing.T) { +func TestReadWritePluginConfigCache(t *testing.T) { t.Run("Should cache plugin config and read from cache", func(t *testing.T) { const path = "/path/to/awesome/plugin" unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") @@ -26,6 +26,7 @@ func TestPluginCacheAdd(t *testing.T) { c := hplugin.ReattachConfig{} err = ReadPluginConfigCache(path, &c) require.NoError(t, err) + require.Equal(t, rc, c) }) t.Run("Should error writing bad plugin config to cache", func(t *testing.T) { From 9f92e7b74b346ee3621232368d5a3ca1d118f705 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Wed, 21 Dec 2022 14:41:21 -0500 Subject: [PATCH 30/40] update property comment --- ignite/services/plugin/interface.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ignite/services/plugin/interface.go b/ignite/services/plugin/interface.go index f1807259aa..e47c357fa4 100644 --- a/ignite/services/plugin/interface.go +++ b/ignite/services/plugin/interface.go @@ -62,7 +62,10 @@ type Manifest struct { // SharedHost enables sharing a single plugin server across all running instances // of a plugin. Useful if a plugin adds or extends long running commands // - // example: `ignite chain serve` + // Example: if a plugin defines a hook on `ignite chain serve`, a plugin server is instanciated + // when the command is run. Now if you want to interact with that instance from commands + // defined in that plugin, you need to enable `SharedHost`, or else the commands will just + // instantiate separate plugin servers. // // When enabled, all plugins of the same `Path` loaded from the same configuration will // attach it's rpc client to a an existing rpc server. From 3d245581fc0d5b40b5145edf67bfc1d7c221e4c5 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Wed, 21 Dec 2022 15:02:23 -0500 Subject: [PATCH 31/40] update plugin cache to use full plugin path --- ignite/services/plugin/cache.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/ignite/services/plugin/cache.go b/ignite/services/plugin/cache.go index a319658b23..14935a6a8f 100644 --- a/ignite/services/plugin/cache.go +++ b/ignite/services/plugin/cache.go @@ -4,7 +4,6 @@ import ( "encoding/gob" "fmt" "net" - "path" "path/filepath" hplugin "github.com/hashicorp/go-plugin" @@ -34,9 +33,8 @@ type ConfigContext struct { } func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) error { - name := path.Base(pluginPath) - if name == "." { + if pluginPath == "" { return fmt.Errorf("provided path is invalid: %s", pluginPath) } @@ -61,15 +59,14 @@ func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) erro return err } - cache.Put(name, confCont) + cache.Put(pluginPath, confCont) return err } func ReadPluginConfigCache(pluginPath string, ref *hplugin.ReattachConfig) error { - name := path.Base(pluginPath) - if name == "." { + if pluginPath == "" { return fmt.Errorf("provided path is invalid: %s", pluginPath) } @@ -78,7 +75,7 @@ func ReadPluginConfigCache(pluginPath string, ref *hplugin.ReattachConfig) error return err } - confCont, err := cache.Get(name) + confCont, err := cache.Get(pluginPath) if err != nil { return err } @@ -90,9 +87,7 @@ func ReadPluginConfigCache(pluginPath string, ref *hplugin.ReattachConfig) error } func CheckPluginConfCache(pluginPath string) bool { - name := path.Base(pluginPath) - - if name == "." { + if pluginPath == "" { return false } @@ -100,16 +95,14 @@ func CheckPluginConfCache(pluginPath string) bool { if err != nil { return false } - if _, err := cache.Get(name); err != nil { + if _, err := cache.Get(pluginPath); err != nil { return false } return true } func DeletePluginConfCache(pluginPath string) error { - name := path.Base(pluginPath) - - if name == "." { + if pluginPath == "" { return fmt.Errorf("provided path is invalid: %s", pluginPath) } cache, err := newCache() @@ -117,7 +110,7 @@ func DeletePluginConfCache(pluginPath string) error { return err } - if err := cache.Delete(name); err != nil { + if err := cache.Delete(pluginPath); err != nil { return err } From a7b20a5050b8508a005e48623ef1dd8003337419 Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Wed, 21 Dec 2022 15:02:44 -0500 Subject: [PATCH 32/40] cleanup of plugin tests --- ignite/services/plugin/plugin_test.go | 129 +++++++++++++------------- 1 file changed, 63 insertions(+), 66 deletions(-) diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index 6ec0b1bf78..c402d82d88 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -136,36 +136,12 @@ func TestPluginLoad(t *testing.T) { wd, err := os.Getwd() require.NoError(t, err) - // scaffoldPlugin runs Scaffold and updates the go.mod so it uses the - // current ignite/cli sources. - scaffoldPlugin := func(t *testing.T, dir, name string) string { - require := require.New(t) - path, err := Scaffold(dir, name, false) - require.NoError(err) - // We want the scaffolded plugin to use the current version of ignite/cli, - // for that we need to update the plugin go.mod and add a replace to target - // current ignite/cli - gomod, err := gomodule.ParseAt(path) - require.NoError(err) - // use GOMOD env to get current directory module path - modpath, err := gocmd.Env(gocmd.EnvGOMOD) - require.NoError(err) - modpath = filepath.Dir(modpath) - gomod.AddReplace("github.com/ignite/cli", "", modpath, "") - // Save go.mod - data, err := gomod.Format() - require.NoError(err) - err = os.WriteFile(filepath.Join(path, "go.mod"), data, 0o644) - require.NoError(err) - return path - } - // Helper to make a local git repository with gofile committed. // Returns the repo directory and the git.Repository makeGitRepo := func(t *testing.T, name string) (string, *git.Repository) { require := require.New(t) repoDir := t.TempDir() - scaffoldPlugin(t, repoDir, "github.com/ignite/"+name) + scaffoldPlugin(t, repoDir, "github.com/ignite/"+name, false) require.NoError(err) repo, err := git.PlainInit(repoDir, false) require.NoError(err) @@ -210,7 +186,7 @@ func TestPluginLoad(t *testing.T) { { name: "ok: from local", buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar") + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar", false) return Plugin{ Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, @@ -363,74 +339,63 @@ func TestPluginLoad(t *testing.T) { } func TestPluginLoadSharedHost(t *testing.T) { - // wd, err := os.Getwd() - // require.NoError(t, err) - - // scaffoldPlugin runs Scaffold and updates the go.mod so it uses the - // current ignite/cli sources. - scaffoldPlugin := func(t *testing.T, dir, name string) string { - require := require.New(t) - path, err := Scaffold(dir, name, true) - require.NoError(err) - // We want the scaffolded plugin to use the current version of ignite/cli, - // for that we need to update the plugin go.mod and add a replace to target - // current ignite/cli - gomod, err := gomodule.ParseAt(path) - require.NoError(err) - // use GOMOD env to get current directory module path - modpath, err := gocmd.Env(gocmd.EnvGOMOD) - require.NoError(err) - modpath = filepath.Dir(modpath) - gomod.AddReplace("github.com/ignite/cli", "", modpath, "") - // Save go.mod - data, err := gomod.Format() - require.NoError(err) - err = os.WriteFile(filepath.Join(path, "go.mod"), data, 0o644) - require.NoError(err) - return path - } - tests := []struct { - name string - buildPlugin func(t *testing.T) Plugin - expectedError string - instances int + name string + buildPlugin func(t *testing.T) Plugin + instances int + sharesHost bool }{ { - name: "ok: from local sharedhost is on 1", + name: "ok: from local sharedhost is on 1 instance", buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-1") + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-1", true) return Plugin{ Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, binaryName: "bar-1", } }, - instances: 1, + instances: 1, + sharesHost: true, }, { - name: "ok: from local sharedhost is on 2", + name: "ok: from local sharedhost is on 2 instances", buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-2") + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-2", true) return Plugin{ Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, binaryName: "bar-2", } }, - instances: 2, + instances: 2, + sharesHost: true, }, { - name: "ok: from local sharedhost is on 4", + name: "ok: from local sharedhost is on 4 instances", buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4") + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4", true) return Plugin{ Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, binaryName: "bar-4", } }, - instances: 4, + instances: 4, + sharesHost: true, + }, + { + name: "ok: from local sharedhost is off 4 instances", + buildPlugin: func(t *testing.T) Plugin { + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4", false) + return Plugin{ + Plugin: pluginsconfig.Plugin{Path: path}, + srcPath: path, + binaryName: "bar-4", + } + }, + instances: 4, + sharesHost: false, }, } @@ -444,6 +409,14 @@ func TestPluginLoadSharedHost(t *testing.T) { plugins = append(plugins, &p) } + if !tt.sharesHost { + require.Equal(false, CheckPluginConfCache(plugins[0].Path)) + for i := 0; i < tt.instances; i++ { + require.Equal(false, plugins[i].isHost) + } + return + } + require.Equal(true, CheckPluginConfCache(plugins[0].Path)) for i := len(plugins) - 1; i >= 0; i-- { @@ -501,6 +474,30 @@ func TestPluginClean(t *testing.T) { } } +// scaffoldPlugin runs Scaffold and updates the go.mod so it uses the +// current ignite/cli sources. +func scaffoldPlugin(t *testing.T, dir, name string, sharedHost bool) string { + require := require.New(t) + path, err := Scaffold(dir, name, sharedHost) + require.NoError(err) + // We want the scaffolded plugin to use the current version of ignite/cli, + // for that we need to update the plugin go.mod and add a replace to target + // current ignite/cli + gomod, err := gomodule.ParseAt(path) + require.NoError(err) + // use GOMOD env to get current directory module path + modpath, err := gocmd.Env(gocmd.EnvGOMOD) + require.NoError(err) + modpath = filepath.Dir(modpath) + gomod.AddReplace("github.com/ignite/cli", "", modpath, "") + // Save go.mod + data, err := gomod.Format() + require.NoError(err) + err = os.WriteFile(filepath.Join(path, "go.mod"), data, 0o644) + require.NoError(err) + return path +} + func assertPlugin(t *testing.T, want, have Plugin) { if want.Error != nil { require.Error(t, have.Error) From 0bb2e3a6b706bbd0aa18f66662d554af012724ec Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Wed, 21 Dec 2022 15:05:32 -0500 Subject: [PATCH 33/40] lint --- ignite/services/plugin/cache.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/ignite/services/plugin/cache.go b/ignite/services/plugin/cache.go index 14935a6a8f..feefad1bf3 100644 --- a/ignite/services/plugin/cache.go +++ b/ignite/services/plugin/cache.go @@ -33,7 +33,6 @@ type ConfigContext struct { } func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) error { - if pluginPath == "" { return fmt.Errorf("provided path is invalid: %s", pluginPath) } @@ -65,7 +64,6 @@ func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) erro } func ReadPluginConfigCache(pluginPath string, ref *hplugin.ReattachConfig) error { - if pluginPath == "" { return fmt.Errorf("provided path is invalid: %s", pluginPath) } From 5761bcc7f62196ec3628c28a7ad19e89c93dfc3b Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Wed, 21 Dec 2022 15:15:28 -0500 Subject: [PATCH 34/40] update plugin sharedHost tests --- ignite/services/plugin/plugin_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index c402d82d88..0e40c4dd42 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -341,14 +341,14 @@ func TestPluginLoad(t *testing.T) { func TestPluginLoadSharedHost(t *testing.T) { tests := []struct { name string - buildPlugin func(t *testing.T) Plugin + buildPlugin func(t *testing.T, sharedHost bool) Plugin instances int sharesHost bool }{ { name: "ok: from local sharedhost is on 1 instance", - buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-1", true) + buildPlugin: func(t *testing.T, sharedHost bool) Plugin { + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-1", sharedHost) return Plugin{ Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, @@ -360,8 +360,8 @@ func TestPluginLoadSharedHost(t *testing.T) { }, { name: "ok: from local sharedhost is on 2 instances", - buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-2", true) + buildPlugin: func(t *testing.T, sharedHost bool) Plugin { + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-2", sharedHost) return Plugin{ Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, @@ -373,8 +373,8 @@ func TestPluginLoadSharedHost(t *testing.T) { }, { name: "ok: from local sharedhost is on 4 instances", - buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4", true) + buildPlugin: func(t *testing.T, sharedHost bool) Plugin { + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4", sharedHost) return Plugin{ Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, @@ -386,8 +386,8 @@ func TestPluginLoadSharedHost(t *testing.T) { }, { name: "ok: from local sharedhost is off 4 instances", - buildPlugin: func(t *testing.T) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4", false) + buildPlugin: func(t *testing.T, sharedHost bool) Plugin { + path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4", sharedHost) return Plugin{ Plugin: pluginsconfig.Plugin{Path: path}, srcPath: path, @@ -404,7 +404,7 @@ func TestPluginLoadSharedHost(t *testing.T) { require := require.New(t) var plugins []*Plugin for i := 0; i < tt.instances; i++ { - p := tt.buildPlugin(t) + p := tt.buildPlugin(t, tt.sharesHost) p.load(context.Background()) plugins = append(plugins, &p) } From 85ae39cd0a8397bed68e58151c22026817f1ed5d Mon Sep 17 00:00:00 2001 From: joshLong145 Date: Thu, 22 Dec 2022 10:32:43 -0500 Subject: [PATCH 35/40] update to plugin docs for `Sharedhost` flag --- docs/docs/contributing/01-plugins.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/docs/contributing/01-plugins.md b/docs/docs/contributing/01-plugins.md index 8a80c63d42..01edccc9fd 100644 --- a/docs/docs/contributing/01-plugins.md +++ b/docs/docs/contributing/01-plugins.md @@ -25,7 +25,7 @@ plugins: ``` Now the next time the `ignite` command is run under your project, the declared -plugin will be fetched, compiled and ran. This will result in more avaiable +plugin will be fetched, compiled and ran. This will result in more available commands, and/or hooks attached to existing commands. ### Listing installed plugins @@ -129,6 +129,19 @@ type Manifest struct { // Hooks contains the hooks that will be attached to the existing ignite // commands. Hooks []Hook + // SharedHost enables sharing a single plugin server across all running instances + // of a plugin. Useful if a plugin adds or extends long running commands + // + // Example: if a plugin defines a hook on `ignite chain serve`, a plugin server is instanciated + // when the command is run. Now if you want to interact with that instance from commands + // defined in that plugin, you need to enable `SharedHost`, or else the commands will just + // instantiate separate plugin servers. + // + // When enabled, all plugins of the same `Path` loaded from the same configuration will + // attach it's rpc client to a an existing rpc server. + // + // If a plugin instance has no other running plugin servers, it will create one and it will be the host. + SharedHost bool `yaml:"shared_host"` } ``` @@ -142,6 +155,10 @@ If your plugin adds features to existing commands, feeds the `Hooks` field. Of course a plugin can declare `Commands` *and* `Hooks`. +A plugin may also share a host process by setting `SharedHost` to `true`. +`SharedHost` is desirable if a plugin hooks into, or declares long running commands. +Commands executed from the same plugin context interact with the same plugin server. +Allowing all executing commands to share the same server instance, giving shared execution context. ### Adding new command Plugin commands are custom commands added to the ignite cli by a registered From c1e9351cb01348adf8e34f6fa5f0f14727d24f3c Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Sun, 1 Jan 2023 17:27:46 +0100 Subject: [PATCH 36/40] test: fix TestPluginLoadSharedHost --- ignite/services/plugin/plugin_test.go | 111 ++++++++++++-------------- 1 file changed, 50 insertions(+), 61 deletions(-) diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index 0e40c4dd42..d3ad7fe3c8 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -12,6 +12,7 @@ import ( "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" + hplugin "github.com/hashicorp/go-plugin" "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -340,93 +341,81 @@ func TestPluginLoad(t *testing.T) { func TestPluginLoadSharedHost(t *testing.T) { tests := []struct { - name string - buildPlugin func(t *testing.T, sharedHost bool) Plugin - instances int - sharesHost bool + name string + instances int + sharesHost bool }{ { - name: "ok: from local sharedhost is on 1 instance", - buildPlugin: func(t *testing.T, sharedHost bool) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-1", sharedHost) - return Plugin{ - Plugin: pluginsconfig.Plugin{Path: path}, - srcPath: path, - binaryName: "bar-1", - } - }, + name: "ok: from local sharedhost is on 1 instance", instances: 1, sharesHost: true, }, { - name: "ok: from local sharedhost is on 2 instances", - buildPlugin: func(t *testing.T, sharedHost bool) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-2", sharedHost) - return Plugin{ - Plugin: pluginsconfig.Plugin{Path: path}, - srcPath: path, - binaryName: "bar-2", - } - }, + name: "ok: from local sharedhost is on 2 instances", instances: 2, sharesHost: true, }, { - name: "ok: from local sharedhost is on 4 instances", - buildPlugin: func(t *testing.T, sharedHost bool) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4", sharedHost) - return Plugin{ - Plugin: pluginsconfig.Plugin{Path: path}, - srcPath: path, - binaryName: "bar-4", - } - }, + name: "ok: from local sharedhost is on 4 instances", instances: 4, sharesHost: true, }, { - name: "ok: from local sharedhost is off 4 instances", - buildPlugin: func(t *testing.T, sharedHost bool) Plugin { - path := scaffoldPlugin(t, t.TempDir(), "github.com/foo/bar-4", sharedHost) - return Plugin{ - Plugin: pluginsconfig.Plugin{Path: path}, - srcPath: path, - binaryName: "bar-4", - } - }, + name: "ok: from local sharedhost is off 4 instances", instances: 4, sharesHost: false, }, } - for _, tt := range tests { + for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { - require := require.New(t) - var plugins []*Plugin + var ( + require = require.New(t) + assert = assert.New(t) + // scaffold an unique plugin for all instances + path = scaffoldPlugin(t, t.TempDir(), + fmt.Sprintf("github.com/foo/bar-%d", i), tt.sharesHost) + plugins []*Plugin + ) + // Load one plugin per instance for i := 0; i < tt.instances; i++ { - p := tt.buildPlugin(t, tt.sharesHost) + p := Plugin{ + Plugin: pluginsconfig.Plugin{Path: path}, + srcPath: path, + binaryName: filepath.Base(path), + } p.load(context.Background()) + require.NoError(p.Error) plugins = append(plugins, &p) } - - if !tt.sharesHost { - require.Equal(false, CheckPluginConfCache(plugins[0].Path)) - for i := 0; i < tt.instances; i++ { - require.Equal(false, plugins[i].isHost) + // Ensure all plugins are killed at the end of test case + defer func() { + for i := 0; i < len(plugins); i++ { + plugins[i].KillClient() + assert.False(plugins[i].isHost, "killed plugins are no longer host") } - return - } - - require.Equal(true, CheckPluginConfCache(plugins[0].Path)) - - for i := len(plugins) - 1; i >= 0; i-- { - plugins[i].KillClient() - if i != 0 { - require.Equal(true, plugins[0].isHost) - rconf := plugins[i].client.ReattachConfig() - require.Equal(rconf.Addr.String(), plugins[i].client.ReattachConfig().Addr.String()) + assert.False(CheckPluginConfCache(plugins[0].Path), "once host is killed the cache should be cleared") + }() + + for i := 0; i < len(plugins); i++ { + if tt.sharesHost { + assert.True(CheckPluginConfCache(plugins[i].Path), "sharedHost must have a cache entry") + if i == 0 { + // first plugin is the host + assert.True(plugins[i].isHost, "first plugin is the host") + // Assert reattach config has been saved for non host + rconf := plugins[i].client.ReattachConfig() + var ref hplugin.ReattachConfig + if assert.NoError(ReadPluginConfigCache(plugins[i].Path, &ref)) { + assert.Equal(rconf, &ref, "wrong cache entry for plugin host") + } + } else { + // plugins after first aren't host + assert.False(plugins[i].isHost, "plugin %d can't be host", i) + } } else { - require.Equal(false, plugins[0].isHost) + assert.False(plugins[i].isHost, "plugin %d can't be host if sharedHost is disabled", i) + assert.False(CheckPluginConfCache(plugins[i].Path), "plugin %d can't have a cache entry if sharedHost is disabled", i) } } }) From b8fea7da9150ca43f136cf25b86704df90ec2ba3 Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Sun, 1 Jan 2023 18:16:25 +0100 Subject: [PATCH 37/40] refac: plugin cach --- ignite/services/plugin/cache.go | 79 ++++++--------------------- ignite/services/plugin/cache_test.go | 6 +- ignite/services/plugin/plugin.go | 3 +- ignite/services/plugin/plugin_test.go | 12 ++-- 4 files changed, 27 insertions(+), 73 deletions(-) diff --git a/ignite/services/plugin/cache.go b/ignite/services/plugin/cache.go index feefad1bf3..67853c7487 100644 --- a/ignite/services/plugin/cache.go +++ b/ignite/services/plugin/cache.go @@ -4,11 +4,10 @@ import ( "encoding/gob" "fmt" "net" - "path/filepath" + "path" hplugin "github.com/hashicorp/go-plugin" - chainconfig "github.com/ignite/cli/ignite/config" "github.com/ignite/cli/ignite/pkg/cache" ) @@ -17,86 +16,48 @@ const ( cacheNamespace = "plugin.rpc.context" ) -var ( - storage *cache.Storage - storageCache *cache.Cache[ConfigContext] -) +var storageCache *cache.Cache[hplugin.ReattachConfig] func init() { gob.Register(hplugin.ReattachConfig{}) - gob.Register(net.UnixAddr{}) -} - -type ConfigContext struct { - Plugin hplugin.ReattachConfig - Addr net.UnixAddr + gob.Register(&net.UnixAddr{}) } func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) error { if pluginPath == "" { return fmt.Errorf("provided path is invalid: %s", pluginPath) } - if conf.Addr == nil { return fmt.Errorf("plugin Address info cannot be empty") } - - confCont := ConfigContext{} - // TODO: figure out a better way of resolving the type of network connection is established between plugin server and host - // currently this will always be a unix network socket. but this might not be the case moving forward. - ua, err := net.ResolveUnixAddr(conf.Addr.Network(), conf.Addr.String()) - if err != nil { - return err - } - - confCont.Addr = *ua - conf.Addr = nil - confCont.Plugin = conf - cache, err := newCache() if err != nil { return err } - - cache.Put(pluginPath, confCont) - - return err + return cache.Put(pluginPath, conf) } -func ReadPluginConfigCache(pluginPath string, ref *hplugin.ReattachConfig) error { +func ReadPluginConfigCache(pluginPath string) (hplugin.ReattachConfig, error) { if pluginPath == "" { - return fmt.Errorf("provided path is invalid: %s", pluginPath) + return hplugin.ReattachConfig{}, fmt.Errorf("provided path is invalid: %s", pluginPath) } - cache, err := newCache() if err != nil { - return err + return hplugin.ReattachConfig{}, err } - - confCont, err := cache.Get(pluginPath) - if err != nil { - return err - } - - *ref = confCont.Plugin - ref.Addr = &confCont.Addr - - return nil + return cache.Get(pluginPath) } func CheckPluginConfCache(pluginPath string) bool { if pluginPath == "" { return false } - cache, err := newCache() if err != nil { return false } - if _, err := cache.Get(pluginPath); err != nil { - return false - } - return true + _, err = cache.Get(pluginPath) + return err == nil } func DeletePluginConfCache(pluginPath string) error { @@ -107,27 +68,21 @@ func DeletePluginConfCache(pluginPath string) error { if err != nil { return err } - - if err := cache.Delete(pluginPath); err != nil { - return err - } - - return nil + return cache.Delete(pluginPath) } -func newCache() (*cache.Cache[ConfigContext], error) { - cacheRootDir, err := chainconfig.DirPath() +func newCache() (*cache.Cache[hplugin.ReattachConfig], error) { + cacheRootDir, err := PluginsPath() if err != nil { return nil, err } - if storage == nil { - storageTmp, err := cache.NewStorage(filepath.Join(cacheRootDir, cacheFileName)) + if storageCache == nil { + storage, err := cache.NewStorage(path.Join(cacheRootDir, cacheFileName)) if err != nil { return nil, err } - storage = &storageTmp - cacheTmp := cache.New[ConfigContext](*storage, cacheNamespace) - storageCache = &cacheTmp + c := cache.New[hplugin.ReattachConfig](storage, cacheNamespace) + storageCache = &c } return storageCache, nil diff --git a/ignite/services/plugin/cache_test.go b/ignite/services/plugin/cache_test.go index 998e9d9b89..98432a27ba 100644 --- a/ignite/services/plugin/cache_test.go +++ b/ignite/services/plugin/cache_test.go @@ -23,8 +23,7 @@ func TestReadWritePluginConfigCache(t *testing.T) { err := WritePluginConfigCache(path, rc) require.NoError(t, err) - c := hplugin.ReattachConfig{} - err = ReadPluginConfigCache(path, &c) + c, err := ReadPluginConfigCache(path) require.NoError(t, err) require.Equal(t, rc, c) }) @@ -74,9 +73,8 @@ func TestPluginCacheDelete(t *testing.T) { err = DeletePluginConfCache(path) require.NoError(t, err) - c := hplugin.ReattachConfig{} // there should be an error after deleting the config from the cache - err = ReadPluginConfigCache(path, &c) + _, err = ReadPluginConfigCache(path) require.Error(t, err) }) diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index f90b661fff..1e21e4298a 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -245,8 +245,7 @@ func (p *Plugin) load(ctx context.Context) { }) if CheckPluginConfCache(p.Path) { - rconf := hplugin.ReattachConfig{} - err := ReadPluginConfigCache(p.Path, &rconf) + rconf, err := ReadPluginConfigCache(p.Path) if err != nil { p.Error = err return diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index d3ad7fe3c8..71e5112df5 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -397,21 +397,23 @@ func TestPluginLoadSharedHost(t *testing.T) { assert.False(CheckPluginConfCache(plugins[0].Path), "once host is killed the cache should be cleared") }() + var hostConf *hplugin.ReattachConfig for i := 0; i < len(plugins); i++ { if tt.sharesHost { assert.True(CheckPluginConfCache(plugins[i].Path), "sharedHost must have a cache entry") if i == 0 { // first plugin is the host assert.True(plugins[i].isHost, "first plugin is the host") - // Assert reattach config has been saved for non host - rconf := plugins[i].client.ReattachConfig() - var ref hplugin.ReattachConfig - if assert.NoError(ReadPluginConfigCache(plugins[i].Path, &ref)) { - assert.Equal(rconf, &ref, "wrong cache entry for plugin host") + // Assert reattach config has been saved + hostConf = plugins[i].client.ReattachConfig() + ref, err := ReadPluginConfigCache(plugins[i].Path) + if assert.NoError(err) { + assert.Equal(hostConf, &ref, "wrong cache entry for plugin host") } } else { // plugins after first aren't host assert.False(plugins[i].isHost, "plugin %d can't be host", i) + assert.Equal(hostConf, plugins[i].client.ReattachConfig(), "ReattachConfig different from host plugin") } } else { assert.False(plugins[i].isHost, "plugin %d can't be host if sharedHost is disabled", i) From c23ab898f232dc620375d3f6d7e8dc15f7d8d000 Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Sun, 1 Jan 2023 18:19:21 +0100 Subject: [PATCH 38/40] Update docs/docs/contributing/01-plugins.md --- docs/docs/contributing/01-plugins.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/docs/contributing/01-plugins.md b/docs/docs/contributing/01-plugins.md index 01edccc9fd..39184d727d 100644 --- a/docs/docs/contributing/01-plugins.md +++ b/docs/docs/contributing/01-plugins.md @@ -159,6 +159,7 @@ A plugin may also share a host process by setting `SharedHost` to `true`. `SharedHost` is desirable if a plugin hooks into, or declares long running commands. Commands executed from the same plugin context interact with the same plugin server. Allowing all executing commands to share the same server instance, giving shared execution context. + ### Adding new command Plugin commands are custom commands added to the ignite cli by a registered From 66e9463f5d5fb7d8314df43e10f9cd8373a5615a Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Sun, 1 Jan 2023 18:26:35 +0100 Subject: [PATCH 39/40] refac: plugin cache func should be private --- ignite/services/plugin/cache.go | 9 ++++---- ignite/services/plugin/cache_test.go | 30 +++++++++++++-------------- ignite/services/plugin/plugin.go | 10 ++++----- ignite/services/plugin/plugin_test.go | 8 +++---- 4 files changed, 28 insertions(+), 29 deletions(-) diff --git a/ignite/services/plugin/cache.go b/ignite/services/plugin/cache.go index 67853c7487..6d38303d40 100644 --- a/ignite/services/plugin/cache.go +++ b/ignite/services/plugin/cache.go @@ -23,7 +23,7 @@ func init() { gob.Register(&net.UnixAddr{}) } -func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) error { +func writeConfigCache(pluginPath string, conf hplugin.ReattachConfig) error { if pluginPath == "" { return fmt.Errorf("provided path is invalid: %s", pluginPath) } @@ -37,7 +37,7 @@ func WritePluginConfigCache(pluginPath string, conf hplugin.ReattachConfig) erro return cache.Put(pluginPath, conf) } -func ReadPluginConfigCache(pluginPath string) (hplugin.ReattachConfig, error) { +func readConfigCache(pluginPath string) (hplugin.ReattachConfig, error) { if pluginPath == "" { return hplugin.ReattachConfig{}, fmt.Errorf("provided path is invalid: %s", pluginPath) } @@ -48,7 +48,7 @@ func ReadPluginConfigCache(pluginPath string) (hplugin.ReattachConfig, error) { return cache.Get(pluginPath) } -func CheckPluginConfCache(pluginPath string) bool { +func checkConfCache(pluginPath string) bool { if pluginPath == "" { return false } @@ -60,7 +60,7 @@ func CheckPluginConfCache(pluginPath string) bool { return err == nil } -func DeletePluginConfCache(pluginPath string) error { +func deleteConfCache(pluginPath string) error { if pluginPath == "" { return fmt.Errorf("provided path is invalid: %s", pluginPath) } @@ -84,6 +84,5 @@ func newCache() (*cache.Cache[hplugin.ReattachConfig], error) { c := cache.New[hplugin.ReattachConfig](storage, cacheNamespace) storageCache = &c } - return storageCache, nil } diff --git a/ignite/services/plugin/cache_test.go b/ignite/services/plugin/cache_test.go index 98432a27ba..c11ca8b033 100644 --- a/ignite/services/plugin/cache_test.go +++ b/ignite/services/plugin/cache_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestReadWritePluginConfigCache(t *testing.T) { +func TestReadWriteConfigCache(t *testing.T) { t.Run("Should cache plugin config and read from cache", func(t *testing.T) { const path = "/path/to/awesome/plugin" unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") @@ -20,10 +20,10 @@ func TestReadWritePluginConfigCache(t *testing.T) { Pid: 24464, } - err := WritePluginConfigCache(path, rc) + err := writeConfigCache(path, rc) require.NoError(t, err) - c, err := ReadPluginConfigCache(path) + c, err := readConfigCache(path) require.NoError(t, err) require.Equal(t, rc, c) }) @@ -37,7 +37,7 @@ func TestReadWritePluginConfigCache(t *testing.T) { Pid: 24464, } - err := WritePluginConfigCache(path, rc) + err := writeConfigCache(path, rc) require.Error(t, err) }) @@ -50,12 +50,12 @@ func TestReadWritePluginConfigCache(t *testing.T) { Pid: 24464, } - err := WritePluginConfigCache(path, rc) + err := writeConfigCache(path, rc) require.Error(t, err) }) } -func TestPluginCacheDelete(t *testing.T) { +func TestDeleteConfCache(t *testing.T) { t.Run("Delete plugin config after write to cache should remove from cache", func(t *testing.T) { const path = "/path/to/awesome/plugin" unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") @@ -67,25 +67,25 @@ func TestPluginCacheDelete(t *testing.T) { Pid: 24464, } - err := WritePluginConfigCache(path, rc) + err := writeConfigCache(path, rc) require.NoError(t, err) - err = DeletePluginConfCache(path) + err = deleteConfCache(path) require.NoError(t, err) // there should be an error after deleting the config from the cache - _, err = ReadPluginConfigCache(path) + _, err = readConfigCache(path) require.Error(t, err) }) t.Run("Delete plugin config should return error given empty path", func(t *testing.T) { const path = "" - err := DeletePluginConfCache(path) + err := deleteConfCache(path) require.Error(t, err) }) } -func TestPluginCacheCheck(t *testing.T) { +func TestCheckConfCache(t *testing.T) { const path = "/path/to/awesome/plugin" unixFD, _ := net.ResolveUnixAddr("unix", "/var/folders/5k/sv4bxrs102n_6rr7430jc7j80000gn/T/plugin193424090") @@ -97,13 +97,13 @@ func TestPluginCacheCheck(t *testing.T) { } t.Run("Cache should be hydrated", func(t *testing.T) { - err := WritePluginConfigCache(path, rc) + err := writeConfigCache(path, rc) require.NoError(t, err) - require.Equal(t, true, CheckPluginConfCache(path)) + require.Equal(t, true, checkConfCache(path)) }) t.Run("Cache should be empty", func(t *testing.T) { - _ = DeletePluginConfCache(path) - require.Equal(t, false, CheckPluginConfCache(path)) + _ = deleteConfCache(path) + require.Equal(t, false, checkConfCache(path)) }) } diff --git a/ignite/services/plugin/plugin.go b/ignite/services/plugin/plugin.go index 1e21e4298a..d1ced5a970 100644 --- a/ignite/services/plugin/plugin.go +++ b/ignite/services/plugin/plugin.go @@ -181,7 +181,7 @@ func (p *Plugin) KillClient() { } if p.isHost { - DeletePluginConfCache(p.Path) + deleteConfCache(p.Path) p.isHost = false } } @@ -244,8 +244,8 @@ func (p *Plugin) load(ctx context.Context) { Level: logLevel, }) - if CheckPluginConfCache(p.Path) { - rconf, err := ReadPluginConfigCache(p.Path) + if checkConfCache(p.Path) { + rconf, err := readConfigCache(p.Path) if err != nil { p.Error = err return @@ -301,8 +301,8 @@ func (p *Plugin) load(ctx context.Context) { // write the rpc context to cache if the plugin is declared as host. // writing it to cache as lost operation within load to assure rpc client's reattach config // is hydrated. - if m.SharedHost && !CheckPluginConfCache(p.Path) { - err := WritePluginConfigCache(p.Path, *p.client.ReattachConfig()) + if m.SharedHost && !checkConfCache(p.Path) { + err := writeConfigCache(p.Path, *p.client.ReattachConfig()) if err != nil { p.Error = err return diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index 71e5112df5..6903491901 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -394,19 +394,19 @@ func TestPluginLoadSharedHost(t *testing.T) { plugins[i].KillClient() assert.False(plugins[i].isHost, "killed plugins are no longer host") } - assert.False(CheckPluginConfCache(plugins[0].Path), "once host is killed the cache should be cleared") + assert.False(checkConfCache(plugins[0].Path), "once host is killed the cache should be cleared") }() var hostConf *hplugin.ReattachConfig for i := 0; i < len(plugins); i++ { if tt.sharesHost { - assert.True(CheckPluginConfCache(plugins[i].Path), "sharedHost must have a cache entry") + assert.True(checkConfCache(plugins[i].Path), "sharedHost must have a cache entry") if i == 0 { // first plugin is the host assert.True(plugins[i].isHost, "first plugin is the host") // Assert reattach config has been saved hostConf = plugins[i].client.ReattachConfig() - ref, err := ReadPluginConfigCache(plugins[i].Path) + ref, err := readConfigCache(plugins[i].Path) if assert.NoError(err) { assert.Equal(hostConf, &ref, "wrong cache entry for plugin host") } @@ -417,7 +417,7 @@ func TestPluginLoadSharedHost(t *testing.T) { } } else { assert.False(plugins[i].isHost, "plugin %d can't be host if sharedHost is disabled", i) - assert.False(CheckPluginConfCache(plugins[i].Path), "plugin %d can't have a cache entry if sharedHost is disabled", i) + assert.False(checkConfCache(plugins[i].Path), "plugin %d can't have a cache entry if sharedHost is disabled", i) } } }) From 17ef1174e3bc874cb69255bef52076aaa12591ed Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Sun, 1 Jan 2023 19:59:41 +0100 Subject: [PATCH 40/40] test: improve plugin kill assertions --- ignite/services/plugin/plugin_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ignite/services/plugin/plugin_test.go b/ignite/services/plugin/plugin_test.go index 6903491901..b92328dbea 100644 --- a/ignite/services/plugin/plugin_test.go +++ b/ignite/services/plugin/plugin_test.go @@ -390,8 +390,14 @@ func TestPluginLoadSharedHost(t *testing.T) { } // Ensure all plugins are killed at the end of test case defer func() { - for i := 0; i < len(plugins); i++ { + for i := len(plugins) - 1; i >= 0; i-- { plugins[i].KillClient() + if tt.sharesHost && i > 0 { + assert.False(plugins[i].client.Exited(), "non host plugin can't kill host plugin") + assert.True(checkConfCache(plugins[i].Path), "non host plugin doesn't remove config cache when killed") + } else { + assert.True(plugins[i].client.Exited(), "plugin should be killed") + } assert.False(plugins[i].isHost, "killed plugins are no longer host") } assert.False(checkConfCache(plugins[0].Path), "once host is killed the cache should be cleared")