diff --git a/.github/workflows/tox.yaml b/.github/workflows/tox.yaml index 130ac9aa..65c1f7f7 100644 --- a/.github/workflows/tox.yaml +++ b/.github/workflows/tox.yaml @@ -38,6 +38,7 @@ jobs: # - 3.1/stable # - 3.2/stable # - 3.3/stable + - 3.4/stable bundle: - first - second @@ -59,6 +60,10 @@ jobs: # - juju_channel: 3.3/stable # snap_install_flags: "" # pip_constraints: constraints-juju33.txt + - juju_channel: 3.4/stable + snap_install_flags: "" + pip_constraints: constraints-juju34.txt + juju3: 1 env: TEST_ZAZA_BUG_LP1987332: "on" # http://pad.lv/1987332 needs: build @@ -93,6 +98,7 @@ jobs: set -euxo pipefail mkdir logs export PIP_CONSTRAINTS=$(pwd)/${{ matrix.pip_constraints }} + export TEST_JUJU3=${{ matrix.juju3 }} tox -e func-target -- ${{ matrix.bundle }} | tee logs/tox-output.txt - name: crashdump on failure if: failure() diff --git a/constraints-juju34.txt b/constraints-juju34.txt new file mode 100644 index 00000000..0d24a418 --- /dev/null +++ b/constraints-juju34.txt @@ -0,0 +1,9 @@ +# NOTE: this constraints file can be (and will be) consumed by downstream users. +# +# Known consumers: +# * zosci-config: job definitions that declare what juju version (snap channel) +# is used in tandem with this constraints file to lockdown python-libjuju +# version. +# * zaza-openstack-tests +# +juju>=3.4.0,<3.5.0 diff --git a/tox.ini b/tox.ini index 323a9cd0..85167558 100644 --- a/tox.ini +++ b/tox.ini @@ -23,6 +23,11 @@ deps = -r{toxinidir}/test-requirements.txt commands = pytest --cov=./zaza/ {posargs} {toxinidir}/unit_tests +[testenv:.pkg] +pass_env = + # NOTE: This is required because tox>=4 will not pass env. See https://github.com/tox-dev/tox/issues/2543. + TEST_JUJU3 + [testenv:py3] basepython = python3 diff --git a/unit_tests/test_zaza_model.py b/unit_tests/test_zaza_model.py index f46d5158..b66f1ceb 100644 --- a/unit_tests/test_zaza_model.py +++ b/unit_tests/test_zaza_model.py @@ -117,9 +117,18 @@ async def _scp_from(source, destination, user=None, proxy=None, scp_opts=None): return - async def _run(command, timeout=None): + async def _run(command, timeout=None, block=False): return self.action + async def _run_juju2_x(command, timeout=None): + return self.action + + async def _run_wrapper(*args, **kwargs): + if self.juju_version >= 3: + return await _run(*args, **kwargs) + else: + return await _run_juju2_x(*args, **kwargs) + async def _run_action(command, **params): return self.run_action @@ -146,6 +155,7 @@ async def _inner_is_leader(): return leader return _inner_is_leader + self.juju_version = 3 self.run_action = mock.MagicMock() self.run_action.wait.side_effect = _wait self.action = mock.MagicMock() @@ -191,8 +201,8 @@ def fail_on_use(): self.unit2.name = 'app/4' self.unit2.entity_id = 'app/4' self.unit2.machine = self.machine7 - self.unit2.run.side_effect = _run - self.unit1.run.side_effect = _run + self.unit2.run.side_effect = _run_wrapper + self.unit1.run.side_effect = _run_wrapper self.unit1.scp_to.side_effect = _scp_to self.unit2.scp_to.side_effect = _scp_to self.unit1.scp_from.side_effect = _scp_from @@ -785,17 +795,14 @@ def test_run_on_unit(self): self.cmd = cmd = 'somecommand someargument' self.patch_object(model, 'Model') self.patch_object(model, 'get_unit_from_name') - self.patch("inspect.isawaitable", name="isawaitable", - return_value=True) self.get_unit_from_name.return_value = self.unit1 self.Model.return_value = self.Model_mock - self.assertEqual(model.run_on_unit('app/2', cmd), - expected) - self.unit1.run.assert_called_once_with(cmd, timeout=None) - self.action.wait.assert_called_once() + self.assertEqual(model.run_on_unit('app/2', cmd), expected) + self.unit1.run.assert_called_once_with(cmd, timeout=None, block=True) def test_run_on_unit_juju2_x(self): del self.action.results + self.juju_version = 2 self.patch_object(model, 'get_juju_model', return_value='mname') expected = { 'Code': '0', @@ -806,13 +813,12 @@ def test_run_on_unit_juju2_x(self): self.cmd = cmd = 'somecommand someargument' self.patch_object(model, 'Model') self.patch_object(model, 'get_unit_from_name') - self.patch("inspect.isawaitable", name="isawaitable", - return_value=False) self.get_unit_from_name.return_value = self.unit1 self.Model.return_value = self.Model_mock - self.assertEqual(model.run_on_unit('app/2', cmd), - expected) - self.unit1.run.assert_called_once_with(cmd, timeout=None) + self.assertEqual(model.run_on_unit('app/2', cmd), expected) + self.unit1.run.assert_has_calls([ + mock.call(cmd, timeout=None, block=True), + mock.call(cmd, timeout=None)]) self.action.wait.assert_not_called() def test_run_on_unit_lc_keys(self): @@ -832,12 +838,13 @@ def test_run_on_unit_lc_keys(self): 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', cmd), - expected) - self.unit1.run.assert_called_once_with(cmd, timeout=None) + self.assertEqual(model.run_on_unit('app/2', cmd), expected) + self.unit1.run.assert_called_once_with(cmd, timeout=None, block=True) + self.action.wait.assert_not_called() def test_run_on_unit_lc_keys_juju2_x(self): del self.action.results + self.juju_version = 2 self.patch_object(model, 'get_juju_model', return_value='mname') self.action.data['results'] = { 'Code': '0', @@ -854,9 +861,11 @@ def test_run_on_unit_lc_keys_juju2_x(self): 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', cmd), - expected) - self.unit1.run.assert_called_once_with(cmd, timeout=None) + self.assertEqual(model.run_on_unit('app/2', cmd), expected) + self.unit1.run.assert_has_calls([ + mock.call(cmd, timeout=None, block=True), + mock.call(cmd, timeout=None)]) + self.action.wait.assert_not_called() def test_run_on_unit_missing_stderr(self): self.patch_object(model, 'get_juju_model', return_value='mname') @@ -872,12 +881,13 @@ def test_run_on_unit_missing_stderr(self): 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', cmd), - expected) - self.unit1.run.assert_called_once_with(cmd, timeout=None) + self.assertEqual(model.run_on_unit('app/2', cmd), expected) + self.unit1.run.assert_called_once_with(cmd, timeout=None, block=True) + self.action.wait.assert_not_called() def test_run_on_unit_missing_stderr_juju2_x(self): del self.action.results + self.juju_version = 2 self.patch_object(model, 'get_juju_model', return_value='mname') expected = { 'Code': '0', @@ -891,9 +901,11 @@ def test_run_on_unit_missing_stderr_juju2_x(self): 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', cmd), - expected) - self.unit1.run.assert_called_once_with(cmd, timeout=None) + self.assertEqual(model.run_on_unit('app/2', cmd), expected) + self.unit1.run.assert_has_calls([ + mock.call(cmd, timeout=None, block=True), + mock.call(cmd, timeout=None)]) + self.action.wait.assert_not_called() def test_run_on_leader(self): self.patch_object(model, 'get_juju_model', return_value='mname') @@ -906,14 +918,12 @@ def test_run_on_leader(self): self.cmd = cmd = 'somecommand someargument' self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock - self.patch('inspect.isawaitable', return_value=True, - name='isawaitable') - self.assertEqual(model.run_on_leader('app', cmd), - expected) - self.unit2.run.assert_called_once_with(cmd, timeout=None) + self.assertEqual(model.run_on_leader('app', cmd), expected) + self.unit2.run.assert_called_once_with(cmd, timeout=None, block=True) def test_run_on_leader_juju2_x(self): del self.action.results + self.juju_version = 2 self.patch_object(model, 'get_juju_model', return_value='mname') expected = { 'Code': '0', @@ -924,9 +934,11 @@ def test_run_on_leader_juju2_x(self): self.cmd = cmd = 'somecommand someargument' self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock - self.assertEqual(model.run_on_leader('app', cmd), - expected) - self.unit2.run.assert_called_once_with(cmd, timeout=None) + self.assertEqual(model.run_on_leader('app', cmd), expected) + self.unit2.run.assert_has_calls([ + mock.call(cmd, timeout=None, block=True), + mock.call(cmd, timeout=None)]) + self.action.wait.assert_not_called() def test_get_relation_id(self): self.patch_object(model, 'get_juju_model', return_value='mname') @@ -1641,11 +1653,12 @@ def test_file_contents_success(self): '/tmp/src/myfile.txt', timeout=0.1) self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt', timeout=0.1) + 'cat /tmp/src/myfile.txt', timeout=0.1, block=True) self.assertEqual('somestring', contents) def test_file_contents_success_juju2_x(self): del self.action.results + self.juju_version = 2 self.action.data = { 'results': {'Code': '0', 'Stderr': '', 'Stdout': 'somestring'} } @@ -1657,8 +1670,9 @@ def test_file_contents_success_juju2_x(self): 'app/2', '/tmp/src/myfile.txt', timeout=0.1) - self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt', timeout=0.1) + self.unit1.run.assert_has_calls([ + mock.call('cat /tmp/src/myfile.txt', timeout=0.1, block=True), + mock.call('cat /tmp/src/myfile.txt', timeout=0.1)]) self.assertEqual('somestring', contents) def test_file_contents_fault(self): @@ -1674,11 +1688,12 @@ def test_file_contents_fault(self): with self.assertRaises(model.RemoteFileError) as ctxtmgr: model.file_contents('app/2', '/tmp/src/myfile.txt', timeout=0.1) self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt', timeout=0.1) + 'cat /tmp/src/myfile.txt', timeout=0.1, block=True) self.assertEqual(ctxtmgr.exception.args, ('fault',)) def test_file_contents_fault_juju2_x(self): del self.action.results + self.juju_version = 2 self.action.data = { 'results': {'Code': '0', 'Stderr': 'fault', 'Stdout': ''} } @@ -1688,8 +1703,9 @@ def test_file_contents_fault_juju2_x(self): self.patch_object(model, 'get_juju_model', return_value='mname') with self.assertRaises(model.RemoteFileError) as ctxtmgr: model.file_contents('app/2', '/tmp/src/myfile.txt', timeout=0.1) - self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt', timeout=0.1) + self.unit1.run.assert_has_calls([ + mock.call('cat /tmp/src/myfile.txt', timeout=0.1, block=True), + mock.call('cat /tmp/src/myfile.txt', timeout=0.1)]) self.assertEqual(ctxtmgr.exception.args, ('fault',)) def test_block_until_file_has_contents(self): @@ -1708,20 +1724,19 @@ def test_block_until_file_has_contents(self): _fileobj = mock.MagicMock() _fileobj.__enter__().read.return_value = "somestring" self._open.return_value = _fileobj - self.patch('inspect.isawaitable', return_value=True, - name='isawaitable') model.block_until_file_has_contents( 'app', '/tmp/src/myfile.txt', 'somestring', timeout=0.1) self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) self.unit2.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) def test_block_until_file_has_contents_juju2_x(self): del self.action.results + self.juju_version = 2 self.action.data = { 'results': {'Code': '0', 'Stderr': '', 'Stdout': 'somestring'} } @@ -1735,17 +1750,17 @@ def test_block_until_file_has_contents_juju2_x(self): _fileobj = mock.MagicMock() _fileobj.__enter__().read.return_value = "somestring" self._open.return_value = _fileobj - self.patch('inspect.isawaitable', return_value=False, - name='isawaitable') model.block_until_file_has_contents( 'app', '/tmp/src/myfile.txt', 'somestring', timeout=0.1) - self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') - self.unit2.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + self.unit1.run.assert_has_calls([ + mock.call('cat /tmp/src/myfile.txt', timeout=None, block=True), + mock.call('cat /tmp/src/myfile.txt', timeout=None)]) + self.unit2.run.assert_has_calls([ + mock.call('cat /tmp/src/myfile.txt', timeout=None, block=True), + mock.call('cat /tmp/src/myfile.txt', timeout=None)]) def test_block_until_file_has_no_contents(self): self.action.results = {'return-code': '0', 'stderr': ''} @@ -1765,12 +1780,13 @@ def test_block_until_file_has_no_contents(self): '', timeout=0.1) self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) self.unit2.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) def test_block_until_file_has_no_contents_juju2_x(self): del self.action.results + self.juju_version = 2 self.action.data = { 'results': {'Code': '0', 'Stderr': ''} } @@ -1789,10 +1805,12 @@ def test_block_until_file_has_no_contents_juju2_x(self): '/tmp/src/myfile.txt', '', timeout=0.1) - self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') - self.unit2.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + self.unit1.run.assert_has_calls([ + mock.call('cat /tmp/src/myfile.txt', timeout=None, block=True), + mock.call('cat /tmp/src/myfile.txt', timeout=None)]) + self.unit2.run.assert_has_calls([ + mock.call('cat /tmp/src/myfile.txt', timeout=None, block=True), + mock.call('cat /tmp/src/myfile.txt', timeout=None)]) def test_block_until_file_has_contents_missing(self): self.patch_object(model, 'Model') @@ -1810,34 +1828,37 @@ def test_block_until_file_has_contents_missing(self): '/tmp/src/myfile.txt', 'somestring', timeout=0.1) - self.unit1.run.assert_called_once_with('cat /tmp/src/myfile.txt') + self.unit1.run.assert_called_once_with( + 'cat /tmp/src/myfile.txt', timeout=None, block=True) def test_block_until_file_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.action.results = {'stdout': "1"} - self.patch('inspect.isawaitable', return_value=True, - name='isawaitable') model.block_until_file_missing( 'app', '/tmp/src/myfile.txt', timeout=0.1) self.unit1.run.assert_called_once_with( - 'test -e "/tmp/src/myfile.txt"; echo $?') + 'test -e "/tmp/src/myfile.txt"; echo $?', timeout=None, block=True) def test_block_until_file_missing_juju2_x(self): self.patch_object(model, 'Model') self.Model.return_value = self.Model_mock self.patch_object(model, 'get_juju_model', return_value='mname') del self.action.results + self.juju_version = 2 self.action.data['results']['Stdout'] = "1" model.block_until_file_missing( 'app', '/tmp/src/myfile.txt', timeout=0.1) - self.unit1.run.assert_called_once_with( - 'test -e "/tmp/src/myfile.txt"; echo $?') + self.unit1.run.assert_has_calls([ + mock.call( + 'test -e "/tmp/src/myfile.txt"; echo $?', + timeout=None, block=True), + mock.call('test -e "/tmp/src/myfile.txt"; echo $?', timeout=None)]) def test_block_until_file_missing_isnt_missing(self): self.patch_object(model, 'Model') @@ -1866,12 +1887,13 @@ def test_block_until_file_matches_re(self): 's.*string', timeout=0.1) self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) self.unit2.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) def test_block_until_file_matches_re_juju2_x(self): del self.action.results + self.juju_version = 2 self.action.data = { 'results': {'Code': '0', 'Stderr': '', 'Stdout': 'somestring'} } @@ -1884,10 +1906,12 @@ def test_block_until_file_matches_re_juju2_x(self): '/tmp/src/myfile.txt', 's.*string', timeout=0.1) - self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') - self.unit2.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + self.unit1.run.assert_has_calls([ + mock.call('cat /tmp/src/myfile.txt', timeout=None, block=True), + mock.call('cat /tmp/src/myfile.txt', timeout=None)]) + self.unit2.run.assert_has_calls([ + mock.call('cat /tmp/src/myfile.txt', timeout=None, block=True), + mock.call('cat /tmp/src/myfile.txt', timeout=None)]) def test_async_block_until_all_units_idle(self): @@ -2235,9 +2259,9 @@ def test_block_until_oslo_config_entries_match(self): file_contents, expected_contents) self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) self.unit2.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) def test_block_until_oslo_config_entries_match_fail(self): file_contents = """ @@ -2267,7 +2291,7 @@ def test_block_until_oslo_config_entries_match_fail(self): file_contents, expected_contents) self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) def test_block_until_oslo_config_entries_match_missing_entry(self): file_contents = """ @@ -2296,7 +2320,7 @@ def test_block_until_oslo_config_entries_match_missing_entry(self): file_contents, expected_contents) self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) def test_block_until_oslo_config_entries_match_missing_section(self): file_contents = """ @@ -2320,7 +2344,7 @@ def test_block_until_oslo_config_entries_match_missing_section(self): file_contents, expected_contents) self.unit1.run.assert_called_once_with( - 'cat /tmp/src/myfile.txt') + 'cat /tmp/src/myfile.txt', timeout=None, block=True) def block_until_services_restarted_base(self, gu_return=None, gu_raise_exception=False): diff --git a/zaza/model.py b/zaza/model.py index 7e7526b6..89b048ff 100644 --- a/zaza/model.py +++ b/zaza/model.py @@ -569,9 +569,7 @@ async def async_run_on_unit(unit_name, command, model_name=None, timeout=None): """ model = await get_model(model_name) unit = await async_get_unit_from_name(unit_name, model) - action = await unit.run(command, timeout=timeout) - if inspect.isawaitable(action): - await action.wait() + action = await generic_utils.unit_run(unit, command, timeout) action = _normalise_action_object(action) results = action.data.get('results') return _normalise_action_results(results) @@ -598,9 +596,7 @@ async def async_run_on_leader(application_name, command, model_name=None, for unit in model.applications[application_name].units: is_leader = await unit.is_leader_from_status() if is_leader: - action = await unit.run(command, timeout=timeout) - if inspect.isawaitable(action): - await action.wait() + action = await generic_utils.unit_run(unit, command, timeout) action = _normalise_action_object(action) results = action.data.get('results') return _normalise_action_results(results) @@ -2159,9 +2155,8 @@ async def _check_file(): units = model.applications[application_name].units for unit in units: try: - output = await unit.run('cat {}'.format(remote_file)) - if inspect.isawaitable(output): - await output.wait() + output = await generic_utils.unit_run( + unit, 'cat {}'.format(remote_file)) results = {} try: results = output.results @@ -2301,9 +2296,8 @@ async def _check_for_file(model): results = [] for unit in units: try: - output = await unit.run('test -e "{}"; echo $?'.format(path)) - if inspect.isawaitable(output): - await output.wait() + output = await generic_utils.unit_run( + unit, 'test -e "{}"; echo $?'.format(path)) output = _normalise_action_object(output) output_result = _normalise_action_results( output.data.get('results', {})) diff --git a/zaza/utilities/generic.py b/zaza/utilities/generic.py index dc75197a..4addec67 100644 --- a/zaza/utilities/generic.py +++ b/zaza/utilities/generic.py @@ -729,3 +729,23 @@ async def check_output(cmd, log_stdout=True, log_stderr=True): 'Stderr': stderr, 'Stdout': stdout, } + + +async def unit_run(unit, command, timeout=None): + """Help function to help support Juju 2.9 and Juju 3>. + + :param unit: juju unit + :type unit: juju.unit.Unit + :param command: Command to execute + :type command: str + :param timeout: How long in seconds to wait for command to complete + :type timeout: int + :returns: completed Action object + :rtype: juju.action.Action + """ + try: + # block supported in juju>=3.0 + return await unit.run(command, timeout=timeout, block=True) + except TypeError: + # juju 2.9 fallback + return await unit.run(command, timeout=timeout)