From 398678d9df5ee1cad8c704945a17eeecbf96a077 Mon Sep 17 00:00:00 2001 From: Gabriel Cocenza Date: Wed, 24 Apr 2024 14:09:03 -0300 Subject: [PATCH] Raise ApplicationError on auxiliary channel_codename on invalid channel (#389) - Auxiliary subordinates charms like ceph-dashboard uses the channel_codename to calculate the current OpenStack release. When the current channel is not expected, it can return an empty list that breaks the COU execution with a traceback that is hard to understand. - This issue doesn't happen on charms that are not subornitate because it can rely on the workload_version to calculate current OpenStack release - Closes #388 --- cou/apps/auxiliary.py | 9 ++++++ cou/apps/base.py | 3 +- tests/unit/apps/test_auxiliary.py | 16 +++++----- tests/unit/apps/test_auxiliary_subordinate.py | 30 +++++++++++++++++++ tests/unit/apps/test_base.py | 4 +-- tests/unit/apps/test_subordinate.py | 4 +-- 6 files changed, 52 insertions(+), 14 deletions(-) diff --git a/cou/apps/auxiliary.py b/cou/apps/auxiliary.py index 272ed896..dbc07373 100644 --- a/cou/apps/auxiliary.py +++ b/cou/apps/auxiliary.py @@ -128,6 +128,15 @@ def channel_codename(self) -> OpenStackRelease: track: str = self._get_track_from_channel(self.channel) compatible_os_releases = TRACK_TO_OPENSTACK_MAPPING[(self.charm, self.series, track)] + + if not compatible_os_releases: + raise ApplicationError( + f"Channel: {self.channel} for charm '{self.charm}' on series '{self.series}' is " + f"not supported by COU. Please take a look at the documentation: " + "https://docs.openstack.org/charm-guide/latest/project/charm-delivery.html to see " + "if you are using the right track." + ) + return max(compatible_os_releases) def generate_upgrade_plan( diff --git a/cou/apps/base.py b/cou/apps/base.py index 3c2375ca..05a4ff53 100644 --- a/cou/apps/base.py +++ b/cou/apps/base.py @@ -797,8 +797,7 @@ def _check_channel(self) -> None: raise ApplicationError( f"Channel: {self.channel} for charm '{self.charm}' on series " - f"'{self.series}' is currently not supported in this tool. Please take a look at the " - "documentation: " + f"'{self.series}' is not supported by COU. Please take a look at the documentation: " "https://docs.openstack.org/charm-guide/latest/project/charm-delivery.html to see if " "you are using the right track." ) diff --git a/tests/unit/apps/test_auxiliary.py b/tests/unit/apps/test_auxiliary.py index 99dbfe98..e612c3d7 100644 --- a/tests/unit/apps/test_auxiliary.py +++ b/tests/unit/apps/test_auxiliary.py @@ -335,8 +335,8 @@ def test_auxiliary_upgrade_plan_unknown_track(model): """Test auxiliary upgrade plan with unknown track.""" channel = "2.0/stable" exp_msg = ( - f"Channel: {channel} for charm 'rabbitmq-server' on series 'focal' is currently " - "not supported in this tool. Please take a look at the documentation: " + f"Channel: {channel} for charm 'rabbitmq-server' on series 'focal' is not supported by " + "COU. Please take a look at the documentation: " "https://docs.openstack.org/charm-guide/latest/project/charm-delivery.html " "to see if you are using the right track." ) @@ -398,8 +398,8 @@ def test_auxiliary_raise_error_unknown_series(model): series = "foo" channel = "3.8/stable" exp_msg = ( - f"Channel: {channel} for charm 'rabbitmq-server' on series '{series}' is currently " - "not supported in this tool. Please take a look at the documentation: " + f"Channel: {channel} for charm 'rabbitmq-server' on series '{series}' is not supported by " + "COU. Please take a look at the documentation: " "https://docs.openstack.org/charm-guide/latest/project/charm-delivery.html " "to see if you are using the right track." ) @@ -436,8 +436,8 @@ def test_auxiliary_raise_error_os_not_on_lookup(current_os_release, model): """ current_os_release.return_value = OpenStackRelease("diablo") exp_error_msg = ( - "Channel: 3.8/stable for charm 'rabbitmq-server' on series 'focal' is currently not " - "supported in this tool. Please take a look at the documentation: " + "Channel: 3.8/stable for charm 'rabbitmq-server' on series 'focal' is not supported by " + "COU. Please take a look at the documentation: " "https://docs.openstack.org/charm-guide/latest/project/charm-delivery.html to see if you " "are using the right track." ) @@ -895,8 +895,8 @@ def test_ovn_no_compatible_os_release(channel, model): charm = "ovn-central" machines = {"0": MagicMock(spec_set=Machine)} exp_msg = ( - f"Channel: {channel} for charm '{charm}' on series 'focal' is currently " - "not supported in this tool. Please take a look at the documentation: " + f"Channel: {channel} for charm '{charm}' on series 'focal' is not supported by COU. " + "Please take a look at the documentation: " "https://docs.openstack.org/charm-guide/latest/project/charm-delivery.html " "to see if you are using the right track." ) diff --git a/tests/unit/apps/test_auxiliary_subordinate.py b/tests/unit/apps/test_auxiliary_subordinate.py index b848f6e0..5df4ebbf 100644 --- a/tests/unit/apps/test_auxiliary_subordinate.py +++ b/tests/unit/apps/test_auxiliary_subordinate.py @@ -344,3 +344,33 @@ def test_auxiliary_subordinate_latest_stable(model): ) plan = app.generate_upgrade_plan(target, force=False) assert str(plan) == exp_plan + + +def test_auxiliary_subordinate_channel_codename_raise(model): + app = AuxiliarySubordinateApplication( + name="ceph-dashboard", + can_upgrade_to="", + charm="ceph-dashboard", + channel="luminous/stable", + config={}, + machines={"0": MagicMock(spec_set=Machine)}, + model=model, + origin="ch", + series="focal", + subordinate_to=["nova-compute"], + units={}, + workload_version="", + ) + + exp_msg = ( + "Channel: luminous/stable for charm 'ceph-dashboard' on series 'focal' is not supported " + "by COU. Please take a look at the documentation: " + "https://docs.openstack.org/charm-guide/latest/project/charm-delivery.html " + "to see if you are using the right track." + ) + + with pytest.raises(ApplicationError, match=exp_msg): + app.channel_codename + + with pytest.raises(ApplicationError, match=exp_msg): + app.current_os_release diff --git a/tests/unit/apps/test_base.py b/tests/unit/apps/test_base.py index e1f3da78..8302a637 100644 --- a/tests/unit/apps/test_base.py +++ b/tests/unit/apps/test_base.py @@ -315,8 +315,8 @@ def test_check_channel_error(_): channel = "stable" series = "focal" exp_error_msg = ( - f"Channel: {channel} for charm '{name}' on series '{series}' is currently not supported " - "in this tool. Please take a look at the documentation: " + f"Channel: {channel} for charm '{name}' on series '{series}' is not supported by COU. " + "Please take a look at the documentation: " "https://docs.openstack.org/charm-guide/latest/project/charm-delivery.html to see if you " "are using the right track." ) diff --git a/tests/unit/apps/test_subordinate.py b/tests/unit/apps/test_subordinate.py index eac6f66e..7cdabdb1 100644 --- a/tests/unit/apps/test_subordinate.py +++ b/tests/unit/apps/test_subordinate.py @@ -129,8 +129,8 @@ def test_channel_setter_invalid(model, channel): """Test unsuccessful validation of channel upgrade plan for SubordinateApplication.""" machines = {"0": MagicMock(spec_set=Machine)} exp_error_msg = ( - f"Channel: {channel} for charm 'keystone-ldap' on series 'focal' is currently not " - "supported in this tool. Please take a look at the documentation: " + f"Channel: {channel} for charm 'keystone-ldap' on series 'focal' is not supported by COU. " + "Please take a look at the documentation: " "https://docs.openstack.org/charm-guide/latest/project/charm-delivery.html to see if you " "are using the right track." )