From b0a012b04ce21c05d0199d765c0a861fa25230f4 Mon Sep 17 00:00:00 2001 From: Phan Trung Thanh Date: Thu, 21 Mar 2024 12:06:09 +0100 Subject: [PATCH 01/11] 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 From 307dc6231fc5608b635c5c55b107aaab6f1b3321 Mon Sep 17 00:00:00 2001 From: Phan Trung Thanh Date: Mon, 25 Mar 2024 12:54:11 +0100 Subject: [PATCH 02/11] Reset restart failed count on update status (#29) --- src/charm.py | 5 +++++ src/service.py | 16 +++++++++++++++- tests/unit/test_charm.py | 22 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index dd623bf..376e6f0 100755 --- a/src/charm.py +++ b/src/charm.py @@ -109,6 +109,11 @@ def _on_update_status(self, _: ops.UpdateStatusEvent) -> None: self.model.unit.status = ops.BlockedStatus("Waiting for relation.") return + # set NRestart of the service back to 0 + # We do it here because at this point we can be certain that + # the service is up and running + self.jenkins_agent_service.reset_failed_state() + self.model.unit.status = ops.ActiveStatus() diff --git a/src/service.py b/src/service.py index 1d6e6d7..910f083 100644 --- a/src/service.py +++ b/src/service.py @@ -17,7 +17,7 @@ logger = logging.getLogger(__name__) AGENT_SERVICE_NAME = "jenkins-agent" -APT_PACKAGE_VERSION = "1.0.8" +APT_PACKAGE_VERSION = "1.0.9" 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/" @@ -166,6 +166,20 @@ def restart(self) -> None: if not self._startup_check(): raise ServiceRestartError("Error waiting for the agent service to start") + def reset_failed_state(self) -> None: + """Reset NRestart count of service back to 0. + + The service keeps track of the 'restart-count' and blocks further restarts + if the maximum allowed is reached. This count is not reset when the service restarts + so we need to do it manually. + """ + try: + # Disable protected-access here because reset-failed is not implemented in the lib + systemd._systemctl("reset-failed", AGENT_SERVICE_NAME) # pylint: disable=W0212 + except systemd.SystemdError: + # We only log the exception here as this is not critical + logger.error("Failed to reset failed state") + def reset(self) -> None: """Stop the agent service and clear its configuration file. diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 7e3f1bf..f17cd27 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -10,6 +10,7 @@ import ops import ops.testing import pytest +from charms.operator_libs_linux.v1 import systemd import charm_state import service @@ -173,6 +174,27 @@ def test_update_status_service_active( """ harness.add_relation(charm_state.AGENT_RELATION, "jenkins-k8s") monkeypatch.setattr(service.JenkinsAgentService, "is_active", PropertyMock(return_value=True)) + monkeypatch.setattr(systemd, "_systemctl", MagicMock(side_effect=systemd.SystemdError)) + + harness.begin() + + harness.charm.on.update_status.emit() + + assert harness.charm.unit.status.name == ops.ActiveStatus.name + + +def test_update_status_reset_failed_state_systemd_error( + 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 with reset-failed raising an error. + assert: The charm correctly ignore the error and sets the status to active. + """ + harness.add_relation(charm_state.AGENT_RELATION, "jenkins-k8s") + monkeypatch.setattr(service.JenkinsAgentService, "is_active", PropertyMock(return_value=True)) + monkeypatch.setattr(systemd, "_systemctl", MagicMock(side_effect=systemd.SystemdError)) + harness.begin() harness.charm.on.update_status.emit() From f2a42f5486dea95c316032c81fe64459c6a3160d Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 1 Apr 2024 09:42:52 +0200 Subject: [PATCH 03/11] chore(deps): update dependency ops to v2.12.0 (#30) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 69550df..09ddcce 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -ops==2.11.0 +ops==2.12.0 requests==2.31.0 pydantic==1.10.14 jinja2==3.1.3 From 8f2737d4a6933f237380d93718b60a10efe03d52 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 3 Apr 2024 17:06:51 +0200 Subject: [PATCH 04/11] chore(deps): update dependency pydantic to v1.10.15 (#31) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 09ddcce..325c04c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ ops==2.12.0 requests==2.31.0 -pydantic==1.10.14 +pydantic==1.10.15 jinja2==3.1.3 python-dotenv==1.0.1 From 349ba33367e0f1d9ee29bf028bcbf89e3a191ecb Mon Sep 17 00:00:00 2001 From: Phan Trung Thanh Date: Wed, 24 Apr 2024 18:13:04 +0200 Subject: [PATCH 05/11] bump package version to 1.0.10 (#32) --- src/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/service.py b/src/service.py index 910f083..4f8b8d4 100644 --- a/src/service.py +++ b/src/service.py @@ -17,7 +17,7 @@ logger = logging.getLogger(__name__) AGENT_SERVICE_NAME = "jenkins-agent" -APT_PACKAGE_VERSION = "1.0.9" +APT_PACKAGE_VERSION = "1.0.10" 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/" From 63e69b517c4d7bfbaf617a0c3696f84140b38467 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 09:23:17 +0200 Subject: [PATCH 06/11] chore(deps): update dependency jinja2 to v3.1.4 (#34) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 325c04c..22fe475 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ ops==2.12.0 requests==2.31.0 pydantic==1.10.15 -jinja2==3.1.3 +jinja2==3.1.4 python-dotenv==1.0.1 From f18f7dfb787a2e2ab4bad519f350ba7c85f8bb1c Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 21 May 2024 09:58:53 +0200 Subject: [PATCH 07/11] chore(deps): update dependency requests to v2.32.1 (#36) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 22fe475..f8500e6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ ops==2.12.0 -requests==2.31.0 +requests==2.32.1 pydantic==1.10.15 jinja2==3.1.4 python-dotenv==1.0.1 From 72cacbf5aa26e69c011d8d21a55c5693dcb083d3 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 22 May 2024 10:02:54 +0200 Subject: [PATCH 08/11] chore(deps): update dependency requests to v2.32.2 (#37) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index f8500e6..88066af 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ ops==2.12.0 -requests==2.32.1 +requests==2.32.2 pydantic==1.10.15 jinja2==3.1.4 python-dotenv==1.0.1 From 1a641d5756ceca71644332669b419b24f4ef6c8f Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 29 May 2024 19:36:12 +0200 Subject: [PATCH 09/11] chore(deps): update dependency requests to v2.32.3 (#38) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 88066af..0191f62 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ ops==2.12.0 -requests==2.32.2 +requests==2.32.3 pydantic==1.10.15 jinja2==3.1.4 python-dotenv==1.0.1 From a055bd71ce6c2b22e5aae3fad6024cb949bf1b5f Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 3 Jun 2024 09:19:28 +0200 Subject: [PATCH 10/11] chore(deps): update dependency ops to v2.14.0 (#33) * chore(deps): update dependency ops to v2.13.0 * Update requirements.txt --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: arturo-seijas <102022572+arturo-seijas@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 0191f62..5ec708b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -ops==2.12.0 +ops==2.13.0 requests==2.32.3 pydantic==1.10.15 jinja2==3.1.4 From affdcb2d46a101224cbbb6397ac39274ef7a1813 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 3 Jun 2024 14:16:41 +0200 Subject: [PATCH 11/11] chore(deps): update dependency ops to v2.14.0 (#39) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 5ec708b..7709a43 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -ops==2.13.0 +ops==2.14.0 requests==2.32.3 pydantic==1.10.15 jinja2==3.1.4