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 e508b8e99..ada06c6ac 100644 --- a/tripleo_common/actions/ansible.py +++ b/tripleo_common/actions/ansible.py @@ -347,8 +347,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]) @@ -380,11 +386,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 2fac74a06..a7181f925 100644 --- a/tripleo_common/tests/actions/test_ansible.py +++ b/tripleo_common/tests/actions/test_ansible.py @@ -85,6 +85,90 @@ class AnsiblePlaybookActionTest(base.TestCase): '--check', '--diff', 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, config_download_args=['--check', + '--diff']) + 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_dense,tripleo_profile_tasks,tripleo_states', + 'ANSIBLE_STDOUT_CALLBACK': 'tripleo_dense', + 'PROFILE_TASKS_TASK_OUTPUT_LIMIT': '20', + } + 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), + '--check', '--diff', 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, config_download_args=['--check', + '--diff']) + 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_dense,tripleo_profile_tasks,tripleo_states', + 'ANSIBLE_STDOUT_CALLBACK': 'tripleo_dense', + 'PROFILE_TASKS_TASK_OUTPUT_LIMIT': '20', + } + 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), '--check', '--diff', + 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):