From d12a0d3e83332d10b4354f58c6a14c1dd8b6071a Mon Sep 17 00:00:00 2001 From: javierdelapuente Date: Tue, 30 Apr 2024 16:38:55 +0200 Subject: [PATCH] Integration test for upgrades (#239) --- .github/workflows/integration_test.yaml | 2 +- discourse_rock/patches/sigterm.patch | 28 ++++++++ discourse_rock/rockcraft.yaml | 8 +++ src/charm.py | 8 +-- tests/integration/conftest.py | 6 +- tests/integration/test_charm.py | 88 ++++++++++++++++++++++++- 6 files changed, 128 insertions(+), 12 deletions(-) create mode 100644 discourse_rock/patches/sigterm.patch diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index a8450787..489bd5a0 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -8,7 +8,7 @@ jobs: uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit with: - extra-arguments: --localstack-address 172.17.0.1 + extra-arguments: -x --localstack-address 172.17.0.1 pre-run-script: localstack-installation.sh trivy-image-config: "trivy.yaml" juju-channel: 3.1/stable diff --git a/discourse_rock/patches/sigterm.patch b/discourse_rock/patches/sigterm.patch new file mode 100644 index 00000000..7ddbdf19 --- /dev/null +++ b/discourse_rock/patches/sigterm.patch @@ -0,0 +1,28 @@ +--- a/config/unicorn.conf.rb ++++ b/config/unicorn.conf.rb +@@ -52,6 +52,15 @@ check_client_connection false + + initialized = false + before_fork do |server, worker| ++ Signal.trap 'TERM' do ++ puts 'Unicorn master intercepting TERM and sending myself QUIT after 5s instead' ++ Thread.new do ++ sleep 15 ++ puts 'Send QUIT signal to master' ++ Process.kill 'QUIT', Process.pid ++ end ++ end ++ + unless initialized + Discourse.preload_rails! + +@@ -266,6 +275,9 @@ before_fork do |server, worker| + end + + after_fork do |server, worker| ++ Signal.trap 'TERM' do ++ puts 'Unicorn worker intercepting TERM and doing nothing. Wait for master to sent QUIT' ++ end + DiscourseEvent.trigger(:web_fork_started) + Discourse.after_fork + end diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 514a3246..054c01ed 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -187,6 +187,7 @@ parts: override-stage: | git -C srv/discourse/app apply patches/lp1903695.patch git -C srv/discourse/app apply patches/anonymize_user.patch + git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation # https://github.com/lautis/uglifier/issues/127#issuecomment-352224986 sed -i 's/config.assets.js_compressor = :uglifier/config.assets.js_compressor = Uglifier.new(:harmony => true)/g' srv/discourse/app/config/environments/production.rb @@ -267,3 +268,10 @@ parts: chown -R 584792:584792 var/lib/pebble/default/.npm chown -R 584792:584792 var/lib/pebble/default/.yarn chown 584792:584792 var/lib/pebble/default/.yarnrc +checks: + discourse-setup-completed: + override: replace + level: ready + threshold: 1 + exec: + command: ls /run/discourse-k8s-operator/setup_completed diff --git a/src/charm.py b/src/charm.py index 639606cd..92f44fac 100755 --- a/src/charm.py +++ b/src/charm.py @@ -487,6 +487,7 @@ def _create_layer_config(self) -> ops.pebble.LayerDict: "user": CONTAINER_APP_USERNAME, "startup": "enabled", "environment": self._create_discourse_environment_settings(), + "kill-delay": "20s", } }, "checks": { @@ -495,13 +496,6 @@ def _create_layer_config(self) -> ops.pebble.LayerDict: "level": "ready", "http": {"url": f"http://localhost:{SERVICE_PORT}/srv/status"}, }, - "discourse-setup-completed": { - "override": "replace", - "level": "ready", - "exec": { - "command": f"ls {SETUP_COMPLETED_FLAG_FILE}", - }, - }, }, } return typing.cast(ops.pebble.LayerDict, layer_config) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 9d435e02..959e32af 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -152,7 +152,8 @@ async def app_fixture( trust=True, config={"profile": "testing"}, ) - await model.wait_for_idle(apps=[postgres_app.name], status="active") + async with ops_test.fast_forward(): + await model.wait_for_idle(apps=[postgres_app.name], status="active") redis_app = await model.deploy("redis-k8s", series="jammy", channel="latest/edge") await model.wait_for_idle(apps=[redis_app.name], status="active") @@ -234,7 +235,7 @@ async def setup_saml_config(app: Application, model: Model): @pytest_asyncio.fixture(scope="module", name="admin_credentials") async def admin_credentials_fixture(app: Application) -> types.Credentials: """Admin user credentials.""" - email = "admin-user@test.internal" + email = f"admin-user{secrets.randbits(32)}@test.internal" password = secrets.token_urlsafe(16) discourse_unit: Unit = app.units[0] action: Action = await discourse_unit.run_action( @@ -277,6 +278,7 @@ async def admin_api_key_fixture( ) # pylint doesn't see the "ok" member assert res.status_code == requests.codes.ok, res.text # pylint: disable=no-member + assert "error" not in res.json() # Create global key res = sess.post( f"{discourse_address}/admin/api/keys", diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index b242deab..b8f4f6d9 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -7,6 +7,8 @@ import re import socket import unittest.mock +from datetime import datetime, timedelta +from pathlib import Path from typing import Dict import pytest @@ -14,8 +16,9 @@ import urllib3.exceptions from boto3 import client from botocore.config import Config -from ops.model import ActiveStatus, Application -from pytest_operator.plugin import Model +from juju.application import Application +from ops.model import ActiveStatus +from pytest_operator.plugin import Model, OpsTest from saml_test_helper import SamlK8sTestHelper # pylint: disable=import-error from charm import PROMETHEUS_PORT @@ -379,3 +382,84 @@ def test_discourse_srv_status_ok(): await model.add_relation(app.name, "nginx-ingress-integrator") await model.wait_for_idle(status="active") test_discourse_srv_status_ok() + + +async def test_upgrade( + app: Application, + model: Model, + pytestconfig: Config, + ops_test: OpsTest, +): + """ + arrange: Given discourse application with three units + act: Refresh the application (upgrade) + assert: The application upgrades and over all the upgrade, the application replies + correctly through the ingress. + """ + + await app.scale(3) + await model.wait_for_idle(status="active") + + resources = { + "discourse-image": pytestconfig.getoption("--discourse-image"), + } + + if charm_file := pytestconfig.getoption("--charm-file"): + charm_path: str | Path | None = f"./{charm_file}" + else: + charm_path = await ops_test.build_charm(".") + + host = app.name + + def check_alive(): + response = requests.get("http://127.0.0.1/srv/status", headers={"Host": host}, timeout=2) + logger.info("check_alive response: %s", response.content) + assert response.status_code == 200 + + check_alive() + await app.refresh(path=charm_path, resources=resources) + + def upgrade_finished(idle_seconds=15): + """Check that the upgrade finishes correctly (active) + + This function checks continuously during the upgrade (in every iteration + every 0.5 seconds) that Discourse is replying correctly to the /srv/status endpoint. + + The upgrade is considered done when the units have been idle for + `idle_seconds` and all the units workloads and the app are active. + """ + idle_start = None + + def _upgrade_finished(): + nonlocal idle_start + check_alive() + + idle_period = timedelta(seconds=idle_seconds) + is_idle = all(unit.agent_status == "idle" for unit in app.units) + + now = datetime.now() + + if not is_idle: + idle_start = None + return False + + if not idle_start: + idle_start = now + return False + + if now - idle_start < idle_period: + # Not idle for long enough + return False + + is_active = app.status == "active" and all( + unit.workload_status == "active" for unit in app.units + ) + if is_active: + return True + + return False + + return _upgrade_finished + + await model.block_until(upgrade_finished(), timeout=10 * 60) + check_alive()