Add version upgrade test (#113)

* add version upgrade test * add version upgrade test * add version upgrade test * add version upgrade test * add version upgrade test * address comments * address comments * address comments * address comments * pin juju to fix CI * address comments * address comments * address comments * test possible CI failure * test CI * enable tmate debug * address comments * test CI * test CI * test CI * test CI * test CI * test CI * test CI * test CI * test CI with libjuju 3.1 * test CI with libjuju 3.1 * try deferring as possible fix for CI * storage mount fix attempt * storage mount fix attempt * fix linting * address comment * address comment * address comment * fix jenkins-agent test fails * fix jenkins-agent test fails * Include temporary for issue 989 in pylibjuju to see if that addresses test failures * Revert to pylibjuju version in main --------- Co-authored-by: Phan Trung Thanh <[email protected]> Co-authored-by: Tom Haddon <[email protected]>
canonical · Mar 21, 2024 · 866d05a · 866d05a
1 parent cc59cac
commit 866d05a
Show file tree

Hide file tree

Showing 8 changed files with 237 additions and 18 deletions.
diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml
@@ -11,7 +11,7 @@ jobs:
       channel: 1.28-strict/stable
       extra-arguments: |
         --kube-config ${GITHUB_WORKSPACE}/kube-config
-      modules: '["test_cos.py", "test_ingress.py", "test_jenkins.py", "test_k8s_agent.py", "test_machine_agent.py", "test_plugins.py", "test_proxy.py"]'
+      modules: '["test_ingress.py", "test_jenkins.py", "test_k8s_agent.py", "test_machine_agent.py", "test_plugins.py", "test_proxy.py", "test_cos.py", "test_upgrade.py"]'
       pre-run-script: |
         -c "sudo microk8s config > ${GITHUB_WORKSPACE}/kube-config
         chmod +x tests/integration/pre_run_script.sh

diff --git a/.trivyignore b/.trivyignore
@@ -1,5 +1,7 @@
 # Jenkins CVEs
 CVE-2016-1000027
+CVE-2024-22259
+CVE-2024-22257
 # Jenkins Plugin Manager CVEs
 CVE-2023-5072
 GHSA-4jq9-2xhw-jpx7
@@ -10,4 +12,4 @@ CVE-2024-26308
 # https://github.com/jenkinsci/jenkins/pull/8696
 # Fixed in 5.3.32
 CVE-2024-22243
-CVE-2024-22201
+CVE-2024-22201
diff --git a/src/charm.py b/src/charm.py
@@ -69,6 +69,7 @@ def __init__(self, *args: typing.Any):
         )
         self.framework.observe(self.on.jenkins_pebble_ready, self._on_jenkins_pebble_ready)
         self.framework.observe(self.on.update_status, self._on_update_status)
+        self.framework.observe(self.on.upgrade_charm, self._upgrade_charm)
 
     def _get_pebble_layer(self) -> ops.pebble.Layer:
         """Return a dictionary representing a Pebble layer.
@@ -213,7 +214,27 @@ def _on_jenkins_home_storage_attached(self, event: ops.StorageAttachedEvent) ->
         Args:
             event: The event fired when the storage is attached.
         """
+        self.jenkins_set_storage_config(event)
+
+    def _upgrade_charm(self, event: ops.UpgradeCharmEvent) -> None:
+        """Correctly set permissions when charm is upgraded.
+
+        Args:
+            event: The event fired when the charm is upgraded.
+        """
+        container = self.unit.get_container(JENKINS_SERVICE_NAME)
+        if not jenkins.is_storage_ready(container):
+            self.jenkins_set_storage_config(event)
+
+    def jenkins_set_storage_config(self, event: ops.framework.EventBase) -> None:
+        """Correctly set permissions when storage is attached.
+
+        Args:
+            event: The event fired when the permission change is needed.
+        """
         container = self.unit.get_container(JENKINS_SERVICE_NAME)
+        container_meta = self.framework.meta.containers["jenkins"]
+        storage_path = container_meta.mounts["jenkins-home"].location
         if not container.can_connect():
             self.unit.status = ops.WaitingStatus("Waiting for pebble.")
             # This event should be handled again once the container becomes available.
@@ -224,7 +245,7 @@ def _on_jenkins_home_storage_attached(self, event: ops.StorageAttachedEvent) ->
             "chown",
             "-R",
             f"{jenkins.USER}:{jenkins.GROUP}",
-            str(event.storage.location.resolve()),
+            str(storage_path),
         ]
 
         container.exec(

diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
@@ -25,11 +25,10 @@
 from pytest import FixtureRequest
 from pytest_operator.plugin import OpsTest
 
-import jenkins
 import state
 
 from .constants import ALLOWED_PLUGINS
-from .helpers import get_pod_ip
+from .helpers import generate_jenkins_client_from_application, get_pod_ip
 from .types_ import KeycloakOIDCMetadata, LDAPSettings, ModelAppUnit, UnitWebClient
 
 KUBECONFIG = os.environ.get("TESTING_KUBECONFIG", "~/.kube/config")
@@ -136,17 +135,10 @@ async def jenkins_client_fixture(
     web_address: str,
 ) -> jenkinsapi.jenkins.Jenkins:
     """The Jenkins API client."""
-    jenkins_unit: Unit = application.units[0]
-    ret, api_token, stderr = await ops_test.juju(
-        "ssh",
-        "--container",
-        "jenkins",
-        jenkins_unit.name,
-        "cat",
-        str(jenkins.API_TOKEN_PATH),
+    jenkins_client = await generate_jenkins_client_from_application(
+        ops_test, application, web_address
     )
-    assert ret == 0, f"Failed to get Jenkins API token, {stderr}"
-    return jenkinsapi.jenkins.Jenkins(web_address, "admin", api_token, timeout=60)
+    return jenkins_client
 
 
 @pytest_asyncio.fixture(scope="function", name="jenkins_user_client")
@@ -184,6 +176,7 @@ async def jenkins_k8s_agents_fixture(
     """The Jenkins k8s agent."""
     agent_app: Application = await model.deploy(
         "jenkins-agent-k8s",
+        base="[email protected]",
         config={"jenkins_agent_labels": "k8s"},
         channel="latest/edge",
         application_name=f"jenkins-agent-k8s-{app_suffix}",
@@ -215,6 +208,7 @@ async def extra_jenkins_k8s_agents_fixture(
     """The Jenkins k8s agent."""
     agent_app: Application = await model.deploy(
         "jenkins-agent-k8s",
+        base="[email protected]",
         config={"jenkins_agent_labels": "k8s-extra"},
         channel="latest/edge",
         application_name="jenkins-agent-k8s-extra",

diff --git a/tests/integration/constants.py b/tests/integration/constants.py
@@ -6,3 +6,20 @@
 ALLOWED_PLUGINS = ("git", "blueocean", "openid")
 INSTALLED_PLUGINS = ("git", "timestamper", "blueocean", "openid")
 REMOVED_PLUGINS = set(INSTALLED_PLUGINS) - set(ALLOWED_PLUGINS)
+ALL_PLUGINS = [
+    "bazaar",
+    "blueocean",
+    "dependency-check-jenkins-plugin",
+    "docker-build-publish",
+    "git",
+    "kubernetes",
+    "ldap",
+    "matrix-combinations-parameter",
+    "oic-auth",
+    "openid",
+    "pipeline-groovy-lib",
+    "postbuildscript",
+    "rebuild",
+    "ssh-agent",
+    "thinBackup",
+]
diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py
@@ -12,8 +12,10 @@
 import jenkinsapi.jenkins
 import kubernetes.client
 import requests
+from juju.application import Application
 from juju.model import Model
 from juju.unit import Unit
+from pytest_operator.plugin import OpsTest
 
 import jenkins
 
@@ -49,7 +51,7 @@ async def install_plugins(
             client.requester.post_url(f"{web}/manage/pluginManager/updates/body").content,
             encoding="utf-8",
         ),
-        timeout=60 * 5,
+        timeout=60 * 10,
         wait_period=10,
     )
 
@@ -58,11 +60,27 @@ async def install_plugins(
     client.safe_restart()
     await unit.model.block_until(
         lambda: requests.get(web, timeout=10).status_code == 403,
-        timeout=300,
+        timeout=60 * 10,
         wait_period=10,
     )
 
 
+async def get_model_jenkins_unit_address(model: Model, app_name: str):
+    """Extract the address of a given unit.
+
+    Args:
+        model: Juju model
+        app_name: Juju application name
+
+    Returns:
+        the IP address of the Jenkins unit.
+    """
+    status = await model.get_status()
+    unit = list(status.applications[app_name].units)[0]
+    address = status["applications"][app_name]["units"][unit]["address"]
+    return address
+
+
 def gen_test_job_xml(node_label: str):
     """Generate a job xml with target node label.
 
@@ -251,6 +269,54 @@ async def wait_for(
     raise TimeoutError()
 
 
+async def generate_jenkins_client_from_application(
+    ops_test: OpsTest, jenkins_app: Application, address: str
+):
+    """Generate a Jenkins client directly from the Juju application.
+
+    Args:
+        ops_test: OpsTest framework
+        jenkins_app: Juju Jenkins-k8s application.
+        address: IP address of the jenkins unit.
+
+    Returns:
+        A Jenkins web client.
+    """
+    jenkins_unit = jenkins_app.units[0]
+    ret, api_token, stderr = await ops_test.juju(
+        "ssh",
+        "--container",
+        "jenkins",
+        jenkins_unit.name,
+        "cat",
+        str(jenkins.API_TOKEN_PATH),
+    )
+    assert ret == 0, f"Failed to get Jenkins API token, {stderr}"
+    return jenkinsapi.jenkins.Jenkins(address, "admin", api_token, timeout=60)
+
+
+async def generate_unit_web_client_from_application(
+    ops_test: OpsTest, model: Model, jenkins_app: Application
+) -> UnitWebClient:
+    """Generate a UnitWebClient client directly from the Juju application.
+
+    Args:
+        ops_test: OpsTest framework
+        model: Juju model
+        jenkins_app: Juju Jenkins-k8s application.
+
+    Returns:
+        A Jenkins web client.
+    """
+    assert model
+    unit_ip = await get_model_jenkins_unit_address(model, jenkins_app.name)
+    address = f"http://{unit_ip}:8080"
+    jenkins_unit = jenkins_app.units[0]
+    jenkins_client = await generate_jenkins_client_from_application(ops_test, jenkins_app, address)
+    unit_web_client = UnitWebClient(unit=jenkins_unit, web=address, client=jenkins_client)
+    return unit_web_client
+
+
 def get_job_invoked_unit(job: jenkins.jenkinsapi.job.Job, units: typing.List[Unit]) -> Unit | None:
     """Get the jenkins unit that has run the latest job.
 

diff --git a/tests/integration/test_upgrade.py b/tests/integration/test_upgrade.py
@@ -0,0 +1,73 @@
+# Copyright 2024 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Integration test relation file."""
+
+
+import logging
+
+import ops
+import pytest
+import pytest_asyncio
+import requests
+from juju.application import Application
+from juju.model import Model
+from pytest_operator.plugin import OpsTest
+
+from .helpers import (
+    gen_git_test_job_xml,
+    generate_unit_web_client_from_application,
+    get_model_jenkins_unit_address,
+)
+
+LOGGER = logging.getLogger(__name__)
+JENKINS_APP_NAME = "jenkins-k8s-upgrade"
+JOB_NAME = "test_job"
+
+
+@pytest_asyncio.fixture(scope="module")
+async def jenkins_upgrade_depl(ops_test: OpsTest, model: Model):
+    """
+    arrange: given a juju model.
+
+    act: deploy Jenkins, instantiate the Jenkins client and define a job.
+
+    assert: the deployment has no errors.
+    """
+    application: Application = await model.deploy(
+        "jenkins-k8s",
+        application_name=JENKINS_APP_NAME,
+        channel="stable",
+    )
+    await model.wait_for_idle(status="active", timeout=10 * 60)
+    unit_web_client = await generate_unit_web_client_from_application(ops_test, model, application)
+    unit_web_client.client.create_job(JOB_NAME, gen_git_test_job_xml("k8s"))
+
+
+@pytest.mark.usefixtures("jenkins_upgrade_depl")
+async def test_jenkins_upgrade_check_job(
+    ops_test: OpsTest, jenkins_image: str, model: Model, charm: ops.CharmBase
+):
+    """
+    arrange: given charm has been built, deployed and a job has been defined.
+
+    act: get Jenkins' version and upgrade the charm.
+
+    assert: if Jenkins versions differ, the job persists.
+    """
+    application = model.applications[JENKINS_APP_NAME]
+    unit_ip = await get_model_jenkins_unit_address(model, JENKINS_APP_NAME)
+    address = f"http://{unit_ip}:8080"
+    response = requests.get(address, timeout=60)
+    old_version = response.headers["X-Jenkins"]
+    await application.refresh(path=charm, resources={"jenkins-image": jenkins_image})
+    await model.wait_for_idle(status="active", timeout=10 * 60)
+    unit_ip = await get_model_jenkins_unit_address(model, JENKINS_APP_NAME)
+    address = f"http://{unit_ip}:8080"
+    response = requests.get(address, timeout=60)
+    if old_version != response.headers["X-Jenkins"]:
+        unit_web_client = await generate_unit_web_client_from_application(
+            ops_test, model, application
+        )
+        job = unit_web_client.client.get_job(JOB_NAME)
+        assert job.name == JOB_NAME
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
@@ -345,14 +345,60 @@ def test__on_jenkins_home_storage_attached(harness: Harness, monkeypatch: pytest
 
     event = MagicMock()
     mock_jenkins_home_path = "/var/lib/jenkins"
-    event.storage.location.resolve = lambda: mock_jenkins_home_path
     jenkins_charm._on_jenkins_home_storage_attached(event)
 
     exec_handler.assert_called_once_with(
         ["chown", "-R", "jenkins:jenkins", mock_jenkins_home_path], timeout=120
     )
 
 
+def test_upgrade_charm(harness: Harness, monkeypatch: pytest.MonkeyPatch):
+    """
+    arrange: given a base jenkins charm.
+    act: when _upgrade_charm is called.
+    assert: The chown command was ran on the jenkins container with correct parameters.
+    """
+    harness.begin()
+    jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm)
+    container = jenkins_charm.unit.containers["jenkins"]
+    harness.set_can_connect(container, True)
+    # We don't use harness.handle_exec here because we want to assert
+    # the parameters passed to exec()
+    exec_handler = MagicMock()
+    monkeypatch.setattr(container, "exec", exec_handler)
+    monkeypatch.setattr(jenkins, "is_storage_ready", lambda x: False)
+
+    event = MagicMock()
+    mock_jenkins_home_path = "/var/lib/jenkins"
+    jenkins_charm._upgrade_charm(event)
+
+    exec_handler.assert_called_once_with(
+        ["chown", "-R", "jenkins:jenkins", mock_jenkins_home_path], timeout=120
+    )
+
+
+def test_upgrade_charm_storage_ready(harness: Harness, monkeypatch: pytest.MonkeyPatch):
+    """
+    arrange: given a base jenkins charm.
+    act: when _upgrade_charm is called.
+    assert: The chown command was not ran.
+    """
+    harness.begin()
+    jenkins_charm = typing.cast(JenkinsK8sOperatorCharm, harness.charm)
+    container = jenkins_charm.unit.containers["jenkins"]
+    harness.set_can_connect(container, True)
+    # We don't use harness.handle_exec here because we want to assert
+    # the parameters passed to exec()
+    exec_handler = MagicMock()
+    monkeypatch.setattr(container, "exec", exec_handler)
+    monkeypatch.setattr(jenkins, "is_storage_ready", lambda x: True)
+
+    event = MagicMock()
+    jenkins_charm._upgrade_charm(event)
+
+    exec_handler.assert_not_called()
+
+
 def test__on_jenkins_home_storage_attached_container_not_ready(
     harness: Harness, monkeypatch: pytest.MonkeyPatch
 ):