From 9d06bb890f19c7839dc3d17b7e3363627e240c19 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 12 Jun 2018 11:04:35 +0100 Subject: [PATCH] Move call to get_juju_model() down to run_in_model Currently interacting with functions in zaza.model requires the model to be passed in. This has resulted in multiple calls to get_juju_model(). It is cleaner to push these calls down into the model wrapper and make the model_name an optional argument. In addition, the current model name is now cached without having to check the os.env each time. Unfortunately this has resulted in the signature changing on a great many function so this diff is bigger than would normally be desirable. --- unit_tests/test_zaza_charm_lifecycle_utils.py | 19 --- unit_tests/test_zaza_model.py | 97 +++++++++---- .../utilities/test_zaza_utilities_generic.py | 3 - .../utilities/test_zaza_utilities_juju.py | 12 +- unit_tests/utils.py | 2 + zaza/charm_lifecycle/configure.py | 3 +- zaza/charm_lifecycle/deploy.py | 2 +- zaza/charm_lifecycle/utils.py | 29 ---- zaza/charm_tests/dragent/test.py | 4 +- zaza/charm_tests/test_utils.py | 3 +- zaza/charm_tests/vault/tests.py | 6 - zaza/charm_tests/vault/utils.py | 15 +- zaza/model.py | 128 ++++++++++++------ zaza/utilities/generic.py | 4 +- zaza/utilities/juju.py | 21 +-- zaza/utilities/openstack.py | 15 +- 16 files changed, 181 insertions(+), 182 deletions(-) diff --git a/unit_tests/test_zaza_charm_lifecycle_utils.py b/unit_tests/test_zaza_charm_lifecycle_utils.py index 0bd1594..abf57bf 100644 --- a/unit_tests/test_zaza_charm_lifecycle_utils.py +++ b/unit_tests/test_zaza_charm_lifecycle_utils.py @@ -30,22 +30,3 @@ class TestCharmLifecycleUtils(ut_utils.BaseTestCase): 'test_zaza_charm_lifecycle_utils.' 'TestCharmLifecycleUtils')()), type(self)) - - def test_get_juju_model(self): - self.patch_object(lc_utils.os, 'environ') - self.patch_object(lc_utils.model, 'get_current_model') - self.get_current_model.return_value = 'modelsmodel' - - def _get_env(key): - return _env.get(key) - self.environ.__getitem__.side_effect = _get_env - _env = {"JUJU_MODEL": 'envmodel'} - - # JUJU_ENV environment variable set - self.assertEqual(lc_utils.get_juju_model(), 'envmodel') - self.get_current_model.assert_not_called() - - # No envirnment variable - self.environ.__getitem__.side_effect = KeyError - self.assertEqual(lc_utils.get_juju_model(), 'modelsmodel') - self.get_current_model.assert_called_once() diff --git a/unit_tests/test_zaza_model.py b/unit_tests/test_zaza_model.py index 1dd5cd5..b038c68 100644 --- a/unit_tests/test_zaza_model.py +++ b/unit_tests/test_zaza_model.py @@ -123,6 +123,26 @@ class TestModel(ut_utils.BaseTestCase): self.Controller_mock.add_model.side_effect = _ctrl_add_model self.Controller_mock.destroy_models.side_effect = _ctrl_destroy_models + def test_get_juju_model(self): + self.patch_object(model.os, 'environ') + self.patch_object(model, 'get_current_model') + model.CURRENT_MODEL = None + self.get_current_model.return_value = 'modelsmodel' + + def _get_env(key): + return _env.get(key) + self.environ.__getitem__.side_effect = _get_env + _env = {"JUJU_MODEL": 'envmodel'} + + # JUJU_ENV environment variable set + self.assertEqual(model.get_juju_model(), 'envmodel') + self.get_current_model.assert_not_called() + + # No envirnment variable + self.environ.__getitem__.side_effect = KeyError + self.assertEqual(model.get_juju_model(), 'modelsmodel') + self.get_current_model.assert_called_once() + def test_run_in_model(self): self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock @@ -135,47 +155,53 @@ class TestModel(ut_utils.BaseTestCase): self.Model_mock.disconnect.assert_called_once_with() def test_scp_to_unit(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.patch_object(model, 'get_unit_from_name') self.get_unit_from_name.return_value = self.unit1 self.Model.return_value = self.Model_mock - model.scp_to_unit('modelname', 'app/1', '/tmp/src', '/tmp/dest') + model.scp_to_unit('app/1', '/tmp/src', '/tmp/dest') self.unit1.scp_to.assert_called_once_with( '/tmp/src', '/tmp/dest', proxy=False, scp_opts='', user='ubuntu') def test_scp_to_all_units(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock - model.scp_to_all_units('modelname', 'app', '/tmp/src', '/tmp/dest') + model.scp_to_all_units('app', '/tmp/src', '/tmp/dest') self.unit1.scp_to.assert_called_once_with( '/tmp/src', '/tmp/dest', proxy=False, scp_opts='', user='ubuntu') self.unit2.scp_to.assert_called_once_with( '/tmp/src', '/tmp/dest', proxy=False, scp_opts='', user='ubuntu') def test_scp_from_unit(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.patch_object(model, 'get_unit_from_name') self.get_unit_from_name.return_value = self.unit1 self.Model.return_value = self.Model_mock - model.scp_from_unit('modelname', 'app/1', '/tmp/src', '/tmp/dest') + model.scp_from_unit('app/1', '/tmp/src', '/tmp/dest') self.unit1.scp_from.assert_called_once_with( '/tmp/src', '/tmp/dest', proxy=False, scp_opts='', user='ubuntu') def test_get_units(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock self.assertEqual( - model.get_units('modelname', 'app'), + model.get_units('app'), self.units) def test_get_machines(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock self.assertEqual( - model.get_machines('modelname', 'app'), + model.get_machines('app'), ['machine3', 'machine7']) def test_get_first_unit_name(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'get_units') self.get_units.return_value = self.units self.assertEqual( @@ -183,62 +209,76 @@ class TestModel(ut_utils.BaseTestCase): 'app/2') def test_get_unit_from_name(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.assertEqual( model.get_unit_from_name('app/4', self.mymodel), self.unit2) def test_get_app_ips(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'get_units') self.get_units.return_value = self.units self.assertEqual(model.get_app_ips('model', 'app'), ['ip1', 'ip2']) def test_run_on_unit(self): + self.patch_object(model, 'get_juju_model', return_value='mname') expected = {'Code': '0', 'Stderr': '', 'Stdout': 'RESULT'} self.cmd = cmd = 'somecommand someargument' self.patch_object(model, 'Model') self.patch_object(model, 'get_unit_from_name') self.get_unit_from_name.return_value = self.unit1 self.Model.return_value = self.Model_mock - self.assertEqual(model.run_on_unit('app/2', 'modelname', cmd), + self.assertEqual(model.run_on_unit('app/2', cmd), expected) self.unit1.run.assert_called_once_with(cmd, timeout=None) def test_get_relation_id(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock - self.assertEqual(model.get_relation_id('testmodel', 'app', 'app'), 42) + self.assertEqual(model.get_relation_id('app', 'app'), 42) def test_get_relation_id_interface(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock - self.assertEqual(model.get_relation_id('testmodel', 'app', 'app', - 'interface'), 51) + self.assertEqual( + model.get_relation_id('app', 'app', + remote_interface_name='interface'), + 51) def test_run_action(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.patch_object(model, 'get_unit_from_name') self.get_unit_from_name.return_value = self.unit1 self.Model.return_value = self.Model_mock - model.run_action('app/2', 'modelname', 'backup', - {'backup_dir': '/dev/null'}) + model.run_action( + 'app/2', + 'backup', + action_params={'backup_dir': '/dev/null'}) self.unit1.run_action.assert_called_once_with( 'backup', backup_dir='/dev/null') def test_get_actions(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model.subprocess, 'check_output') self.check_output.return_value = 'action: "action desc"' self.assertEqual( - model.get_actions('mname', 'myapp'), + model.get_actions('myapp'), {'action': "action desc"}) self.check_output.assert_called_once_with( ['juju', 'actions', '-m', 'mname', 'myapp', '--format', 'yaml']) def test_run_action_on_leader(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock - model.run_action_on_leader('modelname', 'app', 'backup', - {'backup_dir': '/dev/null'}) + model.run_action_on_leader( + 'app', + 'backup', + action_params={'backup_dir': '/dev/null'}) self.assertFalse(self.unit1.called) self.unit2.run_action.assert_called_once_with( 'backup', @@ -419,6 +459,7 @@ class TestModel(ut_utils.BaseTestCase): def test_block_until_file_has_contents(self): self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch("builtins.open", new_callable=mock.mock_open(), name="_open") @@ -426,7 +467,6 @@ class TestModel(ut_utils.BaseTestCase): _fileobj.__enter__().read.return_value = "somestring" self._open.return_value = _fileobj model.block_until_file_has_contents( - 'modelname', 'app', '/tmp/src/myfile.txt', 'somestring', @@ -439,6 +479,7 @@ class TestModel(ut_utils.BaseTestCase): def test_block_until_file_has_contents_missing(self): self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch("builtins.open", new_callable=mock.mock_open(), name="_open") @@ -447,7 +488,6 @@ class TestModel(ut_utils.BaseTestCase): self._open.return_value = _fileobj with self.assertRaises(asyncio.futures.TimeoutError): model.block_until_file_has_contents( - 'modelname', 'app', '/tmp/src/myfile.txt', 'somestring', @@ -503,35 +543,35 @@ class TestModel(ut_utils.BaseTestCase): self.async_block_until.side_effect = _block_until def test_block_until_service_status_check_running(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.block_until_service_status_base({'Stdout': '152 409 54'}) model.block_until_service_status( - 'modelname', 'app/2', ['test_svc'], 'running') def test_block_until_service_status_check_running_fail(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.block_until_service_status_base({'Stdout': ''}) with self.assertRaises(asyncio.futures.TimeoutError): model.block_until_service_status( - 'modelname', 'app/2', ['test_svc'], 'running') def test_block_until_service_status_check_stopped(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.block_until_service_status_base({'Stdout': ''}) model.block_until_service_status( - 'modelname', 'app/2', ['test_svc'], 'stopped') def test_block_until_service_status_check_stopped_fail(self): + self.patch_object(model, 'get_juju_model', return_value='mname') self.block_until_service_status_base({'Stdout': '152 409 54'}) with self.assertRaises(asyncio.futures.TimeoutError): model.block_until_service_status( - 'modelname', 'app/2', ['test_svc'], 'stopped') @@ -542,7 +582,7 @@ class TestModel(ut_utils.BaseTestCase): self.patch_object(model, 'async_run_on_unit') self.async_run_on_unit.side_effect = _run_on_unit self.assertEqual( - model.get_unit_time('mymodel', 'app/2'), + model.get_unit_time('app/2'), 1524409654) def test_get_unit_service_start_time(self): @@ -551,8 +591,7 @@ class TestModel(ut_utils.BaseTestCase): self.patch_object(model, 'async_run_on_unit') self.async_run_on_unit.side_effect = _run_on_unit self.assertEqual( - model.get_unit_service_start_time('mymodel', 'app/2', 'mysvc1'), - 1524409654) + model.get_unit_service_start_time('app/2', 'mysvc1'), 1524409654) def test_get_unit_service_start_time_not_running(self): async def _run_on_unit(model_name, unit_name, cmd, timeout=None): @@ -560,7 +599,7 @@ class TestModel(ut_utils.BaseTestCase): self.patch_object(model, 'async_run_on_unit') self.async_run_on_unit.side_effect = _run_on_unit with self.assertRaises(model.ServiceNotRunning): - model.get_unit_service_start_time('mymodel', 'app/2', 'mysvc1') + model.get_unit_service_start_time('app/2', 'mysvc1') def block_until_oslo_config_entries_match_base(self, file_contents, expected_contents): @@ -568,11 +607,11 @@ class TestModel(ut_utils.BaseTestCase): with open('{}/myfile.txt'.format(tmpdir), 'w') as f: f.write(file_contents) self.patch_object(model, 'Model') + self.patch_object(model, 'get_juju_model', return_value='mname') self.Model.return_value = self.Model_mock self.unit1.scp_from.side_effect = _scp_from self.unit2.scp_from.side_effect = _scp_from model.block_until_oslo_config_entries_match( - 'modelname', 'app', '/tmp/src/myfile.txt', expected_contents, @@ -706,6 +745,7 @@ disk_formats = ami,ari,aki,vhd,vmdk,raw,qcow2,vdi,iso,root-tar raise model.ServiceNotRunning('sv1') else: return gu_return + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'async_get_unit_service_start_time') self.async_get_unit_service_start_time.side_effect = \ _async_get_unit_service_start_time @@ -715,7 +755,6 @@ disk_formats = ami,ari,aki,vhd,vmdk,raw,qcow2,vdi,iso,root-tar def test_block_until_services_restarted(self): self.block_until_services_restarted_base(gu_return=10) model.block_until_services_restarted( - 'modelname', 'app', 8, ['svc1', 'svc2']) @@ -724,7 +763,6 @@ disk_formats = ami,ari,aki,vhd,vmdk,raw,qcow2,vdi,iso,root-tar self.block_until_services_restarted_base(gu_return=10) with self.assertRaises(asyncio.futures.TimeoutError): model.block_until_services_restarted( - 'modelname', 'app', 12, ['svc1', 'svc2']) @@ -733,7 +771,6 @@ disk_formats = ami,ari,aki,vhd,vmdk,raw,qcow2,vdi,iso,root-tar self.block_until_services_restarted_base(gu_raise_exception=True) with self.assertRaises(asyncio.futures.TimeoutError): model.block_until_services_restarted( - 'modelname', 'app', 12, ['svc1', 'svc2']) @@ -742,6 +779,7 @@ disk_formats = ami,ari,aki,vhd,vmdk,raw,qcow2,vdi,iso,root-tar async def _block_until(f, timeout=None): if not f(): raise asyncio.futures.TimeoutError + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock self.Model_mock.block_until.side_effect = _block_until @@ -749,7 +787,6 @@ disk_formats = ami,ari,aki,vhd,vmdk,raw,qcow2,vdi,iso,root-tar self.get_unit_from_name.return_value = mock.MagicMock( workload_status='active') model.block_until_unit_wl_status( - 'modelname', 'app/2', 'active', timeout=0.1) @@ -758,6 +795,7 @@ disk_formats = ami,ari,aki,vhd,vmdk,raw,qcow2,vdi,iso,root-tar async def _block_until(f, timeout=None): if not f(): raise asyncio.futures.TimeoutError + self.patch_object(model, 'get_juju_model', return_value='mname') self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock self.Model_mock.block_until.side_effect = _block_until @@ -766,7 +804,6 @@ disk_formats = ami,ari,aki,vhd,vmdk,raw,qcow2,vdi,iso,root-tar workload_status='maintenance') with self.assertRaises(asyncio.futures.TimeoutError): model.block_until_unit_wl_status( - 'modelname', 'app/2', 'active', timeout=0.1) diff --git a/unit_tests/utilities/test_zaza_utilities_generic.py b/unit_tests/utilities/test_zaza_utilities_generic.py index e4a1d8a..6aa51a2 100644 --- a/unit_tests/utilities/test_zaza_utilities_generic.py +++ b/unit_tests/utilities/test_zaza_utilities_generic.py @@ -48,15 +48,12 @@ class TestGenericUtils(ut_utils.BaseTestCase): def test_get_pkg_version(self): self.patch_object(generic_utils.model, "get_units") - self.patch_object(generic_utils.lifecycle_utils, "get_juju_model") self.patch_object(generic_utils.juju_utils, "remote_run") _pkg = "os-thingy" _version = "2:27.0.0-0ubuntu1~cloud0" _dpkg_output = ("ii {} {} all OpenStack thingy\n" .format(_pkg, _version)) self.remote_run.return_value = _dpkg_output - _model_name = "model-name" - self.get_juju_model.return_value = _model_name _unit1 = mock.MagicMock() _unit1.entity_id = "os-thingy/7" _unit2 = mock.MagicMock() diff --git a/unit_tests/utilities/test_zaza_utilities_juju.py b/unit_tests/utilities/test_zaza_utilities_juju.py index b895db8..b43988d 100644 --- a/unit_tests/utilities/test_zaza_utilities_juju.py +++ b/unit_tests/utilities/test_zaza_utilities_juju.py @@ -27,9 +27,7 @@ class TestJujuUtils(ut_utils.BaseTestCase): # Model self.patch_object(juju_utils, "model") - self.patch_object(juju_utils.lifecycle_utils, "get_juju_model") self.model_name = "model-name" - self.get_juju_model.return_value = self.model_name self.model.get_status.return_value = self.juju_status self.run_output = {"Code": "0", "Stderr": "", "Stdout": "RESULT"} self.error_run_output = {"Code": "1", "Stderr": "ERROR", "Stdout": ""} @@ -80,7 +78,7 @@ class TestJujuUtils(ut_utils.BaseTestCase): def test_get_full_juju_status(self): self.assertEqual(juju_utils.get_full_juju_status(), self.juju_status) - self.model.get_status.assert_called_once_with(self.model_name) + self.model.get_status.assert_called_once_with() def test_get_machines_for_application(self): self.patch_object(juju_utils, "get_application_status") @@ -145,7 +143,7 @@ class TestJujuUtils(ut_utils.BaseTestCase): self.assertEqual(juju_utils.remote_run(self.unit, _cmd), self.run_output["Stdout"]) self.model.run_on_unit.assert_called_once_with( - self.model_name, self.unit, _cmd, timeout=None) + self.unit, _cmd, timeout=None) # Non-fatal failure self.model.run_on_unit.return_value = self.error_run_output @@ -168,7 +166,6 @@ class TestJujuUtils(ut_utils.BaseTestCase): self.model.get_first_unit_name.assert_called() def test_get_relation_from_unit(self): - self.patch_object(juju_utils, 'lifecycle_utils') self.patch_object(juju_utils, '_get_unit_names') self.patch_object(juju_utils, 'yaml') self.patch_object(juju_utils, 'model') @@ -179,12 +176,11 @@ class TestJujuUtils(ut_utils.BaseTestCase): juju_utils.get_relation_from_unit('aunit/0', 'otherunit/0', 'arelation') self.model.run_on_unit.assert_called_with( - self.lifecycle_utils.get_juju_model(), 'aunit/0', + 'aunit/0', 'relation-get --format=yaml -r "42" - "otherunit/0"') self.yaml.load.assert_called_with(str(data)) def test_get_relation_from_unit_fails(self): - self.patch_object(juju_utils, 'lifecycle_utils') self.patch_object(juju_utils, '_get_unit_names') self.patch_object(juju_utils, 'yaml') self.patch_object(juju_utils, 'model') @@ -195,6 +191,6 @@ class TestJujuUtils(ut_utils.BaseTestCase): juju_utils.get_relation_from_unit('aunit/0', 'otherunit/0', 'arelation') self.model.run_on_unit.assert_called_with( - self.lifecycle_utils.get_juju_model(), 'aunit/0', + 'aunit/0', 'relation-get --format=yaml -r "42" - "otherunit/0"') self.assertFalse(self.yaml.load.called) diff --git a/unit_tests/utils.py b/unit_tests/utils.py index e1d0fb6..370262c 100644 --- a/unit_tests/utils.py +++ b/unit_tests/utils.py @@ -47,6 +47,7 @@ class BaseTestCase(unittest.TestCase): def tearDown(self): for k, v in self._patches.items(): + print("Stopping patch {} {}".format(k, v)) v.stop() setattr(self, k, None) self._patches = None @@ -65,6 +66,7 @@ class BaseTestCase(unittest.TestCase): if new is None: started.return_value = return_value self._patches_start[name] = started + print("Starting patch {}".format(name)) setattr(self, name, started) def patch(self, item, return_value=None, name=None, new=None, **kwargs): diff --git a/zaza/charm_lifecycle/configure.py b/zaza/charm_lifecycle/configure.py index 50000c0..05246c8 100644 --- a/zaza/charm_lifecycle/configure.py +++ b/zaza/charm_lifecycle/configure.py @@ -4,6 +4,7 @@ import argparse import logging import sys +import zaza.model import zaza.charm_lifecycle.utils as utils @@ -26,7 +27,7 @@ def configure(model_name, functions): :param functions: List of configure functions functions :type tests: ['zaza.charms_tests.svc.setup', ...] """ - utils.set_juju_model(model_name) + zaza.model.set_juju_model(model_name) run_configure_list(functions) diff --git a/zaza/charm_lifecycle/deploy.py b/zaza/charm_lifecycle/deploy.py index 8ba3c61..7a72f7c 100755 --- a/zaza/charm_lifecycle/deploy.py +++ b/zaza/charm_lifecycle/deploy.py @@ -225,7 +225,7 @@ def deploy(bundle, model, wait=True): if wait: test_config = utils.get_charm_config() logging.info("Waiting for environment to settle") - utils.set_juju_model(model) + zaza.model.set_juju_model(model) zaza.model.wait_for_application_states( model, test_config.get('target_deploy_status', {})) diff --git a/zaza/charm_lifecycle/utils.py b/zaza/charm_lifecycle/utils.py index 8f4c92b..2487d23 100644 --- a/zaza/charm_lifecycle/utils.py +++ b/zaza/charm_lifecycle/utils.py @@ -1,10 +1,7 @@ """Utilities to support running lifecycle phases.""" import importlib -import os import yaml -from zaza import model - BUNDLE_DIR = "./tests/bundles/" DEFAULT_TEST_CONFIG = "./tests/tests.yaml" @@ -38,29 +35,3 @@ def get_class(class_str): class_name = class_str.split('.')[-1] module = importlib.import_module(module_name) return getattr(module, class_name) - - -def set_juju_model(model_name): - """Point environment at the given model. - - :param model_name: Model to point environment at - :type model_name: str - """ - os.environ["JUJU_MODEL"] = model_name - - -def get_juju_model(): - """Retrieve current model. - - First check the environment for JUJU_MODEL. If this is not set, get the - current active model. - - :returns: In focus model name - :rtype: str - """ - try: - # Check the environment - return os.environ["JUJU_MODEL"] - except KeyError: - # If unset connect get the current active model - return model.get_current_model() diff --git a/zaza/charm_tests/dragent/test.py b/zaza/charm_tests/dragent/test.py index 0ccd0e3..28c766a 100644 --- a/zaza/charm_tests/dragent/test.py +++ b/zaza/charm_tests/dragent/test.py @@ -6,7 +6,6 @@ import sys import tenacity from zaza import model -from zaza.charm_lifecycle import utils as lifecycle_utils from zaza.utilities import ( cli as cli_utils, juju as juju_utils, @@ -35,8 +34,7 @@ def test_bgp_routes(peer_application_name="quagga", keystone_session=None): keystone_session) # Get the peer unit - peer_unit = model.get_units( - lifecycle_utils.get_juju_model(), peer_application_name)[0].entity_id + peer_unit = model.get_units(peer_application_name)[0].entity_id # Get expected advertised routes private_cidr = neutron_client.list_subnets( diff --git a/zaza/charm_tests/test_utils.py b/zaza/charm_tests/test_utils.py index b587a83..80860fd 100644 --- a/zaza/charm_tests/test_utils.py +++ b/zaza/charm_tests/test_utils.py @@ -11,7 +11,6 @@ def skipIfNotHA(service_name): def _skipIfNotHA_inner_1(f): def _skipIfNotHA_inner_2(*args, **kwargs): ips = zaza.model.get_app_ips( - lifecycle_utils.get_juju_model(), service_name) if len(ips) > 1: return f(*args, **kwargs) @@ -29,7 +28,7 @@ class OpenStackBaseTest(unittest.TestCase): @classmethod def setUpClass(cls): cls.keystone_session = openstack_utils.get_overcloud_keystone_session() - cls.model_name = lifecycle_utils.get_juju_model() + cls.model_name = model.get_juju_model() cls.test_config = lifecycle_utils.get_charm_config() cls.application_name = cls.test_config['charm_name'] cls.first_unit = model.get_first_unit_name( diff --git a/zaza/charm_tests/vault/tests.py b/zaza/charm_tests/vault/tests.py index 126068f..d9cf522 100644 --- a/zaza/charm_tests/vault/tests.py +++ b/zaza/charm_tests/vault/tests.py @@ -28,13 +28,11 @@ class VaultTest(unittest.TestCase): def test_csr(self): vault_actions = zaza.model.get_actions( - lifecycle_utils.get_juju_model(), 'vault') if 'get-csr' not in vault_actions: raise unittest.SkipTest('Action not defined') try: zaza.model.get_application( - lifecycle_utils.get_juju_model(), 'keystone') except KeyError: raise unittest.SkipTest('No client to test csr') @@ -59,15 +57,12 @@ class VaultTest(unittest.TestCase): test_config = lifecycle_utils.get_charm_config() del test_config['target_deploy_status']['vault'] zaza.model.block_until_file_has_contents( - lifecycle_utils.get_juju_model(), 'keystone', '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt', cacert.decode().strip()) zaza.model.wait_for_application_states( - lifecycle_utils.get_juju_model(), test_config.get('target_deploy_status', {})) ip = zaza.model.get_app_ips( - lifecycle_utils.get_juju_model(), 'keystone')[0] with tempfile.NamedTemporaryFile(mode='w') as fp: fp.write(cacert.decode()) @@ -127,7 +122,6 @@ class VaultTest(unittest.TestCase): def test_vault_authorize_charm_action(self): vault_actions = zaza.model.get_actions( - lifecycle_utils.get_juju_model(), 'vault') if 'authorize-charm' not in vault_actions: raise unittest.SkipTest('Action not defined') diff --git a/zaza/charm_tests/vault/utils.py b/zaza/charm_tests/vault/utils.py index 4b1fc8d..9666c2b 100644 --- a/zaza/charm_tests/vault/utils.py +++ b/zaza/charm_tests/vault/utils.py @@ -10,7 +10,6 @@ import yaml import collections -import zaza.charm_lifecycle.utils as utils import zaza.model AUTH_FILE = "vault_tests.yaml" @@ -47,8 +46,7 @@ def get_vip_client(): :rtype: CharmVaultClient or None """ client = None - vault_config = zaza.model.get_application_config( - utils.get_juju_model(), 'vault') + vault_config = zaza.model.get_application_config('vault') vip = vault_config.get('vip', {}).get('value') if vip: client = CharmVaultClient( @@ -82,7 +80,7 @@ def get_clients(units=None): :rtype: [CharmVaultClient, ...] """ if not units: - units = zaza.model.get_app_ips(utils.get_juju_model(), 'vault') + units = zaza.model.get_app_ips('vault') clients = [] for unit in units: vault_url = get_unit_api_url(unit) @@ -124,11 +122,10 @@ def get_credentails(): :returns: Tokens and keys for accessing test environment :rtype: dict """ - unit = zaza.model.get_first_unit_name(utils.get_juju_model(), 'vault') + unit = zaza.model.get_first_unit_name('vault') with tempfile.TemporaryDirectory() as tmpdirname: tmp_file = '{}/{}'.format(tmpdirname, AUTH_FILE) zaza.model.scp_from_unit( - utils.get_juju_model(), unit, '~/{}'.format(AUTH_FILE), tmp_file) @@ -144,12 +141,11 @@ def store_credentails(creds): :param creds: Keys and token to store :type creds: dict """ - unit = zaza.model.get_first_unit_name(utils.get_juju_model(), 'vault') + unit = zaza.model.get_first_unit_name('vault') with tempfile.NamedTemporaryFile(mode='w') as fp: fp.write(yaml.dump(creds)) fp.flush() zaza.model.scp_to_unit( - utils.get_juju_model(), unit, fp.name, '~/{}'.format(AUTH_FILE)) @@ -205,7 +201,6 @@ def auth_all(clients, token): def run_charm_authorize(token): return zaza.model.run_action_on_leader( - utils.get_juju_model(), 'vault', 'authorize-charm', action_params={'token': token}) @@ -213,7 +208,6 @@ def run_charm_authorize(token): def run_get_csr(): return zaza.model.run_action_on_leader( - utils.get_juju_model(), 'vault', 'get-csr', action_params={}) @@ -221,7 +215,6 @@ def run_get_csr(): def run_upload_signed_csr(pem, root_ca, allowed_domains): return zaza.model.run_action_on_leader( - utils.get_juju_model(), 'vault', 'upload-signed-csr', action_params={ diff --git a/zaza/model.py b/zaza/model.py index 7e0b76f..f4fea03 100644 --- a/zaza/model.py +++ b/zaza/model.py @@ -13,6 +13,39 @@ from juju.model import Model from zaza import sync_wrapper +CURRENT_MODEL = None + +def set_juju_model(model_name): + """Point environment at the given model + + :param model_name: Model to point environment at + :type model_name: str + """ + os.environ["JUJU_MODEL"] = model_name + + +def get_juju_model(): + """Retrieve current model + + First check the environment for JUJU_MODEL. If this is not set, get the + current active model. + + :returns: In focus model name + :rtype: str + """ + if CURRENT_MODEL: + return CURRENT_MODEL + # LY: I think we should remove the KeyError handling. I don't think we + # should ever fall back to the model in focus because it will lead + # to functions being added which do not explicitly set a model and + # zaza will loose the ability to do concurrent runs. + try: + # Check the environment + return os.environ["JUJU_MODEL"] + except KeyError: + # If unset connect get the current active model + return get_current_model() + async def deployed(filter=None): # Create a Model instance. We need to connect our Model to a Juju api @@ -63,12 +96,14 @@ async def run_in_model(model_name): :rtype: Iterator[:class:'juju.Model()'] """ model = Model() + if not model_name: + model_name = get_juju_model() await model.connect_model(model_name) await yield_(model) await model.disconnect() -async def async_scp_to_unit(model_name, unit_name, source, destination, +async def async_scp_to_unit(unit_name, source, destination, model_name=None, user='ubuntu', proxy=False, scp_opts=''): """Transfer files to unit_name in model_name. @@ -95,8 +130,8 @@ async def async_scp_to_unit(model_name, unit_name, source, destination, scp_to_unit = sync_wrapper(async_scp_to_unit) -async def async_scp_to_all_units(model_name, application_name, source, - destination, user='ubuntu', proxy=False, +async def async_scp_to_all_units(application_name, source, destination, + model_name=None, user='ubuntu', proxy=False, scp_opts=''): """Transfer files from to all units of an application @@ -124,7 +159,7 @@ async def async_scp_to_all_units(model_name, application_name, source, scp_to_all_units = sync_wrapper(async_scp_to_all_units) -async def async_scp_from_unit(model_name, unit_name, source, destination, +async def async_scp_from_unit(unit_name, source, destination, model_name=None, user='ubuntu', proxy=False, scp_opts=''): """Transfer files from to unit_name in model_name. @@ -152,7 +187,7 @@ async def async_scp_from_unit(model_name, unit_name, source, destination, scp_from_unit = sync_wrapper(async_scp_from_unit) -async def async_run_on_unit(model_name, unit_name, command, timeout=None): +async def async_run_on_unit(unit_name, command, model_name=None, timeout=None): """Juju run on unit :param model_name: Name of model unit is in @@ -177,7 +212,7 @@ async def async_run_on_unit(model_name, unit_name, command, timeout=None): run_on_unit = sync_wrapper(async_run_on_unit) -async def async_get_unit_time(model_name, unit_name, timeout=None): +async def async_get_unit_time(unit_name, model_name=None, timeout=None): """ Get the current time (in seconds since Epoch) on the given unit :param model_name: Name of model to query. @@ -197,8 +232,8 @@ async def async_get_unit_time(model_name, unit_name, timeout=None): get_unit_time = sync_wrapper(async_get_unit_time) -async def async_get_unit_service_start_time(model_name, unit_name, service, - timeout=None): +async def async_get_unit_service_start_time(unit_name, service, + model_name=None, timeout=None): """Return the time that the given service was started on a unit. Return the time (in seconds since Epoch) that the given service was @@ -228,7 +263,7 @@ async def async_get_unit_service_start_time(model_name, unit_name, service, get_unit_service_start_time = sync_wrapper(async_get_unit_service_start_time) -async def async_get_application(model_name, application_name): +async def async_get_application(application_name, model_name=None): """Return an application object :param model_name: Name of model to query. @@ -244,7 +279,7 @@ async def async_get_application(model_name, application_name): get_application = sync_wrapper(async_get_application) -async def async_get_units(model_name, application_name): +async def async_get_units(application_name, model_name=None): """Return all the units of a given application :param model_name: Name of model to query. @@ -260,7 +295,7 @@ async def async_get_units(model_name, application_name): get_units = sync_wrapper(async_get_units) -async def async_get_machines(model_name, application_name): +async def async_get_machines(application_name, model_name=None): """Return all the machines of a given application :param model_name: Name of model to query. @@ -279,7 +314,7 @@ async def async_get_machines(model_name, application_name): get_machines = sync_wrapper(async_get_machines) -def get_first_unit_name(model_name, application_name): +def get_first_unit_name(application_name, model_name=None): """Return name of lowest numbered unit of given application :param model_name: Name of model to query. @@ -292,7 +327,7 @@ def get_first_unit_name(model_name, application_name): return get_units(model_name, application_name)[0].name -def get_app_ips(model_name, application_name): +def get_app_ips(application_name, model_name=None): """Return public address of all units of an application :param model_name: Name of model to query. @@ -305,7 +340,7 @@ def get_app_ips(model_name, application_name): return [u.public_address for u in get_units(model_name, application_name)] -async def async_get_application_config(model_name, application_name): +async def async_get_application_config(application_name, model_name=None): """Return application configuration :param model_name: Name of model to query. @@ -321,8 +356,8 @@ async def async_get_application_config(model_name, application_name): get_application_config = sync_wrapper(async_get_application_config) -async def async_set_application_config(model_name, application_name, - configuration): +async def async_set_application_config(application_name, configuration, + model_name=None): """Set application configuration :param model_name: Name of model to query. @@ -339,7 +374,7 @@ async def async_set_application_config(model_name, application_name, set_application_config = sync_wrapper(async_set_application_config) -async def async_get_status(model_name): +async def async_get_status(model_name=None): """Return full status :param model_name: Name of model to query. @@ -353,7 +388,7 @@ async def async_get_status(model_name): get_status = sync_wrapper(async_get_status) -async def async_run_action(model_name, unit_name, action_name, +async def async_run_action(unit_name, action_name, model_name=None, action_params=None): """Run action on given unit @@ -377,8 +412,8 @@ async def async_run_action(model_name, unit_name, action_name, run_action = sync_wrapper(async_run_action) -async def async_run_action_on_leader(model_name, application_name, action_name, - action_params=None): +async def async_run_action_on_leader(application_name, action_name, + model_name=None, action_params=None): """Run action on lead unit of the given application :param model_name: Name of model to query. @@ -501,7 +536,7 @@ def check_unit_workload_status_message(model, unit, message=None, raise ValueError("Must be called with message or prefixes") -async def async_wait_for_application_states(model_name, states=None, +async def async_wait_for_application_states(model_name=None, states=None, timeout=2700): """Wait for model to achieve the desired state @@ -568,7 +603,7 @@ async def async_wait_for_application_states(model_name, states=None, wait_for_application_states = sync_wrapper(async_wait_for_application_states) -async def async_block_until_all_units_idle(model_name, timeout=2700): +async def async_block_until_all_units_idle(model_name=None, timeout=2700): """Block until all units in the given model are idle An example accessing this function via its sync wrapper:: @@ -587,8 +622,8 @@ async def async_block_until_all_units_idle(model_name, timeout=2700): block_until_all_units_idle = sync_wrapper(async_block_until_all_units_idle) -async def async_block_until_service_status(model_name, unit_name, services, - target_status, timeout=2700): +async def async_block_until_service_status(unit_name, services, target_status, + model_name=None, timeout=2700): """Block until all services on the unit are in the desired state. Block until all services on the unit are in the desired state (stopped @@ -630,7 +665,7 @@ async def async_block_until_service_status(model_name, unit_name, services, block_until_service_status = sync_wrapper(async_block_until_service_status) -def get_actions(model_name, application_name): +def get_actions(application_name, model_name=None): """Get the actions an applications supports :param model_name: Name of model to query. @@ -640,6 +675,8 @@ def get_actions(model_name, application_name): :returns: Dictionary of actions and their descriptions :rtype: dict """ + if not model_name: + model_name = get_juju_model() # libjuju has not implemented get_actions yet # https://github.com/juju/python-libjuju/issues/226 cmd = ['juju', 'actions', '-m', model_name, application_name, @@ -696,8 +733,8 @@ async def async_block_until(*conditions, timeout=None, wait_period=0.5, await asyncio.wait_for(_block(), timeout, loop=loop) -async def async_block_until_file_ready(model_name, application_name, - remote_file, check_function, +async def async_block_until_file_ready(application_name, remote_file, + check_function, model_name=None, timeout=2700): """Block until the check_function passes against. @@ -740,9 +777,9 @@ async def async_block_until_file_ready(model_name, application_name, await async_block_until(_check_file, timeout=timeout) -async def async_block_until_file_has_contents(model_name, application_name, - remote_file, expected_contents, - timeout=2700): +async def async_block_until_file_has_contents(application_name, remote_file, + expected_contents, + model_name=None, timeout=2700): """Block until the expected_contents are present on all units Block until the given string (expected_contents) is present in the file @@ -770,17 +807,21 @@ async def async_block_until_file_has_contents(model_name, application_name, """ def f(x): return expected_contents in x - return await async_block_until_file_ready(model_name, application_name, - remote_file, f, timeout) + return await async_block_until_file_ready( + application_name, + remote_file, + f, + timeout=timeout, + model_name=model_name) block_until_file_has_contents = sync_wrapper( async_block_until_file_has_contents) -async def async_block_until_oslo_config_entries_match(model_name, - application_name, +async def async_block_until_oslo_config_entries_match(application_name, remote_file, expected_contents, + model_name=None, timeout=2700): """Block until dict is represented in the file using oslo.config parser @@ -835,16 +876,21 @@ async def async_block_until_oslo_config_entries_match(model_name, if sections.get(section, {}).get(key) != value: return False return True - return await async_block_until_file_ready(model_name, application_name, - remote_file, f, timeout) + return await async_block_until_file_ready( + application_name, + remote_file, + f, + timeout=timeout, + model_name=model_name) block_until_oslo_config_entries_match = sync_wrapper( async_block_until_oslo_config_entries_match) -async def async_block_until_services_restarted(model_name, application_name, - mtime, services, timeout=2700): +async def async_block_until_services_restarted(application_name, mtime, + services, model_name=None, + timeout=2700): """Block until the given services have a start time later then mtime For example to check that the glance-api service has been restarted:: @@ -888,7 +934,7 @@ block_until_services_restarted = sync_wrapper( async_block_until_services_restarted) -async def async_block_until_unit_wl_status(model_name, unit_name, status, +async def async_block_until_unit_wl_status(unit_name, status, model_name=None, timeout=2700): """Block until the given unit has the desired workload status @@ -920,8 +966,8 @@ block_until_unit_wl_status = sync_wrapper( async_block_until_unit_wl_status) -async def async_get_relation_id(model_name, application_name, - remote_application_name, +async def async_get_relation_id(application_name, remote_application_name, + model_name=None, remote_interface_name=None): """ Get relation id of relation from model diff --git a/zaza/utilities/generic.py b/zaza/utilities/generic.py index 24167c5..0878358 100644 --- a/zaza/utilities/generic.py +++ b/zaza/utilities/generic.py @@ -5,7 +5,6 @@ import os import yaml from zaza import model -from zaza.charm_lifecycle import utils as lifecycle_utils from zaza.utilities import juju as juju_utils @@ -63,8 +62,7 @@ def get_pkg_version(application, pkg): :rtype: list """ versions = [] - units = model.get_units( - lifecycle_utils.get_juju_model(), application) + units = model.get_units(application) for unit in units: cmd = 'dpkg -l | grep {}'.format(pkg) out = juju_utils.remote_run(unit.entity_id, cmd) diff --git a/zaza/utilities/juju.py b/zaza/utilities/juju.py index 31d2ab6..8d724c2 100644 --- a/zaza/utilities/juju.py +++ b/zaza/utilities/juju.py @@ -22,7 +22,6 @@ from zaza import ( model, controller, ) -from zaza.charm_lifecycle import utils as lifecycle_utils from zaza.utilities import generic as generic_utils @@ -73,7 +72,7 @@ def get_full_juju_status(): :returns: Full juju status output :rtype: dict """ - status = model.get_status(lifecycle_utils.get_juju_model()) + status = model.get_status() return status @@ -165,10 +164,7 @@ def remote_run(unit, remote_cmd, timeout=None, fatal=None): """ if fatal is None: fatal = True - result = model.run_on_unit(lifecycle_utils.get_juju_model(), - unit, - remote_cmd, - timeout=timeout) + result = model.run_on_unit(unit, remote_cmd, timeout=timeout) if result: if int(result.get("Code")) == 0: return result.get("Stdout") @@ -195,9 +191,7 @@ def _get_unit_names(names): if '/' in name: result.append(name) else: - result.append( - model.get_first_unit_name(lifecycle_utils.get_juju_model(), - name)) + result.append(model.get_first_unit_name(name)) return result @@ -223,15 +217,12 @@ def get_relation_from_unit(entity, remote_entity, remote_interface_name): """ application = entity.split('/')[0] remote_application = remote_entity.split('/')[0] - rid = model.get_relation_id(lifecycle_utils.get_juju_model(), application, - remote_application, + rid = model.get_relation_id(application, remote_application, remote_interface_name=remote_interface_name) (unit, remote_unit) = _get_unit_names([entity, remote_entity]) result = model.run_on_unit( - lifecycle_utils.get_juju_model(), unit, - 'relation-get --format=yaml -r "{}" - "{}"' - .format(rid, remote_unit) - ) + unit, + 'relation-get --format=yaml -r "{}" - "{}"' .format(rid, remote_unit)) if result and int(result.get('Code')) == 0: return yaml.load(result.get('Stdout')) else: diff --git a/zaza/utilities/openstack.py b/zaza/utilities/openstack.py index b0ea06e..4d00ffb 100644 --- a/zaza/utilities/openstack.py +++ b/zaza/utilities/openstack.py @@ -30,7 +30,6 @@ import tenacity import urllib from zaza import model -from zaza.charm_lifecycle import utils as lifecycle_utils from zaza.utilities import ( exceptions, generic as generic_utils, @@ -413,7 +412,7 @@ def configure_gateway_ext_port(novaclient, neutronclient, logging.info('Config already set to value') return model.set_application_config( - lifecycle_utils.get_juju_model(), application_name, + application_name, configuration={config_key: ext_br_macs_str}) juju_wait.wait(wait_for_workload=True) @@ -848,8 +847,7 @@ def create_bgp_peer(neutron_client, peer_application_name='quagga', :returns: BGP peer object :rtype: dict """ - peer_unit = model.get_units( - lifecycle_utils.get_juju_model(), peer_application_name)[0] + peer_unit = model.get_units(peer_application_name)[0] peer_ip = peer_unit.public_address bgp_peers = neutron_client.list_bgp_peers(name=peer_application_name) if len(bgp_peers['bgp_peers']) == 0: @@ -1086,8 +1084,7 @@ def get_application_config_keys(application): :returns: List of aplication configuration keys :rtype: list """ - application_config = model.get_application_config( - lifecycle_utils.get_juju_model(), application) + application_config = model.get_application_config(application) return list(application_config.keys()) @@ -1101,8 +1098,7 @@ def get_application_config_option(application, option): :returns: Value of configuration option :rtype: Configuration option value type """ - application_config = model.get_application_config( - lifecycle_utils.get_juju_model(), application) + application_config = model.get_application_config(application) try: return application_config.get(option).get('value') except AttributeError: @@ -1184,8 +1180,7 @@ def get_keystone_ip(): """ if get_application_config_option('keystone', 'vip'): return get_application_config_option('keystone', 'vip') - unit = model.get_units( - lifecycle_utils.get_juju_model(), 'keystone')[0] + unit = model.get_units('keystone')[0] return unit.public_address