From 75e56314166b3316b718c5ab70dd60008e1bc815 Mon Sep 17 00:00:00 2001 From: Ben Nemec Date: Thu, 14 May 2015 20:01:48 -0500 Subject: [PATCH] Test _generate_environment Adds test coverage for _generate_environment, and fixes a few issues with the code, both real bugs and things that made testing harder. Also prevents the code from logging to the real log file, although it does still seem to be creating the log file anyway. Change-Id: I9bbfdbb29c32ce71a24fcd15a95f5e11ce406d14 --- .testr.conf | 2 +- instack_undercloud/tests/test_undercloud.py | 102 ++++++++++++++++++++ instack_undercloud/undercloud.py | 51 +++++----- 3 files changed, 131 insertions(+), 24 deletions(-) diff --git a/.testr.conf b/.testr.conf index a709def84..8c84c21cc 100644 --- a/.testr.conf +++ b/.testr.conf @@ -1,4 +1,4 @@ [DEFAULT] -test_command=OS_STDOUT_CAPTURE=1 OS_STDERR_CAPTURE=1 OS_TEST_TIMEOUT=60 ${PYTHON:-python} -m subunit.run discover -t ./instack_undercloud ./instack_undercloud $LISTOPT $IDOPTION +test_command=OS_STDOUT_CAPTURE=1 OS_STDERR_CAPTURE=1 OS_TEST_TIMEOUT=60 OS_LOG_CAPTURE=1 ${PYTHON:-python} -m subunit.run discover -t ./instack_undercloud ./instack_undercloud $LISTOPT $IDOPTION test_id_option=--load-list $IDFILE test_list_option=--list diff --git a/instack_undercloud/tests/test_undercloud.py b/instack_undercloud/tests/test_undercloud.py index 7ce62550b..8bcf1f9ef 100644 --- a/instack_undercloud/tests/test_undercloud.py +++ b/instack_undercloud/tests/test_undercloud.py @@ -13,10 +13,13 @@ # under the License. import io +import os +import tempfile import fixtures import mock from oslotest import base +from oslotest import mockpatch from instack_undercloud import undercloud @@ -44,6 +47,11 @@ class TestUndercloud(base.BaseTestCase): mock_run_command.assert_called_with( ['sudo', 'rm', '-f', '/tmp/svc-map-services'], None, 'rm') + def test_generate_password(self): + first = undercloud._generate_password() + second = undercloud._generate_password() + self.assertNotEqual(first, second) + class TestCheckHostname(base.BaseTestCase): @mock.patch('instack_undercloud.undercloud._run_command') @@ -96,3 +104,97 @@ class TestCheckHostname(base.BaseTestCase): with mock.patch('instack_undercloud.undercloud.open', return_value=fake_hosts, create=True): self.assertRaises(RuntimeError, undercloud._check_hostname) + + +class TestGenerateEnvironment(base.BaseTestCase): + def setUp(self): + super(TestGenerateEnvironment, self).setUp() + # Things that need to always be mocked out, but that the tests + # don't want to care about. + self.useFixture(mockpatch.Patch( + 'instack_undercloud.undercloud._write_password_file')) + self.useFixture(mockpatch.Patch( + 'instack_undercloud.undercloud._load_config')) + mock_isdir = mockpatch.Patch('os.path.isdir') + self.useFixture(mock_isdir) + mock_isdir.mock.return_value = False + # Some tests do care about this, but they can override the default + # return value, and then the tests that don't care can ignore it. + self.mock_distro = mockpatch.Patch('platform.linux_distribution') + self.useFixture(self.mock_distro) + self.mock_distro.mock.return_value = [ + 'Red Hat Enterprise Linux Server 7.1'] + + @mock.patch('socket.gethostname') + def test_hostname_set(self, mock_gethostname): + fake_hostname = 'crazy-test-hostname-!@#$%12345' + mock_gethostname.return_value = fake_hostname + env = undercloud._generate_environment('.') + self.assertEqual(fake_hostname, env['HOSTNAME']) + + def test_elements_path_input(self): + test_path = '/test/elements/path' + self.useFixture(fixtures.EnvironmentVariable('ELEMENTS_PATH', + test_path)) + env = undercloud._generate_environment('.') + self.assertEqual(test_path, env['ELEMENTS_PATH']) + + def test_default_elements_path(self): + env = undercloud._generate_environment('.') + test_path = ('%s:%s:/usr/share/tripleo-image-elements:' + '/usr/share/diskimage-builder/elements' % + (os.path.join(os.getcwd(), 'tripleo-puppet-elements', + 'elements'), + './elements')) + self.assertEqual(test_path, env['ELEMENTS_PATH']) + + def test_rhel7_distro(self): + self.useFixture(fixtures.EnvironmentVariable('NODE_DIST', None)) + env = undercloud._generate_environment('.') + self.assertEqual('rhel7', env['NODE_DIST']) + self.assertEqual('./json-files/rhel-7-undercloud-packages.json', + env['JSONFILE']) + self.assertEqual('disable', env['REG_METHOD']) + self.assertEqual('1', env['REG_HALT_UNREGISTER']) + + def test_centos7_distro(self): + self.useFixture(fixtures.EnvironmentVariable('NODE_DIST', None)) + self.mock_distro.mock.return_value = ['CentOS Linux release 7.1'] + env = undercloud._generate_environment('.') + self.assertEqual('centos7', env['NODE_DIST']) + self.assertEqual('./json-files/centos-7-undercloud-packages.json', + env['JSONFILE']) + + def test_fedora_distro(self): + self.useFixture(fixtures.EnvironmentVariable('NODE_DIST', None)) + self.mock_distro.mock.return_value = ['Fedora Infinity + 1'] + self.assertRaises(RuntimeError, undercloud._generate_environment, '.') + + def test_other_distro(self): + self.useFixture(fixtures.EnvironmentVariable('NODE_DIST', None)) + self.mock_distro.mock.return_value = ['Gentoo'] + self.assertRaises(RuntimeError, undercloud._generate_environment, '.') + + def test_opts_in_env(self): + env = undercloud._generate_environment('.') + # Just spot check, we don't want to replicate the entire opt list here + self.assertEqual(env['DEPLOYMENT_MODE'], 'poc') + self.assertEqual(env['DISCOVERY_RUNBENCH'], '0') + self.assertEqual('192.0.2.1/24', env['PUBLIC_INTERFACE_IP']) + self.assertEqual('192.0.2.1', env['LOCAL_IP']) + + def test_answers_file_support(self): + fake_answers = tempfile.mkstemp()[1] + with open(fake_answers, 'w') as f: + f.write('DEPLOYMENT_MODE=scale\n') + # NOTE(bnemec): For some reason, mocking ANSWERS_PATH with a + # PropertyMock isn't working. This accomplishes the same thing. + save_path = undercloud.ANSWERS_PATH + undercloud.ANSWERS_PATH = fake_answers + + def reset_answers(): + undercloud.ANSWERS_PATH = save_path + + self.addCleanup(reset_answers) + env = undercloud._generate_environment('.') + self.assertEqual('scale', env['DEPLOYMENT_MODE']) diff --git a/instack_undercloud/undercloud.py b/instack_undercloud/undercloud.py index d3e3ad5c4..a12a313e2 100644 --- a/instack_undercloud/undercloud.py +++ b/instack_undercloud/undercloud.py @@ -331,6 +331,29 @@ def _generate_password(length=40): return hashlib.sha1(uuid_str).hexdigest()[:length] +def _write_password_file(answers_parser, instack_env): + with open(PASSWORD_PATH, 'w') as password_file: + password_file.write('[auth]\n') + for opt in _auth_opts: + env_name = opt.name.upper() + if answers_parser.has_option('answers', env_name): + LOG.warning('Using value for %s from instack.answers. This ' + 'behavior is deprecated. undercloud.conf should ' + 'now be used for configuration.', env_name) + value = answers_parser.get('answers', env_name) + else: + value = CONF.auth[opt.name] + if not value: + # Heat requires this encryption key to be a specific length + if env_name == 'UNDERCLOUD_HEAT_ENCRYPTION_KEY': + value = _generate_password(32) + else: + value = _generate_password() + LOG.info('Generated new password for %s', opt.name) + instack_env[env_name] = value + password_file.write('%s=%s\n' % (opt.name, value)) + + def _generate_environment(instack_root): """Generate an environment dict for instack @@ -340,7 +363,7 @@ def _generate_environment(instack_root): :param instack_root: The path containing the instack-undercloud elements and json files. """ - instack_env = os.environ + instack_env = dict(os.environ) # Rabbit uses HOSTNAME, so we need to make sure it's right instack_env['HOSTNAME'] = socket.gethostname() @@ -383,9 +406,9 @@ def _generate_environment(instack_root): ) elif distro.startswith('Fedora'): instack_env['NODE_DIST'] = os.environ.get('NODE_DIST') or 'fedora' - raise Exception('Fedora is not currently supported') + raise RuntimeError('Fedora is not currently supported') else: - raise Exception('%s is not supported' % distro) + raise RuntimeError('%s is not supported' % distro) # Do some fiddling to retain answers file support for now answers_parser = ConfigParser.ConfigParser() @@ -413,26 +436,8 @@ def _generate_environment(instack_root): else '0') instack_env['PUBLIC_INTERFACE_IP'] = instack_env['LOCAL_IP'] instack_env['LOCAL_IP'] = instack_env['LOCAL_IP'].split('/')[0] - with open(PASSWORD_PATH, 'w') as password_file: - password_file.write('[auth]\n') - for opt in _auth_opts: - env_name = opt.name.upper() - if answers_parser.has_option('answers', env_name): - LOG.warning('Using value for %s from instack.answers. This ' - 'behavior is deprecated. undercloud.conf should ' - 'now be used for configuration.', env_name) - value = answers_parser.get('answers', env_name) - else: - value = CONF.auth[opt.name] - if not value: - # Heat requires this encryption key to be a specific length - if env_name == 'UNDERCLOUD_HEAT_ENCRYPTION_KEY': - value = _generate_password(32) - else: - value = _generate_password() - LOG.info('Generated new password for %s', opt.name) - instack_env[env_name] = value - password_file.write('%s=%s\n' % (opt.name, value)) + + _write_password_file(answers_parser, instack_env) return instack_env