diff --git a/releasenotes/notes/limit_over_blacklist-3ce81ecf04b09997.yaml b/releasenotes/notes/limit_over_blacklist-3ce81ecf04b09997.yaml new file mode 100644 index 000000000..734cc406f --- /dev/null +++ b/releasenotes/notes/limit_over_blacklist-3ce81ecf04b09997.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fix `bug 1887692 `__ so + limit_hosts will take precedence over the blacklisted_hostnames. + And therefore Ansible won't be run with two --limit if both limit hosts + and blacklisted hostnames are in use. When we want to run Ansible on + specific hosts, we will ignore the blacklisted nodes and assume we know + what we do. In the case of the scale-down scenario, the unreachable nodes + are ignored. diff --git a/tripleo_common/actions/ansible.py b/tripleo_common/actions/ansible.py index 3793a9ffa..2a87510d8 100644 --- a/tripleo_common/actions/ansible.py +++ b/tripleo_common/actions/ansible.py @@ -527,8 +527,14 @@ class AnsiblePlaybookAction(base.TripleOAction): else: command = [ansible_playbook_cmd, self.playbook] + # --limit should always take precedence over blacklisted hosts. + # https://bugzilla.redhat.com/show_bug.cgi?id=1857298 if self.limit_hosts: command.extend(['--limit', self.limit_hosts]) + elif self.blacklisted_hostnames: + host_pattern = ':'.join( + ['!%s' % h for h in self.blacklisted_hostnames if h]) + command.extend(['--limit', host_pattern]) if self.module_path: command.extend(['--module-path', self.module_path]) @@ -560,11 +566,6 @@ class AnsiblePlaybookAction(base.TripleOAction): if self.inventory: command.extend(['--inventory-file', self.inventory]) - if self.blacklisted_hostnames: - host_pattern = ':'.join( - ['!%s' % h for h in self.blacklisted_hostnames if h]) - command.extend(['--limit', host_pattern]) - if self.tags: command.extend(['--tags', self.tags]) diff --git a/tripleo_common/tests/actions/test_ansible.py b/tripleo_common/tests/actions/test_ansible.py index 45697a688..9fd9efe29 100644 --- a/tripleo_common/tests/actions/test_ansible.py +++ b/tripleo_common/tests/actions/test_ansible.py @@ -126,6 +126,86 @@ class AnsiblePlaybookActionTest(base.TestCase): env_variables=env, cwd=action.work_dir, log_errors=processutils.LogErrors.ALL) + @mock.patch("tripleo_common.actions.ansible.write_default_ansible_cfg") + @mock.patch("oslo_concurrency.processutils.execute") + def test_run_with_limit(self, mock_execute, mock_write_cfg): + + mock_execute.return_value = ('', '') + + action = ansible.AnsiblePlaybookAction( + playbook=self.playbook, limit_hosts=['compute35'], + blacklisted_hostnames=['compute21'], + remote_user=self.remote_user, become=self.become, + become_user=self.become_user, extra_vars=self.extra_vars, + verbosity=self.verbosity) + ansible_config_path = os.path.join(action.work_dir, 'ansible.cfg') + mock_write_cfg.return_value = ansible_config_path + + action.run(self.ctx) + + mock_write_cfg.assert_called_once_with(action.work_dir, + self.remote_user, + ssh_private_key=None, + override_ansible_cfg=None) + + pb = os.path.join(action.work_dir, 'playbook.yaml') + env = { + 'HOME': action.work_dir, + 'ANSIBLE_LOCAL_TEMP': action.work_dir, + 'ANSIBLE_CONFIG': ansible_config_path, + 'ANSIBLE_CALLBACK_WHITELIST': 'tripleo,profile_tasks', + 'ANSIBLE_STDOUT_CALLBACK': 'tripleo', + 'PROFILE_TASKS_TASK_OUTPUT_LIMIT': '0', + } + python_version = sys.version_info.major + ansible_playbook_cmd = 'ansible-playbook-{}'.format(python_version) + mock_execute.assert_called_once_with( + ansible_playbook_cmd, '-v', pb, '--limit', "['compute35']", + '--become', '--become-user', + self.become_user, '--extra-vars', json.dumps(self.extra_vars), + env_variables=env, cwd=action.work_dir, + log_errors=processutils.LogErrors.ALL) + + @mock.patch("tripleo_common.actions.ansible.write_default_ansible_cfg") + @mock.patch("oslo_concurrency.processutils.execute") + def test_run_with_blacklist(self, mock_execute, mock_write_cfg): + + mock_execute.return_value = ('', '') + + action = ansible.AnsiblePlaybookAction( + playbook=self.playbook, limit_hosts=None, + blacklisted_hostnames=['compute21'], + remote_user=self.remote_user, become=self.become, + become_user=self.become_user, extra_vars=self.extra_vars, + verbosity=self.verbosity) + ansible_config_path = os.path.join(action.work_dir, 'ansible.cfg') + mock_write_cfg.return_value = ansible_config_path + + action.run(self.ctx) + + mock_write_cfg.assert_called_once_with(action.work_dir, + self.remote_user, + ssh_private_key=None, + override_ansible_cfg=None) + + pb = os.path.join(action.work_dir, 'playbook.yaml') + env = { + 'HOME': action.work_dir, + 'ANSIBLE_LOCAL_TEMP': action.work_dir, + 'ANSIBLE_CONFIG': ansible_config_path, + 'ANSIBLE_CALLBACK_WHITELIST': 'tripleo,profile_tasks', + 'ANSIBLE_STDOUT_CALLBACK': 'tripleo', + 'PROFILE_TASKS_TASK_OUTPUT_LIMIT': '0', + } + python_version = sys.version_info.major + ansible_playbook_cmd = 'ansible-playbook-{}'.format(python_version) + mock_execute.assert_called_once_with( + ansible_playbook_cmd, '-v', pb, '--limit', '!compute21', + '--become', '--become-user', + self.become_user, '--extra-vars', json.dumps(self.extra_vars), + env_variables=env, cwd=action.work_dir, + log_errors=processutils.LogErrors.ALL) + @mock.patch("tripleo_common.actions.ansible.write_default_ansible_cfg") @mock.patch("oslo_concurrency.processutils.execute") def test_post_message(self, mock_execute, mock_write_cfg):