Skip to content

Commit

Permalink
fix: defer pebble ready event if container is not ready (#116)
Browse files Browse the repository at this point in the history
* fix: defer pebble ready event if container is not ready

Deferring the pebble ready event will ensure that the required operations
that need to happen before starting the service are in place. In this case,
certificate files need to be present in the container, but sometimes the container
is not reachable, causing the action of pushing these files to the container impossible.
In the past we returned if the container was not reachable, but never tried to reach out
to it and push the files, this commit ensures the retry via defer.
Fixes 110

* tests: add test case for defer event and expand unit tests
  • Loading branch information
DnPlas authored Oct 4, 2023
1 parent cbc7cc2 commit 82e26c8
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,12 +227,14 @@ def _gen_certs(self):
for k, v in certs.items():
setattr(self._stored, k, v)

def _upload_certs_to_container(self):
def _upload_certs_to_container(self, event):
"""Upload generated certs to container."""
try:
self._check_container_connection(self.container)
except ErrorWithStatus as error:
self.model.unit.status = error.status
self.logger.warning("Cannot upload certificates to container, deferring")
event.defer()
return

self.container.push(CONTAINER_CERTS_DEST / "key.pem", self._cert_key, make_dirs=True)
Expand Down Expand Up @@ -303,7 +305,7 @@ def _on_remove(self, _):
def _on_pebble_ready(self, event):
"""Configure started container."""
# upload certs to container
self._upload_certs_to_container()
self._upload_certs_to_container(event)

# proceed with other actions
self._on_event(event)
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/test_operator.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from unittest.mock import MagicMock, patch

import ops
import pytest
from ops.model import ActiveStatus, MaintenanceStatus, WaitingStatus
from ops.pebble import CheckStatus
Expand Down Expand Up @@ -132,6 +133,18 @@ def test_update_status(
harness.charm.on.update_status.emit()
assert harness.charm.model.unit.status == charm_status

@patch("charm.KubernetesServicePatch", lambda x, y, service_name: None)
def test_upload_certs_to_container_defer(self, harness):
"""Checks the event is deferred if container is not reachable."""
harness.set_can_connect("admission-webhook", False)
harness.begin()

# Mock the event
mocked_event = MagicMock(spec=ops.HookEvent)

harness.charm._upload_certs_to_container(mocked_event)
mocked_event.defer.assert_called_once()

@patch("charm.KubernetesServicePatch", lambda x, y, service_name: None)
@pytest.mark.parametrize(
"cert_data_dict, should_certs_refresh",
Expand Down

0 comments on commit 82e26c8

Please sign in to comment.