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

@@ -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')
+        [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(), 'test.pem'),
+            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('.')

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(
+        data_file = CONF.hieradata_override
-            os.path.basename(CONF.hieradata_override))[0]
+        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 = ''