Skip to content

Commit

Permalink
fix: prevent hooks before preconditions ready (#75)
Browse files Browse the repository at this point in the history
* fix: prevent hooks before preconditions ready

* fix: check /proc/mounts for storage mount validation

* chore: refactor storage check to state

* fix: circular imports
  • Loading branch information
yanksyoon committed Dec 5, 2023
1 parent 6978123 commit 8b10218
Show file tree
Hide file tree
Showing 15 changed files with 202 additions and 132 deletions.
3 changes: 3 additions & 0 deletions src-docs/actions.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
# <kbd>module</kbd> `actions.py`
Jenkins charm actions.

**Global Variables**
---------------
- **JENKINS_SERVICE_NAME**


---
Expand Down
1 change: 1 addition & 0 deletions src-docs/agent.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ The Jenkins agent relation observer.
---------------
- **AGENT_RELATION**
- **DEPRECATED_AGENT_RELATION**
- **JENKINS_SERVICE_NAME**


---
Expand Down
5 changes: 4 additions & 1 deletion src-docs/charm.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@
# <kbd>module</kbd> `charm.py`
Charm Jenkins.

**Global Variables**
---------------
- **JENKINS_SERVICE_NAME**


---

## <kbd>class</kbd> `JenkinsK8sOperatorCharm`
Charm Jenkins.

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

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

Expand Down
20 changes: 11 additions & 9 deletions src-docs/state.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Jenkins States.
---------------
- **AGENT_RELATION**
- **DEPRECATED_AGENT_RELATION**
- **JENKINS_SERVICE_NAME**
- **JENKINS_HOME_STORAGE_NAME**


---
Expand All @@ -29,7 +31,7 @@ Metadata for registering Jenkins Agent.

---

<a href="../src/state.py#L119"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/state.py#L123"><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 +56,7 @@ Instantiate AgentMeta from charm relation databag.

---

<a href="../src/state.py#L100"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/state.py#L104"><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 +81,7 @@ Instantiate AgentMeta from charm relation databag.

---

<a href="../src/state.py#L87"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/state.py#L91"><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 @@ -112,7 +114,7 @@ Exception raised when a charm configuration is found to be invalid.

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

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

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

Expand Down Expand Up @@ -143,7 +145,7 @@ Represents an error with invalid number of units deployed.

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

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

Expand Down Expand Up @@ -174,7 +176,7 @@ Represents an error with invalid data in relation data.

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

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

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

Expand Down Expand Up @@ -221,7 +223,7 @@ Configuration for accessing Jenkins through proxy.

---

<a href="../src/state.py#L199"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/state.py#L203"><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 @@ -249,16 +251,16 @@ The Jenkins k8s operator charm state.
- <b>`restart_time_range`</b>: Time range to allow Jenkins to update version.
- <b>`agent_relation_meta`</b>: Metadata of all agents from units related through agent relation.
- <b>`deprecated_agent_relation_meta`</b>: Metadata of all agents from units related through deprecated agent relation.
- <b>`is_storage_ready`</b>: Whether the Jenkins home storage is mounted.
- <b>`proxy_config`</b>: Proxy configuration to access Jenkins upstream through.
- <b>`plugins`</b>: The list of allowed plugins to install.
- <b>`jenkins_service_name`</b>: The Jenkins service name. Note that the container name is the same.




---

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

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

Expand Down
14 changes: 7 additions & 7 deletions src/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import ops

import jenkins
from state import State
from state import JENKINS_SERVICE_NAME, State


class Observer(ops.Object):
Expand Down Expand Up @@ -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})
Expand All @@ -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)
Expand Down
22 changes: 9 additions & 13 deletions src/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
19 changes: 11 additions & 8 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import jenkins
import timerange
from state import (
JENKINS_SERVICE_NAME,
CharmConfigInvalidError,
CharmIllegalNumUnitsError,
CharmRelationDataInvalidError,
Expand Down Expand Up @@ -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} "
Expand Down Expand Up @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand All @@ -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 = [
Expand Down
20 changes: 10 additions & 10 deletions src/jenkins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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:
Expand Down
25 changes: 23 additions & 2 deletions src/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import logging
import os
import typing
from pathlib import Path

import ops
from pydantic import BaseModel, Field, HttpUrl, ValidationError, validator
Expand All @@ -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):
Expand Down Expand Up @@ -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.
Expand All @@ -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]
Expand All @@ -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":
Expand Down Expand Up @@ -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,
)
Loading

0 comments on commit 8b10218

Please sign in to comment.