diff --git a/hooks/neutron_ovs_hooks.py b/hooks/neutron_ovs_hooks.py index 2b31e3ce..064c711b 100755 --- a/hooks/neutron_ovs_hooks.py +++ b/hooks/neutron_ovs_hooks.py @@ -21,7 +21,7 @@ from charmhelpers.core.host import ( ) from charmhelpers.fetch import ( - apt_install, apt_update, apt_purge + apt_purge, ) from charmhelpers.contrib.openstack.utils import ( @@ -31,15 +31,15 @@ from charmhelpers.contrib.openstack.utils import ( from neutron_ovs_utils import ( DVR_PACKAGES, configure_ovs, - determine_packages, git_install, get_topics, - determine_dvr_packages, get_shared_secret, register_configs, restart_map, use_dvr, - enable_metadata, + enable_nova_metadata, + enable_local_dhcp, + install_packages, ) hooks = Hooks() @@ -48,11 +48,7 @@ CONFIGS = register_configs() @hooks.hook() def install(): - apt_update() - pkgs = determine_packages() - for pkg in pkgs: - apt_install(pkg, fatal=True) - + install_packages() git_install(config('openstack-origin-git')) @@ -60,10 +56,7 @@ def install(): @hooks.hook('config-changed') @restart_on_change(restart_map()) def config_changed(): - if determine_dvr_packages(): - apt_update() - apt_install(determine_dvr_packages(), fatal=True) - + install_packages() if git_install_requested(): if config_value_changed('openstack-origin-git'): git_install(config('openstack-origin-git')) @@ -80,8 +73,7 @@ def config_changed(): @restart_on_change(restart_map()) def neutron_plugin_api_changed(): if use_dvr(): - apt_update() - apt_install(DVR_PACKAGES, fatal=True) + install_packages() else: apt_purge(DVR_PACKAGES, fatal=True) configure_ovs() @@ -93,14 +85,12 @@ def neutron_plugin_api_changed(): @hooks.hook('neutron-plugin-relation-joined') def neutron_plugin_joined(relation_id=None): - print "Enable metadata: {}".format(enable_metadata()) - if config('enable-local-dhcp-and-metadata'): - apt_install(['neutron-metadata-agent', 'neutron-dhcp-agent'] fatal=True) - secret = get_shared_secret() if enable_metadata() else None + if enable_local_dhcp(): + install_packages() + secret = get_shared_secret() if enable_nova_metadata() else None rel_data = { 'metadata-shared-secret': secret, } - print rel_data relation_set(relation_id=relation_id, **rel_data) diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index 8ab4ad35..996cc54b 100644 --- a/hooks/neutron_ovs_utils.py +++ b/hooks/neutron_ovs_utils.py @@ -43,6 +43,12 @@ from charmhelpers.core.host import ( from charmhelpers.core.templating import render +from charmhelpers.fetch import ( + apt_install, + apt_update, + filter_installed_packages, +) + BASE_GIT_PACKAGES = [ 'libffi-dev', 'libssl-dev', @@ -102,6 +108,8 @@ METADATA_RESOURCE_MAP = OrderedDict([ 'contexts': [neutron_ovs_context.SharedSecretContext(), neutron_ovs_context.APIIdentityServiceContext()], }), +]) +DHCP_RESOURCE_MAP = OrderedDict([ (NEUTRON_DHCP_AGENT_CONF, { 'services': ['neutron-dhcp-agent'], 'contexts': [], @@ -127,16 +135,17 @@ EXT_BRIDGE = "br-ex" DATA_BRIDGE = 'br-data' -def determine_dvr_packages(): - if not git_install_requested(): - if use_dvr(): - return DVR_PACKAGES - return [] +def install_packages(): + apt_update() + apt_install(filter_installed_packages(determine_packages())) def determine_packages(): pkgs = neutron_plugin_attribute('ovs', 'packages', 'neutron') - pkgs.extend(determine_dvr_packages()) + if use_dvr(): + pkgs.extend(DVR_PACKAGES) + if enable_local_dhcp(): + pkgs.extend(['neutron-metadata-agent', 'neutron-dhcp-agent']) if git_install_requested(): pkgs.extend(BASE_GIT_PACKAGES) @@ -168,9 +177,10 @@ def resource_map(): resource_map.update(METADATA_RESOURCE_MAP) dvr_services = ['neutron-metadata-agent', 'neutron-l3-agent'] resource_map[NEUTRON_CONF]['services'] += dvr_services - if enable_metadata(): + if enable_local_dhcp(): resource_map.update(METADATA_RESOURCE_MAP) - metadata_services = ['neutron-metadata-agent'] + resource_map.update(DHCP_RESOURCE_MAP) + metadata_services = ['neutron-metadata-agent', 'neutron-dhcp-agent'] resource_map[NEUTRON_CONF]['services'] += metadata_services return resource_map @@ -230,10 +240,14 @@ def use_dvr(): return context.NeutronAPIContext()()['enable_dvr'] -def enable_metadata(): +def enable_nova_metadata(): return use_dvr() or config('enable-local-dhcp-and-metadata') +def enable_local_dhcp(): + return config('enable-local-dhcp-and-metadata') + + def git_install(projects_yaml): """Perform setup, and install git repos specified in yaml parameter.""" if git_install_requested(): diff --git a/unit_tests/test_neutron_ovs_context.py b/unit_tests/test_neutron_ovs_context.py index 6bd54936..6a33cbc0 100644 --- a/unit_tests/test_neutron_ovs_context.py +++ b/unit_tests/test_neutron_ovs_context.py @@ -250,11 +250,11 @@ class L3AgentContextTest(CharmTestCase): self.assertEquals(context.L3AgentContext()(), {'agent_mode': 'legacy'}) -class DVRSharedSecretContext(CharmTestCase): +class SharedSecretContext(CharmTestCase): def setUp(self): - super(DVRSharedSecretContext, self).setUp(context, - TO_PATCH) + super(SharedSecretContext, self).setUp(context, + TO_PATCH) self.config.side_effect = self.test_config.get @patch('os.path') @@ -286,7 +286,7 @@ class DVRSharedSecretContext(CharmTestCase): _NeutronAPIContext.side_effect = fake_context({'enable_dvr': True}) _shared_secret.return_value = 'secret_thing' self.resolve_address.return_value = '10.0.0.10' - self.assertEquals(context.DVRSharedSecretContext()(), + self.assertEquals(context.SharedSecretContext()(), {'shared_secret': 'secret_thing', 'local_ip': '10.0.0.10'}) @@ -297,4 +297,4 @@ class DVRSharedSecretContext(CharmTestCase): _NeutronAPIContext.side_effect = fake_context({'enable_dvr': False}) _shared_secret.return_value = 'secret_thing' self.resolve_address.return_value = '10.0.0.10' - self.assertEquals(context.DVRSharedSecretContext()(), {}) + self.assertEquals(context.SharedSecretContext()(), {}) diff --git a/unit_tests/test_neutron_ovs_hooks.py b/unit_tests/test_neutron_ovs_hooks.py index 342b7c8c..3df985ca 100644 --- a/unit_tests/test_neutron_ovs_hooks.py +++ b/unit_tests/test_neutron_ovs_hooks.py @@ -1,4 +1,4 @@ -from mock import MagicMock, patch, call +from mock import MagicMock, patch import yaml from test_utils import CharmTestCase @@ -19,13 +19,9 @@ utils.register_configs = _reg utils.restart_map = _map TO_PATCH = [ - 'apt_update', - 'apt_install', 'apt_purge', 'config', 'CONFIGS', - 'determine_packages', - 'determine_dvr_packages', 'get_shared_secret', 'git_install', 'log', @@ -33,6 +29,9 @@ TO_PATCH = [ 'relation_set', 'configure_ovs', 'use_dvr', + 'install_packages', + 'enable_nova_metadata', + 'enable_local_dhcp', ] NEUTRON_CONF_DIR = "/etc/neutron" @@ -54,19 +53,12 @@ class NeutronOVSHooksTests(CharmTestCase): @patch.object(hooks, 'git_install_requested') def test_install_hook(self, git_requested): git_requested.return_value = False - _pkgs = ['foo', 'bar'] - self.determine_packages.return_value = [_pkgs] self._call_hook('install') - self.apt_update.assert_called_with() - self.apt_install.assert_has_calls([ - call(_pkgs, fatal=True), - ]) + self.install_packages.assert_called_with() @patch.object(hooks, 'git_install_requested') def test_install_hook_git(self, git_requested): git_requested.return_value = True - _pkgs = ['foo', 'bar'] - self.determine_packages.return_value = _pkgs openstack_origin_git = { 'repositories': [ {'name': 'requirements', @@ -81,8 +73,7 @@ class NeutronOVSHooksTests(CharmTestCase): projects_yaml = yaml.dump(openstack_origin_git) self.test_config.set('openstack-origin-git', projects_yaml) self._call_hook('install') - self.apt_update.assert_called_with() - self.assertTrue(self.determine_packages) + self.install_packages.assert_called_with() self.git_install.assert_called_with(projects_yaml) @patch.object(hooks, 'git_install_requested') @@ -124,13 +115,9 @@ class NeutronOVSHooksTests(CharmTestCase): @patch.object(hooks, 'git_install_requested') def test_config_changed_dvr(self, git_requested): git_requested.return_value = False - self.determine_dvr_packages.return_value = ['dvr'] self._call_hook('config-changed') - self.apt_update.assert_called_with() + self.install_packages.assert_called_with() self.assertTrue(self.CONFIGS.write_all.called) - self.apt_install.assert_has_calls([ - call(['dvr'], fatal=True), - ]) self.configure_ovs.assert_called_with() @patch.object(hooks, 'neutron_plugin_joined') @@ -140,9 +127,22 @@ class NeutronOVSHooksTests(CharmTestCase): self.configure_ovs.assert_called_with() self.assertTrue(self.CONFIGS.write_all.called) _plugin_joined.assert_called_with(relation_id='rid') + self.install_packages.assert_called_with() + + @patch.object(hooks, 'neutron_plugin_joined') + def test_neutron_plugin_api_nodvr(self, _plugin_joined): + self.use_dvr.return_value = False + self.relation_ids.return_value = ['rid'] + self._call_hook('neutron-plugin-api-relation-changed') + self.configure_ovs.assert_called_with() + self.assertTrue(self.CONFIGS.write_all.called) + _plugin_joined.assert_called_with(relation_id='rid') + self.apt_purge.assert_called_with(['neutron-l3-agent'], fatal=True) @patch.object(hooks, 'git_install_requested') def test_neutron_plugin_joined(self, git_requested): + self.enable_nova_metadata.return_value = True + self.enable_local_dhcp.return_value = True git_requested.return_value = False self.get_shared_secret.return_value = 'secret' self._call_hook('neutron-plugin-relation-joined') diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index 20e6208c..058a9c45 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/unit_tests/test_neutron_ovs_utils.py @@ -18,8 +18,11 @@ import charmhelpers.core.hookenv as hookenv TO_PATCH = [ 'add_bridge', 'add_bridge_port', + 'apt_install', + 'apt_update', 'config', 'os_release', + 'filter_installed_packages', 'neutron_plugin_attribute', 'full_restart', 'service_restart', @@ -75,12 +78,20 @@ class TestNeutronOVSUtils(CharmTestCase): # Reset cached cache hookenv.cache = {} + @patch.object(nutils, 'determine_packages') + def test_install_packages(self, _determine_packages): + _determine_packages.return_value = 'randompkg' + nutils.install_packages() + self.apt_update.assert_called_with() + self.apt_install.assert_called_with(self.filter_installed_packages()) + @patch.object(nutils, 'use_dvr') @patch.object(nutils, 'git_install_requested') @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release') @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package') def test_determine_packages(self, _head_pkgs, _os_rel, _git_requested, _use_dvr): + self.test_config.set('enable-local-dhcp-and-metadata', False) _git_requested.return_value = False _use_dvr.return_value = False _os_rel.return_value = 'trusty' @@ -89,6 +100,36 @@ class TestNeutronOVSUtils(CharmTestCase): expect = [['neutron-plugin-openvswitch-agent'], [head_pkg]] self.assertItemsEqual(pkg_list, expect) + @patch.object(nutils, 'use_dvr') + @patch.object(nutils, 'git_install_requested') + @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release') + @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package') + def test_determine_packages_metadata(self, _head_pkgs, _os_rel, + _git_requested, _use_dvr): + self.test_config.set('enable-local-dhcp-and-metadata', True) + _git_requested.return_value = False + _use_dvr.return_value = False + _os_rel.return_value = 'trusty' + _head_pkgs.return_value = head_pkg + pkg_list = nutils.determine_packages() + expect = [['neutron-plugin-openvswitch-agent'], [head_pkg], + 'neutron-metadata-agent', 'neutron-dhcp-agent'] + self.assertItemsEqual(pkg_list, expect) + + @patch.object(nutils, 'use_dvr') + @patch.object(nutils, 'git_install_requested') + @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release') + @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package') + def test_determine_packages_git(self, _head_pkgs, _os_rel, + _git_requested, _use_dvr): + self.test_config.set('enable-local-dhcp-and-metadata', False) + _git_requested.return_value = True + _use_dvr.return_value = True + _os_rel.return_value = 'trusty' + _head_pkgs.return_value = head_pkg + pkg_list = nutils.determine_packages() + self.assertFalse('neutron-l3-agent' in pkg_list) + @patch.object(nutils, 'use_dvr') def test_register_configs(self, _use_dvr): class _mock_OSConfigRenderer(): @@ -128,6 +169,19 @@ class TestNeutronOVSUtils(CharmTestCase): [self.assertIn(q_conf, _map.keys()) for q_conf in confs] self.assertEqual(_map[nutils.NEUTRON_CONF]['services'], svcs) + @patch.object(nutils, 'enable_local_dhcp') + @patch.object(nutils, 'use_dvr') + def test_resource_map_dhcp(self, _use_dvr, _enable_local_dhcp): + _enable_local_dhcp.return_value = True + _use_dvr.return_value = False + _map = nutils.resource_map() + svcs = ['neutron-plugin-openvswitch-agent', 'neutron-metadata-agent', + 'neutron-dhcp-agent'] + confs = [nutils.NEUTRON_CONF, nutils.NEUTRON_METADATA_AGENT_CONF, + nutils.NEUTRON_DHCP_AGENT_CONF] + [self.assertIn(q_conf, _map.keys()) for q_conf in confs] + self.assertEqual(_map[nutils.NEUTRON_CONF]['services'], svcs) + @patch.object(nutils, 'use_dvr') def test_restart_map(self, _use_dvr): _use_dvr.return_value = False @@ -213,7 +267,7 @@ class TestNeutronOVSUtils(CharmTestCase): ]) self.add_bridge_port.assert_called_with('br-ex', 'eth0') - @patch.object(neutron_ovs_context, 'DVRSharedSecretContext') + @patch.object(neutron_ovs_context, 'SharedSecretContext') def test_get_shared_secret(self, _dvr_secret_ctxt): _dvr_secret_ctxt.return_value = \ DummyContext(return_value={'shared_secret': 'supersecret'})