ssh connection: use 'correct' host in all cases (#76017)

ssh plugin, use 'correct' information source in all cases
  * still fallback to pc
  * added inventory to new test
  * undef var can still show as parser error on pc
    now task_exectuer has a  more accurate error handling

changelogs/fragments/ssh_use_right_host.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - ssh connection now uses more correct host source as play_context can ignore loop/delegation variations.

lib/ansible/executor/task_executor.py

@@ -480,8 +480,19 @@ class TaskExecutor:
             raise self._loop_eval_error  # pylint: disable=raising-bad-type
 
         # if we ran into an error while setting up the PlayContext, raise it now, unless is known issue with delegation
-        if context_validation_error is not None and not (self._task.delegate_to and isinstance(context_validation_error, AnsibleUndefinedVariable)):
-            raise context_validation_error  # pylint: disable=raising-bad-type
+        # and undefined vars (correct values are in cvars later on and connection plugins, if still error, blows up there)
+        if context_validation_error is not None:
+            raiseit = True
+            if self._task.delegate_to:
+                if isinstance(context_validation_error, AnsibleUndefinedVariable):
+                    raiseit = False
+                elif isinstance(context_validation_error, AnsibleParserError):
+                    # parser error, might be cause by undef too
+                    orig_exc = getattr(context_validation_error, 'orig_exc', None)
+                    if isinstance(orig_exc, AnsibleUndefinedVariable):
+                        raiseit = False
+            if raiseit:
+                raise context_validation_error  # pylint: disable=raising-bad-type
 
         # if this task is a TaskInclude, we just return now with a success code so the
         # main thread can expand the task list for the given host
@@ -502,7 +513,7 @@ class TaskExecutor:
         # Now we do final validation on the task, which sets all fields to their final values.
         try:
             self._task.post_validate(templar=templar)
-        except AnsibleError:
+        except AnsibleError as e:
             raise
         except Exception:
             return dict(changed=False, failed=True, _ansible_no_log=self._play_context.no_log, exception=to_text(traceback.format_exc()))

lib/ansible/plugins/connection/ssh.py

@@ -1283,6 +1283,8 @@ class Connection(ConnectionBase):
 
         super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable)
 
+        self.host = self.get_option('host') or self._play_context.remote_addr
+
         display.vvv(u"ESTABLISH SSH CONNECTION FOR USER: {0}".format(self.user), host=self.host)
 
         if getattr(self._shell, "_IS_WINDOWS", False):
@@ -1327,6 +1329,8 @@ class Connection(ConnectionBase):
 
         super(Connection, self).put_file(in_path, out_path)
 
+        self.host = self.get_option('host') or self._play_context.remote_addr
+
         display.vvv(u"PUT {0} TO {1}".format(in_path, out_path), host=self.host)
         if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')):
             raise AnsibleFileNotFound("file or module does not exist: {0}".format(to_native(in_path)))
@@ -1341,6 +1345,8 @@ class Connection(ConnectionBase):
 
         super(Connection, self).fetch_file(in_path, out_path)
 
+        self.host = self.get_option('host') or self._play_context.remote_addr
+
         display.vvv(u"FETCH {0} TO {1}".format(in_path, out_path), host=self.host)
 
         # need to add / if path is rooted
@@ -1352,6 +1358,7 @@ class Connection(ConnectionBase):
     def reset(self):
 
         run_reset = False
+        self.host = self.get_option('host') or self._play_context.remote_addr
 
         # If we have a persistent ssh connection (ControlPersist), we can ask it to stop listening.
         # only run the reset if the ControlPath already exists or if it isn't configured and ControlPersist is set

test/integration/targets/delegate_to/delegate_facts_loop.yml

@@ -18,4 +18,4 @@
         that:
           - "'test' in hostvars[item]"
           - hostvars[item]['test'] == 123
-      loop: "{{ groups['all'] | difference(['localhost']) }}"
+      loop: "{{ groups['all'] | difference(['localhost'])}}"

test/integration/targets/delegate_to/inventory

@@ -7,3 +7,11 @@ testhost5 ansible_connection=fakelocal
 
 [all:vars]
 ansible_python_interpreter="{{ ansible_playbook_python }}"
+
+[delegated_vars]
+testhost6 myhost=127.0.0.3
+testhost7 myhost=127.0.0.4
+
+[delegated_vars:vars]
+ansible_host={{myhost}}
+ansible_connection=ssh

test/integration/targets/delegate_to/runme.sh

@@ -48,7 +48,7 @@ ANSIBLE_SSH_ARGS='-C -o ControlMaster=auto -o ControlPersist=60s -o UserKnownHos
 # this test is not doing what it says it does, also relies on var that should not be available
 #ansible-playbook test_loop_control.yml -v "$@"
 
-ansible-playbook test_delegate_to_loop_randomness.yml -v "$@"
+ansible-playbook test_delegate_to_loop_randomness.yml -i inventory -v "$@"
 
 ansible-playbook delegate_and_nolog.yml -i inventory -v "$@"
 

test/integration/targets/delegate_to/test_delegate_to.yml

@@ -56,3 +56,27 @@
 
     - name: remove test file
       file: path={{ output_dir }}/tmp.txt state=absent
+
+
+- name: verify delegation with per host vars
+  hosts: testhost6
+  gather_facts: yes
+  tasks:
+    - debug: msg={{ansible_facts['env']}}
+
+    - name: ensure normal facts still work as expected
+      assert:
+        that:
+          - '"127.0.0.3" in ansible_facts["env"]["SSH_CONNECTION"]'
+
+    - name: Test delegate_to with other host defined using same named var
+      setup:
+      register: setup_results
+      delegate_to: testhost7
+
+    - debug: msg={{setup_results.ansible_facts.ansible_env}}
+
+    - name: verify ssh plugin resolves variable for ansible_host correctly
+      assert:
+        that:
+          - '"127.0.0.4" in setup_results.ansible_facts.ansible_env["SSH_CONNECTION"]'