Skip to content

Commit

Permalink
Add support for Juju 3.4 (#410)
Browse files Browse the repository at this point in the history
Fix getting results from `run_on_unit` and `run_action`. Also, series
was replaced with base in juju status, so the small function to convert
base to series was required.

---------

Co-authored-by: james_lin <[email protected]>
  • Loading branch information
rgildein and jneo8 authored May 28, 2024
1 parent b72981e commit 9d8d26b
Show file tree
Hide file tree
Showing 12 changed files with 243 additions and 297 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:
workflow_dispatch:
pull_request:
types: [ opened, synchronize, reopened ]
branches: [ master, main ]
branches: [ main, '*.x' ]
paths-ignore:
- '**.md'
- '**.rst'
Expand Down Expand Up @@ -109,11 +109,11 @@ jobs:
uses: actions/setup-python@v5
with:
python-version: '3.10'
- name: Setup Juju 2.9/stable environment
- name: Setup Juju 3.4/stable environment
uses: charmed-kubernetes/actions-operator@main
with:
provider: lxd
juju-channel: 2.9/stable
juju-channel: 3.4/stable
- name: Remove tox install by actions-operator
run: sudo apt remove tox -y
- name: Install tox
Expand All @@ -125,4 +125,4 @@ jobs:
with:
name: SNAP_FILE
- name: Run func tests
run: TEST_SNAP=$GITHUB_WORKSPACE/charmed-openstack-upgrader.snap tox -e func
run: TEST_JUJU3=1 TEST_SNAP=$GITHUB_WORKSPACE/charmed-openstack-upgrader.snap tox -e func
10 changes: 5 additions & 5 deletions cou/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ def __init__(self, cmd: str, result: dict):
:param cmd: Command that was run
:type cmd: string
:param result: Dict returned by juju containing the output of the command
:type result: dict - {'Code': '0', 'Stdout': '', 'Stderr':''}
:type result: dict - {'return-code': '0', 'stdout': '', 'stderr':''}
"""
code = result.get("Code")
output = result.get("Stdout")
err = result.get("Stderr")
msg = f"Command {cmd} failed with code {code}, output {output} and error {err}"
code = result.get("return-code")
stdout = result.get("stdout")
stderr = result.get("stderr")
msg = f"Command {cmd} failed with code {code}, output {stdout} and error {stderr}"
super().__init__(msg)


Expand Down
4 changes: 2 additions & 2 deletions cou/steps/backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ async def backup(model: Model) -> Path:

logger.info("mysqldump mysql-innodb-cluster DBs ...")
action = await model.run_action(unit_name, "mysqldump")
remote_file = action.data["results"]["mysqldump-file"]
basedir = action.data["parameters"]["basedir"]
remote_file = Path(action.results["mysqldump-file"])
basedir = remote_file.parent

logger.info("Set permissions to read mysql-innodb-cluster:%s ...", basedir)
await model.run_on_unit(unit_name, f"chmod o+rx {basedir}")
Expand Down
4 changes: 2 additions & 2 deletions cou/utils/app_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ async def _get_required_osd_release(unit: str, model: Model) -> str:
check_option_result = await model.run_on_unit(
unit_name=unit, command=check_command, timeout=600
)
current_require_osd_release = json.loads(check_option_result["Stdout"]).get(
current_require_osd_release = json.loads(check_option_result["stdout"]).get(
"require_osd_release", ""
)
logger.debug("Current require-osd-release is set to: %s", current_require_osd_release)
Expand All @@ -109,7 +109,7 @@ async def _get_current_osd_release(unit: str, model: Model) -> str:

check_osd_result = await model.run_on_unit(unit_name=unit, command=check_command, timeout=600)

osd_release_output = json.loads(check_osd_result["Stdout"]).get("osd", None)
osd_release_output = json.loads(check_osd_result["stdout"]).get("osd", None)
# throw exception if ceph-mon doesn't contain osd release information in `ceph`
if not osd_release_output:
raise RunUpgradeError(f"Cannot get OSD release information on ceph-mon unit '{unit}'.")
Expand Down
57 changes: 18 additions & 39 deletions cou/utils/juju_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@

from juju.action import Action
from juju.application import Application as JujuApplication
from juju.client._definitions import FullStatus
from juju.client._definitions import Base, FullStatus
from juju.client.connector import NoConnectionException
from juju.client.jujudata import FileJujuData
from juju.errors import JujuAppError, JujuError, JujuUnitError
from juju.model import Model as JujuModel
from juju.unit import Unit as JujuUnit
from juju.utils import get_version_series
from macaroonbakery.httpbakery import BakeryException
from six import wraps

Expand All @@ -56,37 +57,16 @@
logger = logging.getLogger(__name__)


def _normalize_action_results(results: dict[str, str]) -> dict[str, str]:
"""Unify action results format.
def _convert_base_to_series(base: Base) -> str:
"""Convert base to series.
:param results: Results dictionary to process.
:type results: dict[str, str]
:returns: {
'Code': '',
'Stderr': '',
'Stdout': '',
'stderr': '',
'stdout': ''}
:rtype: dict[str, str]
:param base: Base object
:type base: juju.client._definitions.Base
:return: converted channel to series, e.g. 20.04 -> focal
:rtype: str
"""
if results:
# In Juju 2.7 some keys are dropped from the results if their
# value was empty. This breaks some functions downstream, so
# ensure the keys are always present.
for key in ["Stderr", "Stdout", "stderr", "stdout"]:
results[key] = results.get(key, "")
# Juju has started using a lowercase "stdout" key in new action
# commands in recent versions. Ensure the old capatalised keys and
# the new lowercase keys are present and point to the same value to
# avoid breaking functions downstream.
for key in ["stderr", "stdout"]:
old_key = key.capitalize()
if results.get(key) and not results.get(old_key):
results[old_key] = results[key]
elif results.get(old_key) and not results.get(key):
results[key] = results[old_key]
return results
return {}
version, *_ = base.channel.split("/")
return get_version_series(version)


def retry(
Expand Down Expand Up @@ -379,7 +359,7 @@ async def get_applications(self) -> dict[str, Application]:
},
model=self,
origin=status.charm.split(":")[0],
series=status.series,
series=_convert_base_to_series(status.base),
subordinate_to=status.subordinate_to,
units={
name: Unit(name, machines[unit.machine], unit.workload_version)
Expand Down Expand Up @@ -471,23 +451,22 @@ async def run_on_unit(
:type command: str
:param timeout: How long in seconds to wait for command to complete
:type timeout: Optional[int]
:returns: action.data['results'] {'Code': '', 'Stderr': '', 'Stdout': ''}
:returns: action.results {'return-code': '', 'stderr': '', 'stdout': ''}
:rtype: dict[str, str]
:raises UnitNotFound: When a valid unit cannot be found.
:raises CommandRunFailed: When a command fails to run.
"""
logger.debug("Running '%s' on '%s'", command, unit_name)

unit = await self._get_unit(unit_name)
action = await unit.run(command, timeout=timeout)
results = action.data.get("results")
normalize_results = _normalize_action_results(results)
action = await unit.run(command, timeout=timeout, block=True)
results = action.results
logger.debug("results: %s", results)

if str(normalize_results["Code"]) != "0":
raise CommandRunFailed(cmd=command, result=normalize_results)
logger.debug(normalize_results["Stdout"])
if str(results["return-code"]) != "0":
raise CommandRunFailed(cmd=command, result=results)

return normalize_results
return results

@retry(no_retry_exceptions=(ApplicationNotFound,))
async def set_application_config(self, name: str, configuration: dict[str, str]) -> None:
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ python_requires = >=3.10
packages = find:
install_requires =
oslo.config
juju<3.0
juju
colorama
packaging
aioconsole
Expand Down
3 changes: 2 additions & 1 deletion tests/functional/tests/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@ def test_get_status(self):
def test_run_action(self):
"""Test run action."""
action = zaza.sync_wrapper(self.model.run_action)(TESTED_UNIT, "resume")
self.assertEqual(0, action.results["return-code"])
self.assertEqual("completed", action.data["status"])

def test_run_on_unit(self):
"""Test run command on unit."""
results = zaza.sync_wrapper(self.model.run_on_unit)(TESTED_UNIT, "actions/resume")
self.assertIn("active", results["Stdout"])
self.assertIn("active", results["stdout"])

def test_scp_from_unit(self):
"""Test copy file from unit."""
Expand Down
Loading

0 comments on commit 9d8d26b

Please sign in to comment.