From 2a578defbdf8986421eb4d792ec6f369f65a5da6 Mon Sep 17 00:00:00 2001 From: Trung Thanh Phan Date: Mon, 4 Dec 2023 22:09:09 +0100 Subject: [PATCH] cleanup + refactoring --- .gitignore | 2 +- jenkins_agent_snap/commands/agent.start | 83 +++++++++++++++--------- jenkins_agent_snap/hooks/configure | 2 +- jenkins_agent_snap/scripts/management | 2 +- jenkins_agent_snap/snapcraft.yaml | 9 --- requirements.txt | 4 +- src/charm.py | 67 ++------------------ src/charm_state.py | 42 ------------- src/server.py | 34 ---------- src/service.py | 84 +++++++------------------ tests/integration/conftest.py | 11 ---- 11 files changed, 83 insertions(+), 257 deletions(-) diff --git a/.gitignore b/.gitignore index ee35e45..b1253d5 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,4 @@ __pycache__/ *.egg-info/ */*.rock */*.snap -*/*.charm + diff --git a/jenkins_agent_snap/commands/agent.start b/jenkins_agent_snap/commands/agent.start index f8b2088..19e01a7 100755 --- a/jenkins_agent_snap/commands/agent.start +++ b/jenkins_agent_snap/commands/agent.start @@ -3,41 +3,62 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. +err() { + echo "[$(date +'%Y-%m-%dT%H:%M:%S%z')]: $*" >&2 +} + +info() { + echo "[$(date +'%Y-%m-%dT%H:%M:%S%z')]: $*" +} + +stop() { + snapctl stop jenkins-agent + exit 1 +} + set -eu -o pipefail export LC_ALL=C export TERM=xterm . "$SNAP/bin/management" -# defaults for jenkins-agent component of the jenkins continuous integration -# system - -# Setup -echo "JAVA_HOME : $JAVA_HOME" -echo "snap run user: $(whoami)" -WORKDIR=$(pwd) -echo "snap workdir: ${SNAP}, current dir: ${WORKDIR}" -which java || echo "java not found, exiting" -echo "Fetching environment variables" -# URL of jenkins server to connect to -# Not specifying this parameter will stop the agent -# job from running. -jenkins_url="$(jenkins_url)" -typeset JENKINS_URL="${jenkins_url:?"URL of a jenkins server must be provided"}" -# Agent -jenkins_agent="$(jenkins_agent)" -typeset JENKINS_AGENT="${jenkins_agent:?"Jenkins agent name must be provided"}" -# Token -jenkins_token="$(jenkins_token)" -typeset JENKINS_TOKEN="${jenkins_token:?"Jenkins agent token must be provided"}" -# Download the agent.jar -echo "Downloading agent binary" -curl -sO "${JENKINS_URL}/jnlpJars/agent.jar" -# Inspect -ls -la agent.jar + +readonly JENKINS_HOME="/var/lib/jenkins" +if ! mkdir -p $JENKINS_HOME; then + err "Error initializing the agent's home directory" + stop +fi +cd $JENKINS_HOME + +# fetch snap configuration +JENKINS_URL="$(jenkins_url)" +JENKINS_AGENT="$(jenkins_agent)" +JENKINS_TOKEN="$(jenkins_token)" + +if [[ "${JENKINS_URL}" == "unset" || "${JENKINS_AGENT}" == "unset" || "${JENKINS_TOKEN}" == "unset" ]]; then + if [[ "${JENKINS_URL}" == "unset" ]]; then + err "JENKINS_URL needs to be configured" + fi + if [[ "${JENKINS_AGENT}" == "unset" ]]; then + err "JENKINS_AGENT needs to be configured" + fi + if [[ "${JENKINS_TOKEN}" == "unset" ]]; then + err "JENKINS_TOKEN needs to be configured" + fi + err "Invalid configuration, missing value(s)" + stop +fi + +if ! curl "${JENKINS_URL}/jnlpJars/agent.jar" -o ${JENKINS_HOME}/agent.jar; then + err Unable to download agent binary + stop +fi + # Specify the agent as ready -touch $WORKDIR/.ready -# Start Jenkins agent -java -jar agent.jar -jnlpUrl "${JENKINS_URL}/computer/${JENKINS_AGENT}/slave-agent.jnlp" -workDir "${WORKDIR}" -noReconnect -secret "${JENKINS_TOKEN}" || echo "Invalid or already used credentials." -# Remove ready mark if unsuccessful -rm $WORKDIR/.ready +touch $JENKINS_HOME/.ready +if ! java -jar agent.jar -jnlpUrl "${JENKINS_URL}/computer/${JENKINS_AGENT}/slave-agent.jnlp" -workDir "${JENKINS_HOME}" -noReconnect -secret "${JENKINS_TOKEN}"; then + err "Error connecting to jenkins" + # Remove ready mark if unsuccessful + rm $JENKINS_HOME/.ready + stop +fi diff --git a/jenkins_agent_snap/hooks/configure b/jenkins_agent_snap/hooks/configure index a0d4235..705b66f 100644 --- a/jenkins_agent_snap/hooks/configure +++ b/jenkins_agent_snap/hooks/configure @@ -53,4 +53,4 @@ handle_jenkins_token_config() handle_jenkins_url_config handle_jenkins_agent_config -handle_jenkins_token_config \ No newline at end of file +handle_jenkins_token_config diff --git a/jenkins_agent_snap/scripts/management b/jenkins_agent_snap/scripts/management index c1fd566..57d2ce2 100644 --- a/jenkins_agent_snap/scripts/management +++ b/jenkins_agent_snap/scripts/management @@ -40,4 +40,4 @@ jenkins_token() set_jenkins_token() { snapctl set jenkins.token="$1" -} \ No newline at end of file +} diff --git a/jenkins_agent_snap/snapcraft.yaml b/jenkins_agent_snap/snapcraft.yaml index c5551bc..23b6e6c 100644 --- a/jenkins_agent_snap/snapcraft.yaml +++ b/jenkins_agent_snap/snapcraft.yaml @@ -22,9 +22,6 @@ parts: stage-snaps: - openjdk plugin: dump - override-prime: | - snapcraftctl prime - ls -la $SNAPCRAFT_PRIME source: commands organize: '*': bin/ @@ -46,12 +43,6 @@ parts: stage-packages: - curl - git - # stage-snaps: - # - charmcraft - # - juju - # - rockcraft - # - lxd - # - snapcraft stage: - -etc/bash_completion.d - -etc/cron.d diff --git a/requirements.txt b/requirements.txt index aaa16b1..4b47197 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,3 @@ -ops >= 2.2.0 +ops>=2,<3 +requests>=2,<3 +pydantic>=1,<2 diff --git a/src/charm.py b/src/charm.py index e58f5e3..23c8185 100755 --- a/src/charm.py +++ b/src/charm.py @@ -35,79 +35,19 @@ def __init__(self, *args: typing.Any): self.unit.status = ops.BlockedStatus(exc.msg) return - self.pebble_service = service.Service(self.state) self.agent_observer = agent_observer.Observer(self, self.state, self.pebble_service) self.framework.observe(self.on.config_changed, self._on_config_changed) self.framework.observe(self.on.upgrade_charm, self._on_upgrade_charm) - def _register_via_config( - self, event: typing.Union[ops.ConfigChangedEvent, ops.UpgradeCharmEvent] - ) -> None: - """Register the agent to server from configuration values. - - Args: - event: The event fired on config changed or upgrade charm. - - Raises: - RuntimeError: if the Jenkins agent failed to download. - """ - container = self.unit.get_container(self.state.jenkins_agent_service_name) - if not container.can_connect(): - logger.warning("Jenkins agent container not yet ready. Deferring.") - event.defer() - return - - if ( - not self.state.jenkins_config - and not self.model.get_relation(AGENT_RELATION) - ): - self.model.unit.status = ops.BlockedStatus("Waiting for config/relation.") - return - - if not self.state.jenkins_config: - # Support fallback relation to AGENT_RELATION. - self.model.unit.status = ops.BlockedStatus( - "Please remove and re-relate agent relation." - ) - return - - try: - server.download_jenkins_agent( - server_url=self.state.jenkins_config.server_url, - container=container, - ) - except server.AgentJarDownloadError as exc: - logger.error("Failed to download Agent JAR executable, %s", exc) - raise RuntimeError("Failed to download Jenkins agent. Fix issue ") from exc - - valid_agent_token = server.find_valid_credentials( - agent_name_token_pairs=self.state.jenkins_config.agent_name_token_pairs, - server_url=self.state.jenkins_config.server_url, - container=container, - ) - if not valid_agent_token: - logger.error("No valid agent-token pair found.") - self.model.unit.status = ops.BlockedStatus( - "Additional valid agent-token pairs required." - ) - return - - self.model.unit.status = ops.MaintenanceStatus("Starting agent pebble service.") - self.pebble_service.reconcile( - server_url=self.state.jenkins_config.server_url, - agent_token_pair=valid_agent_token, - container=container, - ) - self.model.unit.status = ops.ActiveStatus() - def _on_config_changed(self, event: ops.ConfigChangedEvent) -> None: """Handle config changed event. Args: event: The event fired on configuration change. """ - self._register_via_config(event) + # TODO: implement lifecycle management with snap + self._reconcile(event) def _on_upgrade_charm(self, event: ops.UpgradeCharmEvent) -> None: """Handle upgrade charm event. @@ -115,7 +55,8 @@ def _on_upgrade_charm(self, event: ops.UpgradeCharmEvent) -> None: Args: event: The event fired on upgrade charm. """ - self._register_via_config(event) + # TODO: implement lifecycle management with snap + self._reconcile(event) if __name__ == "__main__": # pragma: no cover diff --git a/src/charm_state.py b/src/charm_state.py index 38ce682..74a0d7a 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -36,40 +36,6 @@ def __init__(self, msg: str = ""): self.msg = msg -class JenkinsConfig(BaseModel): - """The Jenkins config from juju config values. - - Attrs: - server_url: The Jenkins server url. - agent_name_token_pairs: Jenkins agent names paired with corresponding token value. - """ - - server_url: str = Field(..., min_length=1) - agent_name_token_pairs: typing.List[typing.Tuple[str, str]] = Field(..., min_items=1) - - @classmethod - def from_charm_config(cls, config: ops.ConfigData) -> typing.Optional["JenkinsConfig"]: - """Instantiate JenkinsConfig from charm config. - - Args: - config: Charm configuration data. - - Returns: - JenkinsConfig if configuration exists, None otherwise. - """ - server_url = config.get("jenkins_url") - agent_name_config = config.get("jenkins_agent_name") - agent_token_config = config.get("jenkins_agent_token") - # None represents an unset Jenkins configuration values, meaning configuration values from - # relation would be used. - if not server_url and not agent_name_config and not agent_token_config: - return None - agent_names = agent_name_config.split(":") if agent_name_config else [] - agent_tokens = agent_token_config.split(":") if agent_token_config else [] - agent_name_token_pairs = list(zip(agent_names, agent_tokens)) - return cls(server_url=server_url or "", agent_name_token_pairs=agent_name_token_pairs) - - def _get_jenkins_unit( all_units: typing.Set[ops.Unit], current_app_name: str ) -> typing.Optional[ops.Unit]: @@ -123,7 +89,6 @@ class State: """ agent_meta: metadata.Agent - jenkins_config: typing.Optional[JenkinsConfig] agent_relation_credentials: typing.Optional[server.Credentials] jenkins_agent_service_name: str = "jenkins-agent" @@ -150,12 +115,6 @@ def from_charm(cls, charm: ops.CharmBase) -> "State": logging.error("Invalid executor state, %s", exc) raise InvalidStateError("Invalid executor state.") from exc - try: - jenkins_config = JenkinsConfig.from_charm_config(charm.config) - except ValidationError as exc: - logging.error("Invalid jenkins config values, %s", exc) - raise InvalidStateError("Invalid jenkins config values.") from exc - agent_relation = charm.model.get_relation(AGENT_RELATION) agent_relation_credentials: typing.Optional[server.Credentials] = None if agent_relation and ( @@ -167,6 +126,5 @@ def from_charm(cls, charm: ops.CharmBase) -> "State": return cls( agent_meta=agent_meta, - jenkins_config=jenkins_config, agent_relation_credentials=agent_relation_credentials, ) diff --git a/src/server.py b/src/server.py index 4d7bc58..0c43201 100644 --- a/src/server.py +++ b/src/server.py @@ -4,13 +4,10 @@ """Functions to interact with jenkins server.""" import logging -import random -import time import typing from pathlib import Path import ops -import requests from pydantic import BaseModel logger = logging.getLogger(__name__) @@ -39,37 +36,10 @@ class ServerBaseError(Exception): """Represents errors with interacting with Jenkins server.""" -class AgentJarDownloadError(ServerBaseError): - """Represents an error downloading agent JAR executable.""" - - -def download_jenkins_agent(server_url: str, container: ops.Container) -> None: - """Download Jenkins agent JAR executable from server. - - Args: - server_url: The Jenkins server URL address. - container: The agent workload container. - - Raises: - AgentJarDownloadError: If an error occurred downloading the JAR executable. - """ - try: - res = requests.get(f"{server_url}/jnlpJars/agent.jar", timeout=300) - res.raise_for_status() - except (requests.HTTPError, requests.Timeout, requests.ConnectionError) as exc: - logger.error("Failed to download agent JAR executable from server, %s", exc) - raise AgentJarDownloadError( - "Failed to download agent JAR executable from server." - ) from exc - - container.push(path=AGENT_JAR_PATH, make_dirs=True, source=res.content, user=USER) - - def validate_credentials( agent_name: str, credentials: Credentials, container: ops.Container, - add_random_delay: bool = False, ) -> bool: """Check if the credentials can be used to register to the server. @@ -83,10 +53,6 @@ def validate_credentials( Returns: True if credentials and agent_name pairs are valid, False otherwise. """ - # IMPORTANT: add random delay to prevent parallel execution. - if add_random_delay: - # It's okay to use random since it's not used for sensitive data. - time.sleep(random.random()) # nosec proc: ops.pebble.ExecProcess = container.exec( [ "java", diff --git a/src/service.py b/src/service.py index 8e43db4..f8f591b 100644 --- a/src/service.py +++ b/src/service.py @@ -5,81 +5,39 @@ import logging import typing - -import ops - -import server from charm_state import State +from charms.operator_libs_linux.v2 import snap + logger = logging.getLogger(__name__) +SNAP_NAME = "jenkins-agent" -class Service: - """The charm pebble service manager.""" +class JenkinsAgentService: + """Jenkins agent service class.""" def __init__(self, state: State): - """Initialize the pebble service. + """Initialize the jenkins agent service. Args: state: The Jenkins agent state. """ self.state = state - def _get_pebble_layer( - self, server_url: str, agent_token_pair: typing.Tuple[str, str] - ) -> ops.pebble.Layer: - """Return a dictionary representing a Pebble layer. - - Args: - server_url: The Jenkins server address. - agent_token_pair: Matching pair of agent name to agent token. - - Returns: - The pebble layer defining Jenkins service layer. - """ - layer: ops.pebble.LayerDict = { - "summary": "Jenkins agent layer", - "description": "pebble config layer for Jenkins agent.", - "services": { - self.state.jenkins_agent_service_name: { - "override": "replace", - "summary": "Jenkins agent", - "command": str(server.ENTRYSCRIPT_PATH), - "environment": { - "JENKINS_URL": server_url, - "JENKINS_AGENT": agent_token_pair[0], - "JENKINS_TOKEN": agent_token_pair[1], - }, - "startup": "enabled", - "user": server.USER, - }, - }, - } - return ops.pebble.Layer(layer) - - def reconcile( - self, server_url: str, agent_token_pair: typing.Tuple[str, str], container: ops.Container - ) -> None: - """Reconcile the Jenkins agent service. - - Args: - server_url: The Jenkins server address. - agent_token_pair: Matching pair of agent name to agent token. - container: The agent workload container. - """ - agent_layer = self._get_pebble_layer( - server_url=server_url, agent_token_pair=agent_token_pair - ) - container.add_layer( - label=self.state.jenkins_agent_service_name, layer=agent_layer, combine=True + def start(self, server_url: str, agent_token_pair: typing.Tuple[str, str]): + cache = snap.SnapCache() + agent = cache[SNAP_NAME] + agent_name, agent_token = agent_token_pair + agent.set( + { + "jenkins.token": agent_token, + "jenkins.url": server_url, + "jenkins.agent": agent_name, + } ) - container.replan() + agent.start() - def stop_agent(self, container: ops.Container) -> None: - """Stop Jenkins agent. - - Args: - container: The agent workload container. - """ - container.stop(self.state.jenkins_agent_service_name) - container.remove_path(str(server.AGENT_READY_PATH)) + def stop(self): + cache = snap.SnapCache() + agent = cache[SNAP_NAME] + agent.stop() diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index c4bc277..814bd10 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -39,17 +39,6 @@ def model_fixture(ops_test: OpsTest) -> Model: return ops_test.model -@pytest.fixture(scope="module", name="agent_image") -def agent_image_fixture(request: pytest.FixtureRequest) -> str: - """The OCI image for jenkins-agent charm.""" - agent_k8s_image = request.config.getoption("--jenkins-agent-image") - assert agent_k8s_image, ( - "--jenkins-agent-image argument is required which should contain the name of the OCI " - "image." - ) - return agent_k8s_image - - @pytest.fixture(scope="module", name="num_agents") def num_agents_fixture() -> int: """The number of agents to deploy."""