diff --git a/devops_bench/chaos/faults/generate_load.py b/devops_bench/chaos/faults/generate_load.py index 68215c69..7ab149ee 100644 --- a/devops_bench/chaos/faults/generate_load.py +++ b/devops_bench/chaos/faults/generate_load.py @@ -28,6 +28,7 @@ import contextlib import json import os +import re import shlex import textwrap import threading @@ -57,9 +58,19 @@ # active. The harness watches the shared event to coordinate measurements. _LOAD_MARKER = "fortio load" -# Wall-clock ceiling for a single chaos command. +# Wall-clock ceiling for a single chaos command that is not a load spike. _COMMAND_TIMEOUT = 40 +# A load spike has to outlive its own ``-t`` duration, so its ceiling is derived +# from the command rather than fixed. The flat 40s ceiling silently killed every +# spike a task declared for longer than that: optimize-scale asks for 300s +# deliberately (so the spike is still running when verification starts), fortio +# was SIGKILLed at 40s, the fault recorded exit -1, and the run was scored +# ``chaos_invalidated`` with the reason "load did not reach the workload" — which +# reads as unreachable rather than cut short. +_LOAD_TIMEOUT_SLACK_SEC = 60 +_LOAD_TIMEOUT_CEILING_SEC = 900 + # The workload's in-cluster (remote) port for chaos load generation, and the # default local side of the port-forward. Parallel runs override only the local # side via ``_ENV_LOCAL_PORT`` so two concurrent forwards do not contend. @@ -85,6 +96,41 @@ _TARGET_READY_TIMEOUT_SEC = 120 +def _go_duration_seconds(value: str) -> float | None: + """Parse a Go-style duration (``300s``, ``5m``, ``1h30m``) into seconds. + + fortio takes its ``-t`` in Go's format. Returns ``None`` for anything not + understood, so the caller falls back to the fixed ceiling rather than + inventing a budget from a value it misread. + """ + parts = re.findall(r"([0-9]*\.?[0-9]+)\s*(ms|h|m|s)", value.strip()) + if not parts: + return None + unit = {"h": 3600.0, "m": 60.0, "s": 1.0, "ms": 0.001} + total = 0.0 + for amount, suffix in parts: + total += float(amount) * unit[suffix] + return total or None + + +def _command_timeout(argv: list[str], *, is_load: bool) -> float: + """Wall-clock ceiling for this command. + + A load spike gets its declared duration plus slack (bounded), so the + generator is never killed mid-spike. Everything else keeps the flat + ceiling. + """ + if not is_load: + return _COMMAND_TIMEOUT + for index, token in enumerate(argv): + if token == "-t" and index + 1 < len(argv): + declared = _go_duration_seconds(argv[index + 1]) + if declared is None: + break + return min(declared + _LOAD_TIMEOUT_SLACK_SEC, _LOAD_TIMEOUT_CEILING_SEC) + return _COMMAND_TIMEOUT + + def build_system_instruction(target_url: str = _DEFAULT_TARGET_URL) -> str: """Build the SRE system instruction, targeting ``target_url`` for load. @@ -199,7 +245,7 @@ def run_chaos_command( _log.info("load spike detected; signaling harness via chaos event") chaos_active_event.set() - completed = run(argv, check=False, timeout=_COMMAND_TIMEOUT) + completed = run(argv, check=False, timeout=_command_timeout(argv, is_load=is_load)) if is_load and load_result is not None: # Record the spike's real exit status so the fault can fail closed: # a non-zero fortio exit means it could not reach the workload. diff --git a/tasks/common/optimize-scale/README.md b/tasks/common/optimize-scale/README.md index 3578857f..d85fe026 100644 --- a/tasks/common/optimize-scale/README.md +++ b/tasks/common/optimize-scale/README.md @@ -92,24 +92,28 @@ at 15s, the objectives above are still evaluated against a cluster that was neve entry's status in `results.json` before reading a passing score as evidence the workload absorbed anything. -## Why GKE +## Why kind -This task is pinned to `gcp`, which is what every published run of it was graded against. +Two things used to argue for GKE, and neither survives measurement. -The metrics objective needs a working metrics pipeline. GKE ships metrics-server; a stock kind -cluster does not, so `ScalingActive` would read `False` for reasons that have nothing to do with -the agent. The stack now installs metrics-server itself under `infra_provider=kind`, so that -reason alone no longer forces GKE. +Metrics: the HPA objective grades `ScalingActive=True`, which needs a live +metrics pipeline. A stock kind cluster ships none — but the stack now installs +metrics-server itself under `infra_provider=kind`, so the objective is decided +by the agent again. -The load path is what still does. On GKE the Service is a `LoadBalancer` and the chaos generator -reaches it directly; on kind it is a `ClusterIP` behind the harness port-forward. The planned load -spike failed to inject in **8 of 8** published runs, so the fewer moving parts in that path the -better until a spike is demonstrably landing. +The load path: on GKE the Service is a LoadBalancer the chaos generator was +meant to reach directly. In practice it cannot — fortio times out on the +external IP (`dial tcp :8080: i/o timeout`) because the project's firewall +admits only 22/3389/443, so the spike never injects. On kind the Service is a +`ClusterIP` behind the harness port-forward, which connects and serves the +spike. -`INFRA_PROVIDER=kind` is a genuinely working alternative now — metrics included — and much cheaper -for local iteration on the fixture. Whichever you pick, **every runner must pick the same one**: -the provider changes the Service type and the load path, so a kind arm and a GKE arm are not -comparable on this task. +The historic 8-of-8 injection failures were a third thing entirely: every chaos +command shared a 40s ceiling, and this task declares a 300s spike, so fortio was +killed mid-run and the fault reported "load did not reach the workload". + +`INFRA_PROVIDER=gcp` still selects GKE and is the better load path once a runner +can reach a LoadBalancer. ## Run diff --git a/tasks/common/optimize-scale/task.yaml b/tasks/common/optimize-scale/task.yaml index e22da6b7..824aa54f 100644 --- a/tasks/common/optimize-scale/task.yaml +++ b/tasks/common/optimize-scale/task.yaml @@ -7,28 +7,31 @@ name: "optimize-scale" # target_deployment_name/namespace can be substituted straight into the # {{TARGET_DEPLOYMENT_NAME}}/{{NAMESPACE}} placeholders below. # -# Pinned to gcp, which is what every published run of this task was graded -# against. Two reasons, and the second is the one that bites: +# Runs on kind. This task was pinned to gcp for two reasons; measurement has +# now retired both: # # 1. The objectives depend on a working metrics pipeline. GKE ships # metrics-server; a stock kind cluster does not, so ScalingActive would # read False for reasons that have nothing to do with the agent. The # stack now installs metrics-server itself under infra_provider=kind, so # this reason no longer holds on its own. -# 2. On GKE the Service is a LoadBalancer and the chaos load reaches it -# directly. On kind it needs a port-forward — one more moving part in the -# exact path that already failed to inject the spike in 8 of 8 published -# runs. Until a spike demonstrably lands on kind, gcp is the arm to -# compare on. +# 2. The load path was the stronger argument for GKE: a LoadBalancer the +# chaos generator reaches directly, versus a port-forward on kind. It ran +# the other way in practice. On GKE fortio never connects at all — the +# LB's external IP is unreachable from the runner (`dial tcp :8080: +# i/o timeout`; the project's firewall admits only 22/3389/443). On kind +# the port-forward connects and serves the spike. The 8-of-8 injection +# failures were not the port-forward: every chaos command shared a 40s +# ceiling that killed this task's declared 300s spike outright. # -# INFRA_PROVIDER=kind is now a genuinely working alternative (metrics included) -# for cheap local iteration. Whichever is used, every runner must use the SAME -# one: the provider changes the Service type and the load path, so a kind arm -# and a GKE arm are not comparable on this task. +# INFRA_PROVIDER=gcp still selects GKE, and remains the better load path the +# day the runner can reach a LoadBalancer. Whichever is used, every runner must +# use the SAME one: the provider changes the Service type and the load path, so +# a kind arm and a GKE arm are not comparable on this task. infrastructure: deployer: "tofu" stack: "prebuilt/optimize-scale" - provider: "gcp" + provider: "kind" teardown: true variables: namespace: "default" diff --git a/tests/unit/chaos/test_generate_load.py b/tests/unit/chaos/test_generate_load.py index cbe3ace2..76d1e623 100644 --- a/tests/unit/chaos/test_generate_load.py +++ b/tests/unit/chaos/test_generate_load.py @@ -27,6 +27,8 @@ from typing import Any from unittest.mock import MagicMock, patch +import pytest + from devops_bench.chaos.base import ChaosResult from devops_bench.chaos.faults import generate_load as gl from devops_bench.chaos.faults.generate_load import ( @@ -456,3 +458,37 @@ def test_inject_port_forward_setup_failure_becomes_failed_result() -> None: assert result.success is False assert result.error is not None assert "kubectl missing" in result.error + + +class TestLoadCommandTimeout: + """A spike must outlive its own ``-t``; everything else keeps the flat cap.""" + + def test_spike_timeout_covers_the_declared_duration(self): + # 300s is what optimize-scale declares. Under the old flat 40s ceiling + # fortio was killed mid-spike and the fault reported "load did not reach + # the workload", which reads as unreachable rather than cut short. + argv = ["fortio", "load", "-qps", "300", "-t", "300s", "-c", "2", "http://localhost:8080"] + assert gl._command_timeout(argv, is_load=True) > 300 + + def test_spike_timeout_is_bounded(self): + argv = ["fortio", "load", "-t", "24h", "http://localhost:8080"] + assert gl._command_timeout(argv, is_load=True) == gl._LOAD_TIMEOUT_CEILING_SEC + + def test_non_load_command_keeps_the_flat_ceiling(self): + assert gl._command_timeout(["kubectl", "get", "pods"], is_load=False) == gl._COMMAND_TIMEOUT + + def test_unparsable_duration_falls_back_rather_than_guessing(self): + argv = ["fortio", "load", "-t", "banana", "http://localhost:8080"] + assert gl._command_timeout(argv, is_load=True) == gl._COMMAND_TIMEOUT + + def test_load_without_a_duration_flag_keeps_the_flat_ceiling(self): + assert gl._command_timeout(["fortio", "load", "http://x"], is_load=True) == ( + gl._COMMAND_TIMEOUT + ) + + @pytest.mark.parametrize( + ("value", "expected"), + [("300s", 300.0), ("5m", 300.0), ("1h30m", 5400.0), ("250ms", 0.25), ("nope", None)], + ) + def test_go_duration_parsing(self, value, expected): + assert gl._go_duration_seconds(value) == expected