Skip to content

Commit

Permalink
fix: call self._cni_config_changed and handle exception (#396)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
DnPlas authored Mar 21, 2024
1 parent a3f8904 commit ecf92a0
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
7 changes: 2 additions & 5 deletions charms/istio-pilot/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions charms/istio-pilot/tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit ecf92a0

Please sign in to comment.