From 845f728b4d676c474af34fe20fbe3975cd865d1e Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Sun, 19 Apr 2020 16:03:39 +0200 Subject: [PATCH 1/4] n-ovs: Remove redundant unit references --- zaza/openstack/charm_tests/neutron/tests.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/zaza/openstack/charm_tests/neutron/tests.py b/zaza/openstack/charm_tests/neutron/tests.py index cfbb32b..0cf148e 100644 --- a/zaza/openstack/charm_tests/neutron/tests.py +++ b/zaza/openstack/charm_tests/neutron/tests.py @@ -442,10 +442,6 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): """Run class setup for running Neutron Openvswitch tests.""" super(NeutronOpenvSwitchTest, cls).setUpClass(cls) - cls.compute_unit = zaza.model.get_units('nova-compute')[0] - cls.neutron_api_unit = zaza.model.get_units('neutron-api')[0] - cls.n_ovs_unit = zaza.model.get_units('neutron-openvswitch')[0] - # set up client cls.neutron_client = ( openstack_utils.get_neutron_session_client(cls.keystone_session)) From e3fb0fde92e47ac0c6b34639f805d66c1b095ff5 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Sun, 19 Apr 2020 16:08:35 +0200 Subject: [PATCH 2/4] n-ovs: Await start of execution before awaiting idle At present we may start interrogating the model for a result of a change made by the functional test before all units of the application have started executing. --- zaza/openstack/charm_tests/neutron/tests.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zaza/openstack/charm_tests/neutron/tests.py b/zaza/openstack/charm_tests/neutron/tests.py index 0cf148e..f4f3895 100644 --- a/zaza/openstack/charm_tests/neutron/tests.py +++ b/zaza/openstack/charm_tests/neutron/tests.py @@ -458,6 +458,7 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): self.application_name, {'enable-sriov': 'True'}) + zaza.model.wait_for_agent_status() zaza.model.wait_for_application_states() self._check_settings_in_config( @@ -481,6 +482,7 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): {'enable-sriov': 'False'}) logging.info('Waiting for config-changes to complete...') + zaza.model.wait_for_agent_status() zaza.model.wait_for_application_states() logging.debug('OK') @@ -543,6 +545,7 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): 'neutron-openvswitch', {'disable-security-groups': 'True'}) + zaza.model.wait_for_agent_status() zaza.model.wait_for_application_states() expected = { @@ -565,6 +568,7 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): 'neutron-api', {'neutron-security-groups': 'False'}) + zaza.model.wait_for_agent_status() zaza.model.wait_for_application_states() def test_401_restart_on_config_change(self): From 169dff2d8ec7857e11e45b42687f7538064d37e2 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Mon, 20 Apr 2020 08:31:58 +0200 Subject: [PATCH 3/4] n-ovs: Do not treat ``bool`` config as ``str`` When applying configuration to a model the helpers will compare settings already set to what is being attempted set to be able to accurately predict model behaviour. When passing ``bool`` values as ``str`` this does not work and it may lead to unwanted behaviour depending on the model state at the time the test runs. --- zaza/openstack/charm_tests/neutron/tests.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/zaza/openstack/charm_tests/neutron/tests.py b/zaza/openstack/charm_tests/neutron/tests.py index f4f3895..5e318ad 100644 --- a/zaza/openstack/charm_tests/neutron/tests.py +++ b/zaza/openstack/charm_tests/neutron/tests.py @@ -497,7 +497,7 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): expected = { section: { - config_file_key: [vpair[1]], + config_file_key: [str(vpair[1])], }, } @@ -517,7 +517,7 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): 'neutron-api', 'l2-population', 'l2_population', - ['False', 'True'], + [False, True], 'agent', '/etc/neutron/plugins/ml2/openvswitch_agent.ini') @@ -540,10 +540,10 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): zaza.model.set_application_config( 'neutron-api', - {'neutron-security-groups': 'True'}) + {'neutron-security-groups': True}) zaza.model.set_application_config( 'neutron-openvswitch', - {'disable-security-groups': 'True'}) + {'disable-security-groups': True}) zaza.model.wait_for_agent_status() zaza.model.wait_for_application_states() @@ -563,10 +563,10 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): logging.info('Restoring to default configuration...') zaza.model.set_application_config( 'neutron-openvswitch', - {'disable-security-groups': 'False'}) + {'disable-security-groups': False}) zaza.model.set_application_config( 'neutron-api', - {'neutron-security-groups': 'False'}) + {'neutron-security-groups': False}) zaza.model.wait_for_agent_status() zaza.model.wait_for_application_states() @@ -579,8 +579,8 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): """ self.restart_on_changed( '/etc/neutron/neutron.conf', - {'debug': 'false'}, - {'debug': 'true'}, + {'debug': False}, + {'debug': True}, {'DEFAULT': {'debug': ['False']}}, {'DEFAULT': {'debug': ['True']}}, ['neutron-openvswitch-agent'], @@ -592,8 +592,8 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): logging.debug('Skipping test') return - set_default = {'enable-qos': 'false'} - set_alternate = {'enable-qos': 'true'} + set_default = {'enable-qos': False} + set_alternate = {'enable-qos': True} app_name = 'neutron-api' conf_file = '/etc/neutron/plugins/ml2/openvswitch_agent.ini' From 92038654085eb39849c33d75b12b943e8465a794 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Mon, 20 Apr 2020 09:29:15 +0200 Subject: [PATCH 4/4] n-ovs: Do not use model.set_application_config directly There are two problems with doing so as part of individual functional tests: 1) If the application already have the value set the test will time out waiting for a change that will never be made. 2) python-libjuju ``set_config`` call requires values to be ``str`` regardless of their actual type. Pairing this fact with the requirement to use the actual type when comparing values before attempting to set them makes this very confusing and error prone. juju/python-libjuju#388 openstack-charmers/zaza#348 Use the ``config_change`` helper instead. --- zaza/openstack/charm_tests/neutron/tests.py | 43 ++++++--------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/zaza/openstack/charm_tests/neutron/tests.py b/zaza/openstack/charm_tests/neutron/tests.py index 5e318ad..c7f1648 100644 --- a/zaza/openstack/charm_tests/neutron/tests.py +++ b/zaza/openstack/charm_tests/neutron/tests.py @@ -538,38 +538,17 @@ class NeutronOpenvSwitchTest(NeutronPluginApiSharedTests): else: conf_file = "/etc/neutron/plugins/ml2/ml2_conf.ini" - zaza.model.set_application_config( - 'neutron-api', - {'neutron-security-groups': True}) - zaza.model.set_application_config( - 'neutron-openvswitch', - {'disable-security-groups': True}) - - zaza.model.wait_for_agent_status() - zaza.model.wait_for_application_states() - - expected = { - 'securitygroup': { - 'enable_security_group': ['False'], - }, - } - - zaza.model.block_until_oslo_config_entries_match( - self.application_name, - conf_file, - expected, - ) - - logging.info('Restoring to default configuration...') - zaza.model.set_application_config( - 'neutron-openvswitch', - {'disable-security-groups': False}) - zaza.model.set_application_config( - 'neutron-api', - {'neutron-security-groups': False}) - - zaza.model.wait_for_agent_status() - zaza.model.wait_for_application_states() + with self.config_change( + {'neutron-security-groups': False}, + {'neutron-security-groups': True}, + application_name='neutron-api'): + with self.config_change( + {'disable-security-groups': False}, + {'disable-security-groups': True}): + zaza.model.block_until_oslo_config_entries_match( + self.application_name, + conf_file, + {'securitygroup': {'enable_security_group': ['False']}}) def test_401_restart_on_config_change(self): """Verify that the specified services are restarted.