Ensure play order is obeyed (#49897)

* Ensure play order is obeyed

 it was being ignored depending on other options
 also added tests for each order (except shuffle) both serial and not
 fixes #49846

8 files changed, 59 insertions, 3 deletions

diff --git a/changelogs/fragments/fix_order_serial.yml b/changelogs/fragments/fix_order_serial.yml
new file mode 100644
index 0000000000..6b0de87a54
--- /dev/null
+++ b/changelogs/fragments/fix_order_serial.yml
@@ -0,0 +1,2 @@
+bugfixes:
+    - play order is now applied under all circumstances, fixes #49846
diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py
index ab40ce6723..5a32a1d52b 100644
--- a/lib/ansible/executor/play_iterator.py
+++ b/lib/ansible/executor/play_iterator.py
@@ -190,7 +190,7 @@ class PlayIterator:
 
         self._host_states = {}
         start_at_matched = False
-        batch = inventory.get_hosts(self._play.hosts)
+        batch = inventory.get_hosts(self._play.hosts, order=self._play.order)
         self.batch_size = len(batch)
         for host in batch:
             self._host_states[host.name] = HostState(blocks=self._blocks)
diff --git a/lib/ansible/executor/playbook_executor.py b/lib/ansible/executor/playbook_executor.py
index 51c42b5021..7b2c7e07ea 100644
--- a/lib/ansible/executor/playbook_executor.py
+++ b/lib/ansible/executor/playbook_executor.py
@@ -255,7 +255,7 @@ class PlaybookExecutor:
         '''
 
         # make sure we have a unique list of hosts
-        all_hosts = self._inventory.get_hosts(play.hosts)
+        all_hosts = self._inventory.get_hosts(play.hosts, order=play.order)
         all_hosts_len = len(all_hosts)
 
         # the serial value can be listed as a scalar or a list of
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index 95dc140b2e..d3a136d258 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -226,7 +226,9 @@ class StrategyBase:
         # make sure that all of the hosts are advanced to their final task.
         # This should be safe, as everything should be ITERATING_COMPLETE by
         # this point, though the strategy may not advance the hosts itself.
-        [iterator.get_next_task_for_host(host) for host in self._inventory.get_hosts(iterator._play.hosts) if host.name not in self._tqm._unreachable_hosts]
+
+        inv_hosts = self._inventory.get_hosts(iterator._play.hosts, order=iterator._play.order)
+        [iterator.get_next_task_for_host(host) for host in inv_hosts if host.name not in self._tqm._unreachable_hosts]
 
         # save the failed/unreachable hosts, as the run_handlers()
         # method will clear that information during its execution
diff --git a/test/integration/targets/order/aliases b/test/integration/targets/order/aliases
new file mode 100644
index 0000000000..3581b8a7ed
--- /dev/null
+++ b/test/integration/targets/order/aliases
@@ -0,0 +1,3 @@
+shippable/posix/group1
+shippable/posix/group2
+shippable/posix/group3
diff --git a/test/integration/targets/order/inventory b/test/integration/targets/order/inventory
new file mode 100644
index 0000000000..11f322a192
--- /dev/null
+++ b/test/integration/targets/order/inventory
@@ -0,0 +1,9 @@
+[incremental]
+hostB
+hostA
+hostD
+hostC
+
+[incremental:vars]
+ansible_connection=local
+ansible_python_interpreter='{{ansible_playbook_python}}'
diff --git a/test/integration/targets/order/order.yml b/test/integration/targets/order/order.yml
new file mode 100644
index 0000000000..59691a2a76
--- /dev/null
+++ b/test/integration/targets/order/order.yml
@@ -0,0 +1,27 @@
+- name: just plain order
+  hosts: all
+  gather_facts: false
+  order: '{{ myorder|default("inventory")}}'
+  tasks:
+    - shell: echo '{{inventory_hostname}}' >> hostlist.txt
+
+- name: with serial
+  hosts: all
+  gather_facts: false
+  serial: 1
+  order: '{{ myorder|default("inventory")}}'
+  tasks:
+    - shell: echo '{{inventory_hostname}}' >> shostlist.txt
+
+- name: ensure everything works
+  hosts: localhost
+  gather_facts:  false
+  tasks:
+    - assert:
+        that:
+          - item.1 == hostlist[item.0]
+          - item.1 == shostlist[item.0]
+      loop: '{{ lookup("indexed_items", inputlist) }}'
+      vars:
+        hostlist: '{{lookup("file", "hostlist.txt").splitlines()}}'
+        shostlist: '{{lookup("file", "shostlist.txt").splitlines()}}'
diff --git a/test/integration/targets/order/runme.sh b/test/integration/targets/order/runme.sh
new file mode 100755
index 0000000000..16d86cca83
--- /dev/null
+++ b/test/integration/targets/order/runme.sh
@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+
+set -eux
+
+for EXTRA in '{"inputlist": ["hostB", "hostA", "hostD", "hostC"]}' \
+             '{"myorder": "inventory", "inputlist": ["hostB", "hostA", "hostD", "hostC"]}' \
+             '{"myorder": "sorted", "inputlist": ["hostA", "hostB", "hostC", "hostD"]}'  \
+		     '{"myorder": "reverse_sorted", "inputlist": ["hostD", "hostC", "hostB", "hostA"]}' \
+             '{"myorder": "reverse_inventory", "inputlist": ["hostC", "hostD", "hostA", "hostB"]}'
+do
+	rm shostlist.txt hostlist.txt || true
+	ansible-playbook order.yml --forks 1 -i inventory -e "$EXTRA" "$@"
+done