From 8b1021865002da073b8d64d242fae74bb9f0eb3f Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Tue, 5 Dec 2023 16:06:26 +0800 Subject: [PATCH] fix: prevent hooks before preconditions ready (#75) * fix: prevent hooks before preconditions ready * fix: check /proc/mounts for storage mount validation * chore: refactor storage check to state * fix: circular imports --- src-docs/actions.py.md | 3 + src-docs/agent.py.md | 1 + src-docs/charm.py.md | 5 +- src-docs/state.py.md | 20 ++++--- src/actions.py | 14 ++--- src/agent.py | 22 +++---- src/charm.py | 19 +++--- src/jenkins.py | 20 +++---- src/state.py | 25 +++++++- tests/unit/conftest.py | 6 +- tests/unit/helpers.py | 1 + tests/unit/test_actions.py | 46 ++++++-------- tests/unit/test_charm.py | 119 +++++++++++++++++++++---------------- tests/unit/test_jenkins.py | 2 +- tests/unit/test_state.py | 31 ++++++++++ 15 files changed, 202 insertions(+), 132 deletions(-) diff --git a/src-docs/actions.py.md b/src-docs/actions.py.md index d23dc0bb..0684f53e 100644 --- a/src-docs/actions.py.md +++ b/src-docs/actions.py.md @@ -5,6 +5,9 @@ # module `actions.py` Jenkins charm actions. +**Global Variables** +--------------- +- **JENKINS_SERVICE_NAME** --- diff --git a/src-docs/agent.py.md b/src-docs/agent.py.md index 5cce7905..f9272e1e 100644 --- a/src-docs/agent.py.md +++ b/src-docs/agent.py.md @@ -9,6 +9,7 @@ The Jenkins agent relation observer. --------------- - **AGENT_RELATION** - **DEPRECATED_AGENT_RELATION** +- **JENKINS_SERVICE_NAME** --- diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index 695d79ba..fdc05ed9 100644 --- a/src-docs/charm.py.md +++ b/src-docs/charm.py.md @@ -5,6 +5,9 @@ # module `charm.py` Charm Jenkins. +**Global Variables** +--------------- +- **JENKINS_SERVICE_NAME** --- @@ -12,7 +15,7 @@ Charm Jenkins. ## class `JenkinsK8sOperatorCharm` Charm Jenkins. - + ### function `__init__` diff --git a/src-docs/state.py.md b/src-docs/state.py.md index 8c59a020..531b4f84 100644 --- a/src-docs/state.py.md +++ b/src-docs/state.py.md @@ -9,6 +9,8 @@ Jenkins States. --------------- - **AGENT_RELATION** - **DEPRECATED_AGENT_RELATION** +- **JENKINS_SERVICE_NAME** +- **JENKINS_HOME_STORAGE_NAME** --- @@ -29,7 +31,7 @@ Metadata for registering Jenkins Agent. --- - + ### classmethod `from_agent_relation` @@ -54,7 +56,7 @@ Instantiate AgentMeta from charm relation databag. --- - + ### classmethod `from_deprecated_agent_relation` @@ -79,7 +81,7 @@ Instantiate AgentMeta from charm relation databag. --- - + ### classmethod `numeric_executors` @@ -112,7 +114,7 @@ Exception raised when a charm configuration is found to be invalid. - `msg`: Explanation of the error. - + ### function `__init__` @@ -143,7 +145,7 @@ Represents an error with invalid number of units deployed. - `msg`: Explanation of the error. - + ### function `__init__` @@ -174,7 +176,7 @@ Represents an error with invalid data in relation data. - `msg`: Explanation of the error. - + ### function `__init__` @@ -221,7 +223,7 @@ Configuration for accessing Jenkins through proxy. --- - + ### classmethod `from_env` @@ -249,16 +251,16 @@ The Jenkins k8s operator charm state. - `restart_time_range`: Time range to allow Jenkins to update version. - `agent_relation_meta`: Metadata of all agents from units related through agent relation. - `deprecated_agent_relation_meta`: Metadata of all agents from units related through deprecated agent relation. + - `is_storage_ready`: Whether the Jenkins home storage is mounted. - `proxy_config`: Proxy configuration to access Jenkins upstream through. - `plugins`: The list of allowed plugins to install. - - `jenkins_service_name`: The Jenkins service name. Note that the container name is the same. --- - + ### classmethod `from_charm` diff --git a/src/actions.py b/src/actions.py index b14398c2..481f3cad 100644 --- a/src/actions.py +++ b/src/actions.py @@ -6,7 +6,7 @@ import ops import jenkins -from state import State +from state import JENKINS_SERVICE_NAME, State class Observer(ops.Object): @@ -35,9 +35,9 @@ def on_get_admin_password(self, event: ops.ActionEvent) -> None: Args: event: The event fired from get-admin-password action. """ - container = self.charm.unit.get_container(self.state.jenkins_service_name) - if not container.can_connect(): - event.fail("Container not yet ready.") + container = self.charm.unit.get_container(JENKINS_SERVICE_NAME) + if not container.can_connect() or not self.state.is_storage_ready: + event.fail("Service not yet ready.") return credentials = jenkins.get_admin_credentials(container) event.set_results({"password": credentials.password_or_token}) @@ -48,9 +48,9 @@ def on_rotate_credentials(self, event: ops.ActionEvent) -> None: Args: event: The rotate credentials event. """ - container = self.charm.unit.get_container(self.state.jenkins_service_name) - if not container.can_connect(): - event.fail("Container not yet ready.") + container = self.charm.unit.get_container(JENKINS_SERVICE_NAME) + if not container.can_connect() or not self.state.is_storage_ready: + event.fail("Service not yet ready.") return try: password = jenkins.rotate_credentials(container) diff --git a/src/agent.py b/src/agent.py index 1a350b0f..7bb8e959 100644 --- a/src/agent.py +++ b/src/agent.py @@ -8,7 +8,7 @@ import ops import jenkins -from state import AGENT_RELATION, DEPRECATED_AGENT_RELATION, AgentMeta, State +from state import AGENT_RELATION, DEPRECATED_AGENT_RELATION, JENKINS_SERVICE_NAME, AgentMeta, State logger = logging.getLogger(__name__) @@ -60,9 +60,8 @@ def _on_deprecated_agent_relation_joined(self, event: ops.RelationJoinedEvent) - Args: event: The event fired from an agent joining the relationship. """ - container = self.charm.unit.get_container(self.state.jenkins_service_name) - if not container.can_connect(): - event.defer() + container = self.charm.unit.get_container(JENKINS_SERVICE_NAME) + if not container.can_connect() or not self.state.is_storage_ready: return # The relation is joined, it cannot be None, hence the type casting. deprecated_agent_relation_meta = typing.cast( @@ -100,9 +99,8 @@ def _on_agent_relation_joined(self, event: ops.RelationJoinedEvent) -> None: Args: event: The event fired from an agent joining the relationship. """ - container = self.charm.unit.get_container(self.state.jenkins_service_name) - if not container.can_connect(): - event.defer() + container = self.charm.unit.get_container(JENKINS_SERVICE_NAME) + if not container.can_connect() or not self.state.is_storage_ready: return # The relation is joined, it cannot be None, hence the type casting. agent_relation_meta = typing.cast( @@ -144,9 +142,8 @@ def _on_deprecated_agent_relation_departed(self, event: ops.RelationDepartedEven event: The event fired when a unit in deprecated agent relation is departed. """ # the event unit cannot be None. - container = self.charm.unit.get_container(self.state.jenkins_service_name) - if not container.can_connect(): - event.defer() + container = self.charm.unit.get_container(JENKINS_SERVICE_NAME) + if not container.can_connect() or not self.state.is_storage_ready: return # The relation data is removed before this particular hook runs, making the name set by the @@ -172,9 +169,8 @@ def _on_agent_relation_departed(self, event: ops.RelationDepartedEvent) -> None: event: The event fired when a unit in agent relation is departed. """ # the event unit cannot be None. - container = self.charm.unit.get_container(self.state.jenkins_service_name) - if not container.can_connect(): - event.defer() + container = self.charm.unit.get_container(JENKINS_SERVICE_NAME) + if not container.can_connect() or not self.state.is_storage_ready: return # The relation data is removed before this particular hook runs, making the name set by the diff --git a/src/charm.py b/src/charm.py index b2cf2fd9..907eee27 100755 --- a/src/charm.py +++ b/src/charm.py @@ -17,6 +17,7 @@ import jenkins import timerange from state import ( + JENKINS_SERVICE_NAME, CharmConfigInvalidError, CharmIllegalNumUnitsError, CharmRelationDataInvalidError, @@ -74,7 +75,7 @@ def _get_pebble_layer(self, jenkins_env: jenkins.Environment) -> ops.pebble.Laye "summary": "jenkins layer", "description": "pebble config layer for jenkins", "services": { - self.state.jenkins_service_name: { + JENKINS_SERVICE_NAME: { "override": "replace", "summary": "jenkins", "command": f"java -D{jenkins.SYSTEM_PROPERTY_HEADLESS} " @@ -105,8 +106,9 @@ def _on_jenkins_pebble_ready(self, event: ops.PebbleReadyEvent) -> None: Args: event: The event fired when pebble is ready. """ - container = event.workload - if not container or not container.can_connect(): + container = self.unit.get_container(JENKINS_SERVICE_NAME) + if not container or not container.can_connect() or not self.state.is_storage_ready: + self.unit.status = ops.WaitingStatus("Waiting for container/storage.") event.defer() return @@ -123,7 +125,7 @@ def _on_jenkins_pebble_ready(self, event: ops.PebbleReadyEvent) -> None: self.unit.status = ops.MaintenanceStatus("Configuring Jenkins.") jenkins.bootstrap(container, self.state.proxy_config) # Second Jenkins server start restarts Jenkins to bypass Wizard setup. - container.restart(self.state.jenkins_service_name) + container.restart(JENKINS_SERVICE_NAME) jenkins.wait_ready() except TimeoutError as exc: logger.error("Timed out waiting for Jenkins, %s", exc) @@ -171,8 +173,9 @@ def _on_update_status(self, _: ops.UpdateStatusEvent) -> None: 1. Remove plugins that are installed but are not allowed by plugins config value. 2. Update Jenkins patch version if available and is within restart-time-range config value. """ - container = self.unit.get_container(self.state.jenkins_service_name) - if not container.can_connect(): + container = self.unit.get_container(JENKINS_SERVICE_NAME) + if not container.can_connect() or not self.state.is_storage_ready: + self.unit.status = ops.WaitingStatus("Waiting for container/storage.") return if self.state.restart_time_range and not timerange.check_now_within_bound_hours( @@ -188,9 +191,9 @@ def _on_jenkins_home_storage_attached(self, event: ops.StorageAttachedEvent) -> Args: event: The event fired when the storage is attached. """ - container = self.unit.get_container(self.state.jenkins_service_name) + container = self.unit.get_container(JENKINS_SERVICE_NAME) if not container.can_connect(): - event.defer() + self.unit.status = ops.WaitingStatus("Waiting for pebble.") return command = [ diff --git a/src/jenkins.py b/src/jenkins.py index b914e0e3..d1ece1aa 100644 --- a/src/jenkins.py +++ b/src/jenkins.py @@ -22,30 +22,30 @@ from pydantic import HttpUrl import state +from state import JENKINS_HOME_PATH logger = logging.getLogger(__name__) WEB_PORT = 8080 WEB_URL = f"http://localhost:{WEB_PORT}" LOGIN_URL = f"{WEB_URL}/login?from=%2F" -HOME_PATH = Path("/var/lib/jenkins") EXECUTABLES_PATH = Path("/srv/jenkins/") # Path to initial Jenkins password file -PASSWORD_FILE_PATH = HOME_PATH / "secrets/initialAdminPassword" +PASSWORD_FILE_PATH = JENKINS_HOME_PATH / "secrets/initialAdminPassword" # Path to Jenkins admin API token -API_TOKEN_PATH = HOME_PATH / "secrets/apiToken" +API_TOKEN_PATH = JENKINS_HOME_PATH / "secrets/apiToken" # Path to last executed Jenkins version file, required to override wizard installation -LAST_EXEC_VERSION_PATH = HOME_PATH / Path("jenkins.install.InstallUtil.lastExecVersion") +LAST_EXEC_VERSION_PATH = JENKINS_HOME_PATH / Path("jenkins.install.InstallUtil.lastExecVersion") # Path to Jenkins version file, required to override wizard installation -WIZARD_VERSION_PATH = HOME_PATH / Path("jenkins.install.UpgradeWizard.state") +WIZARD_VERSION_PATH = JENKINS_HOME_PATH / Path("jenkins.install.UpgradeWizard.state") # The Jenkins bootstrapping config path -CONFIG_FILE_PATH = HOME_PATH / "config.xml" +CONFIG_FILE_PATH = JENKINS_HOME_PATH / "config.xml" # The Jenkins plugins installation directory -PLUGINS_PATH = HOME_PATH / "plugins" +PLUGINS_PATH = JENKINS_HOME_PATH / "plugins" # The Jenkins logging configuration path -LOGGING_CONFIG_PATH = HOME_PATH / "logging.properties" +LOGGING_CONFIG_PATH = JENKINS_HOME_PATH / "logging.properties" # The Jenkins logging path as defined in templates/logging.properties file -LOGGING_PATH = HOME_PATH / "jenkins.log" +LOGGING_PATH = JENKINS_HOME_PATH / "jenkins.log" # The plugins that are required for Jenkins to work REQUIRED_PLUGINS = [ @@ -218,7 +218,7 @@ def calculate_env() -> Environment: Returns: The dictionary mapping of environment variables for the Jenkins service. """ - return Environment(JENKINS_HOME=str(HOME_PATH)) + return Environment(JENKINS_HOME=str(JENKINS_HOME_PATH)) def get_version() -> str: diff --git a/src/state.py b/src/state.py index ff558876..85aba182 100644 --- a/src/state.py +++ b/src/state.py @@ -7,6 +7,7 @@ import logging import os import typing +from pathlib import Path import ops from pydantic import BaseModel, Field, HttpUrl, ValidationError, validator @@ -17,6 +18,9 @@ AGENT_RELATION = "agent" DEPRECATED_AGENT_RELATION = "agent-deprecated" +JENKINS_SERVICE_NAME = "jenkins" +JENKINS_HOME_STORAGE_NAME = "jenkins-home" +JENKINS_HOME_PATH = Path("/var/lib/jenkins") class CharmStateBaseError(Exception): @@ -214,6 +218,22 @@ def from_env(cls) -> typing.Optional["ProxyConfig"]: ) +def _is_storage_ready(charm: ops.CharmBase) -> bool: + """Return whether the Jenkins home storage is mounted. + + Args: + charm: The Jenkins k8s charm. + + Returns: + True if storage is mounted, False otherwise. + """ + container = charm.unit.get_container(JENKINS_SERVICE_NAME) + if not container.can_connect(): + return False + mount_info: str = container.pull("/proc/mounts").read() + return str(JENKINS_HOME_PATH) in mount_info + + @dataclasses.dataclass(frozen=True) class State: """The Jenkins k8s operator charm state. @@ -223,9 +243,9 @@ class State: agent_relation_meta: Metadata of all agents from units related through agent relation. deprecated_agent_relation_meta: Metadata of all agents from units related through deprecated agent relation. + is_storage_ready: Whether the Jenkins home storage is mounted. proxy_config: Proxy configuration to access Jenkins upstream through. plugins: The list of allowed plugins to install. - jenkins_service_name: The Jenkins service name. Note that the container name is the same. """ restart_time_range: typing.Optional[Range] @@ -235,7 +255,7 @@ class State: ] proxy_config: typing.Optional[ProxyConfig] plugins: typing.Optional[typing.Iterable[str]] - jenkins_service_name: str = "jenkins" + is_storage_ready: bool @classmethod def from_charm(cls, charm: ops.CharmBase) -> "State": @@ -295,6 +315,7 @@ def from_charm(cls, charm: ops.CharmBase) -> "State": restart_time_range=restart_time_range, agent_relation_meta=agent_relation_meta_map, deprecated_agent_relation_meta=deprecated_agent_meta_map, + is_storage_ready=_is_storage_ready(charm=charm), plugins=plugins, proxy_config=proxy_config, ) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 379655cc..a2fb41de 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -169,7 +169,11 @@ def container_fixture( proxy_config: state.ProxyConfig, ) -> Container: """Harness Jenkins workload container that acts as a Jenkins container.""" + harness.add_storage(state.JENKINS_HOME_STORAGE_NAME, count=1, attach=True) jenkins_root = harness.get_filesystem_root("jenkins") + storage_mount_proc_path = combine_root_paths(jenkins_root, Path("/proc/mounts")) + storage_mount_proc_path.parent.mkdir(parents=True, exist_ok=True) + storage_mount_proc_path.write_text(str(state.JENKINS_HOME_PATH), "utf-8") password_file_path = combine_root_paths(jenkins_root, jenkins.PASSWORD_FILE_PATH) password_file_path.parent.mkdir(parents=True, exist_ok=True) password_file_path.write_text(admin_credentials.password_or_token, encoding="utf-8") @@ -249,7 +253,7 @@ def cmd_handler(argv: list[str]) -> tuple[int, str, str]: @pytest.fixture(scope="function", name="harness_container") def harness_container_fixture(harness: Harness, container: Container) -> HarnessWithContainer: - """Named tuple containing Harness with container.""" + """Named tuple containing Harness with container with container ready and fs mounted.""" return HarnessWithContainer(harness=harness, container=container) diff --git a/tests/unit/helpers.py b/tests/unit/helpers.py index db561738..79ff8771 100644 --- a/tests/unit/helpers.py +++ b/tests/unit/helpers.py @@ -10,6 +10,7 @@ ACTIVE_STATUS_NAME = "active" BLOCKED_STATUS_NAME = "blocked" MAINTENANCE_STATUS_NAME = "maintenance" +WAITING_STATUS_NAME = "waiting" # There aren't enough public methods with this patch class. diff --git a/tests/unit/test_actions.py b/tests/unit/test_actions.py index 0d742de4..23cc5023 100644 --- a/tests/unit/test_actions.py +++ b/tests/unit/test_actions.py @@ -20,22 +20,27 @@ from .types_ import HarnessWithContainer -def test_on_get_admin_password_action_container_not_ready( - harness_container: HarnessWithContainer, -): +@pytest.mark.parametrize( + "event_handler", + [ + pytest.param("on_get_admin_password"), + pytest.param("on_rotate_credentials"), + ], +) +def test_workload_not_ready(harness: Harness, event_handler: str): """ - arrange: given a jenkins container that is not connectable. - act: when get-admin-password action is run. - assert: the event is failed. + arrange: given a charm with no container ready. + act: when an event hook is fired. + assert: the charm falls into waiting status. """ - harness_container.harness.set_can_connect( - harness_container.harness.model.unit.containers["jenkins"], False - ) + harness.begin() + jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) + handler_func = getattr(jenkins_charm.actions_observer, event_handler) mock_event = unittest.mock.MagicMock(spec=ops.ActionEvent) - harness_container.harness.begin() + mock_event.workload = unittest.mock.MagicMock(spec=ops.model.Container) + mock_event.workload.can_connect.return_value = False - jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness_container.harness.charm) - jenkins_charm.actions_observer.on_get_admin_password(mock_event) + handler_func(mock_event) mock_event.fail.assert_called_once() @@ -59,23 +64,6 @@ def test_on_get_admin_password_action( ) -def test_on_rotate_credentials_action_container_not_ready( - harness: Harness, -): - """ - arrange: given a jenkins container that is not connectable. - act: when rotate_credentials action is run. - assert: the event is failed. - """ - mock_event = unittest.mock.MagicMock(spec=ops.ActionEvent) - harness.begin() - - jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) - jenkins_charm.actions_observer.on_rotate_credentials(mock_event) - - mock_event.fail.assert_called_once() - - def test_on_rotate_credentials_action_api_error( harness_container: HarnessWithContainer, monkeypatch: pytest.MonkeyPatch, diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 0b85976d..7183abd6 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -20,7 +20,7 @@ import timerange from charm import JenkinsK8sOperatorCharm -from .helpers import ACTIVE_STATUS_NAME, BLOCKED_STATUS_NAME +from .helpers import ACTIVE_STATUS_NAME, BLOCKED_STATUS_NAME, WAITING_STATUS_NAME from .types_ import Harness, HarnessWithContainer @@ -42,20 +42,56 @@ def test___init___invailid_config( assert jenkins_charm.unit.status.name == BLOCKED_STATUS_NAME, "unit should be in BlockedStatus" -def test__on_jenkins_pebble_ready_no_container(harness_container: HarnessWithContainer): +@pytest.mark.parametrize( + "event_handler", + [ + pytest.param("_on_jenkins_home_storage_attached"), + pytest.param("_on_jenkins_pebble_ready"), + pytest.param("_on_update_status"), + ], +) +def test_workload_not_ready(harness: Harness, event_handler: str): """ - arrange: given a pebble ready event with container unable to connect. - act: when the Jenkins pebble ready event is fired. - assert: the event should be deferred. + arrange: given a charm with no container ready. + act: when an event hook is fired. + assert: the charm falls into waiting status. """ - harness_container.harness.begin() - jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness_container.harness.charm) - mock_event = unittest.mock.MagicMock(spec=ops.PebbleReadyEvent) - mock_event.workload = None + harness.begin() + jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) + handler_func = getattr(jenkins_charm, event_handler) + mock_event = unittest.mock.MagicMock(spec=ops.WorkloadEvent) + mock_event.workload = unittest.mock.MagicMock(spec=ops.model.Container) + mock_event.workload.can_connect.return_value = False - jenkins_charm._on_jenkins_pebble_ready(mock_event) + handler_func(mock_event) - mock_event.defer.assert_called() + assert jenkins_charm.unit.status.name == WAITING_STATUS_NAME + + +@pytest.mark.parametrize( + "event_handler", + [ + pytest.param("_on_jenkins_home_storage_attached"), + pytest.param("_on_jenkins_pebble_ready"), + pytest.param("_on_update_status"), + ], +) +def test_storage_not_ready(harness: Harness, event_handler: str): + """ + arrange: given a charm with no storage ready. + act: when an event hook is fired. + assert: the charm falls into waiting status. + """ + harness.begin() + jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) + handler_func = getattr(jenkins_charm, event_handler) + mock_event = unittest.mock.MagicMock(spec=ops.WorkloadEvent) + mock_event.workload = unittest.mock.MagicMock(spec=ops.model.Container) + mock_event.workload.can_connect.return_value = True + + handler_func(mock_event) + + assert jenkins_charm.unit.status.name == WAITING_STATUS_NAME def test__on_jenkins_pebble_ready_error( @@ -68,6 +104,7 @@ def test__on_jenkins_pebble_ready_error( act: when the jenkins_pebble_ready event is fired. assert: the unit status should be in BlockedStatus. """ + harness = harness_container.harness # speed up waiting by changing default argument values monkeypatch.setattr(jenkins.wait_ready, "__defaults__", (1, 1)) monkeypatch.setattr( @@ -76,10 +113,11 @@ def test__on_jenkins_pebble_ready_error( unittest.mock.MagicMock(side_effect=jenkins.JenkinsBootstrapError()), ) monkeypatch.setattr(requests, "get", functools.partial(mocked_get_request, status_code=200)) + harness.begin() - harness_container.harness.begin_with_initial_hooks() + jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) + jenkins_charm._on_jenkins_pebble_ready(unittest.mock.MagicMock(spec=ops.PebbleReadyEvent)) - jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness_container.harness.charm) assert jenkins_charm.unit.status.name == BLOCKED_STATUS_NAME, "unit should be in BlockedStatus" @@ -93,6 +131,7 @@ def test__on_jenkins_pebble_ready_get_version_error( act: when the jenkins_pebble_ready event is fired. assert: the unit status should be in BlockedStatus. """ + harness = harness_container.harness # speed up waiting by changing default argument values monkeypatch.setattr( jenkins, "get_version", unittest.mock.MagicMock(side_effect=jenkins.JenkinsError) @@ -100,26 +139,25 @@ def test__on_jenkins_pebble_ready_get_version_error( monkeypatch.setattr(jenkins.wait_ready, "__defaults__", (1, 1)) monkeypatch.setattr(jenkins, "bootstrap", lambda *_args: None) monkeypatch.setattr(requests, "get", functools.partial(mocked_get_request, status_code=200)) + harness.begin() - harness_container.harness.begin_with_initial_hooks() + jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) + jenkins_charm._on_jenkins_pebble_ready(unittest.mock.MagicMock(spec=ops.PebbleReadyEvent)) - jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness_container.harness.charm) assert jenkins_charm.unit.status.name == BLOCKED_STATUS_NAME, "unit should be in BlockedStatus" @pytest.mark.parametrize( - "status_code,expected_status", + "exception,expected_status", [ - pytest.param(503, ops.BlockedStatus, id="jenkins not ready"), - pytest.param(200, ops.ActiveStatus, id="jenkins ready"), + pytest.param(TimeoutError, ops.BlockedStatus, id="jenkins not ready"), + pytest.param(None, ops.ActiveStatus, id="jenkins ready"), ], ) -# there are too many dependent fixtures that cannot be merged. -def test__on_jenkins_pebble_ready( # pylint: disable=too-many-arguments +def test__on_jenkins_pebble_ready( harness_container: HarnessWithContainer, - mocked_get_request: typing.Callable[[str, int, typing.Any, typing.Any], requests.Response], monkeypatch: pytest.MonkeyPatch, - status_code: int, + exception: Exception | None, expected_status: ops.StatusBase, ): """ @@ -127,19 +165,22 @@ def test__on_jenkins_pebble_ready( # pylint: disable=too-many-arguments act: when the Jenkins pebble ready event is fired. assert: the unit status should show expected status. """ + harness = harness_container.harness # monkeypatch environment variables because the test is running in self-hosted runners and juju # proxy environment is picked up, making the test fail. monkeypatch.setattr(state.os, "environ", {}) # speed up waiting by changing default argument values - monkeypatch.setattr(jenkins.wait_ready, "__defaults__", (1, 1)) - monkeypatch.setattr(jenkins, "_setup_user_token", lambda *_args, **_kwargs: None) monkeypatch.setattr( - requests, "get", functools.partial(mocked_get_request, status_code=status_code) + jenkins, "wait_ready", unittest.mock.MagicMock(side_effect=[None, exception]) ) + monkeypatch.setattr(jenkins, "bootstrap", lambda *_args, **_kwargs: None) + monkeypatch.setattr(jenkins, "get_version", lambda *_args, **_kwargs: "1") + harness.begin() - harness_container.harness.begin_with_initial_hooks() + jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) + jenkins_charm._on_jenkins_pebble_ready(unittest.mock.MagicMock(spec=ops.PebbleReadyEvent)) - jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness_container.harness.charm) + jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) assert ( jenkins_charm.unit.status.name == expected_status.name ), f"unit should be in {expected_status}" @@ -208,30 +249,6 @@ def test__remove_unlisted_plugins( assert returned_status.message == "" -def test__on_update_status_no_container( - harness_container: HarnessWithContainer, monkeypatch: pytest.MonkeyPatch -): - """ - arrange: given a charm with container not yet ready to connect. - act: when _on_update_status is called. - assert: no further functions are called. - """ - mock_remove_unlisted_plugins_func = unittest.mock.MagicMock( - spec=JenkinsK8sOperatorCharm._remove_unlisted_plugins - ) - harness, container = harness_container.harness, harness_container.container - harness.set_can_connect(container, False) - harness.begin() - - jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness_container.harness.charm) - monkeypatch.setattr( - jenkins_charm, "_remove_unlisted_plugins", mock_remove_unlisted_plugins_func - ) - jenkins_charm._on_update_status(unittest.mock.MagicMock(spec=ops.UpdateStatusEvent)) - - mock_remove_unlisted_plugins_func.assert_not_called() - - def test__on_update_status_not_in_time_range( harness_container: HarnessWithContainer, monkeypatch: pytest.MonkeyPatch ): diff --git a/tests/unit/test_jenkins.py b/tests/unit/test_jenkins.py index 5f78f70b..57c0737b 100644 --- a/tests/unit/test_jenkins.py +++ b/tests/unit/test_jenkins.py @@ -186,7 +186,7 @@ def test_calculate_env(): """ env = jenkins.calculate_env() - assert env == {"JENKINS_HOME": str(jenkins.HOME_PATH)} + assert env == {"JENKINS_HOME": str(state.JENKINS_HOME_PATH)} @pytest.mark.parametrize( diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index 929a91a5..df94930e 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -7,8 +7,39 @@ import ops import pytest +from ops.testing import Harness import state +from charm import JenkinsK8sOperatorCharm + +from .types_ import HarnessWithContainer + + +def test_is_storage_ready_no_container(harness: Harness): + """ + arrange: given Jenkins charm with container not yet ready. + act: when is_storage_ready is called. + assert: Falsy value is returned. + """ + harness.begin() + + jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) + + assert not jenkins_charm.state.is_storage_ready + + +def test_is_storage_ready(harness_container: HarnessWithContainer): + """ + arrange: given Jenkins charm with container ready and storage mounted. + act: when is_storage_ready is called. + assert: Truthy value is returned. + """ + harness = harness_container.harness + harness.begin() + + jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm) + + assert jenkins_charm.state.is_storage_ready def test_state_invalid_time_config():