From 76bc13e3bf88c24ba1733c699deadb364420f14e Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Wed, 15 Jul 2020 12:40:13 -0400 Subject: [PATCH] ansible: limit_hosts now takes precedence over blacklisted_hostnames From now, 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. Note: adding unit tests coverage for both parameters. Change-Id: I2e9fc7b9e9005fce7d956f1b936054e540b39849 Closes-Bug: #1857298 --- ...limit_over_blacklist-3ce81ecf04b09997.yaml | 10 +++ tripleo_common/actions/ansible.py | 11 +-- tripleo_common/tests/actions/test_ansible.py | 84 +++++++++++++++++++ 3 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/limit_over_blacklist-3ce81ecf04b09997.yaml 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 001c41aa9..bffc52c27 100644 --- a/tripleo_common/actions/ansible.py +++ b/tripleo_common/actions/ansible.py @@ -346,8 +346,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]) @@ -379,11 +385,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):