From b0a012b04ce21c05d0199d765c0a861fa25230f4 Mon Sep 17 00:00:00 2001 From: Phan Trung Thanh Date: Thu, 21 Mar 2024 12:06:09 +0100 Subject: [PATCH] refactor is_active check and add update_status hook (#25) --- .github/workflows/integration_test.yaml | 2 + src-docs/charm.py.md | 2 +- src-docs/service.py.md | 27 +++++----- src/charm.py | 17 ++++++ src/service.py | 15 +++--- tests/integration/conftest.py | 2 +- tests/integration/pre_run_script.sh | 8 +-- .../requirements_integration_tests.txt | 2 +- tests/unit/test_charm.py | 53 ++++++++++++++++++- tests/unit/test_service.py | 1 - 10 files changed, 99 insertions(+), 30 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index eb171a0..e12cd6e 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -15,3 +15,5 @@ jobs: -c "sudo microk8s config > ${GITHUB_WORKSPACE}/kube-config chmod +x tests/integration/pre_run_script.sh ./tests/integration/pre_run_script.sh" + juju-channel: 3.1/stable + channel: 1.27-strict/stable diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index 4a22aab..cd064a6 100644 --- a/src-docs/charm.py.md +++ b/src-docs/charm.py.md @@ -72,7 +72,7 @@ Unit that this execution is responsible for. --- - + ### function `restart_agent_service` diff --git a/src-docs/service.py.md b/src-docs/service.py.md index 32f4869..a6df64d 100644 --- a/src-docs/service.py.md +++ b/src-docs/service.py.md @@ -8,7 +8,8 @@ The agent pebble service module. **Global Variables** --------------- - **AGENT_SERVICE_NAME** -- **AGENT_PACKAGE_NAME** +- **APT_PACKAGE_NAME** +- **APT_PACKAGE_VERSION** - **SYSTEMD_SERVICE_CONF_DIR** - **PPA_URI** - **PPA_DEB_SRC** @@ -33,7 +34,7 @@ Jenkins agent service class. Attrs: is_active: Indicate if the agent service is active and running. - + ### function `__init__` @@ -60,7 +61,7 @@ Indicate if the jenkins agent service is active. --- - + ### function `install` @@ -78,39 +79,39 @@ Install and set up the jenkins agent apt package. --- - + -### function `restart` +### function `reset` ```python -restart() → None +reset() → None ``` -Start the agent service. +Stop the agent service and clear its configuration file. **Raises:** - - `ServiceRestartError`: when restarting the service fails + - `ServiceStopError`: if systemctl stop returns a non-zero exit code. --- - + -### function `stop` +### function `restart` ```python -stop() → None +restart() → None ``` -Stop the agent service. +Start the agent service. **Raises:** - - `ServiceStopError`: if systemctl stop returns a non-zero exit code. + - `ServiceRestartError`: when restarting the service fails --- diff --git a/src/charm.py b/src/charm.py index 2fb657a..dd623bf 100755 --- a/src/charm.py +++ b/src/charm.py @@ -43,6 +43,7 @@ def __init__(self, *args: typing.Any): self.framework.observe(self.on.start, self._on_start) self.framework.observe(self.on.config_changed, self._on_config_changed) self.framework.observe(self.on.upgrade_charm, self._on_upgrade_charm) + self.framework.observe(self.on.update_status, self._on_update_status) def _on_install(self, _: ops.InstallEvent) -> None: """Handle install event, setup the agent service. @@ -94,6 +95,22 @@ def restart_agent_service(self) -> None: self.model.unit.status = ops.ActiveStatus() + def _on_update_status(self, _: ops.UpdateStatusEvent) -> None: + """Update status event hook. + + Raises: + RuntimeError: when the service is not running. + """ + if self.model.get_relation(AGENT_RELATION) and not self.jenkins_agent_service.is_active: + logger.error("agent related to Jenkins but service is not active") + raise RuntimeError("jenkins-agent service is not running") + + if not self.model.get_relation(AGENT_RELATION): + self.model.unit.status = ops.BlockedStatus("Waiting for relation.") + return + + self.model.unit.status = ops.ActiveStatus() + if __name__ == "__main__": # pragma: no cover main(JenkinsAgentCharm) diff --git a/src/service.py b/src/service.py index 6d23f9f..1d6e6d7 100644 --- a/src/service.py +++ b/src/service.py @@ -17,8 +17,8 @@ logger = logging.getLogger(__name__) AGENT_SERVICE_NAME = "jenkins-agent" -APT_PACKAGE_NAME = "jenkins-agent" -APT_PACKAGE_VERSION = "1.0.6" +APT_PACKAGE_VERSION = "1.0.8" +APT_PACKAGE_NAME = f"jenkins-agent-{APT_PACKAGE_VERSION}" SYSTEMD_SERVICE_CONF_DIR = "/etc/systemd/system/jenkins-agent.service.d/" PPA_URI = "https://ppa.launchpadcontent.net/canonical-is-devops/jenkins-agent-charm/ubuntu/" PPA_DEB_SRC = "deb-https://ppa.launchpadcontent.net/canonical-is-devops/jenkins-agent-charm/ubuntu/-" # noqa: E501 pylint: disable=line-too-long @@ -88,7 +88,9 @@ def _render_file(self, path: Path, content: str, mode: int) -> None: def is_active(self) -> bool: """Indicate if the jenkins agent service is active.""" try: - return systemd.service_running(AGENT_SERVICE_NAME) + return os.path.exists(str(AGENT_READY_PATH)) and systemd.service_running( + AGENT_SERVICE_NAME + ) except SystemError as exc: logger.error("Failed to call systemctl:\n%s", exc) return False @@ -117,7 +119,7 @@ def install(self) -> None: # Install the necessary packages apt.update() apt.add_package("openjdk-17-jre") - apt.add_package(package_names=APT_PACKAGE_NAME, version=APT_PACKAGE_VERSION) + apt.add_package(package_names=APT_PACKAGE_NAME) except (apt.PackageError, apt.PackageNotFoundError, apt.GPGKeyError) as exc: raise PackageInstallError("Error installing the agent package") from exc @@ -187,7 +189,6 @@ def _startup_check(self) -> bool: timeout = time.time() + STARTUP_CHECK_TIMEOUT while time.time() < timeout: time.sleep(STARTUP_CHECK_INTERVAL) - service_up = os.path.exists(str(AGENT_READY_PATH)) and self.is_active - if service_up: + if self.is_active: break - return os.path.exists(str(AGENT_READY_PATH)) and self.is_active + return self.is_active diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 28e27de..b3b958f 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -100,7 +100,7 @@ async def jenkins_server_model_fixture( @pytest_asyncio.fixture(scope="function", name="jenkins_server") async def jenkins_server_fixture(jenkins_server_model: Model) -> Application: """The jenkins machine server.""" - jenkins = await jenkins_server_model.deploy("jenkins-k8s") + jenkins = await jenkins_server_model.deploy("jenkins-k8s", channel="latest/edge") await jenkins_server_model.wait_for_idle( apps=[jenkins.name], timeout=20 * 60, diff --git a/tests/integration/pre_run_script.sh b/tests/integration/pre_run_script.sh index 6b9b815..5efbbff 100644 --- a/tests/integration/pre_run_script.sh +++ b/tests/integration/pre_run_script.sh @@ -9,13 +9,13 @@ # Jenkins machine agent charm is deployed on lxd and Jenkins-k8s server charm is deployed on # microk8s. -sg microk8s -c "microk8s status --wait-ready" +sg snap_microk8s -c "microk8s status --wait-ready" # lxd should be installed and inited by a previous step in integration test action. echo "bootstrapping lxd juju controller" -sg microk8s -c "juju bootstrap localhost localhost" +sg snap_microk8s -c "juju bootstrap localhost localhost" echo "bootstrapping secondary microk8s controller" -sg microk8s -c "juju bootstrap microk8s controller" +sg snap_microk8s -c "juju bootstrap microk8s controller" echo "Switching to testing model" -sg microk8s -c "juju switch localhost" +sg snap_microk8s -c "juju switch localhost" diff --git a/tests/integration/requirements_integration_tests.txt b/tests/integration/requirements_integration_tests.txt index 8278852..0fa0d75 100644 --- a/tests/integration/requirements_integration_tests.txt +++ b/tests/integration/requirements_integration_tests.txt @@ -1,5 +1,5 @@ jenkinsapi>=0.3,<1 -juju==3.0.4 +juju>=3.2.2 ops pytest pytest-asyncio diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index e1ab802..7e3f1bf 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -5,7 +5,7 @@ """Test for charm hooks.""" -from unittest.mock import MagicMock +from unittest.mock import MagicMock, PropertyMock import ops import ops.testing @@ -41,7 +41,7 @@ def test__on_upgrade_charm(harness: ops.testing.Harness, monkeypatch: pytest.Mon act: when _on_upgrade_charm is called. assert: The agent falls into waiting status with the correct message. """ - monkeypatch.setattr(service.JenkinsAgentService, "is_active", MagicMock(return_value=True)) + monkeypatch.setattr(service.JenkinsAgentService, "is_active", PropertyMock(return_value=True)) monkeypatch.setattr(service.JenkinsAgentService, "restart", MagicMock()) harness.begin() @@ -129,3 +129,52 @@ def test_restart_agent_service_service_restart_error( monkeypatch.setattr(charm.state, "agent_relation_credentials", get_credentials_mock) with pytest.raises(RuntimeError, match="Error restarting the agent service"): charm.restart_agent_service() + + +def test_update_status_service_not_active( + harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatch +): + """ + arrange: given a charm with relation to jenkins and the service is not active. + act: when update-status hook is fired. + assert: The charm correctly raise a runtime error. + """ + harness.add_relation(charm_state.AGENT_RELATION, "jenkins-k8s") + monkeypatch.setattr(service.JenkinsAgentService, "is_active", PropertyMock(return_value=False)) + harness.begin() + + with pytest.raises(RuntimeError, match="jenkins-agent service is not running"): + harness.charm.on.update_status.emit() + + +def test_update_status_service_waiting_for_relation( + harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatch +): + """ + arrange: given a charm without a relation to jenkins. + act: when update-status hook is fired. + assert: The charm correctly sets the status to blocked. + """ + monkeypatch.setattr(service.JenkinsAgentService, "is_active", PropertyMock(return_value=False)) + harness.begin() + + harness.charm.on.update_status.emit() + + assert harness.charm.unit.status.name == ops.BlockedStatus.name + + +def test_update_status_service_active( + harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatch +): + """ + arrange: given a charm with relation to jenkins and the service is active. + act: when update-status hook is fired. + assert: The charm correctly sets the status to active. + """ + harness.add_relation(charm_state.AGENT_RELATION, "jenkins-k8s") + monkeypatch.setattr(service.JenkinsAgentService, "is_active", PropertyMock(return_value=True)) + harness.begin() + + harness.charm.on.update_status.emit() + + assert harness.charm.unit.status.name == ops.ActiveStatus.name diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 6a71a71..fa6c552 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -68,7 +68,6 @@ def test_on_install(harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatc assert apt_add_package_mock.call_count == 2 assert apt_add_package_mock.call_args_list[0][0][0] == "openjdk-17-jre" assert apt_add_package_mock.call_args_list[1][1]["package_names"] == service.APT_PACKAGE_NAME - assert apt_add_package_mock.call_args_list[1][1]["version"] == service.APT_PACKAGE_VERSION assert harness.charm.unit.status.name == ops.BlockedStatus.name