From 8b7a46781144157d70be949b40a6f33d840b121c Mon Sep 17 00:00:00 2001 From: Christian Albrecht Date: Sat, 3 Nov 2018 15:22:05 +0100 Subject: [PATCH 1/2] Add synced_folder_opts to puppet provisioner #10345 Provide the possibility to configure the synced folders created by the puppet provisioner with options supported by any of the existing synced folder plugins. Deprecate synced_folder_type and synced_folder_args. --- plugins/provisioners/puppet/config/puppet.rb | 40 ++++++++---- .../provisioners/puppet/provisioner/puppet.rb | 12 +--- .../provisioners/puppet/config/puppet_test.rb | 64 +++++++++++++++++++ .../docs/provisioning/puppet_apply.html.md | 19 ++++-- 4 files changed, 105 insertions(+), 30 deletions(-) create mode 100644 test/unit/plugins/provisioners/puppet/config/puppet_test.rb diff --git a/plugins/provisioners/puppet/config/puppet.rb b/plugins/provisioners/puppet/config/puppet.rb index 1f31cce6a..4501de9c7 100644 --- a/plugins/provisioners/puppet/config/puppet.rb +++ b/plugins/provisioners/puppet/config/puppet.rb @@ -17,8 +17,7 @@ module VagrantPlugins attr_accessor :environment_variables attr_accessor :module_path attr_accessor :options - attr_accessor :synced_folder_type - attr_accessor :synced_folder_args + attr_accessor :synced_folder_opts attr_accessor :temp_dir attr_accessor :working_directory @@ -35,22 +34,30 @@ module VagrantPlugins @module_path = UNSET_VALUE @options = [] @facter = {} - @synced_folder_type = UNSET_VALUE + @synced_folder_opts = {} @temp_dir = UNSET_VALUE @working_directory = UNSET_VALUE @structured_facts = UNSET_VALUE end - def nfs=(value) - puts "DEPRECATION: The 'nfs' setting for the Puppet provisioner is" - puts "deprecated. Please use the 'synced_folder_type' setting instead." - puts "The 'nfs' setting will be removed in the next version of Vagrant." + def synced_folder_type=(value) + puts "DEPRECATION: The 'synced_folder_type' setting for the Puppet provisioner is" + puts "deprecated. Please use the 'synced_folder_opts' setting instead." + puts "The 'synced_folder_type' setting will be removed in the next version of Vagrant." - if value - @synced_folder_type = "nfs" - else - @synced_folder_type = nil - end + @synced_folder_type = value + end + + def synced_folder_args=(value) + puts "DEPRECATION: The 'synced_folder_args' setting for the Puppet provisioner is" + puts "deprecated. Please use the 'synced_folder_opts' setting instead." + puts "The 'synced_folder_args' setting will be removed in the next version of Vagrant." + + @synced_folder_args = value + end + + def synced_folder_opts(**opts) + @synced_folder_opts = @synced_folder_opts.merge!(opts) end def merge(other) @@ -95,10 +102,15 @@ module VagrantPlugins @environment_variables = {} end + if @synced_folder_opts.eql?({}) + @synced_folder_opts[:type] = @synced_folder_type if @synced_folder_type + @synced_folder_opts[:owner] = "root" if !@synced_folder_type + @synced_folder_opts[:args] = @synced_folder_args if @synced_folder_args + @synced_folder_opts[:nfs__quiet] = true + end + @binary_path = nil if @binary_path == UNSET_VALUE @module_path = nil if @module_path == UNSET_VALUE - @synced_folder_type = nil if @synced_folder_type == UNSET_VALUE - @synced_folder_args = nil if @synced_folder_args == UNSET_VALUE @temp_dir = "/tmp/vagrant-puppet" if @temp_dir == UNSET_VALUE @working_directory = nil if @working_directory == UNSET_VALUE @structured_facts = nil if @structured_facts == UNSET_VALUE diff --git a/plugins/provisioners/puppet/provisioner/puppet.rb b/plugins/provisioners/puppet/provisioner/puppet.rb index be4b8c79f..a53189f74 100644 --- a/plugins/provisioners/puppet/provisioner/puppet.rb +++ b/plugins/provisioners/puppet/provisioner/puppet.rb @@ -28,18 +28,12 @@ module VagrantPlugins @module_paths << [path, File.join(config.temp_dir, "modules-#{key}")] end - folder_opts = {} - folder_opts[:type] = @config.synced_folder_type if @config.synced_folder_type - folder_opts[:owner] = "root" if !@config.synced_folder_type - folder_opts[:args] = @config.synced_folder_args if @config.synced_folder_args - folder_opts[:nfs__quiet] = true - if @config.environment_path.is_a?(Array) # Share the environments directory with the guest if @config.environment_path[0].to_sym == :host root_config.vm.synced_folder( File.expand_path(@config.environment_path[1], root_path), - environments_guest_path, folder_opts) + environments_guest_path, @config.synced_folder_opts) end end if @config.manifest_file @@ -48,13 +42,13 @@ module VagrantPlugins if @config.manifests_path[0].to_sym == :host root_config.vm.synced_folder( File.expand_path(@config.manifests_path[1], root_path), - manifests_guest_path, folder_opts) + manifests_guest_path, @config.synced_folder_opts) end end # Share the module paths @module_paths.each do |from, to| - root_config.vm.synced_folder(from, to, folder_opts) + root_config.vm.synced_folder(from, to, @config.synced_folder_opts) end end diff --git a/test/unit/plugins/provisioners/puppet/config/puppet_test.rb b/test/unit/plugins/provisioners/puppet/config/puppet_test.rb new file mode 100644 index 000000000..fe828af2b --- /dev/null +++ b/test/unit/plugins/provisioners/puppet/config/puppet_test.rb @@ -0,0 +1,64 @@ +require File.expand_path("../../../../../base", __FILE__) + +require Vagrant.source_root.join("plugins/provisioners/puppet/config/puppet") + +describe VagrantPlugins::Puppet::Config::Puppet do + include_context "unit" + + subject { described_class.new } + + let(:machine) { double("machine") } + + def expect_puts_deprecation_msg(setting) + expect($stdout).to receive(:puts).with(/DEPRECATION: The '#{setting}'/) + expect($stdout).to receive(:puts).with(/.*/) + expect($stdout).to receive(:puts).with(/.*/) + end + + describe "#finalize!" do + context "synced_folder_opts is not given" do + it "sets defaults when no other options are given" do + subject.finalize! + expect(subject.synced_folder_opts).to \ + eql({:owner => "root", :nfs__quiet => true}) + end + it "sets defaults when synced_folder_args is given" do + expect_puts_deprecation_msg("synced_folder_args") + subject.synced_folder_args = %w[foo] + subject.finalize! + expect(subject.synced_folder_opts).to \ + eql({:owner => "root", :args => %w[foo], :nfs__quiet => true}) + end + it "sets defaults when synced_folder_type is given" do + expect_puts_deprecation_msg("synced_folder_type") + subject.synced_folder_type = "foo" + subject.finalize! + expect(subject.synced_folder_opts).to \ + eql({:type => "foo", :nfs__quiet => true}) + end + it "sets defaults when synced_folder_type and synced_folder_args is given" do + expect_puts_deprecation_msg("synced_folder_type") + expect_puts_deprecation_msg("synced_folder_args") + subject.synced_folder_type = "foo" + subject.synced_folder_args = %w[foo] + subject.finalize! + expect(subject.synced_folder_opts).to \ + eql({:type => "foo", :args => %w[foo], :nfs__quiet => true}) + end + end + + context "synced_folder_opts is given" do + it "ignores synced_folder_type and synced_folder_args" do + expect_puts_deprecation_msg("synced_folder_type") + expect_puts_deprecation_msg("synced_folder_args") + subject.synced_folder_type = "foo" + subject.synced_folder_args = %w[foo] + subject.synced_folder_opts :foo => true, :bar => "baz" + subject.finalize! + expect(subject.synced_folder_opts).to \ + eql({:foo => true, :bar => "baz"}) + end + end + end + +end diff --git a/website/source/docs/provisioning/puppet_apply.html.md b/website/source/docs/provisioning/puppet_apply.html.md index 76a5caccb..3f9f63289 100644 --- a/website/source/docs/provisioning/puppet_apply.html.md +++ b/website/source/docs/provisioning/puppet_apply.html.md @@ -58,14 +58,19 @@ available below this section. * `options` (array of strings) - Additionally options to pass to the Puppet executable when running Puppet. -* `synced_folder_type` (string) - The type of synced folders to use when - sharing the data required for the provisioner to work properly. By default - this will use the default synced folder type. For example, you can set this - to "nfs" to use NFS synced folders. +* `synced_folder_type` (string) - **Deprecated** (use `synced_folder_opts` instead) + The type of synced folders to use when sharing the data required for the provisioner + to work properly. By default this will use the default synced folder type. + For example, you can set this to "nfs" to use NFS synced folders. -* `synced_folder_args` (array) - Arguments that are passed to the folder sync. - For example ['-a', '--delete', '--exclude=fixtures'] for the rsync sync - command. +* `synced_folder_args` (array) - **Deprecated** (use `synced_folder_opts` instead) + Arguments that are passed to the folder sync. + For example ['-a', '--delete', '--exclude=fixtures'] for the rsync sync command. + +* `synced_folder_opts` (hash) - Options to apply when creating synced folders required + for the provisioner to work properly. If not specified, this will use the default + synced folder options. All [synced folder types](/synced-folders/basic_usage.html) + and their relevant options are supported. * `temp_dir` (string) - The directory where all the data associated with the Puppet run (manifest files, modules, etc.) will be stored on the From edd31f78aa9cb01bdd4c504b9f3252377ee43316 Mon Sep 17 00:00:00 2001 From: Christian Albrecht Date: Mon, 12 Nov 2018 09:28:14 +0100 Subject: [PATCH 2/2] Address review synced_folder_type and synced_folder_args. --- plugins/provisioners/puppet/config/puppet.rb | 50 +++++--- templates/locales/en.yml | 16 +++ .../provisioners/puppet/config/puppet_test.rb | 117 +++++++++++++++--- .../docs/provisioning/puppet_apply.html.md | 29 ++++- 4 files changed, 173 insertions(+), 39 deletions(-) diff --git a/plugins/provisioners/puppet/config/puppet.rb b/plugins/provisioners/puppet/config/puppet.rb index 4501de9c7..16c5dbbf3 100644 --- a/plugins/provisioners/puppet/config/puppet.rb +++ b/plugins/provisioners/puppet/config/puppet.rb @@ -17,6 +17,8 @@ module VagrantPlugins attr_accessor :environment_variables attr_accessor :module_path attr_accessor :options + attr_accessor :synced_folder_type + attr_accessor :synced_folder_args attr_accessor :synced_folder_opts attr_accessor :temp_dir attr_accessor :working_directory @@ -34,29 +36,16 @@ module VagrantPlugins @module_path = UNSET_VALUE @options = [] @facter = {} + @synced_folder_type = UNSET_VALUE + @synced_folder_args = UNSET_VALUE @synced_folder_opts = {} @temp_dir = UNSET_VALUE @working_directory = UNSET_VALUE @structured_facts = UNSET_VALUE end - def synced_folder_type=(value) - puts "DEPRECATION: The 'synced_folder_type' setting for the Puppet provisioner is" - puts "deprecated. Please use the 'synced_folder_opts' setting instead." - puts "The 'synced_folder_type' setting will be removed in the next version of Vagrant." - - @synced_folder_type = value - end - - def synced_folder_args=(value) - puts "DEPRECATION: The 'synced_folder_args' setting for the Puppet provisioner is" - puts "deprecated. Please use the 'synced_folder_opts' setting instead." - puts "The 'synced_folder_args' setting will be removed in the next version of Vagrant." - - @synced_folder_args = value - end - def synced_folder_opts(**opts) + [:nfs, :rsync, :smb, :virtualbox].each {|sym| opts[:type] = sym.to_s if opts.key?(sym)} @synced_folder_opts = @synced_folder_opts.merge!(opts) end @@ -103,10 +92,17 @@ module VagrantPlugins end if @synced_folder_opts.eql?({}) - @synced_folder_opts[:type] = @synced_folder_type if @synced_folder_type - @synced_folder_opts[:owner] = "root" if !@synced_folder_type - @synced_folder_opts[:args] = @synced_folder_args if @synced_folder_args + @synced_folder_opts[:type] = @synced_folder_type unless @synced_folder_type == UNSET_VALUE + @synced_folder_opts[:owner] = "root" if @synced_folder_type == UNSET_VALUE + @synced_folder_opts[:args] = @synced_folder_args unless @synced_folder_args == UNSET_VALUE @synced_folder_opts[:nfs__quiet] = true + else + unless @synced_folder_type == UNSET_VALUE or @synced_folder_opts.key?(:type) + @synced_folder_opts[:type] = @synced_folder_type + end + unless @synced_folder_args == UNSET_VALUE or not @synced_folder_opts[:type] == "rsync" + @synced_folder_opts[:args] = @synced_folder_args + end end @binary_path = nil if @binary_path == UNSET_VALUE @@ -133,6 +129,22 @@ module VagrantPlugins def validate(machine) errors = _detected_errors + unless @synced_folder_type == UNSET_VALUE + machine.ui.warn(I18n.t("vagrant.provisioners.puppet.synced_folder_type_deprecation")) + if @synced_folder_opts.key?(:type) + errors << I18n.t("vagrant.provisioners.puppet.synced_folder_type_ignored", + synced_folder_type: @synced_folder_type, + type: @synced_folder_opts[:type]) + end + end + + unless @synced_folder_args == UNSET_VALUE + machine.ui.warn(I18n.t("vagrant.provisioners.puppet.synced_folder_args_deprecation")) + unless @synced_folder_opts[:type] == "rsync" + errors << I18n.t("vagrant.provisioners.puppet.synced_folder_args_ignored", args: @synced_folder_args) + end + end + # Calculate the manifests and module paths based on env this_expanded_module_paths = expanded_module_paths(machine.env.root_path) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 1a6498ce9..0384fe6e7 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2533,6 +2533,22 @@ en: machine. The fix is to run a `vagrant reload` so that the proper shared folders will be prepared and mounted on the VM. module_path_missing: "The configured module path doesn't exist: %{path}" + synced_folder_type_deprecation: |- + DEPRECATION: The 'synced_folder_type' setting for the Puppet provisioner is + deprecated. Please use the 'synced_folder_opts' setting instead. + The 'synced_folder_type' setting will be removed in the next version of Vagrant. + synced_folder_args_deprecation: |- + DEPRECATION: The 'synced_folder_args' setting for the Puppet provisioner is + deprecated. Please use the 'synced_folder_opts' setting instead. + The 'synced_folder_args' setting will be removed in the next version of Vagrant. + synced_folder_type_ignored: |- + The option 'synced_folder_type' with value '%{synced_folder_type}' was ignored + in favour of the option 'synced_folder_opts[:type]' with value '%{type}'. + Please remove the option 'synced_folder_type' and set valid 'synced_folder_opts' instead'. + synced_folder_args_ignored: |- + The option 'synced_folder_args' with value '%{args}' was ignored + because it is only applicable to the 'rsync' synced_folder type. + Please remove the option 'synced_folder_args' and set valid 'synced_folder_opts' instead'. puppet_server: cert_requires_node: |- diff --git a/test/unit/plugins/provisioners/puppet/config/puppet_test.rb b/test/unit/plugins/provisioners/puppet/config/puppet_test.rb index fe828af2b..a02cdb10e 100644 --- a/test/unit/plugins/provisioners/puppet/config/puppet_test.rb +++ b/test/unit/plugins/provisioners/puppet/config/puppet_test.rb @@ -7,12 +7,36 @@ describe VagrantPlugins::Puppet::Config::Puppet do subject { described_class.new } - let(:machine) { double("machine") } + let(:machine) { double("machine", ui: double("ui"), env: double("env", root_path: "")) } - def expect_puts_deprecation_msg(setting) - expect($stdout).to receive(:puts).with(/DEPRECATION: The '#{setting}'/) - expect($stdout).to receive(:puts).with(/.*/) - expect($stdout).to receive(:puts).with(/.*/) + def puppet_msg(key, options = {}) + I18n.t("vagrant.provisioners.puppet.#{key}", options) + end + def expect_deprecation_warning(setting) + expect(machine.ui).to receive(:warn).with(puppet_msg("#{setting}_deprecation")) + end + def expect_no_deprecation_warning(setting) + expect(machine.ui).not_to receive(:warn).with(puppet_msg("#{setting}_deprecation")) + end + + describe "#synced_folder_opts setter" do + [:nfs, :rsync, :smb, :virtualbox].each do |symbol| + it "should map synced_folder symbol :#{symbol} to :type" do + subject.synced_folder_opts symbol => symbol.to_s + expect(subject.synced_folder_opts[:type]).to eql(symbol.to_s) + end + end + it "should not change single given type" do + subject.synced_folder_opts :type => "foo" + expect(subject.synced_folder_opts[:type]).to eql("foo") + end + it "lets the given type symbol overwrite type" do + subject.synced_folder_opts :type => "foo", :nfs => true + expect(subject.synced_folder_opts[:type]).to eql("nfs") + + subject.synced_folder_opts :nfs => true, :type => "foo" + expect(subject.synced_folder_opts[:type]).to eql("nfs") + end end describe "#finalize!" do @@ -23,22 +47,18 @@ describe VagrantPlugins::Puppet::Config::Puppet do eql({:owner => "root", :nfs__quiet => true}) end it "sets defaults when synced_folder_args is given" do - expect_puts_deprecation_msg("synced_folder_args") subject.synced_folder_args = %w[foo] subject.finalize! expect(subject.synced_folder_opts).to \ eql({:owner => "root", :args => %w[foo], :nfs__quiet => true}) end it "sets defaults when synced_folder_type is given" do - expect_puts_deprecation_msg("synced_folder_type") subject.synced_folder_type = "foo" subject.finalize! expect(subject.synced_folder_opts).to \ eql({:type => "foo", :nfs__quiet => true}) end it "sets defaults when synced_folder_type and synced_folder_args is given" do - expect_puts_deprecation_msg("synced_folder_type") - expect_puts_deprecation_msg("synced_folder_args") subject.synced_folder_type = "foo" subject.synced_folder_args = %w[foo] subject.finalize! @@ -48,17 +68,80 @@ describe VagrantPlugins::Puppet::Config::Puppet do end context "synced_folder_opts is given" do - it "ignores synced_folder_type and synced_folder_args" do - expect_puts_deprecation_msg("synced_folder_type") - expect_puts_deprecation_msg("synced_folder_args") - subject.synced_folder_type = "foo" - subject.synced_folder_args = %w[foo] - subject.synced_folder_opts :foo => true, :bar => "baz" + it "should be able to assign hash" do + subject.synced_folder_opts = { foo: true } subject.finalize! - expect(subject.synced_folder_opts).to \ - eql({:foo => true, :bar => "baz"}) + expect(subject.synced_folder_opts[:foo]).to eql(true) + end + it "should set from function parameters" do + subject.synced_folder_opts :foo => true + subject.finalize! + expect(subject.synced_folder_opts[:foo]).to eql(true) + end + it "does merge synced_folder_type if it does not collide" do + subject.synced_folder_opts :foo => true, :bar => true + subject.synced_folder_type = "foo" + subject.finalize! + expect(subject.synced_folder_opts[:type]).to eql("foo") + end + [:nfs, :rsync, :smb, :virtualbox, :type].each do |symbol| + it "does not merge synced_folder_type if it collides with synced_folder_opts[:#{symbol.to_s}]" do + subject.synced_folder_opts symbol => symbol.to_s + subject.synced_folder_type = "foo" + subject.finalize! + expect(subject.synced_folder_opts[symbol]).to eql(symbol.to_s) + end + end + [:rsync, :type].each do |symbol| + it "does merge synced_folder_args if it does not collide with synced_folder_opts[:#{symbol.to_s}]" do + subject.synced_folder_opts symbol => "rsync" + subject.synced_folder_args = "foo" + subject.finalize! + expect(subject.synced_folder_opts[:args]).to eql("foo") + end end end end + describe "#validate" do + let(:result) { subject.validate(machine) } + let(:errors) { result["puppet provisioner"] } + before do + subject.module_path = "" + subject.manifests_path = [:host, "manifests"]; + end + it "does not warn when synced_folder_type is not set" do + expect_no_deprecation_warning("synced_folder_type") + subject.validate(machine) + end + it "does not warn when synced_folder_args is not set" do + expect_no_deprecation_warning("synced_folder_args") + subject.validate(machine) + end + it "does warn about deprecation when synced_folder_type is set" do + expect_deprecation_warning("synced_folder_type") + subject.synced_folder_type = "foo" + subject.validate(machine) + end + it "does warn about deprecation when synced_folder_args is set" do + expect_deprecation_warning("synced_folder_args") + subject.synced_folder_args = "foo" + subject.validate(machine) + end + [:nfs, :rsync, :smb, :virtualbox, :type].each do |symbol| + it "errors if synced_folder_type collides with synced_folder_opts[:#{symbol.to_s}]" do + expect_deprecation_warning("synced_folder_type") + subject.synced_folder_opts symbol => symbol.to_s + subject.synced_folder_type = "foo" + expect(errors).to include puppet_msg("synced_folder_type_ignored", {synced_folder_type: "foo", type: symbol.to_s}) + end + end + it "errors if synced_folder_args is given and type is not rsync" do + expect_deprecation_warning("synced_folder_args") + subject.synced_folder_opts :type => "bar" + subject.synced_folder_args = "foo" + expect(errors).to include puppet_msg("synced_folder_args_ignored", {args: "foo"}) + end + end + end diff --git a/website/source/docs/provisioning/puppet_apply.html.md b/website/source/docs/provisioning/puppet_apply.html.md index 3f9f63289..7ce07c674 100644 --- a/website/source/docs/provisioning/puppet_apply.html.md +++ b/website/source/docs/provisioning/puppet_apply.html.md @@ -61,15 +61,19 @@ available below this section. * `synced_folder_type` (string) - **Deprecated** (use `synced_folder_opts` instead) The type of synced folders to use when sharing the data required for the provisioner to work properly. By default this will use the default synced folder type. - For example, you can set this to "nfs" to use NFS synced folders. + For example, you can set this to "nfs" to use NFS synced folders. + **Warning**: Setting this option while ```synced_folder_opts[:type]``` is also set, + generates a validation error. * `synced_folder_args` (array) - **Deprecated** (use `synced_folder_opts` instead) Arguments that are passed to the folder sync. - For example ['-a', '--delete', '--exclude=fixtures'] for the rsync sync command. + For example ['-a', '--delete', '--exclude=fixtures'] for the rsync sync command. + **Warning**: Setting this option while ```synced_folder_opts[:type]``` is not set + to ```"rsync"```, generates a validation error. * `synced_folder_opts` (hash) - Options to apply when creating synced folders required for the provisioner to work properly. If not specified, this will use the default - synced folder options. All [synced folder types](/synced-folders/basic_usage.html) + synced folder options. All [synced folder types](/synced-folders/basic_usage.html#options) and their relevant options are supported. * `temp_dir` (string) - The directory where all the data associated with @@ -241,3 +245,22 @@ Vagrant.configure("2") do |config| end end ``` + +## Synced folder options + +The puppet provisioner creates synced folders required to work properly. +You are able to specify the options that are passed to the creation routine, +all [synced folder types](/synced-folders/basic_usage.html) and their +[options](/synced-folders/basic_usage.html) are supported: + +```ruby +Vagrant.configure("2") do |config| + config.vm.provision "puppet" do |puppet| + puppet.synced_folder_opts = { + type: "nfs", + # ... valid nfs options [1] + } + end +end +``` +[1] [nfs-synced-folder-options](/synced-folders/nfs.html#nfs-synced-folder-options)