Skip to content

Commit

Permalink
feat: limit num units deployment to 1 (#50)
Browse files Browse the repository at this point in the history
Co-authored-by: arturo-seijas <[email protected]>
  • Loading branch information
yanksyoon and arturo-seijas authored Nov 16, 2023
1 parent 805cf53 commit a1f3cb0
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src-docs/charm.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Charm Jenkins.
## <kbd>class</kbd> `JenkinsK8sOperatorCharm`
Charm Jenkins.

<a href="../src/charm.py#L30"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/charm.py#L35"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>function</kbd> `__init__`

Expand Down
42 changes: 37 additions & 5 deletions src-docs/state.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Metadata for registering Jenkins Agent.

---

<a href="../src/state.py#L103"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/state.py#L119"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>classmethod</kbd> `from_agent_relation`

Expand All @@ -54,7 +54,7 @@ Instantiate AgentMeta from charm relation databag.

---

<a href="../src/state.py#L84"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/state.py#L100"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>classmethod</kbd> `from_deprecated_agent_relation`

Expand All @@ -79,7 +79,7 @@ Instantiate AgentMeta from charm relation databag.

---

<a href="../src/state.py#L71"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/state.py#L87"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>classmethod</kbd> `numeric_executors`

Expand Down Expand Up @@ -132,6 +132,37 @@ Initialize a new instance of the CharmConfigInvalidError exception.



---

## <kbd>class</kbd> `CharmIllegalNumUnitsError`
Represents an error with invalid number of units deployed.



**Attributes:**

- <b>`msg`</b>: Explanation of the error.

<a href="../src/state.py#L65"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>function</kbd> `__init__`

```python
__init__(msg: str)
```

Initialize a new instance of the CharmIllegalNumUnitsError exception.



**Args:**

- <b>`msg`</b>: Explanation of the error.





---

## <kbd>class</kbd> `CharmRelationDataInvalidError`
Expand Down Expand Up @@ -190,7 +221,7 @@ Configuration for accessing Jenkins through proxy.

---

<a href="../src/state.py#L183"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/state.py#L199"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>classmethod</kbd> `from_env`

Expand Down Expand Up @@ -227,7 +258,7 @@ The Jenkins k8s operator charm state.

---

<a href="../src/state.py#L224"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/state.py#L240"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>classmethod</kbd> `from_charm`

Expand All @@ -254,5 +285,6 @@ Initialize the state from charm.

- <b>`CharmConfigInvalidError`</b>: if invalid state values were encountered.
- <b>`CharmRelationDataInvalidError`</b>: if invalid relation data was received.
- <b>`CharmIllegalNumUnitsError`</b>: if more than 1 unit of Jenkins charm is deployed.


9 changes: 7 additions & 2 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
37 changes: 23 additions & 14 deletions src/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
26 changes: 18 additions & 8 deletions tests/unit/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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)

0 comments on commit a1f3cb0

Please sign in to comment.