From 06350a7afc6e7359331be34932d3fbb219224908 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Mon, 16 May 2022 17:03:37 -0500 Subject: [PATCH] Port default provider selection - Pulls in the SDK changes to Project.DefaultProvider and Project.Target - Implements the hefty default provider method - Un-hard-codes provider from Target, and sets it when a Target is looked up from a Project --- builtin/otherplugin/command.go | 2 +- internal/core/project.go | 171 +++++++++++++++++- internal/core/project_test.go | 6 +- internal/core/target.go | 18 +- lib/vagrant/environment/remote.rb | 6 +- .../proto/vagrant_plugin_sdk/plugin_pb.rb | 8 + .../vagrant_plugin_sdk/plugin_services_pb.rb | 2 +- lib/vagrant/vagrantfile.rb | 1 - plugins/commands/serve/client/project.rb | 29 ++- plugins/commands/serve/client/target.rb | 2 +- plugins/commands/serve/mappers/mapper.rb | 10 +- 11 files changed, 224 insertions(+), 31 deletions(-) diff --git a/builtin/otherplugin/command.go b/builtin/otherplugin/command.go index d8e95cb28..011d44585 100644 --- a/builtin/otherplugin/command.go +++ b/builtin/otherplugin/command.go @@ -132,7 +132,7 @@ func (c *Command) ExecuteInfo(trm terminal.UI, p plugincore.Project) int32 { ptrm.Output("YAY! This is project specific output!") } - t, err := p.Target("one") + t, err := p.Target("one", "") if err != nil { trm.Output("Failed to load `one' target -- " + err.Error()) return 1 diff --git a/internal/core/project.go b/internal/core/project.go index 6cc55ff83..2cea8a78b 100644 --- a/internal/core/project.go +++ b/internal/core/project.go @@ -3,6 +3,8 @@ package core import ( "context" "errors" + "fmt" + "os" "strings" "sync" @@ -99,10 +101,159 @@ func (p *Project) DefaultPrivateKey() (path path.Path, err error) { } // DefaultProvider implements core.Project -func (p *Project) DefaultProvider() (name string, err error) { - // TODO: This needs to implement the default provider algorithm - // from https://www.vagrantup.com/docs/providers/basic_usage.html#default-provider - return "virtualbox", nil +func (p *Project) DefaultProvider(opts *core.DefaultProviderOptions) (string, error) { + logger := p.logger.Named("default-provider") + logger.Debug("Searching for default provider", "options", fmt.Sprintf("%#v", opts)) + // Algorithm ported from Vagrant::Environment#default_provider; structure + // and comments mirrored from there. + + // Implement the algorithm from + // https://www.vagrantup.com/docs/providers/basic_usage.html#default-provider + // with additional steps 2.5 and 3.5 from + // https://bugzilla.redhat.com/show_bug.cgi?id=1444492 + // to allow system-configured provider priorities. + // + // 1. The --provider flag on a vagrant up is chosen above all else, if it is + // present. + // + // (Step 1 is done by the caller; this method is only called if --provider + // wasn't given.) + // + // 2. If the VAGRANT_DEFAULT_PROVIDER environmental variable is set, it + // takes next priority and will be the provider chosen. + defaultProvider := os.Getenv("VAGRANT_DEFAULT_PROVIDER") + if defaultProvider != "" && opts.ForceDefault { + logger.Debug("Using forced default provider", "provider", defaultProvider) + return defaultProvider, nil + } + + // Get the list of providers in our configuration, in order + configProviders := []string{} + for _, m := range p.project.GetConfiguration().GetMachineConfigs() { + // If a MachineName is provided - we're only looking at providers + // scoped to that machine name + if opts.MachineName != "" && opts.MachineName != m.GetName() { + continue + } + for _, p := range m.GetConfigVm().GetProviders() { + configProviders = append(configProviders, p.GetType()) + } + } + + usableProviders := []string{} + pluginProviders, err := p.basis.plugins.ListPlugins("provider") + if err != nil { + return "", err + } + for _, pp := range pluginProviders { + // Skip excluded providers + if opts.IsExcluded(pp.Name) { + continue + } + + // TODO: how to check for defaultable? + + // Skip the providers that aren't usable. + if opts.CheckUsable { + logger.Debug("Checking usable on provider", "provider", pp.Name) + plug, err := p.basis.plugins.GetPlugin(pp.Name, pp.Type) + if err != nil { + return "", err + } + pluginImpl := plug.Plugin.(core.Provider) + usable, err := pluginImpl.Usable() + if err != nil { + return "", err + } + if !usable { + continue + } + } + + // If we made it here we have a candidate usable provider + usableProviders = append(usableProviders, pp.Name) + } + logger.Debug("Initial usable provider list", "usableProviders", usableProviders) + + // TODO: how to get and sort by provider priority? + + // If we're not forcing the default, but it's usable and hasn't been + // otherwise excluded, return it now. + for _, u := range usableProviders { + if u == defaultProvider { + logger.Debug("Using default provider as it was found in usable list", + "provider", u) + return u, nil + } + } + + // 2.5. Vagrant will go through all of the config.vm.provider calls in the + // Vagrantfile and try each in order. It will choose the first + // provider that is usable and listed in VAGRANT_PREFERRED_PROVIDERS. + preferredProviders := strings.Split(os.Getenv("VAGRANT_PREFERRED_PROVIDERS"), ",") + k := 0 + for _, pp := range preferredProviders { + spp := strings.TrimSpace(pp) // .map { s.strip } + if spp != "" { // .select { !s.empty? } + preferredProviders[k] = spp + k++ + } + } + preferredProviders = preferredProviders[:k] + + for _, cp := range configProviders { + for _, up := range usableProviders { + if cp == up { + for _, pp := range preferredProviders { + if cp == pp { + logger.Debug("Using preferred provider detected in configuration and usable", + "provider", pp) + return pp, nil + } + } + } + } + } + + // 3. Vagrant will go through all of the config.vm.provider calls in the + // Vagrantfile and try each in order. It will choose the first provider + // that is usable. For example, if you configure Hyper-V, it will never + // be chosen on Mac this way. It must be both configured and usable. + for _, cp := range configProviders { + for _, up := range usableProviders { + if cp == up { + logger.Debug("Using provider detected in configuration and usable", + "provider", cp) + return cp, nil + } + } + } + + // 3.5. Vagrant will go through VAGRANT_PREFERRED_PROVIDERS and find the + // first plugin that reports it is usable. + for _, pp := range preferredProviders { + for _, up := range usableProviders { + if pp == up { + logger.Debug("Using preffered provider found in usable list", + "provider", pp) + return pp, nil + } + } + } + + // 4. Vagrant will go through all installed provider plugins (including the + // ones that come with Vagrant), and find the first plugin that reports + // it is usable. There is a priority system here: systems that are known + // better have a higher priority than systems that are worse. For + // example, if you have the VMware provider installed, it will always + // take priority over VirtualBox. + if len(usableProviders) > 0 { + logger.Debug("Using the first provider from the usable list", + "provider", usableProviders[0]) + return usableProviders[0], nil + } + + return "", errors.New("No default provider.") } // Home implements core.Project @@ -138,13 +289,22 @@ func (p *Project) RootPath() (path path.Path, err error) { } // Target implements core.Project -func (p *Project) Target(nameOrId string) (core.Target, error) { +func (p *Project) Target(nameOrId string, provider string) (core.Target, error) { if t, ok := p.targets[nameOrId]; ok { return t, nil } // Check for name or id for _, t := range p.targets { if t.target.Name == nameOrId { + // TODO: Because we don't have provider selection fully ported + // over, there are cases where a target is loaded without a + // provider being set on it. For now we're just handling that here + // on lookup, but once we know for sure that any Target that exists + // already knows what its provider is, this should be able to be + // removed. + if t.target.Provider == "" && provider != "" { + t.target.Provider = provider + } return t, nil } if t.target.ResourceId == nameOrId { @@ -160,6 +320,7 @@ func (p *Project) Target(nameOrId string) (core.Target, error) { ResourceId: nameOrId, }, ), + WithProvider(provider), ) } diff --git a/internal/core/project_test.go b/internal/core/project_test.go index 25425a552..ddc444674 100644 --- a/internal/core/project_test.go +++ b/internal/core/project_test.go @@ -45,17 +45,17 @@ func TestProjectGetTarget(t *testing.T) { } // Get by id - one, err := tp.Target("id-one") + one, err := tp.Target("id-one", "") require.NoError(t, err) require.Equal(t, targetOne, one) // Get by name - two, err := tp.Target("target-two") + two, err := tp.Target("target-two", "") require.NoError(t, err) require.Equal(t, targetTwo, two) // Get target that doesn't exist - noexist, err := tp.Target("ohnooooo") + noexist, err := tp.Target("ohnooooo", "") require.Error(t, err) require.Nil(t, noexist) } diff --git a/internal/core/target.go b/internal/core/target.go index e0db5d089..74d3cd511 100644 --- a/internal/core/target.go +++ b/internal/core/target.go @@ -2,6 +2,7 @@ package core import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -98,6 +99,9 @@ func (t *Target) Provider() (p core.Provider, err error) { if err != nil { return nil, err } + if providerName == "" { + return nil, errors.New("cannot fetch provider for target when provider name is blank") + } provider, err := t.project.basis.component( t.ctx, component.ProviderType, providerName) @@ -112,8 +116,7 @@ func (t *Target) Provider() (p core.Provider, err error) { // ProviderName implements core.Target // TODO: Use actual value once provider is set on the go side func (t *Target) ProviderName() (string, error) { - // return t.target.Provider, nil - return "virtualbox", nil + return t.target.Provider, nil } // Communicate implements core.Target @@ -410,7 +413,7 @@ type TargetOption func(*Target) error func WithTargetName(name string) TargetOption { return func(t *Target) (err error) { - if ex, _ := t.project.Target(name); ex != nil { + if ex, _ := t.project.Target(name, ""); ex != nil { if et, ok := ex.(*Target); ok { t.target = et.target } @@ -484,4 +487,13 @@ func WithTargetRef(r *vagrant_plugin_sdk.Ref_Target) TargetOption { } } +func WithProvider(provider string) TargetOption { + return func(t *Target) (err error) { + if provider != "" { + t.target.Provider = provider + } + return nil + } +} + var _ core.Target = (*Target)(nil) diff --git a/lib/vagrant/environment/remote.rb b/lib/vagrant/environment/remote.rb index 25e625caf..2f02d3705 100644 --- a/lib/vagrant/environment/remote.rb +++ b/lib/vagrant/environment/remote.rb @@ -102,7 +102,7 @@ module Vagrant end def default_provider(**opts) - client.respond_to?(:default_provider) && client.default_provider.to_sym + client.respond_to?(:default_provider) && client.default_provider(opts) end # Gets a target (machine) by name @@ -126,8 +126,8 @@ module Vagrant # @param [String] machine name # return [Vagrant::Machine] - def machine(name, *_, **_) - client.machine(name) + def machine(name, provider, **_) + client.machine(name, provider) end def machine_names diff --git a/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb b/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb index 3ccac7622..27f2a6bde 100644 --- a/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb +++ b/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_pb.rb @@ -668,6 +668,12 @@ Google::Protobuf::DescriptorPool.generated_pool.build do add_message "hashicorp.vagrant.sdk.Project.CwdResponse" do optional :path, :string, 1 end + add_message "hashicorp.vagrant.sdk.Project.DefaultProviderRequest" do + optional :check_usable, :bool, 1 + repeated :exclude, :string, 2 + optional :force_default, :bool, 3 + optional :machine_name, :string, 4 + end add_message "hashicorp.vagrant.sdk.Project.DefaultProviderResponse" do optional :provider_name, :string, 1 end @@ -685,6 +691,7 @@ Google::Protobuf::DescriptorPool.generated_pool.build do end add_message "hashicorp.vagrant.sdk.Project.TargetRequest" do optional :name, :string, 1 + optional :provider, :string, 2 end add_message "hashicorp.vagrant.sdk.Project.TargetNamesResponse" do repeated :names, :string, 1 @@ -1114,6 +1121,7 @@ module Hashicorp Project::ActiveTargetsResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("hashicorp.vagrant.sdk.Project.ActiveTargetsResponse").msgclass Project::ConfigResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("hashicorp.vagrant.sdk.Project.ConfigResponse").msgclass Project::CwdResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("hashicorp.vagrant.sdk.Project.CwdResponse").msgclass + Project::DefaultProviderRequest = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("hashicorp.vagrant.sdk.Project.DefaultProviderRequest").msgclass Project::DefaultProviderResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("hashicorp.vagrant.sdk.Project.DefaultProviderResponse").msgclass Project::HomeResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("hashicorp.vagrant.sdk.Project.HomeResponse").msgclass Project::LocalDataResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("hashicorp.vagrant.sdk.Project.LocalDataResponse").msgclass diff --git a/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_services_pb.rb b/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_services_pb.rb index 634f7fc5b..39ccc159b 100644 --- a/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_services_pb.rb +++ b/lib/vagrant/protobufs/proto/vagrant_plugin_sdk/plugin_services_pb.rb @@ -472,7 +472,7 @@ module Hashicorp rpc :CWD, ::Google::Protobuf::Empty, ::Hashicorp::Vagrant::Sdk::Args::Path rpc :DataDir, ::Google::Protobuf::Empty, ::Hashicorp::Vagrant::Sdk::Args::DataDir::Project rpc :DefaultPrivateKey, ::Google::Protobuf::Empty, ::Hashicorp::Vagrant::Sdk::Args::Path - rpc :DefaultProvider, ::Google::Protobuf::Empty, ::Hashicorp::Vagrant::Sdk::Project::DefaultProviderResponse + rpc :DefaultProvider, ::Hashicorp::Vagrant::Sdk::Project::DefaultProviderRequest, ::Hashicorp::Vagrant::Sdk::Project::DefaultProviderResponse rpc :Home, ::Google::Protobuf::Empty, ::Hashicorp::Vagrant::Sdk::Args::Path rpc :Host, ::Google::Protobuf::Empty, ::Hashicorp::Vagrant::Sdk::Args::Host rpc :LocalData, ::Google::Protobuf::Empty, ::Hashicorp::Vagrant::Sdk::Args::Path diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb index 21dfa49ec..1a4d5006b 100644 --- a/lib/vagrant/vagrantfile.rb +++ b/lib/vagrant/vagrantfile.rb @@ -81,7 +81,6 @@ module Vagrant return Machine.new(name, provider, provider_cls, provider_config, provider_options, config, data_path, box, env, self) end - # Returns the configuration for a single machine. # # When loading a box Vagrantfile, it will be prepended to the diff --git a/plugins/commands/serve/client/project.rb b/plugins/commands/serve/client/project.rb index 8b4bd227e..a559c75f8 100644 --- a/plugins/commands/serve/client/project.rb +++ b/plugins/commands/serve/client/project.rb @@ -54,10 +54,15 @@ module VagrantPlugins resp.path end - # return [String] - def default_provider - resp = client.default_provider(Empty.new) - resp.provider_name + def default_provider(opts) + req = ::Hashicorp::Vagrant::Sdk::Project::DefaultProviderRequest.new( + exclude: opts.fetch(:exclude, []), + force_default: opts.fetch(:force_default, true), + check_usable: opts.fetch(:check_usable, true), + machine_name: opts[:machine], + ) + resp = client.default_provider(req) + resp.provider_name.to_sym end # return [String] @@ -79,8 +84,11 @@ module VagrantPlugins end # return [Vagrant::Machine] - def machine(name) - t = client.target(SDK::Project::TargetRequest.new(name: name)) + def machine(name, provider) + t = client.target(SDK::Project::TargetRequest.new( + name: name, + provider: provider, + )) machine = mapper.map(t, to: Vagrant::Machine) return machine end @@ -124,9 +132,14 @@ module VagrantPlugins # Returns a machine client for the given name # return [VagrantPlugins::CommandServe::Client::Target::Machine] - def target(name) + def target(name, provider) target = Target.load( - client.target(SDK::Project::TargetRequest.new(name: name)), + client.target( + SDK::Project::TargetRequest.new( + name: name, + provider: provider, + ) + ), broker: broker ) target.to_machine diff --git a/plugins/commands/serve/client/target.rb b/plugins/commands/serve/client/target.rb index e11c5723c..08ae6dd4e 100644 --- a/plugins/commands/serve/client/target.rb +++ b/plugins/commands/serve/client/target.rb @@ -13,7 +13,7 @@ module VagrantPlugins :DESTROYED, ].freeze - # @return [SDK::Ref::Target] proto reference for this target + # @return [Hashicorp::Vagrant::Sdk::Ref::Target] proto reference for this target def ref SDK::Ref::Target.new(resource_id: resource_id) end diff --git a/plugins/commands/serve/mappers/mapper.rb b/plugins/commands/serve/mappers/mapper.rb index 6e00c3934..85b9548bb 100644 --- a/plugins/commands/serve/mappers/mapper.rb +++ b/plugins/commands/serve/mappers/mapper.rb @@ -20,14 +20,14 @@ module VagrantPlugins attr_reader :name # @return [Class] type of the argument attr_reader :type - # @return [Callable] callable that can validate argument + # @return [#call] callable that can validate argument attr_reader :validator # Create a new input # # @param type [Class] Type of the input - # @param validator [Callable] Callable to validate argument (optional) - # @yield Callable to validate argument (optional) + # @param validator [#call] Callable to validate argument (optional) + # @yield #call to validate argument (optional) def initialize(type:, validator: nil, &block) if !type.is_a?(Class) && !type.is_a?(Module) raise ArgumentError, @@ -107,14 +107,14 @@ module VagrantPlugins attr_reader :inputs # @return [Class, nil] type of output attr_reader :output - # @return [Callable] callable to perform mapping + # @return [#call] callable to perform mapping attr_reader :func # Create a new mapper instance # # @param inputs [Array] List of inputs for mapper # @param output [Class] Type of output value - # @param func [Callable] Callable to perform mapping + # @param func [#call] Callable to perform mapping def initialize(inputs:, output:, func:) Array(inputs).each do |i| if !i.is_a?(Input)