From 6bd7ef48388fabd72d763d7018de9d523d8a8c09 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 27 Feb 2019 18:20:16 -0500 Subject: [PATCH] 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 (cherry picked from commit cfba6dfe9141fb7f66bf10573012bdae3f5a847a) --- changelogs/fragments/fix_order_serial.yml | 2 ++ lib/ansible/executor/play_iterator.py | 2 +- lib/ansible/executor/playbook_executor.py | 2 +- lib/ansible/plugins/strategy/__init__.py | 4 +++- test/integration/targets/order/aliases | 3 +++ test/integration/targets/order/inventory | 9 ++++++++ test/integration/targets/order/order.yml | 27 +++++++++++++++++++++++ test/integration/targets/order/runme.sh | 13 +++++++++++ 8 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/fix_order_serial.yml create mode 100644 test/integration/targets/order/aliases create mode 100644 test/integration/targets/order/inventory create mode 100644 test/integration/targets/order/order.yml create mode 100755 test/integration/targets/order/runme.sh 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 46c25496a1..cdf504828b 100644 --- a/lib/ansible/executor/play_iterator.py +++ b/lib/ansible/executor/play_iterator.py @@ -204,7 +204,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 25020c4a1c..59b178357a 100644 --- a/lib/ansible/executor/playbook_executor.py +++ b/lib/ansible/executor/playbook_executor.py @@ -234,7 +234,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 4a1e39bb30..2383743036 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -232,7 +232,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