From 272beb2d192b965f30435106938a06a2ba051408 Mon Sep 17 00:00:00 2001 From: Alex Schultz Date: Mon, 4 Feb 2019 13:36:29 -0700 Subject: [PATCH] Run hostname check for standalone Run a hostname preflight check when running the standalone as this can cause issues with rabbit/mysql/pacemaker/puppet. The standalone check is not run when an undercloud is being deployed or if output only is specified. The undercloud has a different set of preflight checks run prior to deployment so we do not need to rerun the hostname check when the undercloud is being deployed. Conflicts: tripleoclient/tests/test_utils.py tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py tripleoclient/utils.py tripleoclient/v1/tripleo_deploy.py tripleoclient/v1/undercloud_preflight.py Depends-On: https://review.openstack.org/#/c/637309/ Change-Id: I4de51d792f004575e0e802a1d340478d6ae9f848 Closes-Bug: #1814564 (cherry picked from commit 63f4175e768796970801a36dee9585b3d89a9531) --- tripleoclient/tests/test_utils.py | 40 +++++++++ .../tests/v1/tripleo/test_tripleo_deploy.py | 33 +++++++ tripleoclient/utils.py | 88 +++++++++++++++++++ tripleoclient/v1/tripleo_deploy.py | 24 +++++ tripleoclient/v1/undercloud_config.py | 3 + tripleoclient/v1/undercloud_preflight.py | 69 +-------------- 6 files changed, 189 insertions(+), 68 deletions(-) diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index a00c70b6a..a4b897cb6 100644 --- a/tripleoclient/tests/test_utils.py +++ b/tripleoclient/tests/test_utils.py @@ -1143,3 +1143,43 @@ class TestDeployNameScenarios(TestWithScenarios): observed = self.func() self.assertEqual(self.expected, observed) + + +class TestCheckHostname(TestCase): + @mock.patch('tripleoclient.utils.run_command') + def test_hostname_ok(self, mock_run): + mock_run.side_effect = ['host.domain', 'host.domain'] + mock_open_ctx = mock.mock_open(read_data='127.0.0.1 host.domain') + with mock.patch('tripleoclient.utils.open', mock_open_ctx): + utils.check_hostname(False) + run_calls = [ + mock.call(['hostnamectl', '--static'], name='hostnamectl'), + mock.call(['hostnamectl', '--transient'], name='hostnamectl')] + self.assertEqual(mock_run.mock_calls, run_calls) + + @mock.patch('tripleoclient.utils.run_command') + def test_hostname_fix_hosts_ok(self, mock_run): + mock_run.side_effect = ['host.domain', 'host.domain', ''] + mock_open_ctx = mock.mock_open(read_data='') + with mock.patch('tripleoclient.utils.open', mock_open_ctx): + utils.check_hostname(True) + sed_cmd = 'sed -i "s/127.0.0.1\\(\\s*\\)/127.0.0.1\\\\1host.domain ' \ + 'host /" /etc/hosts' + run_calls = [ + mock.call(['hostnamectl', '--static'], name='hostnamectl'), + mock.call(['hostnamectl', '--transient'], name='hostnamectl'), + mock.call(['sudo', '/bin/bash', '-c', sed_cmd], + name='hostname-to-etc-hosts')] + import pprint + pprint.pprint(mock_run.mock_calls) + self.assertEqual(mock_run.mock_calls, run_calls) + + @mock.patch('tripleoclient.utils.run_command') + def test_hostname_mismatch_fail(self, mock_run): + mock_run.side_effect = ['host.domain', ''] + self.assertRaises(RuntimeError, utils.check_hostname) + + @mock.patch('tripleoclient.utils.run_command') + def test_hostname_short_fail(self, mock_run): + mock_run.side_effect = ['host', 'host'] + self.assertRaises(RuntimeError, utils.check_hostname) diff --git a/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py b/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py index c1504227f..6b71d62de 100644 --- a/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py +++ b/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py @@ -58,6 +58,39 @@ class TestDeployUndercloud(TestPluginV1): self.orc.stacks.create = mock.MagicMock( return_value={'stack': {'id': 'foo'}}) + @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy._is_undercloud_deploy') + @mock.patch('tripleoclient.utils.check_hostname') + def test_run_preflight_checks(self, mock_check_hostname, mock_uc): + parsed_args = self.check_parser(self.cmd, + ['--local-ip', '127.0.0.1/8'], []) + + mock_uc.return_value = False + self.cmd._run_preflight_checks(parsed_args) + mock_check_hostname.called_one_with(False) + + @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy._is_undercloud_deploy') + @mock.patch('tripleoclient.utils.check_hostname') + def test_run_preflight_checks_output_only(self, mock_check_hostname, + mock_uc): + parsed_args = self.check_parser(self.cmd, + ['--local-ip', '127.0.0.1/8', + '--output-only'], []) + + mock_uc.return_value = False + self.cmd._run_preflight_checks(parsed_args) + mock_check_hostname.assert_not_called() + + @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy._is_undercloud_deploy') + @mock.patch('tripleoclient.utils.check_hostname') + def test_run_preflight_checks_undercloud(self, mock_check_hostname, + mock_uc): + parsed_args = self.check_parser(self.cmd, + ['--local-ip', '127.0.0.1/8'], []) + + mock_uc.return_value = True + self.cmd._run_preflight_checks(parsed_args) + mock_check_hostname.assert_not_called() + def test_get_roles_file_path(self): parsed_args = self.check_parser(self.cmd, ['--local-ip', '127.0.0.1/8'], []) diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index d96d2eee7..407abc6a6 100644 --- a/tripleoclient/utils.py +++ b/tripleoclient/utils.py @@ -48,6 +48,8 @@ from six.moves.urllib import request from tripleoclient import constants from tripleoclient import exceptions +LOG = logging.getLogger(__name__ + ".utils") + def run_ansible_playbook(logger, workdir, @@ -1340,3 +1342,89 @@ def update_nodes_deploy_data(imageclient, nodes): if 'ramdisk_id' not in node and ramdisk in img_map: node['ramdisk_id'] = img_map[ramdisk] break + + +def run_command(args, env=None, name=None, logger=None): + """Run the command defined by args and return its output + + :param args: List of arguments for the command to be run. + :param env: Dict defining the environment variables. Pass None to use + the current environment. + :param name: User-friendly name for the command being run. A value of + None will cause args[0] to be used. + """ + if logger is None: + logger = LOG + if name is None: + name = args[0] + try: + output = subprocess.check_output(args, + stderr=subprocess.STDOUT, + env=env) + if isinstance(output, bytes): + output = output.decode('utf-8') + return output + except subprocess.CalledProcessError as e: + message = '%s failed: %s' % (name, e.output) + logger.error(message) + raise RuntimeError(message) + + +def set_hostname(hostname): + """Set system hostname to provided hostname + + :param hostname: The hostname to set + """ + args = ['sudo', 'hostnamectl', 'set-hostname', hostname] + return run_command(args, name='hostnamectl') + + +def check_hostname(fix_etc_hosts=True, logger=None): + """Check system hostname configuration + + Rabbit and Puppet require pretty specific hostname configuration. This + function ensures that the system hostname settings are valid before + continuing with the installation. + + :param fix_etc_hosts: Boolean to to enable adding hostname to /etc/hosts + if not found. + """ + if logger is None: + logger = LOG + logger.info('Checking for a FQDN hostname...') + args = ['hostnamectl', '--static'] + detected_static_hostname = run_command(args, name='hostnamectl').rstrip() + logger.info('Static hostname detected as %s', detected_static_hostname) + args = ['hostnamectl', '--transient'] + detected_transient_hostname = run_command(args, + name='hostnamectl').rstrip() + logger.info('Transient hostname detected as %s', + detected_transient_hostname) + if detected_static_hostname != detected_transient_hostname: + logger.error('Static hostname "%s" does not match transient hostname ' + '"%s".', detected_static_hostname, + detected_transient_hostname) + logger.error('Use hostnamectl to set matching hostnames.') + raise RuntimeError('Static and transient hostnames do not match') + short_hostname = detected_static_hostname.split('.')[0] + if short_hostname == detected_static_hostname: + message = _('Configured hostname is not fully qualified.') + logger.error(message) + raise RuntimeError(message) + with open('/etc/hosts') as hosts_file: + for line in hosts_file: + # check if hostname is in /etc/hosts + if (not line.lstrip().startswith('#') and + detected_static_hostname in line.split()): + break + else: + # hostname not found, add it to /etc/hosts + if not fix_etc_hosts: + return + sed_cmd = (r'sed -i "s/127.0.0.1\(\s*\)/127.0.0.1\\1%s %s /" ' + '/etc/hosts' % + (detected_static_hostname, short_hostname)) + args = ['sudo', '/bin/bash', '-c', sed_cmd] + run_command(args, name='hostname-to-etc-hosts') + logger.info('Added hostname %s to /etc/hosts', + detected_static_hostname) diff --git a/tripleoclient/v1/tripleo_deploy.py b/tripleoclient/v1/tripleo_deploy.py index 189bb22b3..d53025e0e 100644 --- a/tripleoclient/v1/tripleo_deploy.py +++ b/tripleoclient/v1/tripleo_deploy.py @@ -94,6 +94,28 @@ class Deploy(command.Command): ansible_playbook_cmd = "ansible-playbook-{}".format(python_version) python_cmd = "python{}".format(python_version) + def _is_undercloud_deploy(self, parsed_args): + return parsed_args.standalone_role == 'Undercloud' and \ + parsed_args.stack == 'undercloud' + + def _run_preflight_checks(self, parsed_args): + """Run preflight deployment checks + + Perform any pre-deployment checks that we want to run when deploying + standalone deployments. This is skipped when in output only mode or + when used with an undercloud. The undercloud has it's own set of + deployment preflight requirements. + + :param parsed_args: parsed arguments from the cli + """ + # we skip preflight checks for output only and undercloud + if parsed_args.output_only or self._is_undercloud_deploy(parsed_args): + return + + # in standalone we don't want to fixup the /etc/hosts as we'll be + # managing that elsewhere during the deployment + utils.check_hostname(fix_etc_hosts=False, logger=self.log) + # NOTE(cjeanner) Quick'n'dirty way before we have proper # escalation support through oslo.privsep def _set_data_rights(self, file_name, user=None, @@ -1092,6 +1114,8 @@ class Deploy(command.Command): self.log.error(msg) raise exceptions.DeploymentError(msg) + self._run_preflight_checks(parsed_args) + # prepare working spaces self.output_dir = os.path.abspath(parsed_args.output_dir) self._create_working_dirs() diff --git a/tripleoclient/v1/undercloud_config.py b/tripleoclient/v1/undercloud_config.py index 8c89345d8..8a3d22c48 100644 --- a/tripleoclient/v1/undercloud_config.py +++ b/tripleoclient/v1/undercloud_config.py @@ -567,6 +567,9 @@ def prepare_undercloud_deploy(upgrade=False, no_validations=False, deploy_args += ['--hieradata-override=%s' % data_file] + if CONF.get('undercloud_hostname'): + utils.set_hostname(CONF.get('undercloud_hostname')) + if CONF.get('enable_validations') and not no_validations: undercloud_preflight.check(verbose_level, upgrade) deploy_args += ['-e', os.path.join( diff --git a/tripleoclient/v1/undercloud_preflight.py b/tripleoclient/v1/undercloud_preflight.py index 3e4446ce3..e6fdd5810 100644 --- a/tripleoclient/v1/undercloud_preflight.py +++ b/tripleoclient/v1/undercloud_preflight.py @@ -45,27 +45,6 @@ PASSWORD_PATH = '%s/%s' % (constants.UNDERCLOUD_OUTPUT_DIR, LOG = logging.getLogger(__name__ + ".UndercloudSetup") -def _run_command(args, env=None, name=None): - """Run the command defined by args and return its output - - :param args: List of arguments for the command to be run. - :param env: Dict defining the environment variables. Pass None to use - the current environment. - :param name: User-friendly name for the command being run. A value of - None will cause args[0] to be used. - """ - if name is None: - name = args[0] - try: - return subprocess.check_output(args, - stderr=subprocess.STDOUT, - env=env).decode('utf-8') - except subprocess.CalledProcessError as e: - message = '%s failed: %s' % (name, e.output) - LOG.error(message) - raise RuntimeError(message) - - def _run_live_command(args, env=None, name=None, cwd=None, wait=True): """Run the command defined by args, env and cwd @@ -117,52 +96,6 @@ def _check_diskspace(upgrade=False): output_callback='validation_output') -def _check_hostname(): - """Check system hostname configuration - - Rabbit and Puppet require pretty specific hostname configuration. This - function ensures that the system hostname settings are valid before - continuing with the installation. - """ - if CONF.undercloud_hostname is not None: - args = ['sudo', 'hostnamectl', 'set-hostname', - CONF.undercloud_hostname] - _run_command(args, name='hostnamectl') - - LOG.info('Checking for a FQDN hostname...') - args = ['sudo', 'hostnamectl', '--static'] - detected_static_hostname = _run_command(args, name='hostnamectl').rstrip() - LOG.info('Static hostname detected as %s', detected_static_hostname) - args = ['sudo', 'hostnamectl', '--transient'] - detected_transient_hostname = _run_command(args, - name='hostnamectl').rstrip() - LOG.info('Transient hostname detected as %s', detected_transient_hostname) - if detected_static_hostname != detected_transient_hostname: - LOG.error('Static hostname "%s" does not match transient hostname ' - '"%s".', detected_static_hostname, - detected_transient_hostname) - LOG.error('Use hostnamectl to set matching hostnames.') - raise RuntimeError('Static and transient hostnames do not match') - with open('/etc/hosts') as hosts_file: - for line in hosts_file: - if (not line.lstrip().startswith('#') and - detected_static_hostname in line.split()): - break - else: - short_hostname = detected_static_hostname.split('.')[0] - if short_hostname == detected_static_hostname: - message = _('Configured hostname is not fully qualified.') - LOG.error(message) - raise RuntimeError(message) - sed_cmd = ('sed -i "s/127.0.0.1\(\s*\)/127.0.0.1\\1%s %s /" ' - '/etc/hosts' % - (detected_static_hostname, short_hostname)) - args = ['sudo', '/bin/bash', '-c', sed_cmd] - _run_command(args, name='hostname-to-etc-hosts') - LOG.info('Added hostname %s to /etc/hosts', - detected_static_hostname) - - def _check_memory(): """Check system memory @@ -502,7 +435,7 @@ def check(verbose_level, upgrade=False): try: # Other validations _checking_status('Hostname') - _check_hostname() + utils.check_hostname() _checking_status('Memory') _check_memory() _checking_status('Disk space')