From 3f44b59b293a433c16bafc94eb7c6d1d3ea216f9 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 25 Feb 2014 13:39:17 -0500 Subject: [PATCH] Move the code that reads the global environment This belongs in the environment module, not the resources package. Change-Id: I97093cef3a3e6d35bc82df8b8533cff7e5bc6ec9 --- heat/engine/environment.py | 40 ++++++++++++++++++-- heat/engine/resources/__init__.py | 51 ++++---------------------- heat/tests/test_environment.py | 61 ++++++++++++++++--------------- 3 files changed, 76 insertions(+), 76 deletions(-) diff --git a/heat/engine/environment.py b/heat/engine/environment.py index 1d5ef14f59..d362859416 100644 --- a/heat/engine/environment.py +++ b/heat/engine/environment.py @@ -13,10 +13,15 @@ # License for the specific language governing permissions and limitations # under the License. +import glob import itertools +import os.path + +from oslo.config import cfg from heat.openstack.common import log from heat.openstack.common.gettextutils import _ +from heat.common import environment_format from heat.common import exception @@ -235,9 +240,9 @@ class ResourceRegistry(object): def is_a_glob(resource_type): return resource_type.endswith('*') globs = itertools.ifilter(is_a_glob, self._registry.keys()) - for glob in globs: - if self._registry[glob].matches(resource_type): - yield self._registry[glob] + for pattern in globs: + if self._registry[pattern].matches(resource_type): + yield self._registry[pattern] def get_resource_info(self, resource_type, resource_name=None, registry_type=None): @@ -377,3 +382,32 @@ class Environment(object): def get_constraint(self, name): return self.constraints.get(name) + + +def read_global_environment(env, env_dir=None): + if env_dir is None: + cfg.CONF.import_opt('environment_dir', 'heat.common.config') + env_dir = cfg.CONF.environment_dir + + try: + env_files = glob.glob(os.path.join(env_dir, '*')) + except OSError as osex: + LOG.error(_('Failed to read %s') % env_dir) + LOG.exception(osex) + return + + for file_path in env_files: + try: + with open(file_path) as env_fd: + LOG.info(_('Loading %s') % file_path) + env_body = environment_format.parse(env_fd.read()) + environment_format.default_for_missing(env_body) + env.load(env_body) + except ValueError as vex: + LOG.error(_('Failed to parse %(file_path)s') % { + 'file_path': file_path}) + LOG.exception(vex) + except IOError as ioex: + LOG.error(_('Failed to read %(file_path)s') % { + 'file_path': file_path}) + LOG.exception(ioex) diff --git a/heat/engine/resources/__init__.py b/heat/engine/resources/__init__.py index 6e7ff071b6..b9b88d8785 100644 --- a/heat/engine/resources/__init__.py +++ b/heat/engine/resources/__init__.py @@ -13,14 +13,10 @@ # License for the specific language governing permissions and limitations # under the License. -import glob import itertools -import os -import os.path from oslo.config import cfg -from heat.common import environment_format from heat.openstack.common import log from heat.openstack.common.gettextutils import _ from heat.engine import environment @@ -94,55 +90,24 @@ _environment = None def global_env(): - global _environment if _environment is None: initialise() return _environment -def _list_environment_files(env_dir): - try: - return glob.glob(os.path.join(env_dir, '*')) - except OSError as osex: - LOG.error(_('Failed to read %s') % env_dir) - LOG.exception(osex) - return [] - - -def _load_all(env): - _load_global_resources(env) - _load_global_environment(env) - - -def _load_global_environment(env, env_dir=None): - if env_dir is None: - cfg.CONF.import_opt('environment_dir', 'heat.common.config') - env_dir = cfg.CONF.environment_dir - - for file_path in _list_environment_files(env_dir): - try: - with open(file_path) as env_fd: - LOG.info(_('Loading %s') % file_path) - env_body = environment_format.parse(env_fd.read()) - environment_format.default_for_missing(env_body) - env.load(env_body) - except ValueError as vex: - LOG.error(_('Failed to parse %(file_path)s') % { - 'file_path': file_path}) - LOG.exception(vex) - except IOError as ioex: - LOG.error(_('Failed to read %(file_path)s') % { - 'file_path': file_path}) - LOG.exception(ioex) - - def initialise(): global _environment if _environment is not None: return - _environment = environment.Environment({}, user_env=False) - _load_all(_environment) + global_env = environment.Environment({}, user_env=False) + _load_global_environment(global_env) + _environment = global_env + + +def _load_global_environment(env): + _load_global_resources(env) + environment.read_global_environment(env) def _global_modules(): diff --git a/heat/tests/test_environment.py b/heat/tests/test_environment.py index 89f577d9c9..0a177a6f9b 100644 --- a/heat/tests/test_environment.py +++ b/heat/tests/test_environment.py @@ -133,7 +133,7 @@ def constraint_mapping(): cfg.CONF.set_override('plugin_dirs', plugin_dir.path) env = environment.Environment({}) - resources._load_all(env) + resources._load_global_environment(env) self.assertEqual("MyConstraint", env.get_constraint("constraint1").__name__) @@ -152,7 +152,8 @@ def constraint_mapping(): cfg.CONF.set_override('plugin_dirs', plugin_dir.path) env = environment.Environment({}) - error = self.assertRaises(ValueError, resources._load_all, env) + error = self.assertRaises(ValueError, + resources._load_global_environment, env) self.assertEqual("oops", str(error)) @@ -199,50 +200,50 @@ class EnvironmentDuplicateTest(common.HeatTestCase): class GlobalEnvLoadingTest(common.HeatTestCase): def test_happy_path(self): - list_dir = 'heat.engine.resources._list_environment_files' - with mock.patch(list_dir) as m_ldir: + with mock.patch('glob.glob') as m_ldir: m_ldir.return_value = ['/etc_etc/heat/environment.d/a.yaml'] env_dir = '/etc_etc/heat/environment.d' env_content = '{"resource_registry": {}}' - with mock.patch('heat.engine.resources.open', + env = environment.Environment({}, user_env=False) + + with mock.patch('heat.engine.environment.open', mock.mock_open(read_data=env_content), create=True) as m_open: - resources._load_global_environment(resources.global_env(), - env_dir) + environment.read_global_environment(env, env_dir) - m_ldir.assert_called_once_with(env_dir) + m_ldir.assert_called_once_with(env_dir + '/*') m_open.assert_called_once_with('%s/a.yaml' % env_dir) def test_empty_env_dir(self): - list_dir = 'heat.engine.resources._list_environment_files' - with mock.patch(list_dir) as m_ldir: + with mock.patch('glob.glob') as m_ldir: m_ldir.return_value = [] env_dir = '/etc_etc/heat/environment.d' - resources._load_global_environment(resources.global_env(), - env_dir) - m_ldir.assert_called_once_with(env_dir) + env = environment.Environment({}, user_env=False) + environment.read_global_environment(env, env_dir) + + m_ldir.assert_called_once_with(env_dir + '/*') def test_continue_on_ioerror(self): """assert we get all files processed even if there are processing exceptions. """ - list_dir = 'heat.engine.resources._list_environment_files' - with mock.patch(list_dir) as m_ldir: + with mock.patch('glob.glob') as m_ldir: m_ldir.return_value = ['/etc_etc/heat/environment.d/a.yaml', '/etc_etc/heat/environment.d/b.yaml'] env_dir = '/etc_etc/heat/environment.d' env_content = '{}' - with mock.patch('heat.engine.resources.open', + env = environment.Environment({}, user_env=False) + + with mock.patch('heat.engine.environment.open', mock.mock_open(read_data=env_content), create=True) as m_open: m_open.side_effect = IOError - resources._load_global_environment(resources.global_env(), - env_dir) + environment.read_global_environment(env, env_dir) - m_ldir.assert_called_once_with(env_dir) + m_ldir.assert_called_once_with(env_dir + '/*') expected = [mock.call('%s/a.yaml' % env_dir), mock.call('%s/b.yaml' % env_dir)] self.assertEqual(expected, m_open.call_args_list) @@ -251,20 +252,20 @@ class GlobalEnvLoadingTest(common.HeatTestCase): """assert we get all files processed even if there are processing exceptions. """ - list_dir = 'heat.engine.resources._list_environment_files' - with mock.patch(list_dir) as m_ldir: + with mock.patch('glob.glob') as m_ldir: m_ldir.return_value = ['/etc_etc/heat/environment.d/a.yaml', '/etc_etc/heat/environment.d/b.yaml'] env_dir = '/etc_etc/heat/environment.d' env_content = '{@$%#$%' - with mock.patch('heat.engine.resources.open', + env = environment.Environment({}, user_env=False) + + with mock.patch('heat.engine.environment.open', mock.mock_open(read_data=env_content), create=True) as m_open: - resources._load_global_environment(resources.global_env(), - env_dir) + environment.read_global_environment(env, env_dir) - m_ldir.assert_called_once_with(env_dir) + m_ldir.assert_called_once_with(env_dir + '/*') expected = [mock.call('%s/a.yaml' % env_dir), mock.call('%s/b.yaml' % env_dir)] self.assertEqual(expected, m_open.call_args_list) @@ -288,7 +289,7 @@ class GlobalEnvLoadingTest(common.HeatTestCase): # 2. load global env g_env = environment.Environment({}, user_env=False) - resources._load_all(g_env) + resources._load_global_environment(g_env) # 3. assert our resource is in place. self.assertEqual('file:///not_really_here.yaml', @@ -310,7 +311,7 @@ class GlobalEnvLoadingTest(common.HeatTestCase): # 2. load global env g_env = environment.Environment({}, user_env=False) - resources._load_all(g_env) + resources._load_global_environment(g_env) # 3. assert our resource is in now gone. self.assertIsNone(g_env.get_resource_info('OS::Nova::Server')) @@ -335,7 +336,7 @@ class GlobalEnvLoadingTest(common.HeatTestCase): # 2. load global env g_env = environment.Environment({}, user_env=False) - resources._load_all(g_env) + resources._load_global_environment(g_env) # 3. assert our resources are now gone. self.assertIsNone(g_env.get_resource_info('AWS::EC2::Instance')) @@ -376,10 +377,10 @@ class GlobalEnvLoadingTest(common.HeatTestCase): # 2. load global env g_env = environment.Environment({}, user_env=False) - with mock.patch('heat.engine.resources.open', + with mock.patch('heat.engine.environment.open', mock.mock_open(read_data=g_env_content), create=True) as m_open: - resources._load_all(g_env) + resources._load_global_environment(g_env) # 3. assert that the file were ignored expected = [mock.call('%s/a.yaml' % envdir.path),