diff --git a/cou/apps/base.py b/cou/apps/base.py index e0e6f9c8..63abcb31 100644 --- a/cou/apps/base.py +++ b/cou/apps/base.py @@ -18,10 +18,9 @@ import logging from collections import defaultdict from dataclasses import dataclass, field -from io import StringIO from typing import Any, Iterable, Optional -from ruamel.yaml import YAML +import yaml from cou.exceptions import ( ApplicationError, @@ -46,6 +45,8 @@ logger = logging.getLogger(__name__) DEFAULT_WAITING_TIMEOUT = 5 * 60 # 5 min +ORIGIN_SETTINGS = ("openstack-origin", "source") +REQUIRED_SETTINGS = ("enable-auto-restarts", "action-managed-upgrade", *ORIGIN_SETTINGS) @dataclass(frozen=True) @@ -99,23 +100,38 @@ def __str__(self) -> str: summary = { self.name: { "model_name": self.model.name, + "can_upgrade_to": self.can_upgrade_to, "charm": self.charm, - "charm_origin": self.origin, - "os_origin": self.os_origin, "channel": self.channel, + # Note (rgildein): sanitized the config + "config": { + key: self.config[key] for key in self.config if key in REQUIRED_SETTINGS + }, + "origin": self.origin, + "series": self.series, + "subordinate_to": self.subordinate_to, + "workload_version": self.workload_version, "units": { unit.name: { + "name": unit.name, + "machine": unit.machine.machine_id, "workload_version": unit.workload_version, "os_version": str(self._get_latest_os_version(unit)), } for unit in self.units.values() }, + "machines": { + machine.machine_id: { + "id": machine.machine_id, + "apps": machine.apps, + "az": machine.az, + } + for machine in self.machines.values() + }, } } - yaml = YAML() - with StringIO() as stream: - yaml.dump(summary, stream) - return stream.getvalue() + + return yaml.dump(summary, sort_keys=False) def _verify_channel(self) -> None: """Verify app channel from current data. @@ -155,7 +171,7 @@ def origin_setting(self) -> Optional[str]: :return: return name of charm origin setting, e.g. "source", "openstack-origin" or None :rtype: Optional[str] """ - for origin in ("openstack-origin", "source"): + for origin in ORIGIN_SETTINGS: if origin in self.config: return origin @@ -410,18 +426,17 @@ def generate_upgrade_plan( :return: Full upgrade plan if the Application is able to generate it. :rtype: ApplicationUpgradePlan """ - upgrade_steps = ApplicationUpgradePlan( + upgrade_plan = ApplicationUpgradePlan( description=f"Upgrade plan for '{self.name}' to {target}", ) - all_steps = ( - self.pre_upgrade_steps(target, units) - + self.upgrade_steps(target, units, force) - + self.post_upgrade_steps(target, units) - ) - for step in all_steps: - if step: - upgrade_steps.add_step(step) - return upgrade_steps + + upgrade_plan.sub_steps = [ + *self.pre_upgrade_steps(target, units), + *self.upgrade_steps(target, units, force), + *self.post_upgrade_steps(target, units), + ] + + return upgrade_plan def _get_upgrade_current_release_packages_step( self, units: Optional[list[COUUnit]] @@ -471,10 +486,8 @@ def _get_refresh_charm_step(self, target: OpenStackRelease) -> PreUpgradeStep: # corner case for rabbitmq and hacluster. if len(self.possible_current_channels) > 1: logger.info( - ( - "'%s' has more than one channel compatible with the current OpenStack " - "release: '%s'. '%s' will be used" - ), + "'%s' has more than one channel compatible with the current OpenStack release: " + "'%s'. '%s' will be used", self.name, self.current_os_release.codename, channel, @@ -574,7 +587,7 @@ def _get_resume_unit_step(self, unit: COUUnit, dependent: bool = False) -> UnitU :rtype: UnitUpgradeStep """ return UnitUpgradeStep( - description=(f"Resume the unit: '{unit.name}'."), + description=f"Resume the unit: '{unit.name}'.", coro=self.model.run_action( unit_name=unit.name, action_name="resume", raise_on_failure=True ), diff --git a/cou/steps/__init__.py b/cou/steps/__init__.py index 4762236b..b8bff049 100644 --- a/cou/steps/__init__.py +++ b/cou/steps/__init__.py @@ -188,6 +188,7 @@ def description(self, description: str) -> None: """ if not description and self._coro: raise ValueError("Every coroutine should have a description") + self._description = description @property @@ -244,6 +245,10 @@ def add_step(self, step: BaseStep) -> None: if not isinstance(step, BaseStep): raise TypeError("Cannot add an upgrade step that is not derived from BaseStep") + if not step: + logger.debug("skipping adding empty step") + return + self._sub_steps.append(step) def cancel(self, safe: bool = True) -> None: diff --git a/setup.cfg b/setup.cfg index 66cb3ed3..8f7c46aa 100644 --- a/setup.cfg +++ b/setup.cfg @@ -28,7 +28,6 @@ install_requires = oslo.config juju<3.0 colorama - ruamel.yaml packaging aioconsole halo diff --git a/tests/unit/apps/test_core.py b/tests/unit/apps/test_core.py index d39f44a2..9230e112 100644 --- a/tests/unit/apps/test_core.py +++ b/tests/unit/apps/test_core.py @@ -890,7 +890,7 @@ def test_nova_compute_upgrade_plan(model): Upgrade software packages on unit nova-compute/1 Upgrade software packages on unit nova-compute/2 Refresh 'nova-compute' to the latest revision of 'ussuri/stable' - Change charm config of 'nova-compute' 'action-managed-upgrade' to True. + Change charm config of 'nova-compute' 'action-managed-upgrade' to True Upgrade 'nova-compute' to the new channel: 'victoria/stable' Change charm config of 'nova-compute' 'source' to 'cloud:focal-victoria' Upgrade plan for units: nova-compute/0, nova-compute/1, nova-compute/2 @@ -933,7 +933,7 @@ def test_nova_compute_upgrade_plan(model): can_upgrade_to="ussuri/stable", charm="nova-compute", channel="ussuri/stable", - config={"source": {"value": "distro"}}, + config={"source": {"value": "distro"}, "action-managed-upgrade": {"value": False}}, machines=machines, model=model, origin="ch", @@ -957,7 +957,7 @@ def test_nova_compute_upgrade_plan_single_unit(model): Upgrade software packages of 'nova-compute' from the current APT repositories Upgrade software packages on unit nova-compute/0 Refresh 'nova-compute' to the latest revision of 'ussuri/stable' - Change charm config of 'nova-compute' 'action-managed-upgrade' to True. + Change charm config of 'nova-compute' 'action-managed-upgrade' to True Upgrade 'nova-compute' to the new channel: 'victoria/stable' Change charm config of 'nova-compute' 'source' to 'cloud:focal-victoria' Upgrade plan for units: nova-compute/0 @@ -986,7 +986,7 @@ def test_nova_compute_upgrade_plan_single_unit(model): can_upgrade_to="ussuri/stable", charm="nova-compute", channel="ussuri/stable", - config={"source": {"value": "distro"}}, + config={"source": {"value": "distro"}, "action-managed-upgrade": {"value": False}}, machines=machines, model=model, origin="ch", diff --git a/tests/unit/steps/test_steps.py b/tests/unit/steps/test_steps.py index 2c1abf34..70c5ad56 100644 --- a/tests/unit/steps/test_steps.py +++ b/tests/unit/steps/test_steps.py @@ -115,12 +115,14 @@ def test_step_bool(): sub_step = BaseStep(description="a.a") assert bool(sub_step) is False - plan.sub_steps = [sub_step] + plan.add_step(sub_step) assert bool(plan) is False # coroutine in the plan sub_steps tree sub_sub_step = BaseStep(description="a.a.a", coro=mock_coro("a.a.a")) - sub_step.sub_steps = [sub_sub_step] + sub_step.add_step(sub_sub_step) + plan.add_step(sub_step) + assert bool(sub_sub_step) is True assert bool(sub_step) is True assert bool(plan) is True @@ -225,6 +227,16 @@ def test_step_add_step(): exp_sub_steps = 3 plan = BaseStep(description="plan") for i in range(exp_sub_steps): + plan.add_step(BaseStep(description=f"sub-step-{i}", coro=mock_coro())) + + assert len(plan.sub_steps) == exp_sub_steps + + +def test_step_add_step_skipping_empty(): + """Test BaseStep skipping to add empty sub steps.""" + exp_sub_steps = 0 + plan = BaseStep(description="plan") + for i in range(3): plan.add_step(BaseStep(description=f"sub-step-{i}")) assert len(plan.sub_steps) == exp_sub_steps @@ -242,9 +254,13 @@ def test_step_add_step_failed(): def test_step_cancel_safe(): """Test step safe cancel.""" plan = BaseStep(description="plan") - plan.sub_steps = sub_steps = [BaseStep(description=f"sub-{i}") for i in range(10)] + plan.sub_steps = sub_steps = [ + BaseStep(description=f"sub-{i}", coro=mock_coro()) for i in range(10) + ] # add sub-sub-steps to one sub-step - sub_steps[0].sub_steps = [BaseStep(description=f"sub-0.{i}") for i in range(3)] + sub_steps[0].sub_steps = [ + BaseStep(description=f"sub-0.{i}", coro=mock_coro()) for i in range(3) + ] plan.cancel() diff --git a/tests/unit/steps/test_steps_analyze.py b/tests/unit/steps/test_steps_analyze.py index 34276b77..a82caa7a 100644 --- a/tests/unit/steps/test_steps_analyze.py +++ b/tests/unit/steps/test_steps_analyze.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from textwrap import dedent from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -22,61 +23,129 @@ from cou.steps.analyze import Analysis from cou.utils.juju_utils import COUApplication, COUMachine, COUUnit from cou.utils.openstack import OpenStackRelease +from tests.unit.utils import generate_cou_machine def test_analysis_dump(model): """Test analysis dump.""" - expected_result = ( - "Control Plane:\n" - "keystone:\n" - " model_name: test_model\n" - " charm: keystone\n" - " charm_origin: ch\n" - " os_origin: distro\n" - " channel: ussuri/stable\n" - " units:\n" - " keystone/0:\n" - " workload_version: 17.0.1\n" - " os_version: ussuri\n" - " keystone/1:\n" - " workload_version: 17.0.1\n" - " os_version: ussuri\n" - " keystone/2:\n" - " workload_version: 17.0.1\n" - " os_version: ussuri\n" - "\n" - "cinder:\n" - " model_name: test_model\n" - " charm: cinder\n" - " charm_origin: ch\n" - " os_origin: distro\n" - " channel: ussuri/stable\n" - " units:\n" - " cinder/0:\n" - " workload_version: 16.4.2\n" - " os_version: ussuri\n" - " cinder/1:\n" - " workload_version: 16.4.2\n" - " os_version: ussuri\n" - " cinder/2:\n" - " workload_version: 16.4.2\n" - " os_version: ussuri\n" - "\n" - "rabbitmq-server:\n" - " model_name: test_model\n" - " charm: rabbitmq-server\n" - " charm_origin: ch\n" - " os_origin: distro\n" - " channel: 3.8/stable\n" - " units:\n" - " rabbitmq-server/0:\n" - " workload_version: '3.8'\n" - " os_version: yoga\n" - "Data Plane:\n" - "\nCurrent minimum OS release in the cloud: ussuri\n" - "\nCurrent minimum Ubuntu series in the cloud: focal\n" + expected_result = dedent( + """\ + Control Plane: + keystone: + model_name: test_model + can_upgrade_to: ussuri/stable + charm: keystone + channel: ussuri/stable + config: + source: + value: distro + origin: ch + series: focal + subordinate_to: [] + workload_version: 17.0.1 + units: + keystone/0: + name: keystone/0 + machine: '0' + workload_version: 17.0.1 + os_version: ussuri + keystone/1: + name: keystone/1 + machine: '1' + workload_version: 17.0.1 + os_version: ussuri + keystone/2: + name: keystone/2 + machine: '2' + workload_version: 17.0.1 + os_version: ussuri + machines: + '0': + id: '0' + apps: !!python/tuple [] + az: null + '1': + id: '1' + apps: !!python/tuple [] + az: null + '2': + id: '2' + apps: !!python/tuple [] + az: null + + cinder: + model_name: test_model + can_upgrade_to: ussuri/stable + charm: cinder + channel: ussuri/stable + config: + source: + value: distro + origin: ch + series: focal + subordinate_to: [] + workload_version: 16.4.2 + units: + cinder/0: + name: cinder/0 + machine: '0' + workload_version: 16.4.2 + os_version: ussuri + cinder/1: + name: cinder/1 + machine: '1' + workload_version: 16.4.2 + os_version: ussuri + cinder/2: + name: cinder/2 + machine: '2' + workload_version: 16.4.2 + os_version: ussuri + machines: + '0': + id: '0' + apps: !!python/tuple [] + az: null + '1': + id: '1' + apps: !!python/tuple [] + az: null + '2': + id: '2' + apps: !!python/tuple [] + az: null + + rabbitmq-server: + model_name: test_model + can_upgrade_to: 3.8/stable + charm: rabbitmq-server + channel: 3.8/stable + config: + source: + value: distro + origin: ch + series: focal + subordinate_to: [] + workload_version: '3.8' + units: + rabbitmq-server/0: + name: rabbitmq-server/0 + machine: '0' + workload_version: '3.8' + os_version: yoga + machines: + '0': + id: '0' + apps: !!python/tuple [] + az: null + Data Plane: + + Current minimum OS release in the cloud: ussuri + + Current minimum Ubuntu series in the cloud: focal + """ ) - machines = {f"{i}": MagicMock(spec_set=COUMachine) for i in range(3)} + machines = {f"{i}": generate_cou_machine(f"{i}") for i in range(3)} keystone = Keystone( name="keystone", can_upgrade_to="ussuri/stable", @@ -142,6 +211,7 @@ def test_analysis_dump(model): apps_control_plane=[keystone, cinder, rabbitmq_server], apps_data_plane=[], ) + assert str(result) == expected_result diff --git a/tests/unit/utils.py b/tests/unit/utils.py index 6909932d..8061a051 100644 --- a/tests/unit/utils.py +++ b/tests/unit/utils.py @@ -29,6 +29,7 @@ def generate_cou_machine(machine_id: str, az: str | None = None) -> MagicMock: machine = MagicMock(spec_set=COUMachine)() machine.machine_id = machine_id machine.az = az + machine.apps = tuple() return machine