Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions .github/workflows/python-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ on:
- 'delphi/**/*.py'
- 'delphi/requirements*.txt'
- 'delphi/Dockerfile'
- 'scripts/*install.sh'
- 'scripts/application_stop.sh'
- 'scripts/validate_service.sh'
- 'scripts/test-deploy-hooks.sh'
- 'docker-compose*.yml'
- '.github/workflows/python-ci.yml'
# server/src so a new server-side wildcard runs the projection-gate sweep
- 'server/src/**'
Expand Down Expand Up @@ -105,13 +110,8 @@ jobs:
echo "Copying script to be tested into container..."
docker compose -f docker-compose.test.yml cp delphi/polismath/run_math_pipeline.py delphi:/app/run_math_pipeline.py
docker compose -f docker-compose.test.yml cp delphi/umap_narrative delphi:/app/umap_narrative
echo "Copying the compose files into container..."
# tests/test_compose_math_env.py asserts that every stack's delphi service
# receives the MATH_ENV its math service writes under. /app/tests has no
# checkout above it, so put the two files it parses beside it; without
# them the test skips instead of guarding the report pipeline's math_env.
docker compose -f docker-compose.test.yml cp docker-compose.yml delphi:/app/docker-compose.yml
docker compose -f docker-compose.test.yml cp docker-compose.test.yml delphi:/app/docker-compose.test.yml
# The same container-layout preparation and hook tests run on mm5.
bash scripts/test-deploy-hooks.sh

# Wire a checkout-shaped root so delphi/tests/scripts collects AND RUNS the
# pure-Python inventory sweep + the DB-free gate unit tests (the conftest
Expand All @@ -127,9 +127,6 @@ jobs:
# its first item (observed: only server/src copied).
# Recordings tests also load the packer and its digest helper from ci/.
# battery_coverage.py is already included in the declared delphi/scripts.
# test_compose_math_env.py reads the deploy hook's per-role compose lines.
# test_before_install_hook.py runs scripts/before_install.sh against a fake docker.
# test_compose_math_env.py also reads the stop hook's per-role stop lines.
# Representative payload tests import the box planner/gate and their
# helpers from this same checkout root; keep their relative paths intact.
# Light-shadow triage tests also import the compare/triage modules.
Expand All @@ -141,8 +138,7 @@ jobs:
ci/private_cert/image_admission.py ci/probe_box/receipt.py \
ci/probe_box/contracts.py ci/probe_box/light_shadow.py ci/probe_box/light_shadow_queries.py \
ci/private_cert/images/light_shadow_compare.py ci/private_cert/images/light_shadow_triage.py \
ci/private_cert/images/recipe.py \
scripts/after_install.sh scripts/before_install.sh scripts/application_stop.sh; do
ci/private_cert/images/recipe.py; do
docker compose -f docker-compose.test.yml exec -T delphi mkdir -p "/app/projgate/$(dirname "$rel")"
docker compose -f docker-compose.test.yml cp "$rel" "delphi:/app/projgate/$rel" \
|| { echo "failed to copy scan input: $rel"; exit 1; }
Expand Down
6 changes: 5 additions & 1 deletion appspec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,8 @@ hooks:
ApplicationStop:
- location: scripts/application_stop.sh
timeout: 300
runas: root
runas: root
ValidateService:
- location: scripts/validate_service.sh
timeout: 1200
runas: root
48 changes: 27 additions & 21 deletions delphi/tests/test_after_install_hook.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""scripts/after_install.sh selects one branch per service type, and a queue
"""scripts/after_install.sh migrates before replacing any service, and a queue
worker box (delphi-large, delphi-worker) starts only its polis-jobs.service.

Runs the real deploy hook (and the stop hook) with every external command
Expand Down Expand Up @@ -75,6 +75,7 @@ def _find(rel):
' *) echo \'{"username":"u","password":"p","dbname":"d"}\';;\n'
'esac\n'),
"docker": RECORD + (
'if [ "$1" = "$FAKE_MIGRATION_FAILURE" ]; then exit 17; fi\n'
'if [ "$1" = ps ]; then\n'
' filter=""\n'
' while [ $# -gt 0 ]; do [ "$1" = --filter ] && filter="${2#name=}"; shift; done\n'
Expand Down Expand Up @@ -124,9 +125,10 @@ def _run(tmp_path, script_path, rules, root, env):
return proc, log


def _deploy(tmp_path, service_type, **kw):
def _deploy(tmp_path, service_type, migration_failure="", **kw):
root, env = _box(tmp_path, service_type, **kw)
rules = [("/usr/local/bin/docker-compose", "docker-compose", 4),
env["FAKE_MIGRATION_FAILURE"] = migration_failure
rules = [("/usr/local/bin/docker-compose", "docker-compose", 3),
("/etc/app-info/", f"{root}/etc/app-info/", 2),
("/etc/systemd/system/", f"{root}/etc/systemd/system/", 0),
("/opt/polis", f"{root}/opt/polis", 2)]
Expand All @@ -145,10 +147,10 @@ def _removed(log):

ALL_IDS = {i for i, _ in CONTAINERS}
UNCHANGED = {
"server": ["up -d server nginx-proxy client-participation-alpha --build --force-recreate"],
"delphi": ["up -d delphi math-python --build --force-recreate"],
"server": ["up -d server nginx-proxy client-participation-alpha --no-build --force-recreate"],
"delphi": ["up -d delphi math-python --no-build --force-recreate"],
"math": [],
"ollama": ["up -d --build --force-recreate"], # unknown type: the start-everything catch-all

}


Expand All @@ -158,9 +160,12 @@ def test_the_other_service_types_behave_as_before(tmp_path, service_type):
assert proc.returncode == 0, proc.stdout + proc.stderr
compose = _calls(log, "docker-compose")
assert [c for c in compose if c.startswith("up")] == UNCHANGED[service_type]
assert not [c for c in compose if c.startswith("build")]
builds = [c for c in compose if c.startswith("build")]
assert builds == {"server": ["build server nginx-proxy client-participation-alpha"], "delphi": ["build delphi math-python"], "math": []}[service_type]
if builds:
assert compose.index(builds[0]) < compose.index("down") < compose.index(UNCHANGED[service_type][0])
assert _calls(log, "systemctl") == []
assert _removed(log) == ALL_IDS
assert not [c for c in _calls(log, "docker") if c.startswith(("rm ", "system prune"))]


@pytest.mark.parametrize("service_type,worker_class", [("delphi-large", "large"),
Expand All @@ -176,7 +181,7 @@ def test_a_worker_box_restarts_only_its_daemon_unit(tmp_path, service_type, work
assert log.index("docker-compose build delphi") < log.index(
"systemctl restart --no-block polis-jobs.service")
# The cleanup spares the running daemon; the restart drains it.
assert _removed(log) == ALL_IDS - {JOBS_ID}
assert not [c for c in _calls(log, "docker") if c.startswith(("rm ", "system prune"))]
assert f"worker class '{worker_class}'" in proc.stdout


Expand Down Expand Up @@ -206,17 +211,18 @@ def test_a_worker_box_that_does_not_match_fails_the_deploy(tmp_path, service_typ
assert _calls(log, "systemctl") == []


@pytest.mark.parametrize("service_type,stops", [("delphi", ["stop delphi"]),
("delphi-large", []), ("delphi-worker", [])])
def test_the_stop_hook_stops_no_worker_service(tmp_path, service_type, stops):
@pytest.mark.parametrize("service_type", ["server", "delphi", "math", "delphi-large", "delphi-worker", "unknown"])
def test_the_stop_hook_leaves_every_service_running(tmp_path, service_type):
root, env = _box(tmp_path, service_type)
(root / "opt/polis/polis").mkdir(parents=True)
# command -v needs the compose path to exist; point it at the fake.
rules = [("/usr/local/bin/docker-compose", str(tmp_path / "bin/docker-compose"), 2),
("/etc/app-info/", f"{root}/etc/app-info/", 1),
("/opt/polis/polis", f"{root}/opt/polis/polis", 1)]
proc, log = _run(tmp_path, STOP_PATH, rules, root, env)
# No path rewrite: the revised hook has no external calls or absolute paths.
proc, log = _run(tmp_path, STOP_PATH, [], root, env)
assert proc.returncode == 0, proc.stdout + proc.stderr
assert [c for c in _calls(log, "docker-compose") if c.startswith("stop")] == stops
assert _calls(log, "systemctl") == []
assert "Unknown service type" not in proc.stdout + proc.stderr
assert log == []
assert "AfterInstall" in proc.stdout


def test_unknown_role_refuses_before_migration_or_replacement(tmp_path):
proc, log = _deploy(tmp_path, "unknown")
assert proc.returncode != 0
assert not _calls(log, "docker")
assert not _calls(log, "docker-compose")
146 changes: 21 additions & 125 deletions delphi/tests/test_before_install_hook.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,10 @@
"""scripts/before_install.sh stops only the exact containers it names.
"""BeforeInstall leaves every healthy service running until AfterInstall migrates.

Docker's ``name`` filter is an unanchored regex, so ``--filter name=polis-math``
also matched the Delphi box's ``polis-math-python-1``; the hook then ran
``docker stop polis-math-1``, which exists only on the math box, and the
production BeforeInstall failed with ``No such container: polis-math-1``.

Runs the real hook against a fake ``docker`` on PATH that applies the filter
the way the daemon does (regex search against the name, with and without its
leading ``/``) and fails ``stop`` for a container that is not running. The hook
is located like ``test_compose_math_env.py`` locates after_install.sh:
$POLIS_CHECKOUT_DIR, else an ancestor of this file (CI copies it into the
checkout-shaped root).
Run the exact hook with forbidden external commands on PATH. A reintroduced
stop, removal, daemon restart or migration here must fail these controls.
"""

import os
import re
import shutil
import subprocess
from pathlib import Path
Expand Down Expand Up @@ -45,117 +35,23 @@ def _find_before_install():
pytest.mark.skipif(shutil.which("bash") is None, reason="bash not available"),
]

FAKE_DOCKER = r"""#!/bin/bash
# Fake docker: running containers come from $FAKE_DOCKER_RUNNING.
echo "$*" >> "$FAKE_DOCKER_LOG"
case "$1" in
ps)
filter=""
while [ $# -gt 0 ]; do
if [ "$1" = "--filter" ]; then filter="$2"; shift; fi
shift
done
regex="${filter#name=}"
for n in $FAKE_DOCKER_RUNNING; do
if printf '%s\n' "$n" | grep -Eq -- "$regex" || printf '%s\n' "/$n" | grep -Eq -- "$regex"; then
echo "id-$n"
fi
done
;;
stop)
for n in $FAKE_DOCKER_RUNNING; do
if [ "$n" = "$2" ]; then echo "$2"; exit 0; fi
done
echo "Error response from daemon: No such container: $2" >&2
exit 1
;;
*)
echo "fake docker: unexpected command: $*" >&2
exit 2
;;
esac
"""


@pytest.fixture
def fake_docker(tmp_path):
@pytest.mark.parametrize("running", [
"polis-math-python-1", "polis-delphi-1 polis-math-python-1",
"polis-math-python-large-1 polis-jobs", "polis-math-1",
"polis-server-1 polis-server-helper-1", "",
])
def test_before_install_never_touches_running_services(tmp_path, running):
bin_dir = tmp_path / "bin"
bin_dir.mkdir()
docker = bin_dir / "docker"
docker.write_text(FAKE_DOCKER)
docker.chmod(0o755)
log = tmp_path / "docker.log"
log.write_text("")

def run(*running, script=None):
env = dict(os.environ)
env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}"
env["FAKE_DOCKER_RUNNING"] = " ".join(running)
env["FAKE_DOCKER_LOG"] = str(log)
log.write_text("")
if script is None:
args = ["bash", str(BEFORE_INSTALL_PATH)]
else:
args = ["bash", "-c", script]
result = subprocess.run(args, env=env, capture_output=True, text=True, timeout=30)
stops = [line.split()[1] for line in log.read_text().splitlines() if line.startswith("stop ")]
return result, stops

return run


def test_fake_docker_filters_by_substring_like_the_daemon(fake_docker):
# Guards the fake: an unanchored filter must reproduce the production failure.
result, _ = fake_docker(
"polis-math-python-1",
script="docker ps -q --filter name=polis-math",
)
assert result.stdout.strip() == "id-polis-math-python-1"


def test_delphi_box_with_math_python_stops_nothing_math(fake_docker):
result, stops = fake_docker("polis-math-python-1")
assert result.returncode == 0, result.stderr
assert stops == []


def test_delphi_box_stops_delphi_but_not_math_python(fake_docker):
result, stops = fake_docker("polis-delphi-1", "polis-math-python-1")
assert result.returncode == 0, result.stderr
assert stops == ["polis-delphi-1"]


def test_large_box_stops_nothing(fake_docker):
# A container no anchored filter names (here the former large poller's
# name, which no compose file defines any more) is left alone: the
# large box's queue worker is not this hook's to stop.
result, stops = fake_docker("polis-math-python-large-1")
assert result.returncode == 0, result.stderr
assert stops == []


def test_math_box_stops_math(fake_docker):
result, stops = fake_docker("polis-math-1")
assert result.returncode == 0, result.stderr
assert stops == ["polis-math-1"]


def test_server_box_stops_server(fake_docker):
result, stops = fake_docker("polis-server-1", "polis-server-helper-1")
assert result.returncode == 0, result.stderr
assert stops == ["polis-server-1"]


def test_no_containers_is_a_clean_exit(fake_docker):
result, stops = fake_docker()
assert result.returncode == 0, result.stderr
assert stops == []


def test_every_name_filter_is_anchored_to_the_container_it_stops():
text = BEFORE_INSTALL_PATH.read_text()
filters = re.findall(r'--filter "name=([^"]*)"', text)
stopped = re.findall(r"docker stop (\S+)", text)
assert filters, "before_install.sh no longer filters by name"
assert "--filter name=" not in text and "--filter 'name=" not in text
assert [f"^/?{name}$" for name in stopped] == filters
log = tmp_path / "calls"
for name in ("docker", "docker-compose", "systemctl", "sudo", "polis-migrate"):
stub = bin_dir / name
stub.write_text('#!/bin/sh\necho "$0 $*" >> "$FAKE_LOG"\nexit 91\n')
stub.chmod(0o755)
env = {"PATH": f"{bin_dir}:/usr/bin:/bin", "FAKE_LOG": str(log),
"FAKE_DOCKER_RUNNING": running}
result = subprocess.run(["bash", str(BEFORE_INSTALL_PATH)], env=env,
capture_output=True, text=True, timeout=30)
assert result.returncode == 0, result.stdout + result.stderr
assert not log.exists(), "BeforeInstall must not invoke service or migration commands"
assert "AfterInstall" in result.stdout
46 changes: 19 additions & 27 deletions delphi/tests/test_compose_math_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
``docker compose config`` would print. The checkout is located by walking up
from this file (``POLIS_CHECKOUT_DIR`` overrides), because the CI job copies
``delphi/tests`` into the delphi image at ``/app/tests``, where the repo root is
not an ancestor — ``python-ci.yml`` copies the two compose files in beside it.
not an ancestor. ``scripts/test-deploy-hooks.sh`` copies the compose files,
hooks and Dockerfile into the explicit checkout root in both CI and mm5.
"""

import os
Expand Down Expand Up @@ -279,7 +280,13 @@ def test_delphi_role_starts_delphi_and_the_shadow_poller():
assert len(roles.get("delphi", [])) == 1, "the delphi role must have exactly one compose up line"
(line,) = roles["delphi"]
assert _services_named(line) == {"delphi", "math-python"}
assert {"-d", "--build", "--force-recreate"} <= set(line)
assert {"-d", "--no-build", "--force-recreate"} <= set(line)
assert "--build" not in line
# Release A builds while the old services still serve, then replaces them.
body = _role_bodies()["delphi"]
build = "sudo /usr/local/bin/docker-compose build delphi math-python"
down = "sudo /usr/local/bin/docker-compose down"
assert body.index(build) < body.index(down) < body.index(" ".join(line))


@requires_after_install
Expand Down Expand Up @@ -474,9 +481,10 @@ def test_delphi_llm_selection_defaults_when_unset():
# Datadog agent listening the tracer only logs failed sends, so the default is
# off; the ddtrace package stays installed so tracing can be switched back on.

DOCKERFILE = Path(__file__).resolve().parent.parent / "Dockerfile"
DOCKERFILE = (CHECKOUT / "delphi/Dockerfile" if CHECKOUT is not None
else Path(__file__).resolve().parent.parent / "Dockerfile")
requires_dockerfile = pytest.mark.skipif(
not DOCKERFILE.is_file(), reason=f"{DOCKERFILE} not found (CI copies only tests/)"
not DOCKERFILE.is_file(), reason=f"{DOCKERFILE} not found"
)


Expand Down Expand Up @@ -640,7 +648,6 @@ def test_there_is_no_large_poller_service_and_no_manifest():

APPLICATION_STOP = Path("scripts") / "application_stop.sh"
_ROLE_BLOCK = re.compile(r'^(?:if|elif) \[ "\$SERVICE_FROM_FILE" == "(?P<role>[a-z-]+)" \]; then$')
_STOP_BRANCH = re.compile(r'^ (?:if|elif) \[ "\$SERVICE_TYPE" == "(?P<role>[a-z-]+)" \]; then$')


def _role_bodies() -> dict:
Expand Down Expand Up @@ -710,26 +717,11 @@ def _find_stop_hook():
requires_stop_hook = pytest.mark.skipif(STOP_HOOK is None, reason=f"{APPLICATION_STOP} not found")


def _stop_lines() -> dict:
roles, current = {}, None
for line in STOP_HOOK.read_text().splitlines():
match = _STOP_BRANCH.match(line)
if match:
current = match["role"]
roles[current] = []
continue
if line.startswith((" else", " fi", "else", "fi")):
current = None
continue
code = line.split("#", 1)[0].split()
if current and code[:1] and code[0].endswith("docker-compose") and "stop" in code:
roles[current].append(code)
return roles


@requires_stop_hook
def test_the_stop_hook_stops_nothing_on_the_large_box_and_no_large_poller_anywhere():
roles = _stop_lines()
assert roles.get("delphi-large", []) == []
assert not any("math-python-large" in l for lines in roles.values() for l in lines)
assert "delphi-large" in STOP_HOOK.read_text()
def test_the_stop_hook_defers_shutdown_for_every_role():
# ApplicationStop is now role-independent; the behavioral hook suite also
# executes it for every role and rejects any external command invocation.
text = STOP_HOOK.read_text()
code = "\n".join(line.split("#", 1)[0] for line in text.splitlines())
assert not re.search(r"\b(?:docker(?:-compose)?|systemctl)\b", code)
assert "AfterInstall" in text
Loading
Loading