[AI-6827] Add ddev release test-agent command#23722
Open
AAraKKe wants to merge 21 commits into
Open
Conversation
Dispatch both .github/workflows/test-agent.yml and test-agent-windows.yml against a release branch or tag, with Agent image resolution and validation against registry.datadoghq.com. The command resolves the latest published *-rc.N tag for a branch input, validates Linux and Windows (servercore) manifests, shows a confirmation panel, then fires both workflow_dispatch calls in parallel via the async GitHub client. Extends AsyncGitHubClient.create_workflow_dispatch with a typed return_run_details kwarg (overloads) so the new 200 response shape (workflow_run_id, run_url, html_url) is parsed into a WorkflowDispatchResult when requested, and the result panel links directly to each run.
Contributor
|
…and tests
- Rename _MANIFEST_ACCEPT to MANIFEST_ACCEPT per AGENTS.md (no underscore prefix on module constants).
- Move asyncio import to module level; drop duplicate import.
- Narrow _extract_run_urls signature to Sequence[GitHubResponse[WorkflowDispatchResult] | BaseException]; remove three type:ignore comments and the unused owner/repo/ref params.
- Surface the originating exception (__cause__) in the abort message after a dispatch failure.
- Combine messages when both Linux and Windows dispatches fail.
- tag.lstrip('v') -> tag.removeprefix('v') for accurate intent.
- Document REPO_OWNER design choice with a short comment.
- Add one-line docstrings to manifest_url and tags_list_url.
- Request a large page (n=10000) when listing registry tags; the Agent registry has many years of tags and the default page may not include the current release cycle.
- Type-annotate test fixtures and test functions.
- Parametrize test_branch_resolves_latest_rc over workflow_id; add a mirror test for the Linux-fails partial-dispatch case; add a both-fail case; cover 401/403/503 in the manifest error test; cover null and missing 'tags' key in the tags-list parser.
Review feedback:
- Pass inputs dict through to _print_plan so the plan display is derived
from the same source the dispatch sees (no more hardcoded 'true'/'false'
drifting from the inputs values).
- Use string values ('true', 'false') for type:boolean workflow inputs to
match what the GitHub workflow_dispatch API documents.
- Replace assert-based type narrowing in _extract_run_urls with nested
isinstance checks so the flow stays sound under python -O.
- Rename surviving_label -> failing_label in the parametrized partial-dispatch
test (the assertion targets the failing side's message, not the surviving).
CI fix:
- Reformat with ruff 0.11.10 (the version pinned by the ddev CI workflow);
my local hatch env shipped 0.15.11 which produced minor multi-line layout
differences. Affects test-agent.yml lint for both Linux and Windows
matrices.
- Abort early when github.token is empty instead of leaking the AsyncGitHubClient ValueError out of asyncio.run. - Abort with a friendly message when the public Agent registry returns a non-404 HTTP error from manifest_exists or list_agent_rc_tags, rather than surfacing the raw httpx traceback. - Narrow the inputs dict to dict[str, str] all the way through _dispatch_both / _dispatch_both_async; the workflow_dispatch API rejects non-string values, so an accidental non-string value now becomes a type error at the closest boundary to the API call. - Drop the cause-formatting suffix on the dispatch-failure abort; the RuntimeError messages already embed the underlying error so the cause would render twice. - Drop the unused ref parameter from _print_result. - Add tests covering timeout and connection-error propagation from the registry helpers.
- Introduce REPO_NAME='integrations-core' constant alongside REPO_OWNER and use it in the workflow_dispatch call, so a user with ddev pointed at integrations-extras/marketplace/a fork doesn't silently dispatch to DataDog/<wrong-repo>. Both test-agent.yml workflows only exist on integrations-core, matching the existing owner hardcoding rationale. - Lift the duplicate inline 'import httpx' to a top-level import so the module's external dependencies are visible at a glance. - Add tests for the empty-token guard and for the two registry HTTP error paths that translate to friendly app.abort messages.
Codecov Report❌ Patch coverage is Additional details and impacted files🚀 New features to boost your workflow:
|
- Move generic Docker Registry v2 helpers (manifest probe, tag listing with Link: rel=next pagination) into ddev.utils.docker_registry so they can be reused by other release tooling. The agent-specific RC filter stays in cli.release.test_agent.registry as a thin wrapper. - Read workflow files from origin/<branch> so a branch the user has not yet fetched no longer reads as a missing workflow file. Distinguish "file not in tree" from "ref not in local clone" from other git failures so the abort message points at the real problem. - Move httpx and asyncio imports back inside the functions that use them; the registry module is lazy-imported, so the top-level imports were paid on every ddev invocation (including ddev --help). - Use add_note to keep the Windows traceback attached when both dispatches fail. Wrap the result print in try/except/else so the success-only call site is obvious. - Take versions (not full image refs) into _validate_images_exist; drop the rsplit round-trip. - Note in the inputs dict that test-py2='false' on Windows is intentional.
- Fold the Linux and Windows error reprs into the RuntimeError message so app.abort(str(e)) renders both. The previous add_note(...) approach stored the Windows error in __notes__, which str(exc) does not include, so the Windows side was silently dropped from the abort output. - Re-raise CancelledError and other non-Exception BaseException subclasses from _extract_run_urls before treating results as dispatch failures. asyncio.gather(return_exceptions=True) captures cancellation into the result list; wrapping it in RuntimeError would hide flow-control intent. - Pass the github token directly into _dispatch_both instead of threading the whole Application object through. Decouples the helper. - Centralize the httpx.HTTPError -> app.abort translation in a _registry_errors context manager; removes the duplicated try/except in _resolve_version and _validate_images_exist and keeps the lazy httpx import in one place. - Render the dispatch plan via display_info so it lands on stderr alongside the surrounding progress lines; piping the command no longer splits the pre-dispatch narrative across stdout and stderr. - Replace the hand-rolled Link-header parser in docker_registry.list_tags with httpx.Response.links, which handles RFC 5988 quoting and multi-link rels correctly. - Strengthen test_both_dispatches_fail_combine_messages to assert both Windows: and a count of 2 of the shared error repr so the regression cannot recur.
Move the supporting logic for `ddev release test-agent` into sibling modules so the command file reads as the orchestration story it tells: - validation.py — input regex checks, git ref existence on origin, and workflow-file presence on the resolved ref (including the file-missing vs ref-not-fetched vs unknown-git-failure dispatch in the error path). - images.py — RC version resolution, image ref construction, manifest existence checks, and the registry_errors context manager that translates httpx errors into clean abort messages. - dispatch.py — the parallel workflow_dispatch orchestration. The async coroutine is nested inside dispatch_both so the reader sees the full flow in one function rather than bouncing between two near-empty stack frames. extract_run_urls keeps the partial/total-failure surface next to the dispatch. `__init__.py` now contains only the Click command plus the small `_print_plan`/`_print_result` display helpers. Every sibling module is imported lazily inside the command body so `ddev --help` only pays for `click` from this package. Also narrow `AsyncGitHubClient.create_workflow_dispatch`'s `inputs` parameter from `dict[str, Any] | None` to `dict[str, str] | None` across both `@overload`s and the implementation. The workflow_dispatch API contract is string-to-string (booleans are matched against the lowercase string form), every in-tree caller already passes a `dict[str, str]`, and the wider type silently accepted values that would surface as runtime 422s from GitHub. The fake test client mirror is updated to match.
Make the user's `--branch` or `--tag` choice a typed `Branch | Tag` produced by `validate_input`, and have every downstream helper take that `ReleaseTarget` instead of `(branch: str | None, tag: str | None)`. This puts the "exactly one is set" invariant in the type system and removes the type-narrowing `assert`s that would otherwise turn into an `AssertionError` with no context if anyone broke the invariant. While at it, drop the `git ls-remote` probe + "please run `git fetch` and try again" hint and fetch the ref ourselves. The new `fetch_target` runs `git fetch --quiet --depth=1 origin refs/heads/<branch>:refs/remotes/origin/<branch>` (or the equivalent `refs/tags/...:refs/tags/...` for `--tag`), which both confirms the ref exists on origin and populates the local refs we need to read the workflow files. If `git fetch` reports `couldn't find remote ref`, the abort message stays the same as before (`Branch X not found on origin`); other git errors surface verbatim through `Failed to fetch ... from origin: ...`. Tests now mock `GitRepository.run` (the fetch) instead of `GitRepository.capture` (the ls-remote). Two new tests pin the exact refspec the command must send so a future refactor that breaks the fetch shape can't slip through. The "please-fetch-first" test goes away; that branch is unreachable now.
…celledError - Move WORKFLOW_LINUX/WORKFLOW_WINDOWS from dispatch.py to validation.py and have dispatch.py import them from validation. Inverts the previous arrow so the layer that runs first owns the constants; validation no longer pulls in dispatch's async/HTTP modules at import time. - Make fetch_target's OSError handler use `if/else` for symmetry with verify_workflows_present_on_ref. Behavior unchanged; the two helpers now read identically instead of relying on `app.abort`'s `NoReturn` to keep the trailing abort unreachable. - Add direct unit tests for extract_run_urls. The existing test_command tests only feed httpx.HTTPStatusError (an Exception), so the BaseException-but-not-Exception re-raise that lets CancelledError / KeyboardInterrupt propagate was untested in CI. New tests pin both the flow-control propagation contract and the both-failure message shape.
…blocks - Add blank-line separators between each phase of the command (fetch, version resolution, image validation, dispatch plan, final result) so the output reads as distinct sections rather than one continuous stream. - Replace the trailing `display_success` + two `display_pair` calls with a rich Panel matching the look of `ddev release port-commit`'s completion summary. The two run URLs sit inside a cyan-bordered "Workflows dispatched" panel with bold-aligned Linux/Windows labels.
AAraKKe
changed the title
AAraKKe
force-pushed
the
aarakke/ddev-release-test-agent
branch
3 times, most recently
from
f2a6169 to
8e21071
Compare
AAraKKe
force-pushed
the
aarakke/ddev-release-test-agent
branch
from
8e21071 to
95cf0ea
Compare
- dispatch.py: declare DispatchOutcome as a PEP 695 type alias so mypy recognises it as a type (the previous TYPE_CHECKING-guarded assignment read as a variable on mypy 2.1). - validation.py: validate both branch/tag invariants up front and assert the remaining one is set, removing the trailing app.abort that mypy refused to credit as a function terminator.
- Move REPO_OWNER/REPO_NAME from dispatch.py to validation.py so monitoring no longer pulls dispatch's async/HTTP module just for two strings. - Narrow monitor_workflows / monitor_dispatched_workflows first parameter from Application to Terminal — matches what the body actually uses and what the tests already pass. - Tighten extract_dispatched_workflows input to tuple[DispatchOutcome, DispatchOutcome] so the 2-result contract is explicit at the type level. - Widen the monitor_dispatched_workflows guard from RuntimeError to Exception so httpx network failures abort cleanly with the standard "Failed to monitor workflows" message. - Guard list_tags against non-JSON 2xx responses so registry_errors translates them into a clean abort instead of a raw ValueError.
- Tolerate transient httpx errors during polling: keep the prior monitor state instead of aborting the entire monitor on one bad request. - Emit a single final panel in both interactive and non-interactive paths so CI logs no longer get flooded with per-poll panels. - Move DispatchedWorkflow under TYPE_CHECKING in monitoring.py so importing monitoring does not eagerly load dispatch and its async machinery. - Surface the dispatched-run html_urls in the monitor-failed abort message so the user can resume monitoring manually. - Tighten test_dispatch fixtures from list[...] to tuple[...] to match the production signature. - Annotate the FakeAsyncGitHubClient.list_workflow_run_jobs stub with AsyncIterator[GitHubResponse[Any]] for consistency.
Contributor
Validation ReportAll 21 validations passed. Show details
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds a new
ddev release test-agentcommand that dispatches both Agent test workflows (test-agent.ymlandtest-agent-windows.yml) against a release branch or tag.The command:
--branch 7.80.xor--tag 7.80.0-rc.1/7.80.0(mutually exclusive). Also accepts a leadingvon tags.originand that both workflow files are present at that ref.--tag X-> use X.--branch MAJ.MIN.x-> queryregistry.datadoghq.com/v2/agent/tags/listand pick the highest publishedMAJ.MIN.0-rc.N.agent:<version>) and Windows (agent:<version>-servercore) manifests exist inregistry.datadoghq.com.--yes, or just resolved with--dry-run).workflow_dispatchcalls in parallel via the async GitHub client and prints a result panel with each run'shtml_url.Also extends
AsyncGitHubClient.create_workflow_dispatchwith areturn_run_detailskwarg (typed via@overload), so callers that opt in get aGitHubResponse[WorkflowDispatchResult]with non-optionalworkflow_run_id/run_url/html_url. The default (False) keeps the existing 204 No Content path untouched. See the GitHub changelog.Motivation
Manually dispatching both test workflows from the GitHub UI for each Agent RC is fiddly. The Windows image suffix (
-servercore) is easy to forget, and there is no easy way to know which RC has actually been published. This command resolves and validates the right images once and triggers both runs in a single call.Address AI-6827
Review checklist (to be filled by reviewers)