From b14f2fc47ec2d5d31692d00db02a3b9025871f6a Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 14 Aug 2018 07:39:21 +0000 Subject: [PATCH] Remove nova metadata service The change turns off the local nova metadata service and uses endpoint data recieved from the quantum-network-service relation to point the neutron metadata service at the nova metadata service on the nova cloud controller for Queens+. Depends-On: I5ad15ba782cb87b6fdb3c0941a6482d201670bff Change-Id: I7037a20feac73f3a3f1ed1b8b1b70d0fa534bc46 --- config.yaml | 8 ++- hooks/neutron_contexts.py | 85 ++++++++++++++++---------- hooks/neutron_hooks.py | 33 ++++++---- hooks/neutron_utils.py | 69 ++++++++++++++++----- templates/icehouse/metadata_agent.ini | 5 +- templates/queens/metadata_agent.ini | 5 +- tests/basic_deployment.py | 19 +++++- unit_tests/test_neutron_contexts.py | 86 +++++++++++++++++++++++++-- unit_tests/test_neutron_hooks.py | 31 ++++++++++ unit_tests/test_neutron_utils.py | 62 ++++++++++++++++++- unit_tests/test_utils.py | 29 +++++++++ 11 files changed, 360 insertions(+), 72 deletions(-) diff --git a/config.yaml b/config.yaml index cf656c0c..0d1d56d6 100644 --- a/config.yaml +++ b/config.yaml @@ -257,7 +257,9 @@ options: description: | A JSON-formatted string that will serve as vendor metadata (via "StaticJSON" provider) to all VM's within an OpenStack deployment, - regardless of project or domain. + regardless of project or domain. For deployments of Queens or later + this value is ignored. Please set the corresponding value in the + nova-cloud-controller charm. vendor-data-url: type: string default: @@ -266,4 +268,6 @@ options: (via "DynamicJSON" provider) to all VM's within an OpenStack deployment, regardless of project or domain. . - Only supported in OpenStack Newton and higher. + Only supported in OpenStack Newton and higher. For deployments of Queens or + later this value is ignored. Please set the corresponding value in the + nova-cloud-controller charm. diff --git a/hooks/neutron_contexts.py b/hooks/neutron_contexts.py index f4f32c98..9af9a6e2 100644 --- a/hooks/neutron_contexts.py +++ b/hooks/neutron_contexts.py @@ -4,6 +4,9 @@ import uuid from charmhelpers.core.hookenv import ( log, ERROR, config, + relation_get, + relation_ids, + related_units, unit_get, network_get_primary_address, ) @@ -16,7 +19,7 @@ from charmhelpers.contrib.openstack.utils import ( os_release, CompareOpenStackReleases, ) -from charmhelpers.contrib.hahelpers.cluster import ( +from charmhelpers.contrib.hahelpers.cluster import( eligible_leader ) from charmhelpers.contrib.network.ip import ( @@ -49,6 +52,22 @@ def core_plugin(): return CORE_PLUGIN[config('plugin')] +def get_local_ip(): + fallback = get_host_ip(unit_get('private-address')) + if config('os-data-network'): + # NOTE: prefer any existing use of config based networking + local_ip = get_address_in_network( + config('os-data-network'), + fallback) + else: + # NOTE: test out network-spaces support, then fallback + try: + local_ip = get_host_ip(network_get_primary_address('data')) + except NotImplementedError: + local_ip = fallback + return local_ip + + class L3AgentContext(OSContextGenerator): def __call__(self): @@ -105,21 +124,7 @@ class NeutronGatewayContext(NeutronAPIContext): 'enable_isolated_metadata': config('enable-isolated-metadata'), } - fallback = get_host_ip(unit_get('private-address')) - if config('os-data-network'): - # NOTE: prefer any existing use of config based networking - ctxt['local_ip'] = \ - get_address_in_network(config('os-data-network'), - fallback) - else: - # NOTE: test out network-spaces support, then fallback - try: - ctxt['local_ip'] = get_host_ip( - network_get_primary_address('data') - ) - except NotImplementedError: - ctxt['local_ip'] = fallback - + ctxt['local_ip'] = get_local_ip() mappings = config('bridge-mappings') if mappings: ctxt['bridge_mappings'] = ','.join(mappings.split()) @@ -152,28 +157,46 @@ class NeutronGatewayContext(NeutronAPIContext): class NovaMetadataContext(OSContextGenerator): + def __init__(self, rel_name='quantum-network-service'): + self.rel_name = rel_name + self.interfaces = [rel_name] + def __call__(self): ctxt = {} - ctxt['vendordata_providers'] = [] - vdata = config('vendor-data') - vdata_url = config('vendor-data-url') cmp_os_release = CompareOpenStackReleases(os_release('neutron-common')) + if cmp_os_release < 'rocky': + ctxt['vendordata_providers'] = [] + vdata = config('vendor-data') + vdata_url = config('vendor-data-url') - if vdata: - ctxt['vendor_data'] = True - ctxt['vendordata_providers'].append('StaticJSON') - - if vdata_url: - if cmp_os_release > 'mitaka': - ctxt['vendor_data_url'] = vdata_url - ctxt['vendordata_providers'].append('DynamicJSON') - else: - log('Dynamic vendor data unsupported' - ' for {}.'.format(cmp_os_release), level=ERROR) + if vdata: + ctxt['vendor_data'] = True + ctxt['vendordata_providers'].append('StaticJSON') + if vdata_url: + if cmp_os_release > 'mitaka': + ctxt['vendor_data_url'] = vdata_url + ctxt['vendordata_providers'].append('DynamicJSON') + else: + log('Dynamic vendor data unsupported' + ' for {}.'.format(cmp_os_release), level=ERROR) + for rid in relation_ids(self.rel_name): + for unit in related_units(rid): + rdata = relation_get(rid=rid, unit=unit) + secret = rdata.get('shared-metadata-secret') + if secret: + ctxt['shared_secret'] = secret + ctxt['nova_metadata_host'] = rdata['nova-metadata-host'] + ctxt['nova_metadata_port'] = rdata['nova-metadata-port'] + ctxt['nova_metadata_protocol'] = rdata[ + 'nova-metadata-protocol'] + if not ctxt.get('shared_secret'): + ctxt['shared_secret'] = get_shared_secret() + ctxt['nova_metadata_host'] = get_local_ip() + ctxt['nova_metadata_port'] = '8775' + ctxt['nova_metadata_protocol'] = 'http' return ctxt - SHARED_SECRET = "/etc/{}/secret.txt" diff --git a/hooks/neutron_hooks.py b/hooks/neutron_hooks.py index 3de05178..91ac5fec 100755 --- a/hooks/neutron_hooks.py +++ b/hooks/neutron_hooks.py @@ -69,6 +69,8 @@ from neutron_utils import ( write_vendordata, pause_unit_helper, resume_unit_helper, + remove_legacy_nova_metadata, + disable_nova_metadata, ) hooks = Hooks() @@ -149,6 +151,9 @@ def config_changed(): # Setup legacy ha configurations update_legacy_ha_files() + # Disable nova metadata if possible, + if disable_nova_metadata(): + remove_legacy_nova_metadata() @hooks.hook('upgrade-charm') @@ -224,18 +229,22 @@ def nm_changed(): if config('ha-legacy-mode'): cache_env_data() - # NOTE: nova-api-metadata needs to be restarted - # once the nova-conductor is up and running - # on the nova-cc units. - restart_nonce = relation_get('restart_trigger') - if restart_nonce is not None: - db = kv() - previous_nonce = db.get('restart_nonce') - if previous_nonce != restart_nonce: - if not is_unit_paused_set(): - service_restart('nova-api-metadata') - db.set('restart_nonce', restart_nonce) - db.flush() + # Disable nova metadata if possible, + if disable_nova_metadata(): + remove_legacy_nova_metadata() + else: + # NOTE: nova-api-metadata needs to be restarted + # once the nova-conductor is up and running + # on the nova-cc units. + restart_nonce = relation_get('restart_trigger') + if restart_nonce is not None: + db = kv() + previous_nonce = db.get('restart_nonce') + if previous_nonce != restart_nonce: + if not is_unit_paused_set(): + service_restart('nova-api-metadata') + db.set('restart_nonce', restart_nonce) + db.flush() @hooks.hook("cluster-relation-departed") diff --git a/hooks/neutron_utils.py b/hooks/neutron_utils.py index 91b00630..dc1f5c10 100644 --- a/hooks/neutron_utils.py +++ b/hooks/neutron_utils.py @@ -6,6 +6,7 @@ from shutil import copy2 from charmhelpers.core.host import ( lsb_release, mkdir, + service, service_running, service_stop, service_restart, @@ -20,6 +21,8 @@ from charmhelpers.core.hookenv import ( config, is_relation_made, relation_ids, + related_units, + relation_get, ) from charmhelpers.fetch import ( apt_upgrade, @@ -84,7 +87,6 @@ from copy import deepcopy def valid_plugin(): return config('plugin') in CORE_PLUGIN - NEUTRON_COMMON = 'neutron-common' VERSION_PACKAGE = NEUTRON_COMMON @@ -269,6 +271,8 @@ def get_packages(): # LBaaS v1 dropped in newton packages.remove('neutron-lbaas-agent') packages.append('neutron-lbaasv2-agent') + if disable_nova_metadata(cmp_os_source): + packages.remove('nova-api-metadata') packages.extend(determine_l3ha_packages()) if cmp_os_source >= 'rocky': @@ -295,7 +299,6 @@ def determine_l3ha_packages(): def use_l3ha(): return NeutronAPIContext()()['enable_l3ha'] - EXT_PORT_CONF = '/etc/init/ext-port.conf' PHY_NIC_MTU_CONF = '/etc/init/os-charm-phy-nic-mtu.conf' STOPPED_SERVICES = ['os-charm-phy-nic-mtu', 'ext-port'] @@ -351,7 +354,8 @@ NEUTRON_SHARED_CONFIG_FILES = { NEUTRON_METADATA_AGENT_CONF: { 'hook_contexts': [NetworkServiceContext(), context.WorkerConfigContext(), - NeutronGatewayContext()], + NeutronGatewayContext(), + NovaMetadataContext()], 'services': ['neutron-metadata-agent'] }, NEUTRON_DHCP_AA_PROFILE_PATH: { @@ -619,21 +623,24 @@ def resolve_config_files(plugin, release): else: drop_config.extend([NEUTRON_LBAAS_AA_PROFILE_PATH]) + if disable_nova_metadata(cmp_os_release): + drop_config.extend(NOVA_CONFIG_FILES.keys()) + else: + if is_relation_made('amqp-nova'): + amqp_nova_ctxt = context.AMQPContext( + ssl_dir=NOVA_CONF_DIR, + rel_name='amqp-nova', + relation_prefix='nova') + else: + amqp_nova_ctxt = context.AMQPContext( + ssl_dir=NOVA_CONF_DIR, + rel_name='amqp') + config_files[plugin][NOVA_CONF][ + 'hook_contexts'].append(amqp_nova_ctxt) + for _config in drop_config: if _config in config_files[plugin]: config_files[plugin].pop(_config) - - if is_relation_made('amqp-nova'): - amqp_nova_ctxt = context.AMQPContext( - ssl_dir=NOVA_CONF_DIR, - rel_name='amqp-nova', - relation_prefix='nova') - else: - amqp_nova_ctxt = context.AMQPContext( - ssl_dir=NOVA_CONF_DIR, - rel_name='amqp') - config_files[plugin][NOVA_CONF][ - 'hook_contexts'].append(amqp_nova_ctxt) return config_files @@ -841,6 +848,37 @@ def update_legacy_ha_files(force=False): remove_legacy_ha_files() +def remove_legacy_nova_metadata(): + """Remove nova metadata files.""" + service_name = 'nova-api-metadata' + service_stop(service_name) + service('disable', service_name) + service('mask', service_name) + for f in NOVA_CONFIG_FILES.keys(): + remove_file(f) + + +def disable_nova_metadata(cmp_os_source=None): + """Check whether nova mnetadata service should be disabled.""" + if not cmp_os_source: + cmp_os_source = CompareOpenStackReleases(os_release('neutron-common')) + if cmp_os_source >= 'rocky': + secret = None + for name in ['quantum', 'neutron']: + for rid in relation_ids('{}-network-service'.format(name)): + for unit in related_units(rid): + rdata = relation_get(rid=rid, unit=unit) + # The presence of the secret shows the + # nova-cloud-controller charm is running a metadata + # service so it can be disabled locally. + if rdata.get('shared-metadata-secret'): + secret = rdata.get('shared-metadata-secret') + disable = bool(secret) + else: + disable = False + return disable + + def cache_env_data(): env = NetworkServiceContext()() if not env: @@ -1010,7 +1048,6 @@ def configure_apparmor(): for profile in profiles: context.AppArmorContext(profile).setup_aa_profile() - VENDORDATA_FILE = '/etc/nova/vendor_data.json' diff --git a/templates/icehouse/metadata_agent.ini b/templates/icehouse/metadata_agent.ini index 4f788b21..d4478d08 100644 --- a/templates/icehouse/metadata_agent.ini +++ b/templates/icehouse/metadata_agent.ini @@ -13,8 +13,9 @@ admin_password = {{ service_password }} root_helper = sudo neutron-rootwrap /etc/neutron/rootwrap.conf state_path = /var/lib/neutron # Gateway runs a metadata API server locally -nova_metadata_ip = {{ local_ip }} -nova_metadata_port = 8775 +nova_metadata_ip = {{ nova_metadata_host }} +nova_metadata_port = {{ nova_metadata_port }} +nova_metadata_protocol = {{ nova_metadata_protocol }} metadata_proxy_shared_secret = {{ shared_secret }} cache_url = memory://?default_ttl=5 metadata_workers = {{ workers }} diff --git a/templates/queens/metadata_agent.ini b/templates/queens/metadata_agent.ini index 8ddda574..a58d14db 100644 --- a/templates/queens/metadata_agent.ini +++ b/templates/queens/metadata_agent.ini @@ -8,8 +8,9 @@ root_helper = sudo neutron-rootwrap /etc/neutron/rootwrap.conf state_path = /var/lib/neutron # Gateway runs a metadata API server locally -nova_metadata_ip = {{ local_ip }} -nova_metadata_port = 8775 +nova_metadata_host = {{ nova_metadata_host }} +nova_metadata_port = {{ nova_metadata_port }} +nova_metadata_protocol = {{ nova_metadata_protocol }} metadata_proxy_shared_secret = {{ shared_secret }} cache_url = memory://?default_ttl=5 metadata_workers = {{ workers }} diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index c48f4ac9..189310e3 100644 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -736,14 +736,27 @@ class NeutronGatewayBasicDeployment(OpenStackAmuletDeployment): """Verify the data in the metadata agent config file.""" u.log.debug('Checking neutron gateway metadata agent ' 'config file data...') + cmp_os_release = CompareOpenStackReleases( + self._get_openstack_release_string() + ) unit = self.neutron_gateway_sentry + if cmp_os_release >= 'rocky': + nova_metadata_ip_key = 'nova_metadata_host' + nova_metadata_ip_value = self.get_private_address( + self.nova_cc_sentry) + elif cmp_os_release >= 'queens': + nova_metadata_ip_key = 'nova_metadata_host' + nova_metadata_ip_value = self.get_private_address(unit) + else: + nova_metadata_ip_key = 'nova_metadata_ip' + nova_metadata_ip_value = self.get_private_address(unit) conf = '/etc/neutron/metadata_agent.ini' expected = { 'root_helper': 'sudo neutron-rootwrap ' '/etc/neutron/rootwrap.conf', 'state_path': '/var/lib/neutron', - 'nova_metadata_ip': self.get_private_address(unit), + nova_metadata_ip_key: nova_metadata_ip_value, 'nova_metadata_port': '8775', 'cache_url': 'memory://?default_ttl=5' } @@ -782,6 +795,10 @@ class NeutronGatewayBasicDeployment(OpenStackAmuletDeployment): def test_308_neutron_nova_config(self): """Verify the data in the nova config file.""" u.log.debug('Checking neutron gateway nova config file data...') + if self._get_openstack_release() >= self.bionic_rocky: + u.log.info("Skipping test, gateway has no nova config after " + "Queens") + return unit = self.neutron_gateway_sentry conf = '/etc/nova/nova.conf' diff --git a/unit_tests/test_neutron_contexts.py b/unit_tests/test_neutron_contexts.py index ca86eb57..d2b4d88b 100644 --- a/unit_tests/test_neutron_contexts.py +++ b/unit_tests/test_neutron_contexts.py @@ -345,7 +345,14 @@ class TestNovaMetadataContext(CharmTestCase): TO_PATCH) self.config.side_effect = self.test_config.get - def test_vendordata_static(self): + @patch.object(neutron_contexts, 'get_local_ip') + @patch.object(neutron_contexts, 'get_shared_secret') + @patch.object(neutron_contexts, 'relation_ids') + def test_vendordata_static(self, _relation_ids, _get_shared_secret, + _get_local_ip): + _get_shared_secret.return_value = 'asecret' + _get_local_ip.return_value = '127.0.0.1' + _relation_ids.return_value = [] _vdata = '{"good": "json"}' self.os_release.return_value = 'pike' @@ -355,7 +362,14 @@ class TestNovaMetadataContext(CharmTestCase): self.assertTrue(ctxt['vendor_data']) self.assertEqual(ctxt['vendordata_providers'], ['StaticJSON']) - def test_vendordata_dynamic(self): + @patch.object(neutron_contexts, 'get_local_ip') + @patch.object(neutron_contexts, 'get_shared_secret') + @patch.object(neutron_contexts, 'relation_ids') + def test_vendordata_dynamic(self, _relation_ids, _get_shared_secret, + _get_local_ip): + _get_shared_secret.return_value = 'asecret' + _get_local_ip.return_value = '127.0.0.1' + _relation_ids.return_value = [] _vdata_url = 'http://example.org/vdata' self.os_release.return_value = 'pike' @@ -365,7 +379,14 @@ class TestNovaMetadataContext(CharmTestCase): self.assertEqual(ctxt['vendor_data_url'], _vdata_url) self.assertEqual(ctxt['vendordata_providers'], ['DynamicJSON']) - def test_vendordata_static_and_dynamic(self): + @patch.object(neutron_contexts, 'get_local_ip') + @patch.object(neutron_contexts, 'get_shared_secret') + @patch.object(neutron_contexts, 'relation_ids') + def test_vendordata_static_and_dynamic(self, _relation_ids, + _get_shared_secret, _get_local_ip): + _get_shared_secret.return_value = 'asecret' + _get_local_ip.return_value = '127.0.0.1' + _relation_ids.return_value = [] _vdata = '{"good": "json"}' _vdata_url = 'http://example.org/vdata' self.os_release.return_value = 'pike' @@ -379,11 +400,66 @@ class TestNovaMetadataContext(CharmTestCase): self.assertEqual(ctxt['vendordata_providers'], ['StaticJSON', 'DynamicJSON']) - def test_vendordata_mitaka(self): + @patch.object(neutron_contexts, 'get_local_ip') + @patch.object(neutron_contexts, 'get_shared_secret') + @patch.object(neutron_contexts, 'relation_ids') + def test_vendordata_mitaka(self, _relation_ids, _get_shared_secret, + _get_local_ip): + _get_shared_secret.return_value = 'asecret' + _get_local_ip.return_value = '127.0.0.1' + _relation_ids.return_value = [] _vdata_url = 'http://example.org/vdata' self.os_release.return_value = 'mitaka' self.test_config.set('vendor-data-url', _vdata_url) ctxt = neutron_contexts.NovaMetadataContext()() + self.assertEqual( + ctxt, + { + 'nova_metadata_host': '127.0.0.1', + 'nova_metadata_port': '8775', + 'nova_metadata_protocol': 'http', + 'shared_secret': 'asecret', + 'vendordata_providers': []}) - self.assertEqual(ctxt, {'vendordata_providers': []}) + @patch.object(neutron_contexts, 'relation_get') + @patch.object(neutron_contexts, 'related_units') + @patch.object(neutron_contexts, 'relation_ids') + def test_NovaMetadataContext(self, _relation_ids, _related_units, + _relation_get): + _relation_ids.return_value = ['rid:1'] + _related_units.return_value = ['nova-cloud-contoller/0'] + _relation_get.return_value = { + 'nova-metadata-host': '10.0.0.10', + 'nova-metadata-port': '8775', + 'nova-metadata-protocol': 'http', + 'shared-metadata-secret': 'auuid'} + self.os_release.return_value = 'queens' + + self.assertEqual( + neutron_contexts.NovaMetadataContext()(), + { + 'nova_metadata_host': '10.0.0.10', + 'nova_metadata_port': '8775', + 'nova_metadata_protocol': 'http', + 'shared_secret': 'auuid', + 'vendordata_providers': []}) + + @patch.object(neutron_contexts, 'get_local_ip') + @patch.object(neutron_contexts, 'get_shared_secret') + @patch.object(neutron_contexts, 'relation_ids') + def test_NovaMetadataContext_legacy(self, _relation_ids, + _get_shared_secret, + _get_local_ip): + _relation_ids.return_value = [] + _get_shared_secret.return_value = 'buuid' + _get_local_ip.return_value = '127.0.0.1' + self.os_release.return_value = 'pike' + self.assertEqual( + neutron_contexts.NovaMetadataContext()(), + { + 'nova_metadata_host': '127.0.0.1', + 'nova_metadata_port': '8775', + 'nova_metadata_protocol': 'http', + 'shared_secret': 'buuid', + 'vendordata_providers': []}) diff --git a/unit_tests/test_neutron_hooks.py b/unit_tests/test_neutron_hooks.py index 58e8eeb5..49ac16ad 100644 --- a/unit_tests/test_neutron_hooks.py +++ b/unit_tests/test_neutron_hooks.py @@ -59,6 +59,8 @@ TO_PATCH = [ 'is_unit_paused_set', 'install_systemd_override', 'configure_apparmor', + 'disable_nova_metadata', + 'remove_legacy_nova_metadata', ] @@ -112,6 +114,8 @@ class TestQuantumHooks(CharmTestCase): _exit.assert_called_with(1) def test_config_changed(self): + self.disable_nova_metadata.return_value = False + def mock_relids(rel): return ['relid'] self.test_config.set('sysctl', '{ kernel.max_pid: "1337"}') @@ -129,6 +133,7 @@ class TestQuantumHooks(CharmTestCase): self.configure_apparmor.assert_called_with() def test_config_changed_upgrade(self): + self.disable_nova_metadata.return_value = False self.openstack_upgrade_available.return_value = True self.valid_plugin.return_value = True self._call_hook('config-changed') @@ -148,6 +153,7 @@ class TestQuantumHooks(CharmTestCase): @patch('sys.exit') def test_config_changed_invalid_plugin(self, _exit): + self.disable_nova_metadata.return_value = False self.valid_plugin.return_value = False self._call_hook('config-changed') self.assertTrue(self.log.called) @@ -202,6 +208,8 @@ class TestQuantumHooks(CharmTestCase): self.assertTrue(self.CONFIGS.write_all.called) def test_nm_changed(self): + self.disable_nova_metadata.return_value = False + def _relation_get(key): data = { 'ca_cert': 'cert', @@ -215,6 +223,8 @@ class TestQuantumHooks(CharmTestCase): def test_nm_changed_restart_nonce(self): '''Ensure first set of restart_trigger restarts nova-api-metadata''' + self.disable_nova_metadata.return_value = False + def _relation_get(key): data = { 'ca_cert': 'cert', @@ -237,6 +247,8 @@ class TestQuantumHooks(CharmTestCase): def test_nm_changed_restart_nonce_changed(self): '''Ensure change of restart_trigger restarts nova-api-metadata''' + self.disable_nova_metadata.return_value = False + def _relation_get(key): data = { 'ca_cert': 'cert', @@ -259,6 +271,9 @@ class TestQuantumHooks(CharmTestCase): def test_nm_changed_restart_nonce_nochange(self): '''Ensure no change in restart_trigger skips restarts''' + self.patch_object(hooks, 'disable_nova_metadata', + return_value=False) + def _relation_get(key): data = { 'ca_cert': 'cert', @@ -278,6 +293,20 @@ class TestQuantumHooks(CharmTestCase): self.assertFalse(kv_mock.set.called) self.assertFalse(kv_mock.flush.called) + def test_nm_changed_disable_meta(self): + self.disable_nova_metadata.return_value = True + + def _relation_get(key): + data = { + 'ca_cert': 'cert', + } + return data.get(key) + self.relation_get.side_effect = _relation_get + self._call_hook('quantum-network-service-relation-changed') + self.assertTrue(self.CONFIGS.write_all.called) + self.install_ca_cert.assert_called_with('cert') + self.remove_legacy_nova_metadata.assert_called_once_with() + def test_neutron_plugin_changed(self): self.use_l3ha.return_value = True self._call_hook('neutron-plugin-api-relation-changed') @@ -308,6 +337,8 @@ class TestQuantumHooks(CharmTestCase): self.assertTrue(self.stop_neutron_ha_monitor_daemon.called) def test_quantum_network_service_relation_changed(self): + self.patch_object(hooks, 'disable_nova_metadata', + return_value=False) self.test_config.set('ha-legacy-mode', True) self._call_hook('quantum-network-service-relation-changed') self.assertTrue(self.cache_env_data.called) diff --git a/unit_tests/test_neutron_utils.py b/unit_tests/test_neutron_utils.py index 55e58df2..a8b1452e 100644 --- a/unit_tests/test_neutron_utils.py +++ b/unit_tests/test_neutron_utils.py @@ -60,6 +60,7 @@ class TestNeutronUtils(CharmTestCase): self.maxDiff = None def tearDown(self): + super(TestNeutronUtils, self).tearDown() # Reset cached cache hookenv.cache = {} @@ -96,12 +97,16 @@ class TestNeutronUtils(CharmTestCase): []) def test_get_packages_ovs_icehouse(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.os_release.return_value = 'icehouse' self.assertTrue('neutron-vpn-agent' in neutron_utils.get_packages()) self.assertFalse('neutron-l3-agent' in neutron_utils.get_packages()) def test_get_packages_ovs_juno_utopic(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.os_release.return_value = 'juno' self._set_distrib_codename('utopic') @@ -109,17 +114,23 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue('neutron-l3-agent' in neutron_utils.get_packages()) def test_get_packages_ovs_juno_trusty(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.os_release.return_value = 'juno' self.assertTrue('neutron-vpn-agent' in neutron_utils.get_packages()) self.assertFalse('neutron-l3-agent' in neutron_utils.get_packages()) def test_get_packages_ovs_kilo(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.os_release.return_value = 'kilo' self.assertTrue('python-neutron-fwaas' in neutron_utils.get_packages()) def test_get_packages_ovs_liberty(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.os_release.return_value = 'liberty' packages = neutron_utils.get_packages() @@ -129,6 +140,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue('python-pymysql' in packages) def test_get_packages_ovs_mitaka(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.os_release.return_value = 'mitaka' packages = neutron_utils.get_packages() @@ -140,6 +153,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue('python-pymysql' in packages) def test_get_packages_ovs_newton(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.os_release.return_value = 'newton' packages = neutron_utils.get_packages() @@ -152,6 +167,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue('python-pymysql' in packages) def test_get_packages_ovs_rocky(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=True) self.config.return_value = 'ovs' self.os_release.return_value = 'rocky' packages = neutron_utils.get_packages() @@ -179,6 +196,8 @@ class TestNeutronUtils(CharmTestCase): ) def test_get_packages_ovsodl_icehouse(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs-odl' self.os_release.return_value = 'icehouse' packages = neutron_utils.get_packages() @@ -189,6 +208,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue('neutron-lbaas-agent' in packages) def test_get_packages_ovsodl_newton(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs-odl' self.os_release.return_value = 'newton' packages = neutron_utils.get_packages() @@ -200,6 +221,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue('neutron-lbaasv2-agent' in packages) def test_get_packages_l3ha(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.get_os_codename_install_source.return_value = 'juno' self.os_release.return_value = 'juno' @@ -319,6 +342,8 @@ class TestNeutronUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.templating.OSConfigRenderer') def test_do_openstack_upgrade(self, mock_renderer, mock_register_configs): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) mock_configs = MagicMock() mock_register_configs.return_value = mock_configs self.config.side_effect = self.test_config.get @@ -349,6 +374,8 @@ class TestNeutronUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.templating.OSConfigRenderer') def test_do_openstack_upgrade_rocky(self, mock_renderer, mock_register_configs): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=True) mock_configs = MagicMock() mock_register_configs.return_value = mock_configs self.config.side_effect = self.test_config.get @@ -381,6 +408,8 @@ class TestNeutronUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.templating.OSConfigRenderer') def test_register_configs_ovs(self, mock_renderer): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.os_release.return_value = 'diablo' self.is_relation_made.return_value = False @@ -397,6 +426,8 @@ class TestNeutronUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.templating.OSConfigRenderer') def test_register_configs_ovs_odl(self, mock_renderer): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.side_effect = self.test_config.get self.test_config.set('plugin', 'ovs-odl') self.is_relation_made.return_value = False @@ -414,6 +445,8 @@ class TestNeutronUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.templating.OSConfigRenderer') def test_register_configs_amqp_nova(self, mock_renderer): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.is_relation_made.return_value = True self.os_release.return_value = 'diablo' @@ -430,6 +463,8 @@ class TestNeutronUtils(CharmTestCase): @patch.object(neutron_utils, 'get_packages') def test_restart_map_ovs(self, mock_get_packages): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.get_os_codename_install_source.return_value = 'havana' mock_get_packages.return_value = ['neutron-vpn-agent'] @@ -475,6 +510,8 @@ class TestNeutronUtils(CharmTestCase): @patch.object(neutron_utils, 'get_packages') def test_restart_map_ovs_mitaka(self, mock_get_packages): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' mock_get_packages.return_value = ['neutron-vpn-agent'] self.os_release.return_value = 'mitaka' @@ -518,6 +555,8 @@ class TestNeutronUtils(CharmTestCase): @patch.object(neutron_utils, 'get_packages') def test_restart_map_ovs_newton(self, mock_get_packages): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' mock_get_packages.return_value = ['neutron-vpn-agent'] self.os_release.return_value = 'newton' @@ -561,6 +600,8 @@ class TestNeutronUtils(CharmTestCase): @patch.object(neutron_utils, 'get_packages') def test_restart_map_ovs_post_trusty(self, mock_get_packages): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' # No VPN agent after trusty mock_get_packages.return_value = ['neutron-l3-agent'] @@ -571,6 +612,8 @@ class TestNeutronUtils(CharmTestCase): @patch.object(neutron_utils, 'get_packages') def test_restart_map_ovs_odl(self, mock_get_packages): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs-odl' mock_get_packages.return_value = ['neutron-vpn-agent'] self.os_release.return_value = 'icehouse' @@ -609,6 +652,8 @@ class TestNeutronUtils(CharmTestCase): @patch.object(neutron_utils, 'get_packages') def test_restart_map_ovs_odl_newton(self, mock_get_packages): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs-odl' mock_get_packages.return_value = ['neutron-vpn-agent'] self.os_release.return_value = 'newton' @@ -647,6 +692,8 @@ class TestNeutronUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.templating.OSConfigRenderer') def test_register_configs_nsx(self, mock_renderer): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'nsx' self.os_release.return_value = 'diablo' configs = neutron_utils.register_configs() @@ -658,6 +705,8 @@ class TestNeutronUtils(CharmTestCase): configs.register.assert_any_call(conf, ANY) def test_stop_services_ovs(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.os_release.return_value = 'diablo' neutron_utils.stop_services() @@ -673,6 +722,8 @@ class TestNeutronUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.templating.OSConfigRenderer') def test_register_configs_pre_install(self, mock_renderer): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self.config.return_value = 'ovs' self.is_relation_made.return_value = False self.os_release.return_value = 'diablo' @@ -720,6 +771,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue(self.log.called) def test_resolve_config_files_ovs_liberty(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self._set_distrib_codename('trusty') self.os_release.return_value = 'liberty' self.is_relation_made = False @@ -735,6 +788,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue(config not in actual_configs) def test_resolve_config_files_ovs_mitaka(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self._set_distrib_codename('trusty') self.os_release.return_value = 'mitaka' self.is_relation_made = False @@ -750,6 +805,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue(config not in actual_configs) def test_resolve_config_files_ovs_trusty(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self._set_distrib_codename('trusty') self.os_release.return_value = 'mitaka' self.is_relation_made = False @@ -763,6 +820,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue(config in actual_configs) def test_resolve_config_files_ovs_xenial(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self._set_distrib_codename('xenial') self.os_release.return_value = 'mitaka' self.is_relation_made = False @@ -776,6 +835,8 @@ class TestNeutronUtils(CharmTestCase): self.assertTrue(config not in actual_configs) def test_resolve_config_files_ovs_newton(self): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) self._set_distrib_codename('xenial') self.os_release.return_value = 'newton' self.is_relation_made = False @@ -803,7 +864,6 @@ class TestNeutronUtils(CharmTestCase): with patch_open() as (_open, _file): self.assertEqual(neutron_utils.write_vendordata(_jdata), False) - network_context = { 'service_username': 'foo', 'service_password': 'bar', diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py index 1b5cddea..a4d6aa9f 100644 --- a/unit_tests/test_utils.py +++ b/unit_tests/test_utils.py @@ -53,6 +53,8 @@ class CharmTestCase(unittest.TestCase): self.test_config = TestConfig() self.test_relation = TestRelation() self.patch_all() + self._patches = {} + self._patches_start = {} def patch(self, method): _m = patch.object(self.obj, method) @@ -64,6 +66,33 @@ class CharmTestCase(unittest.TestCase): for method in self.patches: setattr(self, method, self.patch(method)) + def tearDown(self): + for k, v in self._patches.items(): + v.stop() + setattr(self, k, None) + self._patches = None + self._patches_start = None + + def patch_object(self, obj, attr, name=None, **kwargs): + """Patch a patchable thing. Uses mock.patch.object() to do the work. + Automatically unpatches at the end of the test. + + The mock gets added to the test object (self) using 'name' or the attr + passed in the arguments. + + :param obj: an object that needs to have an attribute patched. + :param attr: that represents the attribute being patched. + :param name: optional name to call the mock. + :param **kwargs: any other args to pass to mock.patch() + """ + mocked = patch.object(obj, attr, **kwargs) + if name is None: + name = attr + started = mocked.start() + self._patches[name] = mocked + self._patches_start[name] = started + setattr(self, name, started) + class TestConfig(object):