Repository navigation
Conversation
nevzheng
force-pushed
the
claude/14117-container-accessors
branch
2 times, most recently
from
October 22, 2026 03:28
cac9a3e to
c24289f
Compare
…st libraries each module uses
…s built-in store cleanup
Design doc for apache#13900 and a rumdl config for design-docs/testing.
…e ContainerSuite lifecycle - Log every container start with pull, start and readiness times; add test log4j configs so the lines show. - Reset all ContainerSuite state in close(); key Hive by version; make the PostgreSQL retry wait; pin StarRocks; drop a dead start method. - Poll readiness with no initial delay; run cleanDisk() only on real starts. - idp-basic starts its containers once per run; Ryuk back on, unprivileged. - Drop the suite-wide closes in Hive2, Flink and Spark ITs.
…ve readiness without the CLI
… calls with in-process clients
nevzheng
force-pushed
the
claude/14117-container-accessors
branch
from
October 22, 2026 04:29
c24289f to
286aafd
Compare
…est containers (part 1/6) ContainerSpec, ContainerExtension and @FixtureContainer: a test declares a container with a fixture annotation and receives it as a parameter or field. Fixtures for Kafka, LocalStack, RustFS, StarRocks, OceanBase and Doris, with their callers migrated.
…o, Ranger and Hive (part 2/6) - TestImages declares every test image in one place. - BaseContainer subclasses can supply their own container (ready for the official Testcontainers modules). - Moto, Ranger and Hive (version, auth) fixtures. - A container whose start fails is closed instead of leaking.
…add ClickHouse fixtures and scope (part 3/6) - client-java, fileset, Hudi, Iceberg, Paimon, Iceberg REST, GVFS, Flink, Spark and Trino env tests use @HiveFixture. - ClickHouse (1 or 3 nodes) and ZooKeeper fixtures. - scope = RUN | CLASS | METHOD; HadoopUserAuthenticationIT gets a private Kerberos Hive.
…and PostgreSQL fixtures (part 4/6) - Review follow-ups from the first fixture changes. - A fixture annotation on a field picks that field's container. - startup = EAGER | LAZY; a warning for fixtures no test injects. - MySQL and PostgreSQL fixtures; SparkQueryRunner starts Hive through its spec. - Only the run's closer closes the container suite. - Split container start time into pull, start and readiness; give the Hive metastore check a real time budget.
…remaining Hive variants (part 5/6) - ContainerSpec.dependencies(): dependencies start first, deepest first, with a cycle check. - ClickHouseCluster injected for the three-node cluster. - Hive auth = RANGER and storage = S3 | GCS | ABS; the CatalogHive IT family and the Ranger ITs use them. - HiveUserAuthenticationIT on the Kerberos Hive fixture.
…th TrinoCluster (part 6/6) TrinoCluster starts the Trino stack through Testcontainers instead of launch.sh and docker compose, with real readiness checks and Ryuk cleanup. @TrinoFixture starts lazily and reads the Gravitino server port from the test.
…ners per fixture - durability = DISPOSABLE (default) | DURABLE on every fixture; each container applies its own recipe at start. - Hive keeps HDFS on tmpfs and its metastore MySQL skips per-commit flushes, for consistency (no measurable gain on a local Mac); ZooKeeper sets forceSync=no; MySQL and PostgreSQL use apache#13899's settings everywhere. - Ranger keeps its policies in the shared MySQL. - -PdurableTestContainers makes every container durable, for debugging. Builds on apache#13899.
…-private shared containers Every test now gets its containers through a spec. Test classes declare fixture annotations; code that cannot carry one (BaseIT's Gravitino backend, which the core database lanes use, idp-basic's backend parameter, RangerITEnv, SparkQueryRunner) calls Fixtures.get(spec), and tests that build a container of their own use Fixtures.network(). ContainerSuite becomes the package-private SharedContainers, used only by specs and ContainerExtension: the same images, environment, host names, network, one container per key per run, run-end close and timing lines. Its unused starts (the single Trino container, default Doris and PostgreSQL overloads) go away; disabled TrinoConnectorIT builds its Trino container itself. TestContainerSuite becomes TestSharedContainers.
…ork per-run names
…containers wait strategies Move the post-start poll loops of Hive, Doris, Ranger, StarRocks, OceanBase, RustFS, ClickHouse, Kafka and Trino into wait strategies: the port or log check composed with a ProbeWaitStrategy for checks that need a client. Startup timeouts are the old port budget plus the old poll budget. Readiness time now shows in containerMs of the [CONTAINER-TIMING] line.
…nd OceanBase on official Testcontainers modules
…tainers onto fixtures
…ontainer and durable data
… for it without commands in the container
…ners for readiness and logs
…container start instead of with kadmin in the container
…sors, with a managed or external backing
nevzheng
force-pushed
the
claude/14117-container-accessors
branch
from
October 22, 2026 16:53
286aafd to
9f3192a
Compare
yuqi1129
added a commit
that referenced
this pull request
Oct 23, 2026
### What changes were proposed in this pull request? Publish Fork CI and ASF Allowlist Check as commit statuses with direct run links, retaining custom checks for run/attempt metadata. Repair missing or stale statuses even when the metadata check is unchanged. PR signals and dispatches are processed in order in one queue (`queue: max`). Sweeps poll every minute for 11 minutes, in their own job and concurrency group, so signals never wait behind them. Each polling sweep then dispatches the next one from a separate `rearm` job, the only job with `actions: write` (it checks out and runs no repository code), so polling is continuous instead of following GitHub's 10-36 minute schedule gaps; the schedule restarts the chain if it breaks. Sweeps spend at most 40% of the hourly API quota; with a small quota they stay single-pass scheduled sweeps without a chain. Within ASF limits this is about one `ubuntu-slim` runner, roughly 4% of the weekly allowance; every publisher derives the commit status from the stored check, and the next pass repairs any interleaving. Make results trustworthy for contributors: - The fail-fast watcher retries transient API errors and never fails the run (`continue-on-error`). A run that failed only because of the watcher is judged by its suites. On #13778 a single `Server Error` in the watcher turned a run whose suites all passed into `Failed: Stop on first failure`. - A fork that never ran `fork-ci.yml` (its branch predates Fork CI) gets an `action_required` check asking to merge the latest base branch. Previously the 404 left such PRs without any check and failed every scheduled sweep (#14118, #14177-#14187: 609 `Not Found` warnings in 25 sweeps). - A successful fork run that edits CI definitions is pending, not red, while it waits for trusted verification. Let contributors restart and stop Fork CI themselves. `apache/gravitino` cannot start or cancel runs in a contributor's fork, so reopening a PR cannot restart CI as it did before the migration: - **Run workflow** in the fork (no PR number) starts a new run of the selected branch against the latest base branch, like a push, and cancels the branch's run in progress (push and dispatch share one cancel-in-progress group). The publisher mirrors these runs too. No new commit, rebase or merge is needed; **Re-run all jobs** also tests the latest base branch. - The fail-fast watcher checks the upstream PR every minute and cancels its fork run once the PR is closed. - The **Fork CI** check explains these options with links to the run and to **Run workflow**, and notes that its own **Re-run** button only refreshes the status. CONTRIBUTING describes push, Run workflow, re-run, close and reopen. Keep CI execution progress separate from approval of CI definition changes. A running fork remains pending; a successful fork that edits CI definitions explicitly requests trusted verification. Preserve actual failures, repair premature terminal checks, and observe both fork and trusted reruns before retaining a completed result. Finalize unfinished checks for confirmed canceled previous heads, retry interrupted history-status publication, and remember completed retirements to avoid repeatedly querying their attempts. History work examines at most ten recent branch runs and follows current-head publication. Resolve trusted runs against the default branch's current SHA rather than the PR's older base snapshot. Verify their exact contributor head and tested base, so an older trusted result cannot obscure a newer head's progress. ### Why are the changes needed? On #13506, a running custom check was absent from the latest-check view. On #14190, an existing successful allowlist check did not satisfy the expected commit-status context. On #14192, a fork still running was prematurely marked `action_required` because the PR changed CI definitions. Real-environment testing also reproduced stale run links and trusted dispatch rejection after the base branch advanced. Commit statuses are append-only, so a publisher writes the status from the check as stored, not from its own snapshot. During rollout, drain publisher runs created before this change: those runs retain their old concurrency and permission configuration. GitHub queues up to 100 runs per group; sweeps recover missed Fork CI signals, and committers can manually refresh either publisher. Scheduling and runner delays still affect update latency. Document close/reopen behavior and the exact trigger steps in `CONTRIBUTING.md`: cancel in the fork to stop a run, reopen to refresh results, and rerun in the fork or push a new/empty commit to start CI. **Update Fork CI Status** only refreshes results. ### Does this PR introduce _any_ user-facing change? PRs report queued/running progress with the corresponding run link. Failed jobs remain visible. Successful forks with CI edits report **Fork CI passed; trusted verification required** until verified by a trusted run. Completed old check-run links retain historical results; unfinished previous-head checks are finalized after their exact fork attempt confirms cancellation. The current PR rollup follows the latest check and commit status. ### How was this patch tested? - 86 Node tests pass. Polling is faster than main (one-minute passes in back-to-back sweeps instead of two-minute passes in sweeps 10-36 minutes apart), and each pass still probes every recently updated PR at the apache quota: the trusted-run listing is shared within a pass, settled statuses are verified once per sweep, and history retirement runs only after a refresh (a regression test fails if any of these is reverted). New regressions: transient watcher API errors and failed cancel requests, watcher-only failures judged by the suites, missing fork workflow (mirror, sweep probe and history retirement), status derived from a check stored by a concurrent writer, six polling passes covering all 60 recently updated PRs, and pending trusted verification. Each fix was reverted once to confirm a test fails. - Read-only run of the new `mirror()` against the real API: #13778 changes from `Failed: Stop on first failure` to `All required suites passed`, #14187 from an unhandled 404 to the missing-workflow check; #14128 and #13506 are unchanged. - Live self-service test with the smoke PR #14150 (head = this PR plus an empty commit): the push started [a fork run](https://github.com/yuqi1129/gravitino/actions/runs/39938622027); **Run workflow** in the fork without a PR number started [a new run](https://github.com/yuqi1129/gravitino/actions/runs/39938650089) whose eligibility resolved the PR and cancelled the push run. After #14150 was closed at 10:40:30, the watcher reported `Cancelling the run because #14150 was closed` at 10:41:27 and the run was cancelled. - Live chain probe in the sandbox (timelyflow/gravitino, same job layout capped at three hops, then removed): two chains started together ran seven sweeps strictly one at a time with 3-7 second gaps; each `rearm` dispatched the next sweep with `GITHUB_TOKEN`, and the chain stopped when the sweep output `chain=false` ([first](https://github.com/timelyflow/gravitino/actions/runs/39934761179), [last](https://github.com/timelyflow/gravitino/actions/runs/39935318794)). - Real GitHub probe in yuqi1129/gravitino: three pushes of a job with job-level `concurrency.queue: max` ran one after another ([1](https://github.com/yuqi1129/gravitino/actions/runs/39932213234), [2](https://github.com/yuqi1129/gravitino/actions/runs/39932222889), [3](https://github.com/yuqi1129/gravitino/actions/runs/39932233971)); a failing `continue-on-error` job left the run successful; in a group without `queue`, a newer pending job replaced the older one, as sweeps intend. - Before the latest fix commit, 69 Node tests passed. Independent fresh-context review of documentation head `c44aa723e1ed13cc935294242bad6063fc38551b` found no Blocker/Important. The publisher code is unchanged from `0ab290ae`, whose seven critical mutations were rejected. Regression mutations reject the old premature-error logic, stale default-branch resolution and settled-success shortcut that ignored trusted reruns. - Changed YAML passes actionlint 1.7.12 with its unsupported `concurrency.queue` diagnostic excluded. Real queued publishers executed successfully. - Actual GitHub Actions and GitHub Actions app tokens were used in timelyflow/gravitino PR #18 with contributor pushes to yuqi1129/gravitino. Caller jobs were short test fixtures; publisher code matched this PR, with repository identity adapted. The final scheduled-handler probe used the final publisher without moving the test base. | Real scenario | Observed result | Evidence | | --- | --- | --- | | CI edits while fork jobs run | Reproduced the old completed/error result; repaired the same attempt to `IN_PROGRESS` / `PENDING`, with the same fork URL in the PR rollup | [fork](https://github.com/yuqi1129/gravitino/actions/runs/39912299769), [legacy publisher](https://github.com/timelyflow/gravitino/actions/runs/39912376668), [repair](https://github.com/timelyflow/gravitino/actions/runs/39912544166) | | Cancellation with failed Required CI | Reports failure and names the failed job | [publisher](https://github.com/timelyflow/gravitino/actions/runs/39912797462) | | Advanced base / merge conflict | Reproduced stale-base dispatch rejection; corrected resolution reached frozen merge preflight and reported the actual conflict | [old rejection](https://github.com/timelyflow/gravitino/actions/runs/39912794573), [conflict](https://github.com/timelyflow/gravitino/actions/runs/39913212924) | | New contributor head / successful CI edits | Previous-head trusted results no longer hide progress; fork success still requests trusted verification | [new fork](https://github.com/yuqi1129/gravitino/actions/runs/39913693166), [publisher](https://github.com/timelyflow/gravitino/actions/runs/39913854040) | | Trusted success / full rerun | Exact-head trusted success is green; attempt 2 returns the PR to pending, then green | [trusted attempts](https://github.com/timelyflow/gravitino/actions/runs/39913976321), [running refresh](https://github.com/timelyflow/gravitino/actions/runs/39914440218), [recovery](https://github.com/timelyflow/gravitino/actions/runs/39914573361) | | Same-color stale URL | Injected success pointing to an older run; scheduled handler corrected the URL without recreating the metadata check | [injection](https://github.com/timelyflow/gravitino/actions/runs/39914635708), [repair](https://github.com/timelyflow/gravitino/actions/runs/39914691920) | Additional push/close/reopen lifecycle tests used real GitHub events and the same publisher code: | Scenario | Observed result | Evidence | | --- | --- | --- | | Push B while A is running | GitHub automatically canceled A and started B; automatic PR synchronization pointed to B's pending status | [A, canceled](https://github.com/yuqi1129/gravitino/actions/runs/39918238583), [B](https://github.com/yuqi1129/gravitino/actions/runs/39918372924) | | Close while B runs, then reopen | B continued and succeeded while closed; reopening automatically published success for the same run and attempt, with no new fork run | [B attempt 1](https://github.com/yuqi1129/gravitino/actions/runs/39918372924/attempts/1) | | Cancel B while closed, then reopen | Reopening automatically reported attempt 2's failed Required CI; it did not silently restart or retain the old success. Manually rerunning failed jobs recovered success in attempt 3 | [B attempts 2–3](https://github.com/yuqi1129/gravitino/actions/runs/39918372924) | | Previously stuck old-head check | Reproduced A's old upstream check remaining running after cancellation; the fixed publisher finalized it as cancelled/error while B remained successful | [retirement](https://github.com/timelyflow/gravitino/actions/runs/39920132510) | | Settled history cost | A subsequent real publisher fetched zero historical workflow attempts, rather than repeatedly fetching already-retired attempts | [settled probe](https://github.com/timelyflow/gravitino/actions/runs/39920184500) | Upstream `GITHUB_TOKEN` is repository-scoped and cannot directly control contributors' fork runs. Closing a PR does **not** cancel a push-triggered fork run; while closed, PR status publishing stops. Reopening refreshes its existing run and does **not** start CI. Running CI again requires a fork rerun or another push. These are current capabilities, not a claim that close/reopen remotely controls contributors' Actions. Earlier live tests also covered [failure and partial-rerun recovery](https://github.com/yuqi1129/gravitino/actions/runs/39902849966), [allowlist rejection](https://github.com/timelyflow/gravitino/actions/runs/39903961810), and [status-write permission failure](https://github.com/timelyflow/gravitino/actions/runs/39904765246) followed by [scheduled-handler repair](https://github.com/timelyflow/gravitino/actions/runs/39907658537). The final repair commit also triggered the actual [full Fork CI](https://github.com/yuqi1129/gravitino/actions/runs/39915782013); its [build / changes contract-test step](https://github.com/yuqi1129/gravitino/actions/runs/39915782013/job/120289143994) passed. The run was then canceled to avoid long application builds; full application suites were not completed by these publishing tests. Scheduled-handler tests used explicit dispatch wrappers and do not establish native cron delivery. Native cron delivery and authenticated browser screenshot validation remain unverified. Actual check runs, commit statuses, PR rollups and links were inspected. The production PR continues to use the default branch's old publisher until this change is merged. Accepted review tradeoffs: settled history still needs one branch-run list and up to ten check lookups per visited PR. Its marker avoids repeated attempt/PR/status requests, rather than eliminating all history reads. Unsettled history needs additional validation and writes; current-head publication happens first. Recent probes and full publication re-read trusted-run listings to reduce stale observations. The current upstream list is small and sweeps retain their API budget; paginated calls can exceed individual request estimates, so this is not a strict per-PR request cap. Temporary default branch, PR base and contributor refs are restored after testing; temporary branches are removed and scoped runs are canceled. No secrets are added. The lifecycle repair commit also triggered [full Fork CI](https://github.com/yuqi1129/gravitino/actions/runs/39920441092); its [build / changes contract tests](https://github.com/yuqi1129/gravitino/actions/runs/39920441092/job/120304312035) passed all 69 tests before the remaining long suites were canceled. The latest documentation-only commit `c44aa723e1ed13cc935294242bad6063fc38551b` adds a lifecycle table, fork Actions rerun instructions and an empty-commit push example. Publisher/workflow/test code is unchanged from `0ab290ae`; Markdown structure, shell-example syntax and `git diff --check` were verified. Independent review reran all 69 Node tests on this exact documentation head. --------- Co-authored-by: yuqi <yuqi4733@protonmail.com>
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
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 changes were proposed in this pull request?
Tests can ask a container for its addresses instead of building them from its Docker IP, and the same container can point at a service running elsewhere. This is step 1 of #14117.
HiveContainer.metastoreUri(),hdfsUri(),hdfsConfiguration(),KafkaContainer.bootstrapServers()and the JDBC URL onMySQLContainer. Managed containers answer withgetHost()and mapped ports, or the official module's own accessors.containers: externalor<service>.external: true;SharedContainersthen builds the container without starting it, and its accessors read the file. Docker-only operations (exec, logs, restart) fail with a clear message.gravitino.test.endpoints.file(or-Pgravitino.test.profile=<name>fordev/test-endpoints/<name>.yaml) is read with Jackson YAML into the records inTestEndpoints, which are the only declaration of the keys (service, optionalinstances.<name>, key; lists forbootstrap-servers, maps for*-client-conf).-Pgravitino.test.<service>[.<instance>].<key>(Gradle forwards every-Pgravitino.test.*as a system property) andGRAVITINO_TEST_<SERVICE>_..._<KEY>override the file. Precedence: property > env > file.test-endpoints.schema.jsonis generated from the records with victoolsjsonschema-generatorand checked in;external-endpoints.example.yamlpoints editors at it with a# yaml-language-server: $schema=...modeline; the key table indesign-docs/testing/containers/external-endpoints.mdis generated from the same records. Drift tests fail when either is stale.ExternalEndpoints.forService(service, instance)reads a named instance's block whole and falls back to the service's own keys;ContainerSpec.service()/instance()default to empty.HiveContainer.Builder.withHostReachableHdfs()gives the container the hostnamelocalhostand publishes the NameNode and DataNode on the same free port inside and outside, so anhdfs://address works from the test and from the metastore in the container.CatalogKafkaITandProxyCatalogHiveIT.ContainerExtension, Specs,Fixturesand the fixture annotations are unchanged.Why are the changes needed?
About 97 test files build
ip:portfrom the Docker bridge IP. That only works where the test JVM can route to Docker's network, which is why macOS needs mac-docker-connector and a fixed subnet and allows one test run at a time, and it ties every test to containers on local Docker. Testcontainers' own mechanism is host plus mapped port; the accessors use it, and the external backing lets the same tests run against services deployed elsewhere, such as a Kubernetes test cluster, from a file of addresses.Part of #14117
Does this PR introduce any user-facing change?
No. Test code only.
How was this patch tested?
integration-test-commonunit tests: 177 pass, 10 of them inTestExternalEndpoints(precedence, flat lists and maps, missing-key messages, typo keys in the file, a property and the environment, instance fallback, the example and profiles binding, schema and docs-table drift).CatalogKafkaIT: 10/10 external againstdocker run -p 9092:9092 apache/kafka:3.7.0with-Pgravitino.test.kafka.external=true -Pgravitino.test.kafka.bootstrap-servers=localhost:9092, and 10/10 with-Pgravitino.test.profile=local-kafka.CatalogKafkaIT10/10 managed;ProxyCatalogHiveIT3/3 managed with host-reachable HDFS (0/3 with plain mapped ports, which motivated it) and 3/3 external against a Hive started by hand.compileTestJava,spotlessCheckandcheckstyleTestforintegration-test-common,catalog-hiveandcatalog-kafka.Based on the #13900 stack (#14094); review the last commit only until the stack lands.