From ecf92a0d7aacb95259f88396625416147fb2dadb Mon Sep 17 00:00:00 2001 From: Daniela Plascencia Date: Thu, 21 Mar 2024 18:38:08 +0100 Subject: [PATCH] fix: call self._cni_config_changed and handle exception (#396) * fix: call self._cni_config_changed and handle exception The _cni_config_changed() method was not called correctly because of a typo, which led to the condition evaluating it to always be True, and running upgrades when they are not required. Calling correctly the method ensures the charm executes correct pieces of code. By doing this, the GenericCharmRuntimeError exception that gets raised on an error with CNI configuration must be re-raised when handling the reconcile. Fixes #395 --- charms/istio-pilot/src/charm.py | 7 ++----- charms/istio-pilot/tests/unit/test_charm.py | 5 +++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/charms/istio-pilot/src/charm.py b/charms/istio-pilot/src/charm.py index 32bb2c84..a27c5cc4 100755 --- a/charms/istio-pilot/src/charm.py +++ b/charms/istio-pilot/src/charm.py @@ -287,11 +287,8 @@ def reconcile(self, event): # Call upgrade_charm in case there are new configurations that affect the control plane # only if the CNI configurations have been provided and have changed from a previous state # This is useful when there is a missing configuration during the install process - try: - if self._cni_config_changed: - self.upgrade_charm(event) - except GenericCharmRuntimeError as err: - handled_errors.append(err) + if self._cni_config_changed(): + self.upgrade_charm(event) # Send istiod information to the istio-pilot relation try: diff --git a/charms/istio-pilot/tests/unit/test_charm.py b/charms/istio-pilot/tests/unit/test_charm.py index ca5bc3a3..6a3eaec9 100644 --- a/charms/istio-pilot/tests/unit/test_charm.py +++ b/charms/istio-pilot/tests/unit/test_charm.py @@ -216,6 +216,7 @@ def test_ingress_auth_and_gateway( harness.update_config({"default-gateway": gateway_name}) harness.begin() + harness.add_relation("peers", harness.charm.app.name) mocker.patch("charm.Operator.upgrade_charm") harness.charm.log = MagicMock() @@ -293,6 +294,7 @@ def test_ingress_relation( harness.update_config({"default-gateway": gateway_name}) harness.begin() + harness.add_relation("peers", harness.charm.app.name) mocker.patch("charm.Operator.upgrade_charm") # Do a reconcile @@ -355,6 +357,7 @@ def test_istio_pilot_relation( harness.update_config({"default-gateway": gateway_name}) harness.begin() + harness.add_relation("peers", harness.charm.app.name) mocker.patch("charm.Operator.upgrade_charm") # Do a reconcile @@ -391,6 +394,7 @@ def test_gateway_info_relation( mocked_is_gateway_service_up.return_value = True harness.begin() + harness.add_relation("peers", harness.charm.app.name) mocker.patch("charm.Operator.upgrade_charm") # Act and assert @@ -486,6 +490,7 @@ def test_reconcile_handling_nonfatal_errors( ) harness.begin() + harness.add_relation("peers", harness.charm.app.name) mocker.patch("charm.Operator.upgrade_charm") # Act