diff --git a/.github/workflows/pr-integration-tests.yml b/.github/workflows/pr-integration-tests.yml index f58ea40..1eebe9b 100644 --- a/.github/workflows/pr-integration-tests.yml +++ b/.github/workflows/pr-integration-tests.yml @@ -9,6 +9,26 @@ permissions: pull-requests: read jobs: + unit-tests: + name: Unit tests + runs-on: ubuntu-22.04 + timeout-minutes: 10 + steps: + - name: Checkout matrix + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + + - name: Install test dependencies + run: | + set -euo pipefail + python3 -m pip install --disable-pip-version-check pytest -r scripts/requirements.txt + + - name: Run unit tests + run: | + set -euo pipefail + python3 -m pytest tests/ -q + changes: name: Detect PR changes runs-on: ubuntu-latest diff --git a/.github/workflows/upstream-release-watch.yml b/.github/workflows/upstream-release-watch.yml new file mode 100644 index 0000000..2fb4a1a --- /dev/null +++ b/.github/workflows/upstream-release-watch.yml @@ -0,0 +1,199 @@ +name: Upstream Release Watch + +# Detects a newly published upstream release and runs the regular integration workflow against that +# exact tag. +# +# Upstream releases are the one case no other check reaches. A new Scylla driver tag is caught by the +# release workflow in scylladb/java-driver, and an edited patch is caught by +# changed-driver-version-tests, which runs a full integration leg for every version directory a pull +# request touches — but apache/cassandra-java-driver is tagged by the ASF, so neither a ScyllaDB +# release nor a matrix pull request happens at that moment. The tag simply appears, +# `Run.version_folder` starts falling back to the newest version directory below it, and the first +# sign of trouble is the ~3.5h nightly Jenkins matrix failing before it executes a single test. +# +# Scope, deliberately: only the newest release tag is checked, while the nightly selects the newest +# two (--version-size 2). Widening this would duplicate a line of defence that already exists — a +# second upstream release landing before the first is onboarded is a discrepancy the nightly is there +# to surface. +# +# A missing version directory starts the run; a cache marker stops it repeating. The integration +# workflow takes hours, so each tag gets exactly one: mark-tested records the tag once the run +# reaches a conclusion, success or failure, and the next morning's run finds the marker and does +# nothing. A red run is the signal itself and is already reported — repeating it daily until someone +# onboards the tag would bury it rather than reinforce it. Cancelled and skipped runs record nothing, +# so an infra abort is retried the next day, and an evicted marker (caches expire after 7 days +# unaccessed, though the daily lookup counts as access) costs one extra run at worst. Onboarding the +# directory still makes the whole workflow a no-op, so no marker ever needs cleaning up. Cache scope +# keeps the two triggers apart: a pull request run may read the default branch's markers but writes +# only into its own scope, so it cannot suppress a scheduled run. A workflow_dispatch naming a +# driver_ref bypasses the marker entirely, so any tag can be re-tested on demand. +# +# The decision itself lives in scripts/upstream_release_watch.py and is unit-tested in +# tests/test_upstream_release_watch.py. It has to be tested there: a pull request run only ever sees +# the newest upstream tag as already onboarded, so the branches that matter never execute here. The +# integration job also excludes github.event_name == 'pull_request' explicitly, so a pull request +# can never start the multi-hour run even if that assumption ever stops holding. +# +# This cannot be a webhook: GitHub only delivers those for repositories we own. Polling on a schedule +# is the only mechanism available for a third-party repository. + +on: + schedule: + # 06:00 UTC = 08:00 CEST / 09:00 IDT. Anchored to the start of the working day, not to the + # nightly: the Jenkins driver-matrix jobs run at 21:58 UTC, so a run timed just before them would + # fire when nobody is around to act. Firing in the morning instead means the result is waiting + # when people arrive, it covers tags published the previous evening, and there is a full working + # day to add the missing version directory before that night's matrix run. + - cron: '0 6 * * *' + workflow_dispatch: + inputs: + driver_ref: + description: 'driver_ref: upstream tag to test. When empty, the newest release tag is detected.' + type: string + default: '' + pull_request: + paths: + - '.github/workflows/upstream-release-watch.yml' + - 'scripts/upstream_release_watch.py' + +permissions: + contents: read + +# Queues, never cancels: a second trigger's detect job waits for the first run to finish, so it +# sees the marker the first run wrote and no-ops instead of racing it into a second integration +# run for the same tag. Scoped by ref, so a pull_request run (its own refs/pull/N/merge) never +# queues behind the schedule/workflow_dispatch runs on the default branch, or vice versa. +concurrency: + group: upstream-release-watch-${{ github.ref }} + +jobs: + detect: + name: Detect new upstream release + runs-on: ubuntu-22.04 + timeout-minutes: 10 + outputs: + driver_ref: ${{ steps.resolve.outputs.driver_ref }} + should_run: ${{ steps.decide.outputs.should_run }} + steps: + - name: Checkout matrix + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + + - name: Resolve newest upstream release tag + if: ${{ inputs.driver_ref == '' }} + id: get-upstream-tag + uses: scylladb-actions/get-version@a1dc4fedfb5684242148020b24e4150b1fe7ad08 # v0.4.5 + with: + source: github-tag + repo: apache/cassandra-java-driver + # get-version treats dots as version-component separators, so the three regex components + # below match only clean three-part releases and the LAST.LAST.LAST selector then picks the + # newest one. 71 of the 179 upstream tags are not releases at all (`1.0.0-rc1`, + # `1.0.2-dse2`, bare `2.0`), and extract_n_latest_repo_tags only ever selects N.N.N, so a + # bare LAST filter could hand us a tag the matrix would never test. + filters: '^[0-9]+$.^[0-9]+$.^[0-9]+$ and LAST.LAST.LAST' + github-token: ${{ github.token }} + + - name: Normalize the tag + id: resolve + env: + DRIVER_REF_INPUT: ${{ inputs.driver_ref }} + DRIVER_REF_LATEST: ${{ steps.get-upstream-tag.outputs.versions }} + run: | + set -euo pipefail + python3 - <<'PY' + import os + import sys + + sys.path.insert(0, os.environ["GITHUB_WORKSPACE"]) + from scripts.upstream_release_watch import resolve, write_outputs + + outputs = resolve( + os.environ.get("DRIVER_REF_INPUT", ""), + os.environ.get("DRIVER_REF_LATEST", ""), + ) + + write_outputs(outputs, os.environ["GITHUB_OUTPUT"]) + PY + + # Resolved before the decision, not after, because the key names the tag. + - name: Look up the tested marker + id: marker + uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + with: + path: .upstream-release-tested + key: upstream-release-tested-apache-${{ steps.resolve.outputs.driver_ref }} + lookup-only: true + + - name: Decide whether to run + id: decide + env: + DRIVER_REF: ${{ steps.resolve.outputs.driver_ref }} + FORCED: ${{ steps.resolve.outputs.forced }} + HAS_DIRECTORY: ${{ steps.resolve.outputs.has_directory }} + ALREADY_TESTED: ${{ steps.marker.outputs.cache-hit }} + IS_PULL_REQUEST: ${{ github.event_name == 'pull_request' }} + run: | + set -euo pipefail + python3 - <<'PY' + import os + import sys + + sys.path.insert(0, os.environ["GITHUB_WORKSPACE"]) + from scripts.upstream_release_watch import as_bool, decide, summarize, write_outputs + + driver_ref = os.environ["DRIVER_REF"] + forced = as_bool(os.environ.get("FORCED", "")) + has_directory = as_bool(os.environ.get("HAS_DIRECTORY", "")) + already_tested = as_bool(os.environ.get("ALREADY_TESTED", "")) + is_pull_request = as_bool(os.environ.get("IS_PULL_REQUEST", "")) + + write_outputs(decide(forced, has_directory, already_tested), os.environ["GITHUB_OUTPUT"]) + + with open(os.environ["GITHUB_STEP_SUMMARY"], "a", encoding="utf-8") as summary: + summary.write( + summarize(driver_ref, forced, has_directory, already_tested, is_pull_request=is_pull_request) + ) + PY + + integration: + name: Integration tests for the new tag + needs: detect + # A pull_request run should_run is always false in practice (the newest tag is already onboarded + # by the time a PR runs), but excluding the trigger explicitly means a PR can never start the + # multi-hour run even if that stops holding, instead of relying on it as an assumption. + if: ${{ needs.detect.outputs.should_run == 'true' && github.event_name != 'pull_request' }} + uses: ./.github/workflows/integration-tests.yml + with: + driver_repository: apache/cassandra-java-driver + driver_type: apache + driver_ref: ${{ needs.detect.outputs.driver_ref }} + scylla_version: LATEST + + mark-tested: + name: Record the tag as tested + needs: [detect, integration] + # Only a conclusive run counts. A cancelled or skipped one leaves no marker, so the tag comes back + # tomorrow. + if: >- + ${{ always() && needs.detect.outputs.should_run == 'true' + && (needs.integration.result == 'success' || needs.integration.result == 'failure') }} + runs-on: ubuntu-22.04 + timeout-minutes: 10 + steps: + - name: Write the marker + env: + DRIVER_REF: ${{ needs.detect.outputs.driver_ref }} + RESULT: ${{ needs.integration.result }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + printf '%s %s %s\n' "$DRIVER_REF" "$RESULT" "$RUN_URL" > .upstream-release-tested + cat .upstream-release-tested + + - name: Save the tested marker + uses: actions/cache/save@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + with: + path: .upstream-release-tested + key: upstream-release-tested-apache-${{ needs.detect.outputs.driver_ref }} diff --git a/run.py b/run.py index 376f711..761d0e4 100644 --- a/run.py +++ b/run.py @@ -123,7 +123,7 @@ def version(self) -> str: @cached_property def version_folder(self) -> Path: - version_pattern = re.compile(r"(\d+.)+\d+$") + version_pattern = re.compile(r"(\d+\.)+\d+$") target_version_folder = self._root_path / "versions" / self._driver_type driver_version_dir_path = target_version_folder / self.version if driver_version_dir_path.is_dir(): @@ -140,10 +140,11 @@ def version_folder(self) -> Path: ) for tag in tags_defined: if tag <= target_version: - logging.info("The full directory for '%s' tag is '%s'", self._tag, driver_version_dir_path) - return target_version_folder / str(tag) + fallback_dir_path = target_version_folder / str(tag) + logging.info("No directory for '%s' tag; falling back to '%s'", self._tag, fallback_dir_path) + return fallback_dir_path else: - raise ValueError("Not found directory for python-driver version '%s'", self._tag) + raise ValueError(f"Not found directory for java-driver version '{self._tag}'") @cached_property def ignore_tests(self) -> Set[str]: diff --git a/scripts/upstream_release_watch.py b/scripts/upstream_release_watch.py new file mode 100644 index 0000000..16aea1b --- /dev/null +++ b/scripts/upstream_release_watch.py @@ -0,0 +1,133 @@ +from __future__ import annotations + +import json +import uuid +from pathlib import Path + + +DEFAULT_VERSIONS_DIR = Path("versions") / "apache" + + +def as_bool(value: str) -> bool: + """Read a GitHub Actions boolean. An unset cache-hit output arrives as an empty string.""" + return value.strip().lower() == "true" + + +def write_outputs(outputs: dict[str, str], path: str) -> None: + """Append to $GITHUB_OUTPUT with the delimited form, safe for a value containing a newline. + + driver_ref can come straight from a workflow_dispatch input, so a plain `name=value` line would + let an embedded newline forge extra output lines. + """ + with open(path, "a", encoding="utf-8") as handle: + for name, value in outputs.items(): + delimiter = f"ghadelim_{uuid.uuid4().hex}" + handle.write(f"{name}<<{delimiter}\n{value}\n{delimiter}\n") + + +def pick(input_value: str, latest_json: str, kind: str) -> str: + """Explicit input wins, otherwise take the first resolved version. + + Same shape as the Normalize inputs step in integration-tests.yml. + """ + if input_value: + return input_value + try: + latest = json.loads(latest_json or "[]") + except json.JSONDecodeError as error: + raise SystemExit(f"Unable to resolve {kind}: {error}") from error + if not latest: + raise SystemExit(f"Unable to resolve {kind}") + return latest[0] + + +def as_tuple(name: str) -> tuple[int, ...] | None: + try: + return tuple(int(part) for part in name.split(".")) + except ValueError: + return None + + +def fallback_for(tag: str, versions_dir: Path = DEFAULT_VERSIONS_DIR) -> str: + """The directory Run.version_folder would resolve to: newest defined version <= tag.""" + target = as_tuple(tag) + if target is None: + return "" + candidates = [] + for path in Path(versions_dir).iterdir(): + if not path.is_dir(): + continue + parsed = as_tuple(path.name) + if parsed is not None and parsed <= target: + candidates.append((parsed, path.name)) + return max(candidates)[1] if candidates else "" + + +def resolve( + driver_ref_input: str, + latest_json: str, + versions_dir: Path = DEFAULT_VERSIONS_DIR, +) -> dict[str, str]: + """The tag to consider, and whether it is onboarded. + + Split from decide() because the marker cache key contains the tag, so the lookup step sits + between the two. + """ + driver_ref = pick(driver_ref_input, latest_json, "upstream release tag") + return { + "driver_ref": driver_ref, + "forced": str(bool(driver_ref_input)).lower(), + # The same exact-match test Run.version_folder makes before it starts falling back. + "has_directory": str((Path(versions_dir) / driver_ref).is_dir()).lower(), + } + + +def decide(forced: bool, has_directory: bool, already_tested: bool) -> dict[str, str]: + """An explicit dispatch always runs; otherwise a tag runs once, while it has no directory.""" + return {"should_run": str(forced or not (has_directory or already_tested)).lower()} + + +def summarize( + driver_ref: str, + forced: bool, + has_directory: bool, + already_tested: bool, + versions_dir: Path = DEFAULT_VERSIONS_DIR, + is_pull_request: bool = False, +) -> str: + label = Path(versions_dir).as_posix() + heading = "Requested" if forced else "Newest" + lines = [f"### {heading} `apache` release tag: `{driver_ref}`", ""] + # Reuses decide()'s formula rather than re-deriving it, so the wording can't drift from should_run. + should_run = decide(forced, has_directory, already_tested)["should_run"] == "true" + + if has_directory and not should_run: + lines.append(f"`{label}/{driver_ref}/` exists — nothing to do.") + elif has_directory: + lines.append(f"`{label}/{driver_ref}/` exists; re-testing it on request.") + elif not should_run: + lines.append( + f"No `{label}/{driver_ref}/` directory, and the integration workflow has already run " + f"against this tag. Nothing to do until the directory is added; dispatch this workflow " + f"with `driver_ref: {driver_ref}` to test it again." + ) + else: + fallback = fallback_for(driver_ref, versions_dir) or "nothing" + # forced is always false on a pull_request run (workflow_dispatch inputs aren't populated), + # so this is the only should_run branch a pull_request run can actually reach. + if is_pull_request: + action = ( + "This is a pull_request run, so the integration job is skipped regardless; the " + f"schedule or a workflow_dispatch would run it against `{driver_ref}`." + ) + else: + action = ( + f"Running the integration workflow against `{driver_ref}` to find out whether that " + "still works." + ) + lines.append( + f"No `{label}/{driver_ref}/` directory, so the matrix would patch this tag with " + f"`{label}/{fallback}/`. {action}" + ) + + return "\n".join(lines) + "\n" diff --git a/tests/test_run_command.py b/tests/test_run_command.py index 2db64a4..034dde8 100644 --- a/tests/test_run_command.py +++ b/tests/test_run_command.py @@ -2,6 +2,7 @@ import subprocess import sys +import pytest REPO_ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(REPO_ROOT)) @@ -205,6 +206,19 @@ def test_legacy_driver_type_alias_uses_apache_versions(tmp_path): assert runner.version_folder == REPO_ROOT / "versions" / "apache" / "4.12.0" +def test_version_folder_falls_back_to_the_newest_directory_at_or_below_the_tag(tmp_path): + runner = make_runner(tmp_path, tag="4.19.1.5", driver_type="apache") + + assert runner.version_folder == REPO_ROOT / "versions" / "apache" / "4.19.1" + + +def test_version_folder_raises_below_every_defined_version(tmp_path): + runner = make_runner(tmp_path, tag="4.0.0", driver_type="apache") + + with pytest.raises(ValueError, match="4.0.0"): + runner.version_folder + + def test_environment_uses_java_11_for_add_exports_jvm_config(monkeypatch, tmp_path): java_home_11 = tmp_path / "jdk-11" java_home_11.mkdir() diff --git a/tests/test_upstream_release_watch.py b/tests/test_upstream_release_watch.py new file mode 100644 index 0000000..f4e9f34 --- /dev/null +++ b/tests/test_upstream_release_watch.py @@ -0,0 +1,148 @@ +import sys +from pathlib import Path + +import pytest +import yaml + + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT)) + +from scripts.upstream_release_watch import decide, fallback_for, resolve, summarize + + +@pytest.fixture(name="versions_dir") +def fixture_versions_dir(tmp_path): + versions_dir = tmp_path / "versions" / "apache" + for version in ("4.19.1", "4.19.2", "4.19.3"): + (versions_dir / version).mkdir(parents=True) + return versions_dir + + +def test_resolve_prefers_the_dispatched_tag_over_the_newest_release(versions_dir): + outputs = resolve("4.19.3", '["4.19.9"]', versions_dir=versions_dir) + + assert outputs == {"driver_ref": "4.19.3", "forced": "true", "has_directory": "true"} + + +def test_resolve_takes_the_newest_release_when_no_tag_is_dispatched(versions_dir): + outputs = resolve("", '["4.19.9"]', versions_dir=versions_dir) + + assert outputs == {"driver_ref": "4.19.9", "forced": "false", "has_directory": "false"} + + +def test_resolve_fails_when_no_tag_resolves(versions_dir): + for latest in ("", "[]"): + with pytest.raises(SystemExit): + resolve("", latest, versions_dir=versions_dir) + + +def test_resolve_fails_cleanly_on_malformed_json(versions_dir): + with pytest.raises(SystemExit): + resolve("", "not json", versions_dir=versions_dir) + + +def test_onboarded_tag_does_not_run(): + assert decide(forced=False, has_directory=True, already_tested=False)["should_run"] == "false" + + +def test_tag_without_a_version_directory_runs_once(): + assert decide(forced=False, has_directory=False, already_tested=False)["should_run"] == "true" + assert decide(forced=False, has_directory=False, already_tested=True)["should_run"] == "false" + + +def test_a_dispatched_tag_always_runs(): + for has_directory in (True, False): + for already_tested in (True, False): + outputs = decide(forced=True, has_directory=has_directory, already_tested=already_tested) + + assert outputs["should_run"] == "true", (has_directory, already_tested) + + +def test_fallback_names_the_newest_directory_at_or_below_the_tag(versions_dir): + assert fallback_for("4.19.9", versions_dir=versions_dir) == "4.19.3" + assert fallback_for("4.19.2", versions_dir=versions_dir) == "4.19.2" + + +def test_fallback_is_empty_when_nothing_applies(versions_dir): + assert fallback_for("4.18.0", versions_dir=versions_dir) == "" + assert fallback_for("4.19.3-rc1", versions_dir=versions_dir) == "" + + +def test_summary_names_the_directory_the_matrix_would_patch_with(versions_dir): + summary = summarize( + "4.19.9", forced=False, has_directory=False, already_tested=False, versions_dir=versions_dir + ) + + assert f"{versions_dir.as_posix()}/4.19.3/" in summary + + +def test_summary_points_at_a_dispatch_once_the_tag_has_been_tested(versions_dir): + summary = summarize( + "4.19.9", forced=False, has_directory=False, already_tested=True, versions_dir=versions_dir + ) + + assert "driver_ref: 4.19.9" in summary + assert "4.19.3" not in summary + + +def test_summary_says_the_pull_request_run_will_skip_when_should_run_is_true(versions_dir): + summary = summarize( + "4.19.9", + forced=False, + has_directory=False, + already_tested=False, + versions_dir=versions_dir, + is_pull_request=True, + ) + + assert "pull_request run, so the integration job is skipped" in summary + assert "Running the integration workflow" not in summary + + +def test_summary_defaults_to_the_non_pull_request_wording(versions_dir): + summary = summarize( + "4.19.9", forced=False, has_directory=False, already_tested=False, versions_dir=versions_dir + ) + + assert "Running the integration workflow" in summary + + +def test_watch_workflow_never_runs_integration_from_a_pull_request(): + workflow = yaml.safe_load( + (REPO_ROOT / ".github/workflows/upstream-release-watch.yml").read_text() + ) + + assert "github.event_name != 'pull_request'" in workflow["jobs"]["integration"]["if"] + + +def test_watch_workflow_queues_overlapping_triggers_instead_of_racing(): + workflow = yaml.safe_load( + (REPO_ROOT / ".github/workflows/upstream-release-watch.yml").read_text() + ) + + assert workflow["concurrency"]["group"] == "upstream-release-watch-${{ github.ref }}" + assert "cancel-in-progress" not in workflow["concurrency"] + + +def test_watch_workflow_marks_a_tag_only_after_a_conclusive_run(): + workflow = yaml.safe_load( + (REPO_ROOT / ".github/workflows/upstream-release-watch.yml").read_text() + ) + detect = workflow["jobs"]["detect"]["steps"] + lookup = next(step for step in detect if step.get("id") == "marker") + save = next( + step + for step in workflow["jobs"]["mark-tested"]["steps"] + if step.get("name") == "Save the tested marker" + ) + + # The two keys name the same tag through different contexts: steps.resolve inside detect, + # needs.detect from a separate job. + prefix = "upstream-release-tested-apache-" + assert lookup["with"]["key"] == f"{prefix}${{{{ steps.resolve.outputs.driver_ref }}}}" + assert save["with"]["key"] == f"{prefix}${{{{ needs.detect.outputs.driver_ref }}}}" + assert lookup["with"]["lookup-only"] is True + assert lookup["with"]["path"] == save["with"]["path"] + assert "needs.integration.result == 'success'" in workflow["jobs"]["mark-tested"]["if"] + assert "needs.integration.result == 'failure'" in workflow["jobs"]["mark-tested"]["if"]