From 540e0ea4fcfb73f9f68a308218bd7c25983c709c Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Thu, 19 Mar 2020 17:29:08 +0100 Subject: [PATCH] tidy up style things, add docstring --- .../test_zaza_utilities_upgrade_utils.py | 39 +++++++++++------- zaza/openstack/utilities/generic.py | 2 + zaza/openstack/utilities/series_upgrade.py | 41 +++++++++++-------- zaza/openstack/utilities/upgrade_utils.py | 28 ++++++------- 4 files changed, 65 insertions(+), 45 deletions(-) diff --git a/unit_tests/utilities/test_zaza_utilities_upgrade_utils.py b/unit_tests/utilities/test_zaza_utilities_upgrade_utils.py index 3c27074..1f21b49 100644 --- a/unit_tests/utilities/test_zaza_utilities_upgrade_utils.py +++ b/unit_tests/utilities/test_zaza_utilities_upgrade_utils.py @@ -12,8 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import collections import copy import mock +import pprint import unit_tests.utils as ut_utils import zaza.openstack.utilities.upgrade_utils as openstack_upgrade @@ -81,29 +83,36 @@ class TestUpgradeUtils(ut_utils.BaseTestCase): 'charm': 'cs:cinder-ceph-2'}}}}}} def test_get_upgrade_candidates(self): - self.maxDiff = None - expect = copy.deepcopy(self.juju_status.applications) + expected = copy.deepcopy(self.juju_status.applications) self.assertEqual( openstack_upgrade.get_upgrade_candidates(), - expect) + expected) def test_get_upgrade_groups(self): + expected = collections.OrderedDict([ + ('Core Identity', []), + ('Control Plane', ['cinder']), + ('Data Plane', ['nova-compute']), + ('sweep_up', [])]) + actual = openstack_upgrade.get_upgrade_groups() + pprint.pprint(expected) + pprint.pprint(actual) self.assertEqual( - openstack_upgrade.get_upgrade_groups(), - { - 'Core Identity': [], - 'Control Plane': ['cinder'], - 'Data Plane': ['nova-compute'], - 'sweep_up': []}) + actual, + expected) def test_get_series_upgrade_groups(self): + expected = collections.OrderedDict([ + ('Core Identity', []), + ('Control Plane', ['cinder']), + ('Data Plane', ['nova-compute']), + ('sweep_up', ['mydb', 'ntp'])]) + actual = openstack_upgrade.get_series_upgrade_groups() + pprint.pprint(expected) + pprint.pprint(actual) self.assertEqual( - openstack_upgrade.get_series_upgrade_groups(), - { - 'Core Identity': [], - 'Control Plane': ['cinder'], - 'Data Plane': ['nova-compute'], - 'sweep_up': ['mydb', 'ntp']}) + actual, + expected) def test_extract_charm_name_from_url(self): self.assertEqual( diff --git a/zaza/openstack/utilities/generic.py b/zaza/openstack/utilities/generic.py index a63abc5..3a22404 100644 --- a/zaza/openstack/utilities/generic.py +++ b/zaza/openstack/utilities/generic.py @@ -341,6 +341,8 @@ async def check_call(cmd): stderr=asyncio.subprocess.PIPE) stdout, stderr = await proc.communicate() if proc.returncode != 0: + logging.warn("STDOUT: {}".format(stdout)) + logging.warn("STDERR: {}".format(stderr)) raise subprocess.CalledProcessError(proc.returncode, cmd) diff --git a/zaza/openstack/utilities/series_upgrade.py b/zaza/openstack/utilities/series_upgrade.py index 52e9c49..f217b17 100644 --- a/zaza/openstack/utilities/series_upgrade.py +++ b/zaza/openstack/utilities/series_upgrade.py @@ -43,6 +43,12 @@ def run_post_upgrade_functions(post_upgrade_functions): def get_charm_settings(): + """Application series upgrade settings. + + :returns: A Dict of the settings to manage a series upgrade + :rtype: Dict + + """ default = { 'origin': 'openstack-origin', 'pause_non_leader_subordinate': True, @@ -56,7 +62,7 @@ def get_charm_settings(): 'origin': 'source', 'pause_non_leader_subordinate': False}, 'percona-cluster': {'origin': 'source'}, - 'nova-compute' : { + 'nova-compute': { 'pause_non_leader_primary': False, 'pause_non_leader_subordinate': False}, 'mongodb': { @@ -305,16 +311,17 @@ def series_upgrade_application(application, pause_non_leader_primary=True, model.block_until_all_units_idle() -async def async_series_upgrade_application(application, - pause_non_leader_primary=True, - pause_non_leader_subordinate=True, - from_series="trusty", - to_series="xenial", - origin='openstack-origin', - completed_machines=None, - files=None, workaround_script=None, - post_upgrade_functions=None, - post_application_upgrade_functions=None): +async def async_series_upgrade_application( + application, + pause_non_leader_primary=True, + pause_non_leader_subordinate=True, + from_series="trusty", + to_series="xenial", + origin='openstack-origin', + completed_machines=None, + files=None, workaround_script=None, + post_upgrade_functions=None, + post_application_upgrade_functions=None): """Series upgrade application. Wrap all the functionality to handle series upgrade for a given @@ -433,11 +440,13 @@ async def async_series_upgrade_application(application, # TODO: Move these functions into zaza.model -async def wait_for_unit_idle(unit_name): +async def wait_for_unit_idle(unit_name, timeout=600): """Wait until the unit's agent is idle. :param unit_name: The unit name of the application, ex: mysql/0 :type unit_name: str + :param timeout: How long to wait before timing out + :type timeout: int :returns: None :rtype: None """ @@ -445,10 +454,10 @@ async def wait_for_unit_idle(unit_name): try: await model.async_block_until( _unit_idle(app, unit_name), - timeout=600) + timeout=timeout) except concurrent.futures._base.TimeoutError: - raise model.ModelTimeout("Zaza has timed out waiting on the unit to " - "reach idle state.") + raise model.ModelTimeout("Zaza has timed out waiting on {} to " + "reach idle state.".format(unit_name)) def _unit_idle(app, unit_name): @@ -593,7 +602,7 @@ async def async_series_upgrade(unit_name, machine_num, await wait_for_unit_idle(unit_name) logging.info("Complete series upgrade on {}".format(machine_num)) await async_complete_series_upgrade(machine_num) - await wait_for_unit_idle(unit_name) + await wait_for_unit_idle(unit_name, timeout=1200) logging.info("Running run_post_upgrade_functions {}".format( post_upgrade_functions)) run_post_upgrade_functions(post_upgrade_functions) diff --git a/zaza/openstack/utilities/upgrade_utils.py b/zaza/openstack/utilities/upgrade_utils.py index e345eeb..c611398 100644 --- a/zaza/openstack/utilities/upgrade_utils.py +++ b/zaza/openstack/utilities/upgrade_utils.py @@ -60,6 +60,7 @@ def _include_app(app, app_config, filters, model_name=None): return False return True + def _filter_subordinates(app, app_config, model_name=None): if app_config.get("subordinate-to"): logging.warning( @@ -70,7 +71,6 @@ def _filter_subordinates(app, app_config, model_name=None): def _filter_openstack_upgrade_list(app, app_config, model_name=None): charm_name = extract_charm_name_from_url(app_config['charm']) - print("About to check if {} or {} in {}".format(app, charm_name, UPGRADE_EXCLUDE_LIST)) if app in UPGRADE_EXCLUDE_LIST or charm_name in UPGRADE_EXCLUDE_LIST: print("Excluding {} from upgrade, on the exclude list".format(app)) logging.warning( @@ -80,14 +80,14 @@ def _filter_openstack_upgrade_list(app, app_config, model_name=None): def _filter_non_openstack_services(app, app_config, model_name=None): - charm_options = zaza.model.get_application_config( - app, model_name=model_name).keys() - src_options = ['openstack-origin', 'source'] - if not [x for x in src_options if x in charm_options]: - logging.warning( - "Excluding {} from upgrade, no src option".format(app)) - return True - return False + charm_options = zaza.model.get_application_config( + app, model_name=model_name).keys() + src_options = ['openstack-origin', 'source'] + if not [x for x in src_options if x in charm_options]: + logging.warning( + "Excluding {} from upgrade, no src option".format(app)) + return True + return False def get_upgrade_groups(model_name=None): @@ -107,10 +107,11 @@ def get_upgrade_groups(model_name=None): _filter_subordinates, _filter_openstack_upgrade_list, _filter_non_openstack_services, - ]) + ]) return _build_service_groups(apps_in_model) + def get_series_upgrade_groups(model_name=None): """Place apps in the model into their upgrade groups. @@ -124,9 +125,7 @@ def get_series_upgrade_groups(model_name=None): """ apps_in_model = get_upgrade_candidates( model_name=model_name, - filters=[ - _filter_subordinates, - ]) + filters=[_filter_subordinates]) return _build_service_groups(apps_in_model) @@ -146,10 +145,11 @@ def _build_service_groups(applications): if not (app in [a for group in groups.values() for a in group]): sweep_up.append(app) groups['sweep_up'] = sweep_up + for name, group in groups.items(): + group.sort() return groups - def extract_charm_name_from_url(charm_url): """Extract the charm name from the charm url.