From a1f3cb06d7f4b363f873d9d89911d2a30cc30c67 Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Thu, 16 Nov 2023 22:24:09 +0800 Subject: [PATCH] feat: limit num units deployment to 1 (#50) Co-authored-by: arturo-seijas <102022572+arturo-seijas@users.noreply.github.com> --- src-docs/charm.py.md | 2 +- src-docs/state.py.md | 42 +++++++++++++++++++++++++++++++++++----- src/charm.py | 9 +++++++-- src/state.py | 37 +++++++++++++++++++++-------------- tests/unit/conftest.py | 9 +++++++++ tests/unit/test_state.py | 26 +++++++++++++++++-------- 6 files changed, 95 insertions(+), 30 deletions(-) diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index c0e9ba88..b55265e5 100644 --- a/src-docs/charm.py.md +++ b/src-docs/charm.py.md @@ -12,7 +12,7 @@ Charm Jenkins. ## class `JenkinsK8sOperatorCharm` Charm Jenkins. - + ### function `__init__` diff --git a/src-docs/state.py.md b/src-docs/state.py.md index 3976f3d7..8c59a020 100644 --- a/src-docs/state.py.md +++ b/src-docs/state.py.md @@ -29,7 +29,7 @@ Metadata for registering Jenkins Agent. --- - + ### classmethod `from_agent_relation` @@ -54,7 +54,7 @@ Instantiate AgentMeta from charm relation databag. --- - + ### classmethod `from_deprecated_agent_relation` @@ -79,7 +79,7 @@ Instantiate AgentMeta from charm relation databag. --- - + ### classmethod `numeric_executors` @@ -132,6 +132,37 @@ Initialize a new instance of the CharmConfigInvalidError exception. +--- + +## class `CharmIllegalNumUnitsError` +Represents an error with invalid number of units deployed. + + + +**Attributes:** + + - `msg`: Explanation of the error. + + + +### function `__init__` + +```python +__init__(msg: str) +``` + +Initialize a new instance of the CharmIllegalNumUnitsError exception. + + + +**Args:** + + - `msg`: Explanation of the error. + + + + + --- ## class `CharmRelationDataInvalidError` @@ -190,7 +221,7 @@ Configuration for accessing Jenkins through proxy. --- - + ### classmethod `from_env` @@ -227,7 +258,7 @@ The Jenkins k8s operator charm state. --- - + ### classmethod `from_charm` @@ -254,5 +285,6 @@ Initialize the state from charm. - `CharmConfigInvalidError`: if invalid state values were encountered. - `CharmRelationDataInvalidError`: if invalid relation data was received. + - `CharmIllegalNumUnitsError`: if more than 1 unit of Jenkins charm is deployed. diff --git a/src/charm.py b/src/charm.py index 7987d312..dbe012c7 100755 --- a/src/charm.py +++ b/src/charm.py @@ -15,7 +15,12 @@ import ingress import jenkins import timerange -from state import CharmConfigInvalidError, CharmRelationDataInvalidError, State +from state import ( + CharmConfigInvalidError, + CharmIllegalNumUnitsError, + CharmRelationDataInvalidError, + State, +) if typing.TYPE_CHECKING: from ops.pebble import LayerDict # pragma: no cover @@ -39,7 +44,7 @@ def __init__(self, *args: typing.Any): super().__init__(*args) try: self.state = State.from_charm(self) - except CharmConfigInvalidError as exc: + except (CharmConfigInvalidError, CharmIllegalNumUnitsError) as exc: self.unit.status = ops.BlockedStatus(exc.msg) return except CharmRelationDataInvalidError as exc: diff --git a/src/state.py b/src/state.py index 339a3689..ff558876 100644 --- a/src/state.py +++ b/src/state.py @@ -55,6 +55,22 @@ def __init__(self, msg: str): self.msg = msg +class CharmIllegalNumUnitsError(CharmStateBaseError): + """Represents an error with invalid number of units deployed. + + Attributes: + msg: Explanation of the error. + """ + + def __init__(self, msg: str): + """Initialize a new instance of the CharmIllegalNumUnitsError exception. + + Args: + msg: Explanation of the error. + """ + self.msg = msg + + class AgentMeta(BaseModel): """Metadata for registering Jenkins Agent. @@ -234,6 +250,7 @@ def from_charm(cls, charm: ops.CharmBase) -> "State": Raises: CharmConfigInvalidError: if invalid state values were encountered. CharmRelationDataInvalidError: if invalid relation data was received. + CharmIllegalNumUnitsError: if more than 1 unit of Jenkins charm is deployed. """ time_range_str = charm.config.get("restart-time-range") if time_range_str: @@ -251,24 +268,11 @@ def from_charm(cls, charm: ops.CharmBase) -> "State": agent_relation_meta_map = _get_agent_meta_map_from_relation( charm.model.relations[AGENT_RELATION], charm.app.name ) - except ValidationError as exc: - logger.error( - "Invalid agent relation data received from %s relation, %s", AGENT_RELATION, exc - ) - raise CharmRelationDataInvalidError( - f"Invalid {AGENT_RELATION} relation data." - ) from exc - - try: deprecated_agent_meta_map = _get_agent_meta_map_from_relation( charm.model.relations[DEPRECATED_AGENT_RELATION], charm.app.name ) except ValidationError as exc: - logger.error( - "Invalid agent relation data received from %s relation, %s", - DEPRECATED_AGENT_RELATION, - exc, - ) + logger.error("Invalid agent relation data received, %s", exc) raise CharmRelationDataInvalidError( f"Invalid {DEPRECATED_AGENT_RELATION} relation data." ) from exc @@ -282,6 +286,11 @@ def from_charm(cls, charm: ops.CharmBase) -> "State": plugins_str = charm.config.get("allowed-plugins") plugins = (plugin.strip() for plugin in plugins_str.split(",")) if plugins_str else None + if charm.app.planned_units() > 1: + raise CharmIllegalNumUnitsError( + "The Jenkins charm supports only 1 unit of deployment." + ) + return cls( restart_time_range=restart_time_range, agent_relation_meta=agent_relation_meta_map, diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 36c66f86..3c858f2a 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -13,6 +13,7 @@ import pytest import requests import yaml +from ops.charm import CharmBase from ops.model import Container from ops.pebble import ExecError from ops.testing import Harness @@ -432,3 +433,11 @@ def plugin_groovy_script_result_fixture(): dep-b-b (v0.0.4) => [] """ ) + + +@pytest.fixture(scope="module", name="mock_charm") +def mock_charm_fixture(): + """A valid mock charm.""" + mock_charm = unittest.mock.MagicMock(spec=CharmBase) + mock_charm.app.planned_units.return_value = 1 + return mock_charm diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index 81aad3b4..929a91a5 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -30,13 +30,12 @@ def test_state_invalid_time_config(): pytest.param("", id="empty string"), ], ) -def test_no_time_range_config(time_range: str): +def test_no_time_range_config(time_range: str, mock_charm: unittest.mock.MagicMock): """ arrange: given an empty time range config value. act: when state is instantiated. assert: state without time range is returned. """ - mock_charm = unittest.mock.MagicMock(spec=ops.CharmBase) mock_charm.config = {"restart-time-range": time_range} returned_state = state.State.from_charm(mock_charm) @@ -108,7 +107,9 @@ def test_proxyconfig_none(monkeypatch: pytest.MonkeyPatch): def test_proxyconfig_from_charm_env( - monkeypatch: pytest.MonkeyPatch, proxy_config: state.ProxyConfig + monkeypatch: pytest.MonkeyPatch, + proxy_config: state.ProxyConfig, + mock_charm: unittest.mock.MagicMock, ): """ arrange: given a monkeypatched os.environ with proxy configurations. @@ -124,7 +125,6 @@ def test_proxyconfig_from_charm_env( "JUJU_CHARM_NO_PROXY": str(proxy_config.no_proxy), }, ) - mock_charm = unittest.mock.MagicMock(spec=ops.CharmBase) mock_charm.config = {} config = state.State.from_charm(mock_charm).proxy_config @@ -134,28 +134,38 @@ def test_proxyconfig_from_charm_env( assert config.no_proxy == proxy_config.no_proxy -def test_plugins_config_none(): +def test_plugins_config_none(mock_charm: unittest.mock.MagicMock): """ arrange: given a charm with no plugins config. act: when state is initialized from charm. assert: plugin state is None. """ - mock_charm = unittest.mock.MagicMock(spec=ops.CharmBase) mock_charm.config = {} config = state.State.from_charm(mock_charm) assert config.plugins is None -def test_plugins_config(): +def test_plugins_config(mock_charm: unittest.mock.MagicMock): """ arrange: given a charm with comma separated plugins. act: when state is initialized from charm. assert: plugin state contains an iterable of plugins. """ - mock_charm = unittest.mock.MagicMock(spec=ops.CharmBase) mock_charm.config = {"allowed-plugins": "hello, world"} config = state.State.from_charm(mock_charm) assert config.plugins is not None assert tuple(config.plugins) == ("hello", "world") + + +def test_invalid_num_units(mock_charm: unittest.mock.MagicMock): + """ + arrange: given a mock charm with more than 1 unit of deployment. + act: when state is initialized from charm. + assert: CharmIllegalNumUnitsError is raised. + """ + mock_charm.app.planned_units.return_value = 2 + + with pytest.raises(state.CharmIllegalNumUnitsError): + state.State.from_charm(mock_charm)