From cffd771288c44f8f4e737b892b1998cd5f2fd57a Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 8 Apr 2021 16:09:48 -0700 Subject: [PATCH] Move Vagrant runtime out of server and into runner (via client startup) --- internal/client/basis.go | 26 ++++-- internal/client/runner.go | 1 + internal/client/server.go | 76 +++++++++-------- internal/core/basis.go | 1 - internal/plugin/factory.go | 9 +- internal/runner/operation.go | 2 +- internal/runner/runner.go | 49 ++++++----- internal/server/ruby_vagrant_server.go | 43 ---------- internal/server/singleprocess/service.go | 11 --- .../server/singleprocess/service_server.go | 16 ---- internal/serverclient/ruby_client.go | 83 +++++++------------ 11 files changed, 127 insertions(+), 190 deletions(-) delete mode 100644 internal/server/ruby_vagrant_server.go diff --git a/internal/client/basis.go b/internal/client/basis.go index a43e526bc..fef5de6f4 100644 --- a/internal/client/basis.go +++ b/internal/client/basis.go @@ -6,6 +6,7 @@ import ( // "fmt" "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-plugin" "github.com/hashicorp/vagrant-plugin-sdk/helper/paths" "github.com/hashicorp/vagrant-plugin-sdk/terminal" @@ -20,10 +21,11 @@ type Basis struct { basis *vagrant_server.Basis Project *Project - client *serverclient.VagrantClient - logger hclog.Logger - runner *vagrant_server.Ref_Runner - cleanupFuncs []func() + client *serverclient.VagrantClient + vagrantRubyRuntime *plugin.Client + logger hclog.Logger + runner *vagrant_server.Ref_Runner + cleanupFuncs []func() config *configpkg.Config @@ -34,8 +36,8 @@ type Basis struct { localServer bool // True when a local server is created } -func New(ctx context.Context, opts ...Option) (*Basis, error) { - basis := &Basis{ +func New(ctx context.Context, opts ...Option) (basis *Basis, err error) { + basis = &Basis{ logger: hclog.L().Named("basis"), runner: &vagrant_server.Ref_Runner{ Target: &vagrant_server.Ref_Runner_Any{ @@ -82,6 +84,14 @@ func New(ctx context.Context, opts ...Option) (*Basis, error) { basis.client = serverclient.WrapVagrantClient(conn) } + // If the ruby runtime isn't provided, set it up + if basis.vagrantRubyRuntime == nil { + if basis.vagrantRubyRuntime, err = basis.initVagrantRubyRuntime(); err != nil { + return nil, err + } + basis.cleanup(func() { basis.vagrantRubyRuntime.Kill() }) + } + // Negotiate the version if err := basis.negotiateApiVersion(ctx); err != nil { return nil, err @@ -181,6 +191,10 @@ func (b *Basis) Client() *serverclient.VagrantClient { return b.client } +func (b *Basis) VagrantRubyRuntime() *plugin.Client { + return b.vagrantRubyRuntime +} + // Local is true if the server is an in-process just-in-time server. func (b *Basis) Local() bool { return b.localServer diff --git a/internal/client/runner.go b/internal/client/runner.go index 773defc12..280ee9205 100644 --- a/internal/client/runner.go +++ b/internal/client/runner.go @@ -10,6 +10,7 @@ func (b *Basis) startRunner() (*runner.Runner, error) { // Initialize our runner r, err := runner.New( runner.WithClient(b.client), + runner.WithVagrantRubyRuntime(b.vagrantRubyRuntime), runner.WithLogger(b.logger.Named("runner")), runner.ByIdOnly(), // We'll direct target this runner.WithLocal(b.UI()), // Local mode diff --git a/internal/client/server.go b/internal/client/server.go index b55cabe9e..5110b8190 100644 --- a/internal/client/server.go +++ b/internal/client/server.go @@ -2,7 +2,6 @@ package client import ( "context" - "io" "net" "os/exec" "path/filepath" @@ -17,7 +16,6 @@ import ( "github.com/hashicorp/vagrant-plugin-sdk/proto/vagrant_plugin_sdk" "github.com/hashicorp/vagrant/internal/protocolversion" "github.com/hashicorp/vagrant/internal/server" - sr "github.com/hashicorp/vagrant/internal/server" "github.com/hashicorp/vagrant/internal/server/proto/vagrant_server" "github.com/hashicorp/vagrant/internal/server/singleprocess" "github.com/hashicorp/vagrant/internal/serverclient" @@ -75,21 +73,26 @@ func (b *Basis) initServerClient(ctx context.Context, cfg *config) (*grpc.Client // // If this returns an error, all resources associated with this operation // will be closed, but the project can retry. -func (b *Basis) initLocalServer(ctx context.Context) (*grpc.ClientConn, error) { +func (b *Basis) initLocalServer(ctx context.Context) (_ *grpc.ClientConn, err error) { log := b.logger.Named("server") b.localServer = true // We use this pointer to accumulate things we need to clean up // in the case of an error. On success we nil this variable which // doesn't close anything. - var closers []io.Closer + var cleanups []func() + + // If we encounter an error force all the + // local cleanups to run defer func() { - for _, c := range closers { - c.Close() + if err != nil { + for _, c := range cleanups { + c() + } } }() - // TODO(mitchellh): path to this + // TODO(spox): path to this path := filepath.Join("data.db") log.Debug("opening local mode DB", "path", path) @@ -98,40 +101,40 @@ func (b *Basis) initLocalServer(ctx context.Context) (*grpc.ClientConn, error) { Timeout: 1 * time.Second, }) if err != nil { - return nil, err - } - closers = append(closers, db) - - vrr, err := b.initVagrantRubyRuntime() - if err != nil { - return nil, err + return } + cleanups = append(cleanups, func() { db.Close() }) // Create our server impl, err := singleprocess.New( - singleprocess.WithVagrantRubyRuntime(vrr), singleprocess.WithDB(db), singleprocess.WithLogger(log.Named("singleprocess")), ) if err != nil { log.Trace("failed singleprocess server setup", "error", err) - return nil, err + return } // We listen on a random locally bound port // TODO: we should use Unix domain sockets if supported ln, err := net.Listen("tcp", "127.0.0.1:") if err != nil { - return nil, err + return } - closers = append(closers, ln) + cleanups = append(cleanups, func() { ln.Close() }) // Create a new cancellation context so we can cancel in the case of an error ctx, cancel := context.WithCancel(ctx) + defer func() { + if err != nil { + cancel() + } + }() // Run the server log.Info("starting built-in server for local operations", "addr", ln.Addr().String()) - go server.Run(server.WithContext(ctx), + go server.Run( + server.WithContext(ctx), server.WithLogger(log), server.WithGRPC(ln), server.WithImpl(impl), @@ -141,8 +144,7 @@ func (b *Basis) initLocalServer(ctx context.Context) (*grpc.ClientConn, error) { client, err := serverclient.NewVagrantClient(ctx, log, ln.Addr().String()) if err != nil { - cancel() - return nil, err + return } // Setup our server config. The configuration is specifically set so @@ -158,28 +160,22 @@ func (b *Basis) initLocalServer(ctx context.Context) (*grpc.ClientConn, error) { }, }) if err != nil { - cancel() - return nil, err + return } - // Success, persist the closers - cleanupClosers := closers - closers = nil + // Have the defined cleanups run when the basis is closed b.cleanup(func() { - for _, c := range cleanupClosers { - c.Close() - } - // Force the ruby runtime to shut down - if cl, err := vrr.Client(); err == nil { - cl.Close() + for _, c := range cleanups { + c() } }) + _ = cancel // pacify vet lostcancel return client.Conn(), nil } -func (b *Basis) initVagrantRubyRuntime() (*plugin.Client, error) { +func (b *Basis) initVagrantRubyRuntime() (rubyRuntime *plugin.Client, err error) { // TODO: Update for actual release usage. This is dev only now. // TODO: We should also locate a free port on startup and use that port _, this_dir, _, _ := runtime.Caller(0) @@ -193,15 +189,17 @@ func (b *Basis) initVagrantRubyRuntime() (*plugin.Client, error) { "VAGRANT_LOG_FILE=/tmp/vagrant.log", } - config := sr.RubyVagrantPluginConfig(b.logger) + config := serverclient.RubyVagrantPluginConfig(b.logger) config.Cmd = cmd - rubyServerClient := plugin.NewClient(config) - _, err := rubyServerClient.Start() - if err != nil { - return nil, err + rubyRuntime = plugin.NewClient(config) + if _, err = rubyRuntime.Start(); err != nil { + return } - return rubyServerClient, nil + // Ensure the plugin is halted when the basis is cleaned up + b.cleanup(func() { rubyRuntime.Kill() }) + + return } // negotiateApiVersion negotiates the API version to use and validates diff --git a/internal/core/basis.go b/internal/core/basis.go index d55930302..2aa237c49 100644 --- a/internal/core/basis.go +++ b/internal/core/basis.go @@ -218,7 +218,6 @@ func NewBasis(ctx context.Context, opts ...BasisOption) (b *Basis, err error) { b.env, err = NewEnvironment(ctx, WithHomePath(b.dir.Dir.RootDir()), - WithServerAddr(b.client.ServerTarget()), ) if err != nil { return nil, err diff --git a/internal/plugin/factory.go b/internal/plugin/factory.go index a2cff1781..a9257ee15 100644 --- a/internal/plugin/factory.go +++ b/internal/plugin/factory.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/vagrant-plugin-sdk/component" "github.com/hashicorp/vagrant-plugin-sdk/internal-shared/pluginclient" - "github.com/hashicorp/vagrant/internal/serverclient" ) // exePath contains the value of os.Executable. We cache the value because @@ -101,9 +100,13 @@ type PluginMetadata interface { SetRequestMetadata(k, v string) } -func BuiltinRubyFactory(rubyClient *serverclient.RubyVagrantClient, name string, typ component.Type) interface{} { +func BuiltinRubyFactory(rubyClient *plugin.Client, name string, typ component.Type) interface{} { return func(log hclog.Logger) (interface{}, error) { - raw, err := rubyClient.Dispense(strings.ToLower(typ.String())) + c, err := rubyClient.Client() + if err != nil { + return nil, err + } + raw, err := c.Dispense(strings.ToLower(typ.String())) if err != nil { log.Error("error requesting the ruby plugin", "type", typ, "err", err) return nil, err diff --git a/internal/runner/operation.go b/internal/runner/operation.go index ce1c96d82..c57cecb2d 100644 --- a/internal/runner/operation.go +++ b/internal/runner/operation.go @@ -21,7 +21,7 @@ import ( func (r *Runner) LoadPlugins(cfg *configpkg.Config) error { // Start with loading plugins from the Ruby runtime - plugins, err := r.vagrantRubyRuntime.GetPlugins() + plugins, err := r.vagrantRubyClient.GetPlugins() if err != nil { return err } diff --git a/internal/runner/runner.go b/internal/runner/runner.go index d199273e2..390e40e31 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -6,8 +6,8 @@ import ( "sync" "sync/atomic" - "github.com/golang/protobuf/ptypes/empty" "github.com/hashicorp/go-hclog" + plg "github.com/hashicorp/go-plugin" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -48,7 +48,8 @@ type Runner struct { id string logger hclog.Logger client *serverclient.VagrantClient - vagrantRubyRuntime *serverclient.RubyVagrantClient + vagrantRubyRuntime *plg.Client + vagrantRubyClient *serverclient.RubyVagrantClient builtinPlugins *plugin.Builtin ctx context.Context cleanupFunc func() @@ -183,27 +184,10 @@ func (r *Runner) Start() error { log.Info("runner registered with server") - // Grab Vagrant Ruby runtime information and register plugins - r.logger.Trace("fetching ruby vagrant runtime information from server") - - plugResp, err := r.client.RubyVagrantClientInfo(r.ctx, &empty.Empty{}) - if err != nil { - r.logger.Error("failed to retrieve ruby vagrant runtime information", "error", err) - return err - } - if plugin.IN_PROCESS_PLUGINS { r.builtinPlugins = plugin.NewBuiltins(context.Background(), log) } - r.vagrantRubyRuntime, err = serverclient.NewRubyVagrantClient(r.ctx, r.logger, plugResp.AddrString) - if err != nil { - r.logger.Error("failed to connect to ruby vagrant runtime", "error", err) - return err - } - - r.logger.Trace("loading all ruby based plugins from ruby vagrant runtime") - // track plugins err = r.LoadPlugins(r.opConfig) if err != nil { @@ -266,6 +250,33 @@ func WithClient(client *serverclient.VagrantClient) Option { } } +func WithVagrantRubyRuntime(vrr *plg.Client) Option { + return func(r *Runner, cfg *config) error { + r.vagrantRubyRuntime = vrr + c, err := vrr.Client() + if err != nil { + return err + } + raw, err := c.Dispense("vagrantrubyruntime") + if err != nil { + return err + } + rvc, ok := raw.(serverclient.RubyVagrantClient) + if !ok { + panic("failed to dispense RubyVagrantClient") + } + r.vagrantRubyClient = &rvc + // cln := r.cleanupFunc + // r.cleanupFunc = func() { + // if cln != nil { + // cln() + // } + // c.Close() + // } + return nil + } +} + // WithComponentFactory sets a factory for a component type. If this isn't set for // a component type, then the builtins will be used. func WithComponentFactory(t component.Type, f *factory.Factory) Option { diff --git a/internal/server/ruby_vagrant_server.go b/internal/server/ruby_vagrant_server.go deleted file mode 100644 index 51db33612..000000000 --- a/internal/server/ruby_vagrant_server.go +++ /dev/null @@ -1,43 +0,0 @@ -package server - -import ( - "context" - "errors" - - "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-plugin" - "google.golang.org/grpc" - - "github.com/hashicorp/vagrant-plugin-sdk/internal-shared/pluginclient" - "github.com/hashicorp/vagrant/internal/server/proto/ruby_vagrant" - "github.com/hashicorp/vagrant/internal/serverclient" -) - -type RubyVagrant interface { - GetPlugins() ([]*ruby_vagrant.Plugin, error) - ParseVagrantfile(string) (*ruby_vagrant.Vagrantfile, error) -} - -// This is the implementation of plugin.GRPCPlugin so we can serve/consume this. -type RubyVagrantPlugin struct { - plugin.NetRPCUnsupportedPlugin - - Impl RubyVagrant -} - -func RubyVagrantPluginConfig(log hclog.Logger) *plugin.ClientConfig { - log = log.Named("vagrant-ruby") - config := pluginclient.ClientConfig(log) - config.Logger = log - config.VersionedPlugins[1]["rubyvagrantserver"] = &RubyVagrantPlugin{} - return config -} - -// No go implementation -func (p *RubyVagrantPlugin) GRPCServer(broker *plugin.GRPCBroker, s *grpc.Server) error { - return errors.New("vagrant ruby runtime server not implemented") -} - -func (p *RubyVagrantPlugin) GRPCClient(ctx context.Context, broker *plugin.GRPCBroker, c *grpc.ClientConn) (interface{}, error) { - return serverclient.WrapRubyVagrantClient(c), nil -} diff --git a/internal/server/singleprocess/service.go b/internal/server/singleprocess/service.go index 1f8cb467b..f9e525fd0 100644 --- a/internal/server/singleprocess/service.go +++ b/internal/server/singleprocess/service.go @@ -4,7 +4,6 @@ import ( "github.com/boltdb/bolt" "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-plugin" "github.com/hashicorp/vagrant-plugin-sdk/proto/vagrant_plugin_sdk" "github.com/hashicorp/vagrant/internal/server" @@ -19,9 +18,6 @@ type service struct { // safely mutating server state. state *state.State - // client for the running instance of the Vagrant Ruby runtime - vagrantRubyRuntime *plugin.Client - // id is our unique server ID. id string @@ -133,11 +129,4 @@ func WithAcceptURLTerms(accept bool) Option { } } -func WithVagrantRubyRuntime(vrr *plugin.Client) Option { - return func(s *service, cfg *config) error { - s.vagrantRubyRuntime = vrr - return nil - } -} - var _ vagrant_server.VagrantServer = (*service)(nil) diff --git a/internal/server/singleprocess/service_server.go b/internal/server/singleprocess/service_server.go index 04499a253..be0dc790b 100644 --- a/internal/server/singleprocess/service_server.go +++ b/internal/server/singleprocess/service_server.go @@ -2,7 +2,6 @@ package singleprocess import ( "context" - "fmt" "github.com/golang/protobuf/ptypes/empty" "google.golang.org/grpc/codes" @@ -42,18 +41,3 @@ func (s *service) GetServerConfig( return &vagrant_server.GetServerConfigResponse{Config: cfg}, nil } - -func (s *service) RubyVagrantClientInfo( - ctx context.Context, - _ *empty.Empty, -) (*vagrant_server.RubyVagrantClientInfoResponse, error) { - addr, err := s.vagrantRubyRuntime.Start() - if err != nil { - return nil, fmt.Errorf("failed to determine vagrant ruby runtime server information: " + err.Error()) - } - - return &vagrant_server.RubyVagrantClientInfoResponse{ - AddrNetwork: addr.Network(), - AddrString: addr.String(), - }, nil -} diff --git a/internal/serverclient/ruby_client.go b/internal/serverclient/ruby_client.go index d7f1e6853..ae415a5fb 100644 --- a/internal/serverclient/ruby_client.go +++ b/internal/serverclient/ruby_client.go @@ -2,7 +2,7 @@ package serverclient import ( "context" - "fmt" + "errors" "github.com/golang/protobuf/ptypes/empty" "github.com/hashicorp/go-hclog" @@ -10,63 +10,48 @@ import ( "google.golang.org/grpc" "github.com/hashicorp/vagrant-plugin-sdk/internal-shared/pluginclient" - - "github.com/hashicorp/vagrant/internal/protocolversion" "github.com/hashicorp/vagrant/internal/server/proto/ruby_vagrant" ) +type RubyVagrant interface { + GetPlugins() ([]*ruby_vagrant.Plugin, error) + ParseVagrantfile(string) (*ruby_vagrant.Vagrantfile, error) +} + +// This is the implementation of plugin.GRPCPlugin so we can serve/consume this. +type RubyVagrantPlugin struct { + plugin.NetRPCUnsupportedPlugin + + Impl RubyVagrant +} + type RubyVagrantClient struct { - conn *grpc.ClientConn - client ruby_vagrant.RubyVagrantClient - plugins plugin.PluginSet + broker *plugin.GRPCBroker + client ruby_vagrant.RubyVagrantClient + ctx context.Context } -func NewRubyVagrantClient(ctx context.Context, log hclog.Logger, addr string) (*RubyVagrantClient, error) { +func RubyVagrantPluginConfig(log hclog.Logger) *plugin.ClientConfig { log = log.Named("vagrant-ruby-runtime") - conn, err := grpc.DialContext(ctx, addr, - grpc.WithBlock(), - grpc.WithInsecure(), - grpc.WithUnaryInterceptor(protocolversion.UnaryClientInterceptor(protocolversion.Current())), - grpc.WithStreamInterceptor(protocolversion.StreamClientInterceptor(protocolversion.Current())), - grpc.WithChainUnaryInterceptor( - logClientUnaryInterceptor(log, false), - ), - ) - if err != nil { - return nil, err - } + config := pluginclient.ClientConfig(log) + config.Logger = log + config.VersionedPlugins[1]["vagrantrubyruntime"] = &RubyVagrantPlugin{} + return config +} - return &RubyVagrantClient{ - conn: conn, - client: ruby_vagrant.NewRubyVagrantClient(conn), - plugins: pluginclient.ClientConfig(hclog.L()).VersionedPlugins[1], +// No Go server implementation. Server is provided by the Vagrant Ruby runtime +func (p *RubyVagrantPlugin) GRPCServer(broker *plugin.GRPCBroker, s *grpc.Server) error { + return errors.New("vagrant ruby runtime server not implemented") +} + +func (p *RubyVagrantPlugin) GRPCClient(ctx context.Context, broker *plugin.GRPCBroker, c *grpc.ClientConn) (interface{}, error) { + return RubyVagrantClient{ + broker: broker, + client: ruby_vagrant.NewRubyVagrantClient(c), + ctx: ctx, }, nil } -func WrapRubyVagrantClient(conn *grpc.ClientConn) *RubyVagrantClient { - return &RubyVagrantClient{ - conn: conn, - client: ruby_vagrant.NewRubyVagrantClient(conn), - plugins: pluginclient.ClientConfig(hclog.L()).VersionedPlugins[1], - } -} - -func (r *RubyVagrantClient) Dispense(name string) (interface{}, error) { - raw, ok := r.plugins[name] - if !ok { - hclog.L().Warn("unknown ruby plugin type", "name", name, "plugins", r.plugins) - return nil, fmt.Errorf("unknown ruby runtime plugin type: %s", name) - } - - p, ok := raw.(plugin.GRPCPlugin) - if !ok { - return nil, fmt.Errorf("plugin %s doesn't support ruby runtime grpc", name) - } - - return p.GRPCClient(context.Background(), &plugin.GRPCBroker{}, r.conn) - -} - func (r *RubyVagrantClient) GetPlugins() ([]*ruby_vagrant.Plugin, error) { plugins, err := r.client.GetPlugins(context.Background(), &empty.Empty{}) if err != nil { @@ -86,7 +71,3 @@ func (r *RubyVagrantClient) ParseVagrantfile(path string) (*ruby_vagrant.Vagrant } return vf.Vagrantfile, nil } - -func (r *RubyVagrantClient) ServerTarget() string { - return r.conn.Target() -}