diff --git a/.github/workflows/check-action-file.yaml b/.github/workflows/check-action-file.yaml index f76b18e..12949f0 100644 --- a/.github/workflows/check-action-file.yaml +++ b/.github/workflows/check-action-file.yaml @@ -12,6 +12,7 @@ on: - 'actions/resolve-sca-license-scope/**' - 'actions/restore-sca-analysis-cache/**' - 'actions/seed-go-caches/**' + - 'actions/collect-bvt-coverage/**' - 'actions/upload-artifact-with-retry/action.yaml' - 'actions/upload-artifact-with-retry/state-machine.schema.json' - 'scripts/select_coverage_artifacts.py' @@ -81,6 +82,8 @@ jobs: go vet \ scripts/compact_coverage_profile.go \ scripts/compact_coverage_profile_test.go + - name: Test BVT coverage collection + run: python3 -m unittest discover -s actions/collect-bvt-coverage -p 'test_*.py' -v - name: Test coverage artifact generation selection run: python3 scripts/test_select_coverage_artifacts.py -v - name: Test TKE merge subject identity contract @@ -164,6 +167,7 @@ jobs: actions/resolve-sca-license-scope/action.yaml actions/restore-sca-analysis-cache/action.yaml actions/seed-go-caches/action.yaml + actions/collect-bvt-coverage/action.yaml actions/upload-artifact-with-retry/action.yaml - name: Validate artifact retry state machine uses: dsanders11/json-schema-validate-action@eddf079f55830cc9a916a3c512ba9086240d2fea # v2.0.0 diff --git a/.github/workflows/e2e-compose-parallel.yaml b/.github/workflows/e2e-compose-parallel.yaml index 8ae636e..e263e17 100644 --- a/.github/workflows/e2e-compose-parallel.yaml +++ b/.github/workflows/e2e-compose-parallel.yaml @@ -258,6 +258,8 @@ jobs: # Handle various docker-compose volume formats: /test, /test:options, /test followed by space sed -i 's|:/test\b|:'"$GITHUB_WORKSPACE"'/test|g' ./etc/launch-tae-compose/compose.yaml + # A rerun must never inherit counters from an earlier container generation. + sudo rm -rf -- "$GITHUB_WORKSPACE/coverage" mkdir -p "$GITHUB_WORKSPACE/coverage" # Assemble the runtime image from the shared -cover build when the @@ -479,33 +481,13 @@ jobs: run: | ./optools/check_log_count.sh 1000 60 # {count threshold} {metric collected interval} - - name: shutdown containers + - name: Collect BVT coverage after verified CN shutdown if: ${{ always() }} - run: | - docker ps - docker compose -f etc/launch-tae-compose/compose.yaml --profile launch-multi-cn down --remove-orphans - docker volume rm launch-tae-compose_minio_storage || true - - - name: Generate Compose BVT coverage profile - if: ${{ always() && !cancelled() }} - run: | - set -uo pipefail - coverage_dir="$GITHUB_WORKSPACE/coverage" - coverage_profile="$RUNNER_TEMP/bvt-compose.out" - compose_file="$GITHUB_WORKSPACE/etc/launch-tae-compose/compose.yaml" - if ! grep -Eq 'GOCOVERDIR=/coverage' "${compose_file}" \ - || ! grep -Eq 'coverage:/coverage' "${compose_file}"; then - echo '::warning::Compose BVT coverage is unsupported by this legacy branch; keeping the successful BVT result' - exit 0 - fi - if go tool covdata textfmt -i="${coverage_dir}" -o "${coverage_profile}"; then - test -s "${coverage_profile}" - elif [ "${{ steps.bvt_on_pr_version.conclusion }}" = "success" ]; then - echo '::error::failed to generate Compose BVT coverage after a successful BVT' - exit 1 - else - echo '::warning::Compose BVT coverage unavailable because BVT did not finish successfully' - fi + timeout-minutes: 6 + uses: matrixorigin/CI/actions/collect-bvt-coverage@main + with: + coverage-dir: ${{ github.workspace }}/coverage + output: ${{ runner.temp }}/bvt-compose.out - name: Upload Compose BVT coverage if: ${{ always() && !cancelled() }} diff --git a/.github/workflows/e2e-standalone-parallel.yaml b/.github/workflows/e2e-standalone-parallel.yaml index 9802054..9a5e66e 100644 --- a/.github/workflows/e2e-standalone-parallel.yaml +++ b/.github/workflows/e2e-standalone-parallel.yaml @@ -312,6 +312,8 @@ jobs: cat ./etc/launch-tae-compose/config/cn-1.toml cat ./etc/launch-tae-compose/config/tn.toml sed -i 's|:/test\b|:'"$GITHUB_WORKSPACE"'/test|g' ./etc/launch-tae-compose/compose.yaml + # A rerun must never inherit counters from an earlier container generation. + sudo rm -rf -- "$GITHUB_WORKSPACE/coverage" mkdir -p "$GITHUB_WORKSPACE/coverage" "$GITHUB_WORKSPACE/docker-compose-log" # Assemble the runtime image from the shared -cover build when the @@ -519,33 +521,13 @@ jobs: run: | ./optools/check_log_count.sh 1000 60 # {count threshold} {metric collected interval} - - name: shutdown containers + - name: Collect BVT coverage after verified CN shutdown if: ${{ always() }} - run: | - docker ps - docker compose -f etc/launch-tae-compose/compose.yaml --profile launch-multi-cn down --remove-orphans - docker volume rm launch-tae-compose_minio_storage || true - - - name: Generate Compose BVT coverage profile - if: ${{ always() && !cancelled() }} - run: | - set -uo pipefail - coverage_dir="$GITHUB_WORKSPACE/coverage" - coverage_profile="${RUNNER_TEMP}/bvt-pessimistic.out" - compose_file="$GITHUB_WORKSPACE/etc/launch-tae-compose/compose.yaml" - if ! grep -Eq 'GOCOVERDIR=/coverage' "${compose_file}" \ - || ! grep -Eq 'coverage:/coverage' "${compose_file}"; then - echo '::warning::Compose BVT coverage is unsupported by this legacy branch; keeping the successful BVT result' - exit 0 - fi - if go tool covdata textfmt -i="${coverage_dir}" -o "${coverage_profile}"; then - test -s "${coverage_profile}" - elif [ "${{ steps.bvt_on_pr_version.conclusion }}" = "success" ]; then - echo '::error::failed to generate Compose + Pessimistic BVT coverage after a successful BVT' - exit 1 - else - echo '::warning::Compose + Pessimistic BVT coverage unavailable because BVT did not finish successfully' - fi + timeout-minutes: 6 + uses: matrixorigin/CI/actions/collect-bvt-coverage@main + with: + coverage-dir: ${{ github.workspace }}/coverage + output: ${{ runner.temp }}/bvt-pessimistic.out - name: Upload Compose + Pessimistic BVT coverage if: ${{ always() && !cancelled() }} diff --git a/actions/collect-bvt-coverage/action.yaml b/actions/collect-bvt-coverage/action.yaml new file mode 100644 index 0000000..683b410 --- /dev/null +++ b/actions/collect-bvt-coverage/action.yaml @@ -0,0 +1,48 @@ +name: Collect BVT coverage +description: Verify CN shutdown and SQL execution counters before publishing BVT coverage +inputs: + compose-file: + description: MatrixOne Compose configuration + default: etc/launch-tae-compose/compose.yaml + coverage-dir: + description: Run-owned raw Go coverage directory + required: true + output: + description: Destination coverage profile + required: true +runs: + using: composite + steps: + - name: Stop CNs and verify normal exit + id: stop + if: ${{ !cancelled() }} + shell: bash + env: + ACTION_PATH: ${{ github.action_path }} + COMPOSE_FILE: ${{ inputs.compose-file }} + COVERAGE_OUTPUT: ${{ inputs.output }} + run: | + set -euo pipefail + rm -f -- "${COVERAGE_OUTPUT}" + python3 "${ACTION_PATH}/collect.py" stop --compose-file "${COMPOSE_FILE}" + - name: Remove containers + id: cleanup + if: ${{ always() }} + shell: bash + env: + COMPOSE_FILE: ${{ inputs.compose-file }} + run: | + docker compose -f "${COMPOSE_FILE}" --profile launch-multi-cn down --remove-orphans + docker volume rm launch-tae-compose_minio_storage || true + - name: Generate and validate coverage profile + if: ${{ always() && !cancelled() && steps.stop.outcome == 'success' && steps.cleanup.outcome == 'success' }} + shell: bash + env: + ACTION_PATH: ${{ github.action_path }} + COMPOSE_FILE: ${{ inputs.compose-file }} + COVERAGE_DIR: ${{ inputs.coverage-dir }} + COVERAGE_OUTPUT: ${{ inputs.output }} + run: | + python3 "${ACTION_PATH}/collect.py" profile \ + --compose-file "${COMPOSE_FILE}" --coverage-dir "${COVERAGE_DIR}" \ + --output "${COVERAGE_OUTPUT}" diff --git a/actions/collect-bvt-coverage/collect.py b/actions/collect-bvt-coverage/collect.py new file mode 100644 index 0000000..9ddf09b --- /dev/null +++ b/actions/collect-bvt-coverage/collect.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +"""Preserve CN shutdown evidence before Compose removes the containers.""" + +import argparse +import json +from pathlib import Path +import re +import subprocess +import tempfile + + +CN_SERVICES = ("cn-0", "cn-1") +SQL_EXECUTOR = "/pkg/frontend/mysql_cmd_executor.go:" +BLOCK = re.compile(r"\S+:\d+\.\d+,\d+\.\d+ \d+ (\d+)$") + + +def run(args, timeout=30): + return subprocess.run( + args, check=True, text=True, stdout=subprocess.PIPE, timeout=timeout + ).stdout + + +def supports_coverage(compose_file): + config = compose_file.read_text() + return "GOCOVERDIR=/coverage" in config and "coverage:/coverage" in config + + +def stop_cns(compose_file): + if not supports_coverage(compose_file): + print("::warning::Skipping CN coverage checks for this legacy Compose configuration") + return + compose = ["docker", "compose", "-f", str(compose_file), + "--profile", "launch-multi-cn"] + containers = {} + for service in CN_SERVICES: + ids = run(compose + ["ps", "--all", "--quiet", service]).split() + if len(ids) != 1: + raise ValueError(f"expected one {service} container, found {len(ids)}") + containers[service] = ids[0] + + # CN shutdown allows one minute for draining, plus shutdown profiles. + # Keep TN/log dependencies alive and allow more than Docker's default 10s. + # A hung shutdown still fails the exit-state check below. + print(run(compose + ["stop", "--timeout", "120", *CN_SERVICES], timeout=180), end="") + failures = [] + for service, container in containers.items(): + state = json.loads(run(["docker", "inspect", "--format", "{{json .State}}", container])) + print(f"{service} shutdown: {json.dumps(state, sort_keys=True)}", flush=True) + if (state.get("Status") != "exited" or state.get("ExitCode") != 0 + or state.get("OOMKilled") is not False or state.get("Error")): + failures.append(service) + if failures: + raise ValueError(f"CN shutdown did not complete cleanly: {', '.join(failures)}; " + "coverage may be incomplete") + + +def validate_profile(profile): + sql_executed = False + with profile.open() as stream: + if stream.readline().strip() not in {"mode: set", "mode: count", "mode: atomic"}: + raise ValueError("invalid coverage profile mode") + for line in stream: + block = BLOCK.fullmatch(line.strip()) + if block is None: + raise ValueError("invalid coverage profile block") + if SQL_EXECUTOR in line and int(block.group(1)) > 0: + sql_executed = True + if not sql_executed: + raise ValueError("BVT coverage has no SQL executor hits; CN counters are missing") + + +def generate_profile(compose_file, coverage_dir, output): + output.unlink(missing_ok=True) + if not supports_coverage(compose_file): + print("::warning::BVT coverage is unsupported by this legacy Compose configuration") + return + output.parent.mkdir(parents=True, exist_ok=True) + with tempfile.TemporaryDirectory(dir=output.parent, prefix="bvt-coverage-") as directory: + temporary = Path(directory) / "profile.out" + run(["go", "tool", "covdata", "textfmt", f"-i={coverage_dir}", f"-o={temporary}"], timeout=180) + validate_profile(temporary) + temporary.replace(output) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("operation", choices=("stop", "profile")) + parser.add_argument("--compose-file", type=Path, required=True) + parser.add_argument("--coverage-dir", type=Path) + parser.add_argument("--output", type=Path) + args = parser.parse_args() + if args.operation == "stop": + stop_cns(args.compose_file) + else: + if args.coverage_dir is None or args.output is None: + parser.error("profile requires --coverage-dir and --output") + generate_profile(args.compose_file, args.coverage_dir, args.output) + + +if __name__ == "__main__": + main() diff --git a/actions/collect-bvt-coverage/test_collect.py b/actions/collect-bvt-coverage/test_collect.py new file mode 100644 index 0000000..28b9f7c --- /dev/null +++ b/actions/collect-bvt-coverage/test_collect.py @@ -0,0 +1,128 @@ +import json +from pathlib import Path +import subprocess +import tempfile +import unittest +from unittest.mock import patch + +import collect + + +HEALTHY = {"Status": "exited", "ExitCode": 0, "OOMKilled": False, "Error": ""} +SQL_HIT = "github.com/matrixorigin/matrixone/pkg/frontend/mysql_cmd_executor.go:1.1,2.2 1 1\n" + + +class StopCNsTests(unittest.TestCase): + def setUp(self): + directory = tempfile.TemporaryDirectory() + self.addCleanup(directory.cleanup) + self.compose = Path(directory.name) / "compose.yaml" + self.compose.write_text("GOCOVERDIR=/coverage\n../../coverage:/coverage\n") + + def responses(self, *states): + return ["first\n", "second\n", "", *(json.dumps(s) for s in states)] + + def test_normal_stop_preserves_dependencies_and_checks_both_original_ids(self): + for _ in range(2): + with patch.object(collect, "run", side_effect=self.responses(HEALTHY, HEALTHY)) as run: + collect.stop_cns(self.compose) + stop = run.call_args_list[2] + self.assertEqual(stop.args[0][-5:], ["stop", "--timeout", "120", "cn-0", "cn-1"]) + self.assertEqual(stop.kwargs["timeout"], 180) + self.assertEqual([call.args[0][-1] for call in run.call_args_list[3:]], ["first", "second"]) + self.assertFalse(any("down" in call.args[0] for call in run.call_args_list)) + + def test_missing_or_multiple_containers_fail_before_stop(self): + for ids in ("", "first\nsecond\n"): + with self.subTest(ids=ids), patch.object(collect, "run", return_value=ids) as run: + with self.assertRaisesRegex(ValueError, "expected one cn-0"): + collect.stop_cns(self.compose) + self.assertEqual(run.call_count, 1) + + def test_unclean_exit_is_rejected_after_both_states_are_inspected(self): + for change in ({"ExitCode": 137}, {"ExitCode": 2}, {"Status": "running"}, + {"OOMKilled": True}, {"Error": "daemon failure"}, {"OOMKilled": None}): + for failing_index in (0, 1): + states = [dict(HEALTHY), dict(HEALTHY)] + states[failing_index].update(change) + with self.subTest(change=change, index=failing_index): + with patch.object(collect, "run", side_effect=self.responses(*states)) as run: + with self.assertRaisesRegex(ValueError, "did not complete cleanly"): + collect.stop_cns(self.compose) + self.assertEqual(run.call_count, 5) + + def test_stop_error_and_timeout_are_not_treated_as_normal_exit(self): + for failure in (subprocess.CalledProcessError(1, "docker"), + subprocess.TimeoutExpired("docker", 180)): + with self.subTest(failure=failure): + with patch.object(collect, "run", side_effect=["first", "second", failure]) as run: + with self.assertRaises(type(failure)): + collect.stop_cns(self.compose) + self.assertEqual(run.call_count, 3) + + def test_legacy_compose_preserves_existing_cleanup_without_a_coverage_gate(self): + self.compose.write_text("services: {}\n") + with patch.object(collect, "run") as run: + collect.stop_cns(self.compose) + run.assert_not_called() + + +class ProfileTests(unittest.TestCase): + def setUp(self): + directory = tempfile.TemporaryDirectory() + self.addCleanup(directory.cleanup) + self.root = Path(directory.name) + self.compose = self.root / "compose.yaml" + self.compose.write_text("GOCOVERDIR=/coverage\n../../coverage:/coverage\n") + self.output = self.root / "bvt.out" + + def convert(self, content): + def conversion(args, **kwargs): + self.assertEqual(args[:4], ["go", "tool", "covdata", "textfmt"]) + self.assertEqual(kwargs["timeout"], 180) + Path(args[-1].removeprefix("-o=")).write_text(content) + return "" + return conversion + + def test_publish_valid_profile_and_replace_previous_generation(self): + for mode in ("set", "count", "atomic"): + content = f"mode: {mode}\n" + SQL_HIT + self.output.write_text("stale data") + with patch.object(collect, "run", side_effect=self.convert(content)): + collect.generate_profile(self.compose, self.root, self.output) + self.assertEqual(self.output.read_text(), content) + self.assertEqual(sorted(p.name for p in self.root.iterdir()), ["bvt.out", "compose.yaml"]) + + def test_missing_sql_counters_and_malformed_profiles_are_not_published(self): + # Startup/authentication in a proxy can execute frontend code without + # any CN SQL counters: that is the observed production counterexample. + auth_hit = SQL_HIT.replace("mysql_cmd_executor.go", "authenticate.go") + for content in ("", "mode: set\n", "mode: set\n" + auth_hit, + "mode: set\n" + SQL_HIT[:-2] + "0\n", + "mode: set\n" + SQL_HIT + "invalid\n"): + with self.subTest(content=content): + self.output.write_text("previous valid profile") + with patch.object(collect, "run", side_effect=self.convert(content)): + with self.assertRaises(ValueError): + collect.generate_profile(self.compose, self.root, self.output) + self.assertFalse(self.output.exists()) + self.assertEqual(list(self.root.iterdir()), [self.compose]) + + def test_conversion_failure_removes_old_output_and_temporary_files(self): + self.output.write_text("previous valid profile") + with patch.object(collect, "run", side_effect=subprocess.CalledProcessError(1, "go")): + with self.assertRaises(subprocess.CalledProcessError): + collect.generate_profile(self.compose, self.root, self.output) + self.assertEqual(list(self.root.iterdir()), [self.compose]) + + def test_legacy_compose_does_not_publish_stale_profile(self): + self.compose.write_text("services: {}\n") + self.output.write_text("previous valid profile") + with patch.object(collect, "run") as run: + collect.generate_profile(self.compose, self.root, self.output) + run.assert_not_called() + self.assertFalse(self.output.exists()) + + +if __name__ == "__main__": + unittest.main()