From dc3b6341e2f9f410362a029d487e7409d04456a3 Mon Sep 17 00:00:00 2001 From: Gilles Cornu Date: Fri, 1 Sep 2017 08:05:50 +0200 Subject: [PATCH] provisioners/ansible: Check compatibility conflicts Vagrant will verify that the current Ansible version does support the requested compatibility mode (only applicable if not "auto", of course). As mentioned in the documentation, there is no sanity checks between `version` option and `compatibility_mode` option. With this change, the host-based provisioner is also improved to execute only once the "ansible" command (and store the gathered information for multiple usages like version requirement and compatibility checks). On the other hand, the guest-based provisioner can still potentially execute "ansible" twice (once in the AnsibleInstalled cap, and via "gather_ansible_version" function via Base::set_compatibility_mode). --- plugins/provisioners/ansible/errors.rb | 4 + .../provisioners/ansible/provisioner/base.rb | 93 ++++++++++++------- .../provisioners/ansible/provisioner/guest.rb | 9 +- .../provisioners/ansible/provisioner/host.rb | 19 ++-- templates/locales/en.yml | 7 +- .../provisioners/ansible/provisioner_test.rb | 44 ++++++--- .../docs/provisioning/ansible_common.html.md | 10 +- 7 files changed, 124 insertions(+), 62 deletions(-) diff --git a/plugins/provisioners/ansible/errors.rb b/plugins/provisioners/ansible/errors.rb index efc177ef8..a0504e58d 100644 --- a/plugins/provisioners/ansible/errors.rb +++ b/plugins/provisioners/ansible/errors.rb @@ -26,6 +26,10 @@ module VagrantPlugins class AnsibleVersionMismatch < AnsibleError error_key(:ansible_version_mismatch) end + + class AnsibleCompatibilityModeConflict < AnsibleError + error_key(:ansible_compatibility_mode_conflict) + end end end end \ No newline at end of file diff --git a/plugins/provisioners/ansible/provisioner/base.rb b/plugins/provisioners/ansible/provisioner/base.rb index 021fc29e7..fd4044da5 100644 --- a/plugins/provisioners/ansible/provisioner/base.rb +++ b/plugins/provisioners/ansible/provisioner/base.rb @@ -46,45 +46,31 @@ module VagrantPlugins @environment_variables = {} @inventory_machines = {} @inventory_path = nil + + @gathered_version_stdout = nil + @gathered_version_major = nil + @gathered_version = nil end def set_compatibility_mode - if config.compatibility_mode == Ansible::COMPATIBILITY_MODE_AUTO - detect_compatibility_mode(gather_ansible_version) - end - - unless Ansible::COMPATIBILITY_MODES.slice(1..-1).include?(config.compatibility_mode) - raise "Programming Error: compatibility_mode must correctly set at this stage!" - end - - @lexicon = ANSIBLE_PARAMETER_NAMES[config.compatibility_mode] - end - - def detect_compatibility_mode(ansible_version_stdoutput) - if config.compatibility_mode != Ansible::COMPATIBILITY_MODE_AUTO - raise "Programming Error: detect_compatibility_mode() shouldn't have been called." - end - begin - first_line = ansible_version_stdoutput.lines[0] - full_version = first_line.match(/ansible (\d)(\.\d+){1,}/) + set_gathered_ansible_version(gather_ansible_version) + rescue Exception => e + # Nothing to do here, as the fallback on safe compatibility_mode is done below + @logger.error("Error while gathering the ansible version: #{e.to_s}") + end - if full_version - major_version, _ = full_version.captures - - if major_version.to_i <= 1 - config.compatibility_mode = Ansible::COMPATIBILITY_MODE_V1_8 - else - config.compatibility_mode = Ansible::COMPATIBILITY_MODE_V2_0 - end - - @machine.env.ui.warn(I18n.t("vagrant.provisioners.ansible.compatibility_mode_warning", - compatibility_mode: config.compatibility_mode, - ansible_version: full_version) + - "\n") + if @gathered_version_major + if config.compatibility_mode == Ansible::COMPATIBILITY_MODE_AUTO + detect_compatibility_mode + elsif @gathered_version_major.to_i < 2 && config.compatibility_mode == Ansible::COMPATIBILITY_MODE_V2_0 + # A better version comparator will be needed + # when more compatibility modes come... but so far let's keep it simple! + raise Ansible::Errors::AnsibleCompatibilityModeConflict, + ansible_version: @gathered_version, + system: @control_machine, + compatibility_mode: config.compatibility_mode end - rescue - # Nothing to do here, the fallback to default compatibility_mode is done below end if config.compatibility_mode == Ansible::COMPATIBILITY_MODE_AUTO @@ -92,9 +78,15 @@ module VagrantPlugins @machine.env.ui.warn(I18n.t("vagrant.provisioners.ansible.compatibility_mode_not_detected", compatibility_mode: config.compatibility_mode, - gathered_version: ansible_version_stdoutput) + + gathered_version: @gathered_version_stdout) + "\n") end + + unless Ansible::COMPATIBILITY_MODES.slice(1..-1).include?(config.compatibility_mode) + raise "Programming Error: compatibility_mode must correctly set at this stage!" + end + + @lexicon = ANSIBLE_PARAMETER_NAMES[config.compatibility_mode] end def check_files_existence @@ -357,6 +349,39 @@ module VagrantPlugins end end + private + + def detect_compatibility_mode + if !@gathered_version_major || config.compatibility_mode != Ansible::COMPATIBILITY_MODE_AUTO + raise "Programming Error: detect_compatibility_mode() shouldn't have been called." + end + + if @gathered_version_major.to_i <= 1 + config.compatibility_mode = Ansible::COMPATIBILITY_MODE_V1_8 + else + config.compatibility_mode = Ansible::COMPATIBILITY_MODE_V2_0 + end + + @machine.env.ui.warn(I18n.t("vagrant.provisioners.ansible.compatibility_mode_warning", + compatibility_mode: config.compatibility_mode, + ansible_version: @gathered_version) + + "\n") + end + + def set_gathered_ansible_version(stdout_output) + @gathered_version_stdout = stdout_output + if !@gathered_version_stdout.empty? + first_line = @gathered_version_stdout.lines[0] + ansible_version_pattern = first_line.match(/(^ansible\s+)(.+)$/) + if ansible_version_pattern + _, @gathered_version, _ = ansible_version_pattern.captures + if @gathered_version + @gathered_version_major = @gathered_version.match(/^(\d)\..+$/).captures[0].to_i + end + end + end + end + end end end diff --git a/plugins/provisioners/ansible/provisioner/guest.rb b/plugins/provisioners/ansible/provisioner/guest.rb index c821a256a..ea8d89603 100644 --- a/plugins/provisioners/ansible/provisioner/guest.rb +++ b/plugins/provisioners/ansible/provisioner/guest.rb @@ -66,19 +66,22 @@ module VagrantPlugins if (!config.version.empty? && config.version.to_s.to_sym != :latest && !@machine.guest.capability(:ansible_installed, config.version)) - raise Ansible::Errors::AnsibleVersionMismatch, system: @control_machine, required_version: config.version.to_s + raise Ansible::Errors::AnsibleVersionMismatch, + system: @control_machine, + required_version: config.version, + current_version: @gathered_version end end def gather_ansible_version - raw_output = nil + raw_output = "" result = @machine.communicate.execute("ansible --version", error_check: false) do |type, output| if type == :stdout && output.lines[0] raw_output = output.lines[0] end end if result != 0 - raw_output = nil + raw_output = "" end raw_output end diff --git a/plugins/provisioners/ansible/provisioner/host.rb b/plugins/provisioners/ansible/provisioner/host.rb index dceb92d45..c8f77d190 100644 --- a/plugins/provisioners/ansible/provisioner/host.rb +++ b/plugins/provisioners/ansible/provisioner/host.rb @@ -20,9 +20,9 @@ module VagrantPlugins @ssh_info = @machine.ssh_info warn_for_unsupported_platform - check_required_ansible_version unless config.version.empty? check_files_existence set_compatibility_mode + check_required_ansible_version execute_ansible_galaxy_from_host if config.galaxy_role_file execute_ansible_playbook_from_host @@ -39,15 +39,16 @@ module VagrantPlugins end def check_required_ansible_version - if config.version.to_s.to_sym == :latest - @logger.debug("The :latest version requirement is not supported (yet) by the host-based provisioner") + # Skip this check when not required, nor possible + if !@gathered_version || config.version.empty? || config.version.to_s.to_sym == :latest return end - @logger.info("Checking for Ansible version on Vagrant host...") - found_version = gather_ansible_version - if (!found_version || "ansible #{config.version}\n" != found_version.lines[0]) - raise Ansible::Errors::AnsibleVersionMismatch, system: @control_machine, required_version: config.version.to_s + if config.version != @gathered_version + raise Ansible::Errors::AnsibleVersionMismatch, + system: @control_machine, + required_version: config.version, + current_version: @gathered_version end end @@ -105,7 +106,7 @@ module VagrantPlugins end def gather_ansible_version - raw_output = nil + raw_output = "" command = %w(ansible --version) command << { @@ -119,7 +120,7 @@ module VagrantPlugins end end if result.exit_code != 0 - raw_output = nil + raw_output = "" end rescue Vagrant::Errors::CommandUnavailable raise Ansible::Errors::AnsibleNotFoundOnHost diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 5cb65c0ad..eeea26770 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2366,10 +2366,15 @@ en: https://github.com/mitchellh/vagrant ansible_version_mismatch: |- The requested Ansible version (%{required_version}) was not found on the %{system}. - Please check the Ansible installation on your Vagrant %{system} system, + Please check the Ansible installation on your Vagrant %{system} system (currently: %{current_version}), or adapt the `version` option of this provisioner in your Vagrantfile. See https://docs.vagrantup.com/v2/provisioning/ansible_common.html#version for more information. + ansible_compatibility_mode_conflict: |- + The requested Ansible compatibility mode (%{compatibility_mode}) is in conflict with + the Ansible installation on your Vagrant %{system} system (currently: %{ansible_version}). + See https://docs.vagrantup.com/v2/provisioning/ansible_common.html#compatibility_mode + for more information. config_file_not_found: |- `%{config_option}` does not exist on the %{system}: %{path} extra_vars_invalid: |- diff --git a/test/unit/plugins/provisioners/ansible/provisioner_test.rb b/test/unit/plugins/provisioners/ansible/provisioner_test.rb index 79beba577..deef6cb01 100644 --- a/test/unit/plugins/provisioners/ansible/provisioner_test.rb +++ b/test/unit/plugins/provisioners/ansible/provisioner_test.rb @@ -313,7 +313,8 @@ VF valid_versions = { "0.6": VagrantPlugins::Ansible::COMPATIBILITY_MODE_V1_8, "1.9.4": VagrantPlugins::Ansible::COMPATIBILITY_MODE_V1_8, - "2.2.1.0": VagrantPlugins::Ansible::COMPATIBILITY_MODE_V2_0, + "2.5.0.0-rc1": VagrantPlugins::Ansible::COMPATIBILITY_MODE_V2_0, + "2.x.y.z": VagrantPlugins::Ansible::COMPATIBILITY_MODE_V2_0, "4.3.2.1": VagrantPlugins::Ansible::COMPATIBILITY_MODE_V2_0, } valid_versions.each_pair do |ansible_version, mode| @@ -331,7 +332,7 @@ VF it "warns about compatibility mode auto-detection being used" do expect(machine.env.ui).to receive(:warn).with( I18n.t("vagrant.provisioners.ansible.compatibility_mode_warning", - compatibility_mode: mode, ansible_version: "ansible #{ansible_version}") + + compatibility_mode: mode, ansible_version: ansible_version) + "\n") end end @@ -339,7 +340,7 @@ VF invalid_versions = [ "ansible devel", - "ansible 2.x.y.z\n...\n", + "anything 1.2", "2.9.2.1", ] invalid_versions.each do |unknown_ansible_version| @@ -371,6 +372,7 @@ VF config.compatibility_mode = VagrantPlugins::Ansible::COMPATIBILITY_MODE_V1_8 end + it_should_check_ansible_version it_should_create_and_use_generated_inventory it "doesn't warn about compatibility mode auto-detection" do @@ -381,6 +383,7 @@ VF context "with compatibility_mode '#{VagrantPlugins::Ansible::COMPATIBILITY_MODE_V2_0}'" do before do config.compatibility_mode = VagrantPlugins::Ansible::COMPATIBILITY_MODE_V2_0 + allow(subject).to receive(:gather_ansible_version).and_return("ansible 2.3.0.0\n...\n") end it_should_create_and_use_generated_inventory @@ -389,6 +392,16 @@ VF expect(machine.env.ui).to_not receive(:warn) end + describe "and an incompatible ansible version" do + before do + allow(subject).to receive(:gather_ansible_version).and_return("ansible 1.9.3\n...\n") + end + + it "raises a compatibility conflict error", skip_before: false, skip_after: true do + expect {subject.provision}.to raise_error(VagrantPlugins::Ansible::Errors::AnsibleCompatibilityModeConflict) + end + end + describe "deprecated 'sudo' options are aliases for equivalent 'become' options" do before do # Filter the deprecation notices @@ -413,7 +426,7 @@ VF config.playbook_command = "custom-ansible-playbook" # set the compatibility mode to ensure that only ansible-playbook is excuted - config.compatibility_mode = VagrantPlugins::Ansible::COMPATIBILITY_MODE_V2_0 + config.compatibility_mode = VagrantPlugins::Ansible::COMPATIBILITY_MODE_V1_8 end it "uses custom playbook_command to run playbooks" do @@ -968,20 +981,29 @@ VF allow(subject).to receive(:gather_ansible_version).and_return("ansible 1.9.6\n...\n") end - it "raises an error about the ansible version mismatch", skip_before: true, skip_after: true do - config.finalize! + it "raises an error about the ansible version mismatch", skip_before: false, skip_after: true do expect {subject.provision}.to raise_error(VagrantPlugins::Ansible::Errors::AnsibleVersionMismatch) end end describe "and the installed ansible version cannot be detected" do before do - allow(subject).to receive(:gather_ansible_version).and_return(nil) + allow(subject).to receive(:gather_ansible_version).and_return("") end - it "raises an error about the ansible version mismatch", skip_before: true, skip_after: true do - config.finalize! - expect {subject.provision}.to raise_error(VagrantPlugins::Ansible::Errors::AnsibleVersionMismatch) + it "skips the ansible version check and executes ansible-playbook command" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args).and_return(default_execute_result) + end + end + + describe "with special value: 'latest'" do + before do + config.version = :latest + allow(subject).to receive(:gather_ansible_version).and_return("ansible 2.2.0.1\n...\n") + end + + it "skips the ansible version check and executes ansible-playbook command" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args).and_return(default_execute_result) end end end @@ -1168,7 +1190,7 @@ VF allow(machine.ui).to receive(:warn) # Set the compatibility mode to only get the Windows warning - config.compatibility_mode = VagrantPlugins::Ansible::COMPATIBILITY_MODE_V2_0 + config.compatibility_mode = VagrantPlugins::Ansible::COMPATIBILITY_MODE_V1_8 end it "warns that Windows is not officially supported for the Ansible control machine" do diff --git a/website/source/docs/provisioning/ansible_common.html.md b/website/source/docs/provisioning/ansible_common.html.md index a36f7527f..af3d95393 100644 --- a/website/source/docs/provisioning/ansible_common.html.md +++ b/website/source/docs/provisioning/ansible_common.html.md @@ -35,16 +35,18 @@ Some of these options are for advanced usage only and should not be used unless By default this option is set to `"auto"`. If Vagrant is not able to detect any supported Ansible version, it will falls back on the compatibility mode `"1.8"` with a warning. -
- Compatibility Note: - This option was introduced in Vagrant 2.0. Previous Vagrant versions behave like if this option was set to `"1.8"`. -
+ Vagrant will error if the specified compatibility mode is incompatible with the current Ansible version.
Attention: Vagrant doesn't perform any validation between the `compatibility_mode` value and the value of the [`version`](#version) option.
+
+ Compatibility Note: + This option was introduced in Vagrant 2.0. Previous Vagrant versions behave like if this option was set to `"1.8"`. +
+ - `config_file` (string) - The path to an [Ansible Configuration file](https://docs.ansible.com/intro_configuration.html). By default, this option is not set, and Ansible will [search for a possible configuration file in some default locations](/docs/provisioning/ansible_intro.html#ANSIBLE_CONFIG).