Merge pull request #13740 from taru-garg-hashicorp/fix-test-ansible

Support selection of inventory argument for the ansible-playbook command, based on ansible-core version, as `--inventory` is not the strategic supported argument instead of `--inventory-file` as such this PR updates the behaviour of Vagrant to start using the new argument for when the `ansible-core` version is `>=2.19` i.e. when the deprecation was introduced

Author: taru-garg-hashicorp

plugins/provisioners/ansible/provisioner/base.rb

@@ -53,16 +53,24 @@ def initialize(machine, config)
           @gathered_version_stdout = nil
           @gathered_version_major = nil
           @gathered_version = nil
+
+          @ansible_package_version_map = {}
         end
 
         def set_and_check_compatibility_mode
           begin
-            set_gathered_ansible_version(gather_ansible_version)
+            set_gathered_ansible_version(gather_ansible_version("ansible"))
           rescue StandardError => 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
 
+          begin
+            set_gathered_ansible_package_version("ansible-core", gather_ansible_version("ansible-core"))
+          rescue StandardError => e
+             @logger.error("Error while gathering the ansible-core version: #{e}")
+          end
+          
           if @gathered_version_major
             if config.compatibility_mode == Ansible::COMPATIBILITY_MODE_AUTO
               detect_compatibility_mode
@@ -164,7 +172,7 @@ def prepare_common_command_arguments
             @command_arguments << "--limit=#{@machine.name}"
           end
 
-          @command_arguments << "--inventory-file=#{inventory_path}"
+          @command_arguments << get_inventory_argument(inventory_path)
           @command_arguments << "--extra-vars=#{extra_vars_argument}" if config.extra_vars
           @command_arguments << "--#{@lexicon[:become]}" if config.become
           @command_arguments << "--#{@lexicon[:become_user]}=#{config.become_user}" if config.become_user
@@ -175,6 +183,22 @@ def prepare_common_command_arguments
           @command_arguments << "--start-at-task=#{config.start_at_task}" if config.start_at_task
         end
 
+        def get_inventory_argument(inventory_path)
+          ansible_core_version = @ansible_package_version_map["ansible-core"]
+          # Default to --inventory-file if version info is not available
+          return "--inventory-file=#{inventory_path}" if ansible_core_version.nil? || ansible_core_version.empty?
+          
+          major = ansible_core_version[:major].to_i
+          minor = ansible_core_version[:minor].to_i
+          
+          # Use --inventory for ansible-core >= 2.19
+          if major > 2 || (major == 2 && minor >= 19)
+            "--inventory=#{inventory_path}"
+          else
+            "--inventory-file=#{inventory_path}"
+          end
+        end
+
         def prepare_common_environment_variables
           # Ensure Ansible output isn't buffered so that we receive output
           # on a task-by-task basis.
@@ -399,6 +423,26 @@ def set_gathered_ansible_version(stdout_output)
           end
         end
 
+        def set_gathered_ansible_package_version(ansible_package, stdout_output)
+          if !stdout_output.empty?
+            first_line = stdout_output.lines[0]
+            ansible_version_pattern = first_line.match(/^#{ansible_package}\s+(.+)$/)
+            if ansible_version_pattern
+              gathered_version = ansible_version_pattern.captures.first
+              gathered_version.strip!
+              if !gathered_version.empty?
+                gathered_version_major = gathered_version.match(/(\d+)\..+$/).captures[0].to_i
+                gathered_version_minor = gathered_version.match(/\d+\.(\d+)\..+$/).captures[0].to_i
+                @ansible_package_version_map[ansible_package] = {
+                  version: gathered_version,
+                  major: gathered_version_major,
+                  minor: gathered_version_minor
+                }
+              end
+            end
+          end
+        end
+
       end
     end
   end

plugins/provisioners/ansible/provisioner/guest.rb

@@ -74,11 +74,11 @@ def check_and_install_ansible
           end
         end
 
-        def gather_ansible_version
+        def gather_ansible_version(package = 'ansible')
           raw_output = ""
 
           result = @machine.communicate.execute(
-            "python3 -c \"import importlib.metadata; print('ansible ' + importlib.metadata.version('ansible'))\"",
+            "python3 -c \"import importlib.metadata; print('#{package} ' + importlib.metadata.version('#{package}'))\"",
             error_class: Ansible::Errors::AnsibleNotFoundOnGuest,
             error_key: :ansible_not_found_on_guest) do |type, output|
             if type == :stdout && output.lines[0]

plugins/provisioners/ansible/provisioner/host.rb

@@ -110,10 +110,10 @@ def execute_command_from_host(command)
           end
         end
 
-        def gather_ansible_version
+        def gather_ansible_version(package = 'ansible')
           raw_output = ''
           command = ['python3', '-c',
-                     "import importlib.metadata; print('ansible ' + importlib.metadata.version('ansible'))"]
+                     "import importlib.metadata; print('#{package} ' + importlib.metadata.version('#{package}'))"]
 
           command << {
             notify: [:stdout, :stderr]

test/unit/plugins/provisioners/ansible/provisioner_test.rb

@@ -85,6 +85,17 @@ def self.it_should_check_ansible_version
     end
   end
 
+  def self.it_should_check_ansible_core_version
+    it "executes 'Python ansible-core version check before executing 'ansible-playbook'" do
+      expect(Vagrant::Util::Subprocess).to receive(:execute)
+        .once.with('python3', '-c', "import importlib.metadata; print('ansible-core ' + importlib.metadata.version('ansible-core'))", { notify: %i[
+                     stdout stderr
+                   ] })
+      expect(Vagrant::Util::Subprocess).to receive(:execute)
+        .once.with('ansible-playbook', any_args)
+    end
+  end
+
   def self.it_should_set_arguments_and_environment_variables(
     expected_args_count = 5,
     expected_vars_count = 4,
@@ -286,6 +297,7 @@ def ensure_that_config_is_valid
 
     describe "with default options" do
       it_should_check_ansible_version
+      it_should_check_ansible_core_version
       it_should_set_arguments_and_environment_variables
       it_should_create_and_use_generated_inventory
 
@@ -383,6 +395,7 @@ def ensure_that_config_is_valid
       end
 
       it_should_check_ansible_version
+      it_should_check_ansible_core_version
       it_should_create_and_use_generated_inventory
 
       it "doesn't warn about compatibility mode auto-detection" do
@@ -1262,5 +1275,52 @@ def ensure_that_config_is_valid
         }.and_return(default_execute_result)
       end
     end
+
+    describe '#get_inventory_argument' do
+      context 'when ansible version is not detected' do
+        before do
+          config.version = nil
+          allow(subject).to receive(:gather_ansible_version).and_return("ansible #{config.version}\n...\n")
+          allow(subject).to receive(:gather_ansible_version).with("ansible-core").and_return("ansible #{config.version}\n...\n")
+        end
+
+        it 'returns the default inventory command' do
+          expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args) { |*args|
+            expect(args).to include("--inventory-file=#{generated_inventory_dir}")
+          }.and_return(default_execute_result)
+        end
+      end
+
+      context 'when ansible version is detected' do
+        describe 'version >= 2.19' do
+          before do
+            config.version = "2.19.0"
+            allow(subject).to receive(:gather_ansible_version).with("ansible").and_return("ansible #{config.version}\n...\n")
+            allow(subject).to receive(:gather_ansible_version).with("ansible-core").and_return("ansible-core #{config.version}\n...\n")
+          end
+          
+          it 'returns --inventory as the inventory command' do
+            expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args) { |*args|
+              expect(args).to include("--inventory=#{generated_inventory_dir}")
+            }.and_return(default_execute_result)
+          end
+        end
+
+        describe 'version < 2.19' do
+          before do
+            config.version = "2.18.5"
+            allow(subject).to receive(:gather_ansible_version).with("ansible").and_return("ansible #{config.version}\n...\n")
+            allow(subject).to receive(:gather_ansible_version).with("ansible-core").and_return("ansible-core #{config.version}\n...\n")
+          end
+
+          it 'returns --inventory-file as the inventory command' do
+            expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args) { |*args|
+              expect(args).to include("--inventory-file=#{generated_inventory_dir}")
+            }.and_return(default_execute_result)
+          end
+        end
+      end
+    end
+
   end
 end

test/unit/vagrant/action/builtin/box_add_test.rb

@@ -410,7 +410,7 @@ def with_web_server(path, **opts)
           }.and_return(box)
 
           allow(env[:ui]).to receive(:detail).and_call_original
-          expect(env[:ui]).to receive(:detail).with(%r{.*http://(?!#{username}).+?:(?!#{password}).+?@127\.0\.0\.1:#{port}/#{box_path.basename}.*}).
+          expect(env[:ui]).to receive(:detail).with(/.*http:\/\/\*+(?::\*+)?@127\.0\.0\.1:#{port}\/#{box_path.basename}.*/).
             and_call_original
           expect(app).to receive(:call).with(env)