From 19f983db77f7d75d4d661acd2c23b894ca94dd60 Mon Sep 17 00:00:00 2001 From: Thomas Herve Date: Tue, 10 Oct 2017 11:57:40 +0200 Subject: [PATCH] Handle relative hieradata_override It fixes an issue with hieradata_override when the path is relative. It also aligns the behavior for undercloud_service_certificate. Change-Id: Id7a49617712c5d4c15fcce8c552026f88ca39625 Closes-Bug: #1722489 --- instack_undercloud/tests/test_undercloud.py | 62 +++++++++++++++++++-- instack_undercloud/undercloud.py | 32 ++++++++--- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/instack_undercloud/tests/test_undercloud.py b/instack_undercloud/tests/test_undercloud.py index 992ebcd66..a5105af40 100644 --- a/instack_undercloud/tests/test_undercloud.py +++ b/instack_undercloud/tests/test_undercloud.py @@ -109,6 +109,52 @@ class TestUndercloud(BaseTestCase): mock_die_tuskar_die.assert_called_once() mock_run_validation_groups.assert_called_once() + @mock.patch('instack_undercloud.undercloud._handle_upgrade_fact') + @mock.patch('instack_undercloud.undercloud._configure_logging') + @mock.patch('instack_undercloud.undercloud._validate_configuration') + @mock.patch('instack_undercloud.undercloud._run_command') + @mock.patch('instack_undercloud.undercloud._post_config') + @mock.patch('instack_undercloud.undercloud._run_orc') + @mock.patch('instack_undercloud.undercloud._run_yum_update') + @mock.patch('instack_undercloud.undercloud._run_yum_clean_all') + @mock.patch('instack_undercloud.undercloud._run_instack') + @mock.patch('instack_undercloud.undercloud._generate_environment') + @mock.patch('instack_undercloud.undercloud._load_config') + @mock.patch('instack_undercloud.undercloud._die_tuskar_die') + @mock.patch('instack_undercloud.undercloud._run_validation_groups') + def test_install_upgrade_hieradata(self, mock_run_validation_groups, + mock_die_tuskar_die, mock_load_config, + mock_generate_environment, + mock_run_instack, + mock_run_yum_clean_all, + mock_run_yum_update, mock_run_orc, + mock_post_config, mock_run_command, + mock_validate_configuration, + mock_configure_logging, + mock_upgrade_fact): + conf = config_fixture.Config() + self.useFixture(conf) + conf.config(hieradata_override='override.yaml') + with open(os.path.expanduser('~/override.yaml'), 'w') as f: + f.write('Something\n') + fake_env = mock.MagicMock() + mock_generate_environment.return_value = fake_env + undercloud.install('.', upgrade=True) + self.assertTrue(mock_validate_configuration.called) + mock_generate_environment.assert_called_with('.') + mock_run_instack.assert_called_with(fake_env) + mock_run_orc.assert_called_with(fake_env) + mock_run_command.assert_called_with( + ['sudo', 'rm', '-f', '/tmp/svc-map-services'], None, 'rm') + self.assertNotIn( + mock.call( + ['sudo', 'cp', 'override.yaml', + '/etc/puppet/hieradata/override.yaml']), + mock_run_command.mock_calls) + mock_upgrade_fact.assert_called_with(True) + mock_die_tuskar_die.assert_called_once() + mock_run_validation_groups.assert_called_once() + @mock.patch('instack_undercloud.undercloud._configure_logging') def test_install_exception(self, mock_configure_logging): mock_configure_logging.side_effect = RuntimeError('foo') @@ -539,10 +585,18 @@ class TestGenerateEnvironment(BaseTestCase): def test_relative_cert_path(self): conf = config_fixture.Config() self.useFixture(conf) - conf.config(undercloud_service_certificate='test.pem') - env = undercloud._generate_environment('.') - self.assertEqual(os.path.join(os.getcwd(), 'test.pem'), - env['UNDERCLOUD_SERVICE_CERTIFICATE']) + [cert] = self.create_tempfiles([('test', 'foo')], '.pem') + rel_cert = os.path.basename(cert) + cert_path = os.path.dirname(cert) + cur_dir = os.getcwd() + try: + os.chdir(cert_path) + conf.config(undercloud_service_certificate=rel_cert) + env = undercloud._generate_environment('.') + self.assertEqual(os.path.join(os.getcwd(), rel_cert), + env['UNDERCLOUD_SERVICE_CERTIFICATE']) + finally: + os.chdir(cur_dir) def test_no_cert_path(self): env = undercloud._generate_environment('.') diff --git a/instack_undercloud/undercloud.py b/instack_undercloud/undercloud.py index 93ce18146..90e7e97f6 100644 --- a/instack_undercloud/undercloud.py +++ b/instack_undercloud/undercloud.py @@ -1209,9 +1209,6 @@ def _generate_environment(instack_root): instack_env['PUBLIC_INTERFACE_IP'] = instack_env['LOCAL_IP'] instack_env['LOCAL_IP'] = instack_env['LOCAL_IP'].split('/')[0] instack_env['LOCAL_IP_WRAPPED'] = _wrap_ipv6(instack_env['LOCAL_IP']) - if instack_env['UNDERCLOUD_SERVICE_CERTIFICATE']: - instack_env['UNDERCLOUD_SERVICE_CERTIFICATE'] = os.path.abspath( - instack_env['UNDERCLOUD_SERVICE_CERTIFICATE']) # We're not in a chroot so this doesn't make sense, and it causes weird # errors if it's set. if instack_env.get('DIB_YUM_REPO_CONF'): @@ -1234,6 +1231,19 @@ def _generate_environment(instack_root): public_host = CONF.undercloud_public_host instack_env['UNDERCLOUD_SERVICE_CERTIFICATE'] = ( '/etc/pki/tls/certs/undercloud-%s.pem' % public_host) + elif instack_env['UNDERCLOUD_SERVICE_CERTIFICATE']: + raw_value = instack_env['UNDERCLOUD_SERVICE_CERTIFICATE'] + abs_cert = os.path.abspath(raw_value) + if abs_cert != raw_value: + home_dir = os.path.expanduser('~') + if os.getcwd() != home_dir and os.path.exists(abs_cert): + LOG.warning('Using undercloud_service_certificate from ' + 'current directory, please use an absolute path ' + 'to remove ambiguity') + instack_env['UNDERCLOUD_SERVICE_CERTIFICATE'] = abs_cert + else: + instack_env['UNDERCLOUD_SERVICE_CERTIFICATE'] = os.path.join( + home_dir, raw_value) return instack_env @@ -1257,12 +1267,20 @@ def _generate_init_data(instack_env): context = instack_env.copy() if CONF.hieradata_override: - hiera_entry = os.path.splitext( - os.path.basename(CONF.hieradata_override))[0] + data_file = CONF.hieradata_override + hiera_entry = os.path.splitext(os.path.basename(data_file))[0] dst = os.path.join('/etc/puppet/hieradata', - os.path.basename(CONF.hieradata_override)) + os.path.basename(data_file)) + if os.path.abspath(CONF.hieradata_override) != data_file: + # If we don't have an absolute path, compute it + data_file = os.path.join(os.path.expanduser('~'), data_file) + + if not os.path.exists(data_file): + raise RuntimeError( + "Could not find hieradata_override file '%s'" % data_file) + _run_command(['sudo', 'mkdir', '-p', '/etc/puppet/hieradata']) - _run_command(['sudo', 'cp', CONF.hieradata_override, dst]) + _run_command(['sudo', 'cp', data_file, dst]) _run_command(['sudo', 'chmod', '0644', dst]) else: hiera_entry = ''