Skip to content

Commit

Permalink
Raise errors on unrecoverable problems (#110)
Browse files Browse the repository at this point in the history
Raise errors for snap install or refresh failure.  Raise an error because it's a fatal failure, and juju will auto-retry the hook, which may succeed on retry if the error was transient.

Improve blocked status message for if it detects snap not installed or service not active in the status hook.
I think Blocked is better than Error status here:
- auto retrying the status hook isn't going to fix anything
- blocked status lets the charm provide a more useful status message to the user
- blocked status will automatically update to active on the next update-status hook once the issue is resolved


Partially fixes: #108
  • Loading branch information
samuelallan72 authored Sep 30, 2024
1 parent fea84f2 commit 9dc1d8f
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 16 deletions.
23 changes: 14 additions & 9 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import ops
import yaml
from charms.grafana_agent.v0.cos_agent import COSAgentProvider
from charms.operator_libs_linux.v2.snap import SnapError
from ops.model import ActiveStatus, BlockedStatus, ModelError, WaitingStatus

from service import SNAP_NAME, UPSTREAM_SNAP, get_installed_snap_service, snap_install_or_refresh
Expand Down Expand Up @@ -148,12 +147,10 @@ def get_resource(self) -> Optional[str]:

def install(self) -> None:
"""Install the necessary resources for the charm."""
try:
snap_install_or_refresh(self.get_resource(), self.model.config["snap_channel"])
except SnapError:
self.model.unit.status = BlockedStatus(
"Failed to remove/install openstack-exporter snap"
)
# If this fails, it's not recoverable.
# So we don't catch the error, instead letting this become a charm error status.
# Errored hooks are auto-retried by juju, so the install may work on retry.
snap_install_or_refresh(self.get_resource(), self.model.config["snap_channel"])

def _configure(self, _: ops.HookEvent) -> None:
"""Configure the charm.
Expand Down Expand Up @@ -228,11 +225,19 @@ def _on_collect_unit_status(self, event: ops.CollectStatusEvent) -> None:

if not snap_service.present:
event.add_status(
BlockedStatus("snap service is not installed, please check snap service")
BlockedStatus(
f"{SNAP_NAME} snap is not installed. "
"Please wait for installation to complete, "
"or manually reinstall the snap if the issue persists."
)
)
elif not snap_service.is_active():
event.add_status(
BlockedStatus("snap service is not running, please check snap service")
BlockedStatus(
f"{SNAP_NAME} snap service is not active. "
"Please wait for configuration to complete, "
"or manually start the service the issue persists."
)
)

event.add_status(ActiveStatus())
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/tests/charm_tests/openstack_exporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,9 @@ def test_openstack_exporter_snap_down(self):
model.block_until_unit_wl_status(
self.leader_unit_entity_id, "blocked", timeout=STATUS_TIMEOUT
)
self.assertEqual(
self.assertIn(
"snap service is not active",
self.leader_unit.workload_status_message,
"snap service is not running, please check snap service",
)

# Start the exporter snap
Expand Down
8 changes: 3 additions & 5 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@
import ops
import ops.testing
import pytest
from ops.model import BlockedStatus
from charms.operator_libs_linux.v2.snap import SnapError

from charm import (
CLOUD_NAME,
OS_CLIENT_CONFIG,
SNAP_NAME,
OpenstackExporterOperatorCharm,
SnapError,
)
from service import SnapService

Expand Down Expand Up @@ -106,6 +105,5 @@ def test_install_snap(self, mock_install, _):
def test_install_snap_error(self, mock_install):
mock_install.side_effect = SnapError("My Error")
self.harness.begin()
self.harness.charm.on.install.emit()
expected_msg = "Failed to remove/install openstack-exporter snap"
assert self.harness.charm.unit.status == BlockedStatus(expected_msg)
with pytest.raises(SnapError, match="My Error"):
self.harness.charm.on.install.emit()

0 comments on commit 9dc1d8f

Please sign in to comment.