From e6c69419ed71d75457e8848acb7a22c1bd582b4e Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Thu, 30 Jul 2026 11:09:12 -0700 Subject: [PATCH 01/15] Clean up Cosmos test databases left behind by CI runs Six live test stages in sdk/cosmos/tests.yml run against long lived shared accounts (thin client, thin client canaries, GSI) that are never torn down and are used by several matrix legs and pipeline runs concurrently. Databases a test forgot to delete stayed on those accounts permanently: - cleanupStaleTestDatabases only recognized ids starting with "RxJava.SDKTest.SharedDatabase", so tests naming databases with raw UUIDs or fixed literals leaked forever. - Cleanup only ran at @AfterSuite, so a cancelled job or one hitting its 210 minute timeout killed the JVM and leaked everything it had created. Every test created database id now carries a run id derived from System.JobId, which lets cleanup delete exactly what the current run created without touching resources a concurrently executing leg is still using. Cleanup runs in four layers: an in-JVM registry plus a TestNG listener that deletes leftovers and fails the run naming the offending test, a JVM shutdown hook, an always() post step per stage, and a new six-hourly janitor pipeline for jobs that died before anything in them could run. To stop new tests reintroducing the problem, TestSuiteBase gains sanctioned createTestDatabase helpers that name and register automatically, the arbitrary id creators are deprecated, and TestResourceHygieneTest ratchets against direct database creation using a checked in baseline. Also fixes a latent NPE and an unsafe 2 hour cleanup threshold in the azure-cosmos copy of DatabaseForTest, which the new four segment ids would otherwise have triggered. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- sdk/cosmos/AGENTS.md | 61 +++ sdk/cosmos/azure-cosmos-tests/pom.xml | 12 + .../azure/cosmos/CosmosDatabaseForTest.java | 222 ++++++++- .../cosmos/CosmosDatabaseForTestTest.java | 267 +++++++++++ .../azure/cosmos/CosmosDiagnosticsTest.java | 9 +- .../cosmos/CosmosTestAccountJanitor.java | 193 ++++++++ .../cosmos/CosmosTestResourceJanitor.java | 420 ++++++++++++++++++ .../cosmos/CosmosTestResourceJanitorTest.java | 202 +++++++++ .../cosmos/CosmosTestResourceRegistry.java | 194 ++++++++ .../com/azure/cosmos/CosmosTestRunId.java | 134 ++++++ .../EndToEndTimeOutValidationTests.java | 2 +- ...tionWithAvailabilityStrategyTestsBase.java | 2 +- .../com/azure/cosmos/MaxRetryCountTests.java | 2 +- .../azure/cosmos/ResourceTokenTestForV4.java | 3 +- .../azure/cosmos/TestResourceHygieneTest.java | 261 +++++++++++ .../MetadataRequestRetryPolicyTests.java | 3 +- .../com/azure/cosmos/rx/DocumentCrudTest.java | 4 +- ...GatewayReadConsistencyStrategyE2ETest.java | 3 +- ...wayReadConsistencyStrategySpyWireTest.java | 3 +- .../com/azure/cosmos/rx/OfferQueryTest.java | 4 +- .../azure/cosmos/rx/OfferReadReplaceTest.java | 4 +- .../azure/cosmos/rx/PermissionQueryTest.java | 4 +- .../azure/cosmos/rx/ReadFeedOffersTest.java | 4 +- .../cosmos/rx/ReadFeedPermissionsTest.java | 4 +- .../azure/cosmos/rx/ResourceTokenTest.java | 4 +- .../com/azure/cosmos/rx/TestSuiteBase.java | 112 ++++- .../rx/WorkloadIdDirectInterceptorTests.java | 3 +- .../azure/cosmos/rx/WorkloadIdE2ETests.java | 3 +- .../IncrementalChangeFeedProcessorTest.java | 7 +- .../IncrementalChangeFeedProcessorTest.java | 5 +- .../src/test/resources/cfp-split-testng.xml | 1 + .../circuit-breaker-misc-direct-testng.xml | 1 + .../circuit-breaker-misc-gateway-testng.xml | 1 + ...cuit-breaker-read-all-read-many-testng.xml | 1 + .../consistency-overrides-testng.xml | 1 + .../src/test/resources/direct-testng.xml | 1 + .../src/test/resources/e2e-testng.xml | 1 + .../src/test/resources/emulator-testng.xml | 1 + .../test/resources/emulator-vnext-testng.xml | 1 + .../src/test/resources/examples-testng.xml | 1 + .../src/test/resources/fast-testng.xml | 1 + .../fi-customer-workflows-testng.xml | 1 + .../test/resources/fi-multi-master-testng.xml | 1 + .../fi-sm-customer-workflows-testng.xml | 1 + .../fi-thinclient-multi-master-testng.xml | 1 + .../fi-thinclient-multi-region-testng.xml | 1 + .../resources/flaky-multi-master-testng.xml | 1 + .../src/test/resources/gsi-testng.xml | 1 + .../test/resources/long-emulator-testng.xml | 1 + .../src/test/resources/long-testng.xml | 1 + .../manual-http-network-fault-testng.xml | 1 + .../test/resources/multi-master-testng.xml | 1 + .../test/resources/multi-region-strong.xml | 1 + .../test/resources/multi-region-testng.xml | 1 + .../src/test/resources/query-testng.xml | 1 + .../src/test/resources/split-testng.xml | 1 + .../test-resource-hygiene-baseline.properties | 64 +++ .../thinclient-endpoint-probe-testng.xml | 1 + .../src/test/resources/thinclient-testng.xml | 1 + .../implementation/DatabaseForTest.java | 31 +- sdk/cosmos/cleanup-test-resources.yml | 39 ++ sdk/cosmos/dev.md | 5 + sdk/cosmos/janitor.yml | 100 +++++ sdk/cosmos/tests.yml | 42 ++ 64 files changed, 2396 insertions(+), 64 deletions(-) create mode 100644 sdk/cosmos/AGENTS.md create mode 100644 sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java create mode 100644 sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestAccountJanitor.java create mode 100644 sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitor.java create mode 100644 sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java create mode 100644 sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java create mode 100644 sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java create mode 100644 sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/TestResourceHygieneTest.java create mode 100644 sdk/cosmos/azure-cosmos-tests/src/test/resources/test-resource-hygiene-baseline.properties create mode 100644 sdk/cosmos/cleanup-test-resources.yml create mode 100644 sdk/cosmos/janitor.yml diff --git a/sdk/cosmos/AGENTS.md b/sdk/cosmos/AGENTS.md new file mode 100644 index 000000000000..7fbde9ffc0d9 --- /dev/null +++ b/sdk/cosmos/AGENTS.md @@ -0,0 +1,61 @@ +# AGENTS.md — Azure Cosmos DB SDK for Java + +This file provides Cosmos-specific guidance for AI agents working in the `sdk/cosmos` subtree. +For general repo-wide guidance, see the [root AGENTS.md](../../AGENTS.md). + +## Test resource hygiene (required for any test that touches an account) + +Six live test stages in `tests.yml` run against **long lived shared accounts** that are never torn +down and are used by several matrix legs concurrently. Databases a test forgets to delete stay on +those accounts forever. + +When adding or changing tests in `azure-cosmos-tests`: + +- **Create databases with `TestSuiteBase.createTestDatabase(client, "label")`**, never with + `UUID.randomUUID()`, a fixed literal, or a raw `client.createDatabase(...)` call. The helper embeds + the CI run id in the id so cleanup can attribute the database to this run without deleting another + run's in-flight resources. `TestResourceHygieneTest` fails the build on direct database creation. +- **Still delete what you create** in an `@AfterClass(alwaysRun = true)` using `safeDeleteDatabase`. + `CosmosTestResourceJanitor` cleans up leftovers but **fails the run** when it has to. +- Containers inside a database you delete need no separate cleanup. + +### How cleanup works + +| Layer | Where | Catches | +| --- | --- | --- | +| `CosmosTestResourceRegistry` + `CosmosTestResourceJanitor` | in the test JVM, at the end of the run | normal completion; deletes leftovers and **fails the run** naming the offending test | +| JVM shutdown hook | in the test JVM | crashes and hard aborts | +| `cleanup-test-resources.yml` post step | pipeline, `condition: always()` | failed jobs, and jobs whose JVM died. Runs on cancellation too, but only within `cancelTimeoutInMinutes` (5 by default), so treat it as best effort there | +| `janitor.yml` | scheduled every 6h | cancelled and timed-out jobs, where nothing in the job ever ran | + +Every layer scopes deletion by the run id (`CosmosTestRunId`), derived from `System.JobId` (a GUID that +is unique per job), with the build id carried along so a stray database can be traced back to a build. + +When a leak fails a run, TestNG has already written its reports by the time the janitor runs, so the +**published test results show `Tests run: 0` rather than the leak** — which reads like an infrastructure +fault. The leak is reported in the build log and as an ADO issue annotation; look there, not in the Tests +tab. Databases from *other* runs are only removed by the age based +sweep, whose threshold (8h) is comfortably longer than the longest test stage. + +`CosmosTestAccountJanitor` is the standalone entry point used by both pipeline layers: + +```bash +mvn -f sdk/cosmos/azure-cosmos-tests/pom.xml exec:java \ + -Dexec.args="--account-host --account-key --older-than 8" +``` + +Omit `--older-than` to delete only the current job's databases. + +### Guardrail + +`TestResourceHygieneTest` is a ratchet: +`azure-cosmos-tests/src/test/resources/test-resource-hygiene-baseline.properties` records the +violations that existed when the check was introduced, and the build fails when a file exceeds its +entry or a new offending file appears. Do not add entries for new tests; when you migrate a file, +lower or remove its entry. + +### Escape hatches + +- `-DCOSMOS.TEST_RESOURCE_JANITOR_ENABLED=false` disables the in-process janitor entirely. +- `-DCOSMOS.TEST_RESOURCE_JANITOR_FAIL_ON_LEAK=false` keeps the cleanup but stops it from failing the + run. Use this only to unblock a pipeline while the leaking test is being fixed. diff --git a/sdk/cosmos/azure-cosmos-tests/pom.xml b/sdk/cosmos/azure-cosmos-tests/pom.xml index fee6e44b12c5..90557da8fc9c 100644 --- a/sdk/cosmos/azure-cosmos-tests/pom.xml +++ b/sdk/cosmos/azure-cosmos-tests/pom.xml @@ -256,6 +256,18 @@ Licensed under the MIT License. + + + org.codehaus.mojo + exec-maven-plugin + 3.5.1 + + com.azure.cosmos.CosmosTestAccountJanitor + test + + + org.apache.maven.plugins diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTest.java index 5055166eca39..3bb2e36144ea 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTest.java @@ -16,25 +16,40 @@ import java.time.Duration; import java.time.LocalDateTime; +import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.function.Predicate; import static org.assertj.core.api.Assertions.assertThat; +/** + * Naming and lifetime helper for databases created by the test suite. + *

+ * Ids follow {@code RxJava.SDKTest.SharedDatabase___}. The run id lets + * cleanup delete exactly the databases created by the current run without touching resources that a + * concurrently executing matrix leg or pipeline run is still using on the same shared account. The + * legacy three segment form (without a run id) is still parsed so that databases left behind by + * builds predating this change are picked up by the age based sweep. + */ public class CosmosDatabaseForTest { private static Logger logger = LoggerFactory.getLogger(CosmosDatabaseForTest.class); public static final String SHARED_DB_ID_PREFIX = "RxJava.SDKTest.SharedDatabase"; private static final Duration CLEANUP_THRESHOLD_DURATION = Duration.ofHours(8); private static final String DELIMITER = "_"; + private static final int RANDOM_SUFFIX_LENGTH = 10; private static DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss"); public LocalDateTime createdTime; public CosmosAsyncDatabase createdDatabase; + private final String runId; - private CosmosDatabaseForTest(CosmosAsyncDatabase db, LocalDateTime createdTime) { + private CosmosDatabaseForTest(CosmosAsyncDatabase db, LocalDateTime createdTime, String runId) { this.createdDatabase = db; this.createdTime = createdTime; + this.runId = runId; } private boolean isStale() { @@ -42,25 +57,65 @@ private boolean isStale() { } private boolean isOlderThan(Duration dur) { - return createdTime.isBefore(LocalDateTime.now().minus(dur)); + return createdTime.isBefore(nowUtc().minus(dur)); + } + + /** + * Timestamps are written by the test agent and compared on the janitor agent, which may be a + * different machine. Both sides must use UTC - a local time zone offset would show a database as + * older than it is and could make the age based sweep delete an in-flight run's database. + */ + private static LocalDateTime nowUtc() { + return LocalDateTime.now(ZoneOffset.UTC); } public static String generateId() { - return SHARED_DB_ID_PREFIX + DELIMITER + TIME_FORMATTER.format(LocalDateTime.now()) + DELIMITER + RandomStringUtils.randomAlphabetic(3); + return generateId(null); } - private static CosmosDatabaseForTest from(CosmosAsyncDatabase db) { - if (db == null || db.getId() == null || db.getLink() == null) { - return null; - } + /** + * Generates a run tagged database id. The optional label only makes logs and portal views + * readable - uniqueness and cleanup scoping come from the timestamp and run id. + * + * @param label optional human readable label, may be null. + * @return a database id that cleanup is able to attribute to the current run. + */ + public static String generateId(String label) { + // The timestamp only has second resolution and many call sites now generate ids concurrently. + // A collision is silently swallowed as a 409 by the create helpers, which would make two tests + // share one database, so keep the random part wide enough that collisions do not happen. + String suffix = StringUtils.isEmpty(label) + ? RandomStringUtils.randomAlphabetic(RANDOM_SUFFIX_LENGTH) + : sanitizeLabel(label) + RandomStringUtils.randomAlphabetic(RANDOM_SUFFIX_LENGTH); - String id = db.getId(); + return SHARED_DB_ID_PREFIX + + DELIMITER + TIME_FORMATTER.format(nowUtc()) + + DELIMITER + CosmosTestRunId.get() + + DELIMITER + suffix; + } + + private static String sanitizeLabel(String label) { + String sanitized = label.replaceAll("[^A-Za-z0-9]", ""); + return sanitized.length() <= 40 ? sanitized : sanitized.substring(0, 40); + } + + /** + * @param id the database id to check. + * @return true when the id was produced by {@link #generateId(String)} - and is therefore owned by + * the test suite and safe for automated cleanup to delete. + */ + public static boolean isTestDatabaseId(String id) { + return parse(id) != null; + } + + private static ParsedId parse(String id) { if (id == null) { return null; } String[] parts = StringUtils.split(id, DELIMITER); - if (parts.length != 3) { + // 3 parts: legacy __. 4 parts: ___. + if (parts == null || parts.length < 3 || parts.length > 4) { return null; } if (!StringUtils.equals(parts[0], SHARED_DB_ID_PREFIX)) { @@ -69,12 +124,26 @@ private static CosmosDatabaseForTest from(CosmosAsyncDatabase db) { try { LocalDateTime parsedTime = LocalDateTime.parse(parts[1], TIME_FORMATTER); - return new CosmosDatabaseForTest(db, parsedTime); + String runId = parts.length == 4 ? parts[2] : null; + return new ParsedId(parsedTime, runId); } catch (Exception e) { return null; } } + private static CosmosDatabaseForTest from(CosmosAsyncDatabase db) { + if (db == null || db.getId() == null || db.getLink() == null) { + return null; + } + + ParsedId parsed = parse(db.getId()); + if (parsed == null) { + return null; + } + + return new CosmosDatabaseForTest(db, parsed.createdTime, parsed.runId); + } + public static CosmosDatabaseForTest create(DatabaseManager client) { CosmosDatabaseProperties dbDef = new CosmosDatabaseProperties(generateId()); @@ -82,30 +151,143 @@ public static CosmosDatabaseForTest create(DatabaseManager client) { CosmosAsyncDatabase db = client.getDatabase(dbDef.getId()); CosmosDatabaseForTest dbForTest = CosmosDatabaseForTest.from(db); assertThat(dbForTest).isNotNull(); + CosmosTestResourceRegistry.registerDatabase(dbForTest.createdDatabase.getId()); return dbForTest; } - public static void cleanupStaleTestDatabases(DatabaseManager client) { + /** + * Deletes databases created by other, long finished runs. Databases whose id does not follow the + * test naming convention are never touched, and neither are databases younger than the cleanup + * threshold, since those may belong to a run that is still executing. + * + * @param client the database manager to clean up with. + */ + public static CleanupResult cleanupStaleTestDatabases(DatabaseManager client) { logger.info("Cleaning stale test databases ..."); - List sqlParameterList = new ArrayList<>(); - sqlParameterList.add(new SqlParameter("@PREFIX", CosmosDatabaseForTest.SHARED_DB_ID_PREFIX)); + return deleteMatching(client, CosmosDatabaseForTest::isStale); + } + + /** + * Deletes every database created by the currently executing run, regardless of age. This is the + * safety net for tests that created a database and failed to delete it. + * + * @param client the database manager to clean up with. + * @return the ids of the databases that had been leaked and were deleted here. + */ + public static CleanupResult cleanupDatabasesForCurrentRun(DatabaseManager client) { + return cleanupDatabasesForRun(client, CosmosTestRunId.get()); + } + + /** + * Deletes every database created by the given run, regardless of age. Used by the pipeline post + * step, which knows the run id of the job that just finished. + * + * @param client the database manager to clean up with. + * @param runId the run id to delete databases for. + * @return the ids of the databases that were deleted. + */ + public static CleanupResult cleanupDatabasesForRun(DatabaseManager client, String runId) { + if (StringUtils.isEmpty(runId)) { + // Matching a null run id would match every legacy (pre run id) database, which may belong to + // a run that is still executing. Age based cleanup is the only safe option for those. + throw new IllegalArgumentException("runId must not be empty"); + } + + logger.info("Cleaning test databases for run {} ...", runId); + return deleteMatching(client, dbForTest -> StringUtils.equals(dbForTest.runId, runId)); + } + + /** + * Deletes test databases older than the given duration. Used by the scheduled janitor pipeline to + * recover resources from jobs that were cancelled or timed out. The threshold must be comfortably + * longer than the longest test stage so that in-flight runs are never touched. + * + * @param client the database manager to clean up with. + * @param threshold the minimum age a database must have to be deleted. + * @return the ids of the databases that were deleted. + */ + public static CleanupResult cleanupTestDatabasesOlderThan(DatabaseManager client, Duration threshold) { + logger.info("Cleaning test databases older than {} ...", threshold); + return deleteMatching(client, dbForTest -> dbForTest.isOlderThan(threshold)); + } + + private static CleanupResult deleteMatching( + DatabaseManager client, + Predicate predicate) { + + List deleted = new ArrayList<>(); + int failures = 0; List dbs = client.queryDatabases( - new SqlQuerySpec("SELECT * FROM c WHERE STARTSWITH(c.id, @PREFIX)", sqlParameterList)).collectList().block(); + new SqlQuerySpec( + "SELECT * FROM c WHERE STARTSWITH(c.id, @PREFIX)", + Collections.singletonList(new SqlParameter("@PREFIX", SHARED_DB_ID_PREFIX)))) + .collectList() + .block(); + + if (dbs == null) { + return new CleanupResult(deleted, failures); + } for (CosmosDatabaseProperties db : dbs) { - assertThat(db.getId()).startsWith(CosmosDatabaseForTest.SHARED_DB_ID_PREFIX); + assertThat(db.getId()).startsWith(SHARED_DB_ID_PREFIX); CosmosDatabaseForTest dbForTest = CosmosDatabaseForTest.from(client.getDatabase(db.getId())); + // A null dbForTest means the id does not follow the test convention - leave it alone. + if (dbForTest == null || !predicate.test(dbForTest)) { + continue; + } - if (db != null && dbForTest.isStale()) { - logger.info("Deleting database {}", db.getId()); - dbForTest.deleteDatabase(db.getId()); + logger.info("Deleting database {}", db.getId()); + try { + dbForTest.createdDatabase.delete().block(); + deleted.add(db.getId()); + } catch (Exception e) { + // Keep going - one undeletable database must not strand the rest - but remember that this + // sweep did not fully succeed, so callers do not mistake "found nothing" for "all clean". + failures++; + logger.warn("Failed to delete database {}", db.getId(), e); + } finally { + CosmosTestResourceRegistry.unregisterDatabase(db.getId()); } } + + return new CleanupResult(deleted, failures); + } + + /** + * What a cleanup sweep managed to do. A sweep that deleted nothing because every delete failed is not + * the same as a sweep that found nothing, and callers must be able to tell them apart. + */ + public static final class CleanupResult { + private final List deletedDatabaseIds; + private final int failureCount; + + private CleanupResult(List deletedDatabaseIds, int failureCount) { + this.deletedDatabaseIds = deletedDatabaseIds; + this.failureCount = failureCount; + } + + public List getDeletedDatabaseIds() { + return this.deletedDatabaseIds; + } + + public int getFailureCount() { + return this.failureCount; + } + + public boolean isComplete() { + return this.failureCount == 0; + } } - private void deleteDatabase(String id) { - this.createdDatabase.delete().block(); + private static final class ParsedId { + private final LocalDateTime createdTime; + private final String runId; + + private ParsedId(LocalDateTime createdTime, String runId) { + this.createdTime = createdTime; + this.runId = runId; + } } public interface DatabaseManager { diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java new file mode 100644 index 000000000000..55d0d2aa0e6d --- /dev/null +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java @@ -0,0 +1,267 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.cosmos; + +import com.azure.cosmos.models.CosmosDatabaseProperties; +import com.azure.cosmos.models.CosmosDatabaseResponse; +import com.azure.cosmos.models.SqlQuerySpec; +import com.azure.cosmos.util.CosmosPagedFlux; +import com.azure.cosmos.models.ModelBridgeInternal; +import com.azure.cosmos.util.UtilBridgeInternal; +import org.mockito.Mockito; +import org.testng.annotations.Test; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; + +import java.time.Duration; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Guards the invariant that the whole cleanup design rests on: on the long lived shared accounts, + * cleanup must delete this run's databases and nothing else. Deleting a database belonging to another, + * still-running job would cause confusing cross-run failures, and deleting a hand created fixture would + * be worse. + */ +public class CosmosDatabaseForTestTest { + + private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss"); + + /** + * A hand created database that matches the query prefix but does not parse as a generated id. This is + * the fixture that only the production code protects - it reaches deleteMatching and survives solely + * because ids that fail to parse are skipped. + */ + private static final String PINNED_FIXTURE = "RxJava.SDKTest.SharedDatabase_pinnedFixture"; + + /** A hand created database that does not even match the query prefix. */ + private static final String UNRELATED_FIXTURE = "permanentFixture"; + + @Test(groups = {"unit"}) + public void generatedIdIsRecognizedAndCarriesTheRunId() { + String id = CosmosDatabaseForTest.generateId("myFeature"); + + assertThat(id).startsWith(CosmosDatabaseForTest.SHARED_DB_ID_PREFIX); + assertThat(id).contains("_" + CosmosTestRunId.get() + "_"); + assertThat(CosmosDatabaseForTest.isTestDatabaseId(id)).isTrue(); + // Cosmos ids are capped at 255 characters and may not contain / \ # ? + assertThat(id.length()).isLessThan(255); + assertThat(id).doesNotContain("/").doesNotContain("\\").doesNotContain("#").doesNotContain("?"); + } + + @Test(groups = {"unit"}) + public void runIdNeverContainsTheIdDelimiter() { + // parse() splits on "_". A run id containing one would give every generated id five segments, + // parse() would return null everywhere, and run scoped cleanup would silently stop working. + assertThat(CosmosTestRunId.get()).matches("[a-z0-9]{1,20}"); + } + + @Test(groups = {"unit"}) + public void legacyIdsAreStillRecognized() { + // Builds predating the run id keep creating three segment ids on the same accounts during + // rollout. If these stopped parsing, the age based sweep would silently stop cleaning them up. + assertThat(CosmosDatabaseForTest.isTestDatabaseId( + "RxJava.SDKTest.SharedDatabase_20240101T101010_abc")).isTrue(); + } + + @Test(groups = {"unit"}) + public void nonTestIdsAreNotRecognized() { + assertThat(CosmosDatabaseForTest.isTestDatabaseId("myPermanentFixtureDb")).isFalse(); + assertThat(CosmosDatabaseForTest.isTestDatabaseId("RxJava.SDKTest.SharedDatabase")).isFalse(); + assertThat(CosmosDatabaseForTest.isTestDatabaseId( + "RxJava.SDKTest.SharedDatabase_notatimestamp_run_abc")).isFalse(); + assertThat(CosmosDatabaseForTest.isTestDatabaseId( + "SomethingElse_20240101T101010_run_abc")).isFalse(); + assertThat(CosmosDatabaseForTest.isTestDatabaseId(null)).isFalse(); + } + + @Test(groups = {"unit"}) + public void runScopedCleanupOnlyDeletesThatRunsDatabases() { + LocalDateTime now = LocalDateTime.now(ZoneOffset.UTC); + String mine = idFor(now, "runmine", "aaaaaaaaaa"); + String theirs = idFor(now, "runtheirs", "bbbbbbbbbb"); + // Run ids that are a prefix and an extension of ours - matching must be exact. An extension in + // particular catches a startsWith comparison, which would delete another run's databases. + String prefixOfMine = idFor(now, "runmin", "cccccccccc"); + String extensionOfMine = idFor(now, "runmineextra", "dddddddddd"); + String legacy = "RxJava.SDKTest.SharedDatabase_20240101T101010_abc"; + + FakeDatabaseManager manager = new FakeDatabaseManager( + mine, theirs, prefixOfMine, extensionOfMine, legacy, PINNED_FIXTURE, UNRELATED_FIXTURE); + + List deleted = CosmosDatabaseForTest.cleanupDatabasesForRun(manager, "runmine") + .getDeletedDatabaseIds(); + + assertThat(deleted).containsExactly(mine); + assertThat(manager.deleted).containsExactly(mine); + } + + @Test(groups = {"unit"}) + public void cleanupContinuesAfterAFailedDeleteAndDoesNotReportItAsDeleted() { + LocalDateTime now = LocalDateTime.now(ZoneOffset.UTC); + String first = idFor(now, "runmine", "aaaaaaaaaa"); + String failing = idFor(now, "runmine", "bbbbbbbbbb"); + String last = idFor(now, "runmine", "cccccccccc"); + + FakeDatabaseManager manager = new FakeDatabaseManager(first, failing, last); + manager.failDeleteOf(failing); + + List deleted = CosmosDatabaseForTest.cleanupDatabasesForRun(manager, "runmine") + .getDeletedDatabaseIds(); + + // One bad database must not abort the sweep, and must not be reported as deleted. + assertThat(deleted).containsExactly(first, last); + assertThat(manager.deleted).containsExactly(first, last); + } + + @Test(groups = {"unit"}) + public void runScopedCleanupRejectsAnEmptyRunId() { + // An empty run id would match legacy ids, whose run id is null, and those may belong to a job + // that is still running. + FakeDatabaseManager manager = new FakeDatabaseManager( + "RxJava.SDKTest.SharedDatabase_20240101T101010_abc"); + + try { + CosmosDatabaseForTest.cleanupDatabasesForRun(manager, ""); + org.testng.Assert.fail("Expected an IllegalArgumentException"); + } catch (IllegalArgumentException expected) { + assertThat(manager.deleted).isEmpty(); + } + } + + @Test(groups = {"unit"}) + public void aSweepWhereDeletesFailedIsNotReportedAsComplete() { + // "Deleted nothing because everything failed" must not look like "found nothing to delete", + // otherwise the janitor reports an all clear on a run whose resources are still on the account. + LocalDateTime now = LocalDateTime.now(ZoneOffset.UTC); + String failing = idFor(now, "runmine", "aaaaaaaaaa"); + + FakeDatabaseManager manager = new FakeDatabaseManager(failing); + manager.failDeleteOf(failing); + + CosmosDatabaseForTest.CleanupResult result = + CosmosDatabaseForTest.cleanupDatabasesForRun(manager, "runmine"); + + assertThat(result.getDeletedDatabaseIds()).isEmpty(); + assertThat(result.isComplete()).isFalse(); + assertThat(result.getFailureCount()).isEqualTo(1); + } + + @Test(groups = {"unit"}) + public void aSweepWithNothingToDeleteIsReportedAsComplete() { + FakeDatabaseManager manager = new FakeDatabaseManager(PINNED_FIXTURE, UNRELATED_FIXTURE); + + CosmosDatabaseForTest.CleanupResult result = + CosmosDatabaseForTest.cleanupDatabasesForRun(manager, "runmine"); + + assertThat(result.getDeletedDatabaseIds()).isEmpty(); + assertThat(result.isComplete()).isTrue(); + } + + @Test(groups = {"unit"}) + public void ageBasedCleanupSparesYoungDatabasesAndNonTestIds() { + LocalDateTime now = LocalDateTime.now(ZoneOffset.UTC); + String old = idFor(now.minusHours(9), "runold"); + String young = idFor(now.minusMinutes(30), "runyoung"); + String oldLegacy = "RxJava.SDKTest.SharedDatabase_20200101T101010_abc"; + + FakeDatabaseManager manager = new FakeDatabaseManager( + old, young, oldLegacy, PINNED_FIXTURE, UNRELATED_FIXTURE); + + List deleted = CosmosDatabaseForTest.cleanupTestDatabasesOlderThan(manager, Duration.ofHours(8)) + .getDeletedDatabaseIds(); + + // The young database may belong to a run that is still executing, and neither fixture is ours - + // PINNED_FIXTURE in particular reaches the production code because it matches the query prefix. + assertThat(deleted).containsExactlyInAnyOrder(old, oldLegacy); + } + + private static String idFor(LocalDateTime createdAt, String runId) { + return idFor(createdAt, runId, "abcdefghij"); + } + + private static String idFor(LocalDateTime createdAt, String runId, String randomSuffix) { + return CosmosDatabaseForTest.SHARED_DB_ID_PREFIX + + "_" + TIME_FORMATTER.format(createdAt) + + "_" + runId + + "_" + randomSuffix; + } + + /** + * Stands in for a Cosmos account holding the given databases. Records what cleanup deletes. + */ + private static final class FakeDatabaseManager implements CosmosDatabaseForTest.DatabaseManager { + private final Map databases = new LinkedHashMap<>(); + private final List deleted = new ArrayList<>(); + private final Set failingDeletes = new HashSet<>(); + + private void failDeleteOf(String databaseId) { + failingDeletes.add(databaseId); + } + + private FakeDatabaseManager(String... databaseIds) { + for (String databaseId : databaseIds) { + CosmosAsyncDatabase database = Mockito.mock(CosmosAsyncDatabase.class); + Mockito.when(database.getId()).thenReturn(databaseId); + Mockito.when(database.getLink()).thenReturn("dbs/" + databaseId); + // Recorded on subscription, not on invocation: if someone dropped the .block() in + // deleteMatching the Mono would never run and production would delete nothing, so this + // test has to fail in that case. + CosmosDatabaseResponse response = Mockito.mock(CosmosDatabaseResponse.class); + Mockito.when(database.delete()).thenAnswer(invocation -> Mono.fromRunnable(() -> { + if (failingDeletes.contains(databaseId)) { + throw new IllegalStateException("simulated delete failure for " + databaseId); + } + + deleted.add(databaseId); + }).thenReturn(response)); + + databases.put(databaseId, database); + } + } + + @Override + public CosmosPagedFlux queryDatabases(SqlQuerySpec query) { + // Honour the query the production code actually issues rather than hardcoding the filter. If + // the query lost its prefix binding, production would sweep nothing while these tests still + // passed - exactly the "silently useless janitor" failure this suite exists to prevent. + assertThat(query.getQueryText()).contains("STARTSWITH(c.id, @PREFIX)"); + String prefix = query.getParameters().stream() + .filter(parameter -> "@PREFIX".equals(parameter.getName())) + .findFirst() + .orElseThrow(() -> new AssertionError("query has no @PREFIX parameter")) + .getValue(String.class); + + List matching = new ArrayList<>(); + for (String databaseId : databases.keySet()) { + if (databaseId.startsWith(prefix)) { + matching.add(new CosmosDatabaseProperties(databaseId)); + } + } + + return UtilBridgeInternal.createCosmosPagedFlux( + options -> Flux.just(ModelBridgeInternal.createFeedResponse(matching, new HashMap<>()))); + } + + @Override + public Mono createDatabase(CosmosDatabaseProperties databaseDefinition) { + throw new UnsupportedOperationException(); + } + + @Override + public CosmosAsyncDatabase getDatabase(String id) { + return databases.get(id); + } + } +} diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java index 78c4f2ac9965..4c863508754f 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java @@ -7,7 +7,6 @@ import com.azure.cosmos.implementation.AsyncDocumentClient; import com.azure.cosmos.implementation.ClientSideRequestStatistics; import com.azure.cosmos.implementation.Configs; -import com.azure.cosmos.implementation.DatabaseForTest; import com.azure.cosmos.implementation.FeedResponseDiagnostics; import com.azure.cosmos.implementation.GlobalEndpointManager; import com.azure.cosmos.implementation.HttpConstants; @@ -1510,7 +1509,7 @@ private void validate(CosmosDiagnostics cosmosDiagnostics, int expectedRequestPa public void addressResolutionStatistics() { CosmosClient client1 = null; CosmosClient client2 = null; - String databaseId = DatabaseForTest.generateId(); + String databaseId = CosmosDatabaseForTest.generateId(); String containerId = UUID.randomUUID().toString(); CosmosDatabase cosmosDatabase = null; CosmosContainer cosmosContainer = null; @@ -1592,7 +1591,7 @@ public void addressResolutionStatistics() { @Test(groups = {"emulator"}, timeOut = TIMEOUT) public void responseStatisticRequestStartTimeUTCForDirectCall() { CosmosAsyncClient client = null; - String databaseId = DatabaseForTest.generateId(); + String databaseId = CosmosDatabaseForTest.generateId(); FaultInjectionRule faultInjectionRule = null; try { @@ -1663,7 +1662,7 @@ public void responseStatisticRequestStartTimeUTCForDirectCall() { @Test(groups = {"emulator"}, timeOut = TIMEOUT) public void negativeE2ETimeoutWithPointOperation() { CosmosAsyncClient client = null; - String databaseId = DatabaseForTest.generateId(); + String databaseId = CosmosDatabaseForTest.generateId(); try { client = new CosmosClientBuilder() @@ -1697,7 +1696,7 @@ public void negativeE2ETimeoutWithPointOperation() { @Test(groups = {"emulator"}, timeOut = TIMEOUT) public void negativeE2ETimeoutWithQueryOperation() { CosmosAsyncClient client = null; - String databaseId = DatabaseForTest.generateId(); + String databaseId = CosmosDatabaseForTest.generateId(); try { client = new CosmosClientBuilder() diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestAccountJanitor.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestAccountJanitor.java new file mode 100644 index 000000000000..7c12fc53456d --- /dev/null +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestAccountJanitor.java @@ -0,0 +1,193 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.cosmos; + +import com.azure.cosmos.models.CosmosDatabaseProperties; +import com.azure.cosmos.models.CosmosDatabaseResponse; +import com.azure.cosmos.models.SqlQuerySpec; +import com.azure.cosmos.util.CosmosPagedFlux; +import reactor.core.publisher.Mono; + +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; + +/** + * Standalone cleanup entry point for the long lived shared test accounts. + *

+ * The in-process janitor cannot help when a CI job is cancelled or times out - the JVM is killed + * before any listener or shutdown hook runs. This class is invoked from the pipeline instead: + *

    + *
  • as an always-run post step of a test stage, with {@code --run-id} set to that job's run id, + * to delete exactly what the job created;
    • + *
  • from the scheduled janitor pipeline, with {@code --older-than}, to sweep orphans left by + * jobs that died before their post step could run.
    • + *
+ * Only databases whose id follows the test naming convention are ever deleted, so hand created + * fixtures on these accounts are safe. + *

+ * Usage: + * 

+ * mvn -f sdk/cosmos/azure-cosmos-tests/pom.xml exec:java \
+ *   -Dexec.mainClass=com.azure.cosmos.CosmosTestAccountJanitor \
+ *   -Dexec.classpathScope=test \
+ *   -Dexec.args="--account-host <uri> --account-key <key> --older-than 8"
+ *
+ */ +public final class CosmosTestAccountJanitor { + + private static final int EXIT_BAD_ARGS = 2; + private static final int EXIT_FAILED = 1; + + private CosmosTestAccountJanitor() { + } + + public static void main(String[] args) { + int exitCode = run(args); + if (exitCode != 0) { + // exec:java runs in the Maven JVM, so only exit explicitly when the caller needs a failure. + System.exit(exitCode); + } + } + + private static int run(String[] args) { + Map parsed; + try { + parsed = parseArgs(args); + } catch (IllegalArgumentException e) { + System.err.println(e.getMessage()); + printUsage(); + return EXIT_BAD_ARGS; + } + + String host = parsed.get("account-host"); + String key = parsed.get("account-key"); + String runId = parsed.get("run-id"); + String olderThan = parsed.get("older-than"); + + if (host == null || key == null) { + System.err.println("--account-host and --account-key are required"); + printUsage(); + return EXIT_BAD_ARGS; + } + + // An unresolved pipeline variable arrives as the literal "$(name)". Without this check the client + // build throws, continueOnError swallows it, and a misconfigured janitor pipeline reports success + // while sweeping nothing. + if (!host.startsWith("http://") && !host.startsWith("https://")) { + System.err.println("--account-host must be an http(s) URI but was: " + host); + return EXIT_BAD_ARGS; + } + + if (runId != null && olderThan != null) { + System.err.println("--run-id and --older-than are mutually exclusive"); + printUsage(); + return EXIT_BAD_ARGS; + } + + if (olderThan == null && runId == null) { + // Post step of a test job: the same job environment produces the same run id the tests used. + runId = CosmosTestRunId.get(); + } + + int exitCode = 0; + try (CosmosAsyncClient client = buildClient(host, key)) { + CosmosDatabaseForTest.DatabaseManager manager = new CliDatabaseManager(client); + + CosmosDatabaseForTest.CleanupResult result; + if (olderThan == null) { + System.out.println("Deleting test databases for run " + runId + " on " + host); + result = CosmosDatabaseForTest.cleanupDatabasesForRun(manager, runId); + } else { + Duration threshold = Duration.ofHours(Long.parseLong(olderThan)); + System.out.println("Deleting test databases older than " + threshold + " on " + host); + result = CosmosDatabaseForTest.cleanupTestDatabasesOlderThan(manager, threshold); + } + + result.getDeletedDatabaseIds().forEach(id -> System.out.println(" deleted " + id)); + + if (result.isComplete()) { + System.out.println("Cleanup completed, deleted " + + result.getDeletedDatabaseIds().size() + " database(s)"); + } else { + // Exit non-zero so a sweep that could not delete what it found is not mistaken for a clean + // account. The pipeline steps set continueOnError, so this surfaces as SucceededWithIssues + // rather than failing the job - visible in the UI, but it does not block the run. + System.err.println("Cleanup incomplete: " + result.getFailureCount() + + " database(s) could not be deleted"); + exitCode = EXIT_FAILED; + } + } catch (Exception e) { + // Cleanup is best effort - report loudly, but let the caller decide whether to fail the job. + System.err.println("Cleanup failed: " + e); + e.printStackTrace(); + exitCode = EXIT_FAILED; + } + + return exitCode; + } + + private static Map parseArgs(String[] args) { + Map parsed = new HashMap<>(); + for (int i = 0; i < args.length; i++) { + String arg = args[i]; + if (!arg.startsWith("--")) { + throw new IllegalArgumentException("Unexpected argument: " + arg); + } + if (i + 1 >= args.length) { + throw new IllegalArgumentException("Missing value for " + arg); + } + + parsed.put(arg.substring(2), args[++i]); + } + + return parsed; + } + + private static CosmosAsyncClient buildClient(String host, String key) { + ThrottlingRetryOptions retryOptions = new ThrottlingRetryOptions(); + retryOptions.setMaxRetryAttemptsOnThrottledRequests(200); + retryOptions.setMaxRetryWaitTime(Duration.ofMinutes(5)); + + return new CosmosClientBuilder() + .endpoint(host) + .key(key) + .gatewayMode() + .throttlingRetryOptions(retryOptions) + .consistencyLevel(ConsistencyLevel.SESSION) + .buildAsyncClient(); + } + + private static void printUsage() { + System.err.println("Usage: CosmosTestAccountJanitor --account-host --account-key " + + " [--run-id | --older-than ]"); + System.err.println(" --run-id delete databases created by that run. Defaults to the run id of" + + " the current job environment, which is what a test job post step wants."); + System.err.println(" --older-than delete test databases older than the given number of hours," + + " regardless of which run created them."); + } + + private static final class CliDatabaseManager implements CosmosDatabaseForTest.DatabaseManager { + private final CosmosAsyncClient client; + + private CliDatabaseManager(CosmosAsyncClient client) { + this.client = client; + } + + @Override + public CosmosPagedFlux queryDatabases(SqlQuerySpec query) { + return client.queryDatabases(query, null); + } + + @Override + public Mono createDatabase(CosmosDatabaseProperties databaseDefinition) { + return client.createDatabase(databaseDefinition); + } + + @Override + public CosmosAsyncDatabase getDatabase(String id) { + return client.getDatabase(id); + } + } +} diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitor.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitor.java new file mode 100644 index 000000000000..063e9a044a17 --- /dev/null +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitor.java @@ -0,0 +1,420 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.cosmos; + +import com.azure.cosmos.implementation.TestConfigurations; +import com.azure.cosmos.models.CosmosDatabaseProperties; +import com.azure.cosmos.models.CosmosDatabaseResponse; +import com.azure.cosmos.models.SqlQuerySpec; +import com.azure.cosmos.util.CosmosPagedFlux; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.IExecutionListener; +import org.testng.IInvokedMethod; +import org.testng.IInvokedMethodListener; +import org.testng.ITestResult; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.function.Function; + +import static org.assertj.core.api.Fail.fail; + +/** + * Deletes the databases and containers created by this test run, and fails the run when a test left + * resources behind. + *

+ * Three nets, widest last: + *

    + *
  1. {@link CosmosTestResourceRegistry} contents, deleted at the end of the run.
    2. + *
  2. an account query for databases carrying this run's id, which catches resources created + * without going through the sanctioned helpers.
    3. + *
  3. a JVM shutdown hook, which is the only thing that runs when the JVM dies unexpectedly.
    4. + *
+ * Databases belonging to other runs are only removed by the age based sweep, so this is safe to run + * against the long lived shared accounts where several matrix legs execute concurrently. + *

+ * Registered alongside {@link CosmosNettyLeakDetectorFactory} in every {@code *-testng.xml} suite. + */ +public final class CosmosTestResourceJanitor implements IExecutionListener, IInvokedMethodListener { + + private static final Logger LOGGER = LoggerFactory.getLogger(CosmosTestResourceJanitor.class); + private static final String ENABLED_PROPERTY = "COSMOS.TEST_RESOURCE_JANITOR_ENABLED"; + private static final String FAIL_ON_LEAK_PROPERTY = "COSMOS.TEST_RESOURCE_JANITOR_FAIL_ON_LEAK"; + private static final Duration CLIENT_CLOSE_GRACE = Duration.ofSeconds(30); + private static final Duration ACCOUNT_SWEEP_TIMEOUT = Duration.ofMinutes(5); + + private static final Object SHUTDOWN_HOOK_LOCK = new Object(); + private static volatile boolean shutdownHookRegistered = false; + private static volatile boolean cleanupCompleted = false; + + @Override + public void onExecutionStart() { + if (!isEnabled()) { + LOGGER.info("Cosmos test resource janitor is disabled via -D{}=false", ENABLED_PROPERTY); + return; + } + + registerShutdownHook(); + } + + @Override + public void beforeInvocation(IInvokedMethod method, ITestResult testResult) { + CosmosTestResourceRegistry.setCurrentTest( + testResult.getTestClass().getName() + "." + method.getTestMethod().getMethodName()); + } + + @Override + public void afterInvocation(IInvokedMethod method, ITestResult testResult) { + CosmosTestResourceRegistry.setCurrentTest(null); + } + + @Override + public void onExecutionFinish() { + if (!isEnabled() || cleanupCompleted) { + // Guards against the listener being registered more than once for a suite; only the first + // invocation does the work. + return; + } + + CleanupReport report; + try { + report = cleanup(/* failFast */ false); + } finally { + cleanupCompleted = true; + } + + List leaks = report.leaks; + + if (!report.completed) { + // Never report an all clear here: cleanup did not finish, so "found nothing" and "did not + // look" are indistinguishable. This is the normal outcome on suites that sever connectivity. + LOGGER.warn("Cosmos test resource cleanup did not complete for run {} - resources may have been" + + " left behind and the scheduled janitor is the only remaining backstop for this run", + CosmosTestRunId.get()); + } else if (leaks.isEmpty()) { + LOGGER.info("No leaked Cosmos test resources for run {}{}", + CosmosTestRunId.get(), + supportsDatabaseQueries() ? "" : " (registry-only cleanup; account sweep not supported here)"); + } + + if (leaks.isEmpty()) { + return; + } + + StringBuilder message = new StringBuilder() + .append("Cosmos test resources were leaked by run ") + .append(CosmosTestRunId.get()) + .append(". Cleanup has attempted to delete them, but the tests that created them must delete") + .append(" them themselves - see sdk/cosmos/AGENTS.md. Leaked:"); + for (String leak : leaks) { + message.append(System.lineSeparator()).append(" - ").append(leak); + } + + LOGGER.error(message.toString()); + + // TestNG writes its reports before execution listeners run, so this never reaches the published + // JUnit XML - the ADO Tests tab shows a fully green run on a red job. Raise an ADO issue so the + // leak is visible where triage actually looks. Single line on purpose: ADO logging commands + // terminate at the first newline, which would truncate the list mid-entry. + boolean failing = shouldFailOnLeak(); + if (CosmosTestRunId.isCi()) { + // Downgraded to a warning when the fail-on-leak escape hatch is off, so opting out of failing + // also opts out of a red annotation. + System.out.println("##vso[task.logissue type=" + (failing ? "error" : "warning") + "]" + + "Cosmos test resources leaked by run " + CosmosTestRunId.get() + + " - see the build log for the full list"); + } + + if (failing) { + fail(message.toString()); + } + } + + /** + * Deletes everything this run created that is still around. + * + * @param failFast when true (shutdown hook path) skip the account wide sweeps and only delete what + * the registry knows about, so the JVM is not held open by metadata queries during shutdown. + * @return the leaked resources, and whether cleanup ran to completion. + */ + private static CleanupReport cleanup(boolean failFast) { + List tracked = CosmosTestResourceRegistry.leakedSnapshot(); + boolean shouldSweepAccount = !failFast && supportsDatabaseQueries(); + + if (tracked.isEmpty() && !shouldSweepAccount) { + return new CleanupReport(new ArrayList<>(), true); + } + + Set leaks = new LinkedHashSet<>(); + boolean completed = true; + CosmosAsyncClient client = null; + try { + client = buildHouseKeepingClient(); + final CosmosAsyncClient cleanupClient = client; + leaks.addAll(deleteTrackedResources(tracked, resource -> deleteTracked(cleanupClient, resource))); + + if (shouldSweepAccount) { + SweepResult sweep = sweepAccount(cleanupClient); + leaks.addAll(sweep.leaks); + // Individual deletes inside the sweep are caught and logged, so a sweep can return + // normally having deleted nothing because everything failed. + completed &= sweep.complete; + } + } catch (Exception e) { + completed = false; + LOGGER.error("Cosmos test resource cleanup failed", e); + } finally { + CosmosTestResourceRegistry.clear(); + // On the timeout path the interrupted sweep worker may still be unwinding and can briefly + // touch the client after this closes it. That is benign: the worker's interrupt flag survives, + // so its remaining operations fail fast into deleteMatching's per-database catch, and + // boundedElastic threads are daemons so none of this can hold the JVM open. + closeQuietly(client); + } + + return new CleanupReport(new ArrayList<>(leaks), completed); + } + + /** + * Decides what to delete and what counts as a leak. Split out from the account plumbing so it can be + * unit tested with a fake outcome function - this ordering is load bearing and easy to break. + *

+ * Databases are deleted first and the second loop skips containers underneath a database that is now + * absent: once a database is gone so are its containers, and without this a long suite ends by issuing + * hundreds of serial 404 probes against an account that is already throttling. The two loops must stay + * separate passes - fusing them would make the skip set depend on registration order and silently + * reintroduce the probe storm. + * + * @param tracked everything the registry still holds. + * @param deleter performs the delete and reports what it found. + * @return descriptions of the resources that had genuinely leaked. + */ + static List deleteTrackedResources( + List tracked, + Function deleter) { + + List leaks = new ArrayList<>(); + // "Absent by any means": DELETED and ALREADY_GONE both imply the containers are gone too. + // DELETE_FAILED must not be here - those containers really are still there and were never tried. + Set absentDatabases = new HashSet<>(); + + for (CosmosTestResourceRegistry.TrackedResource resource : tracked) { + if (!resource.isDatabase()) { + continue; + } + + DeleteOutcome outcome = deleter.apply(resource); + if (outcome == DeleteOutcome.DELETED || outcome == DeleteOutcome.ALREADY_GONE) { + absentDatabases.add(resource.getDatabaseId()); + } + if (outcome != DeleteOutcome.ALREADY_GONE) { + leaks.add(describe(resource, outcome)); + } + } + + for (CosmosTestResourceRegistry.TrackedResource resource : tracked) { + if (resource.isDatabase() || absentDatabases.contains(resource.getDatabaseId())) { + continue; + } + + DeleteOutcome outcome = deleter.apply(resource); + if (outcome != DeleteOutcome.ALREADY_GONE) { + leaks.add(describe(resource, outcome)); + } + } + + return leaks; + } + + /** + * Catches databases created without going through the registry, as long as they follow the naming + * convention. Several tests create databases on their own clients and are only covered by this sweep, + * so it must run even when the registry came back empty. + *

+ * Bounded, because this runs at the very end of a job that may already be near its timeout and some + * suites (manual-http-network-fault) deliberately sever connectivity. The result is collected on the + * worker and returned, rather than written into a shared collection, so a timed-out sweep contributes + * nothing and cannot race the caller. + */ + private static SweepResult sweepAccount(CosmosAsyncClient client) { + // The timeout deliberately propagates as an exception: that is what marks the cleanup incomplete + // in cleanup(). Swallowing it here (onErrorReturn, or a catch inside this method) would restore + // the false all-clear, where "found nothing" and "never looked" become indistinguishable - and no + // test would catch that. + return Mono.fromCallable(() -> { + List swept = new ArrayList<>(); + + CosmosDatabaseForTest.CleanupResult runScoped = + CosmosDatabaseForTest.cleanupDatabasesForCurrentRun(new JanitorDatabaseManager(client)); + for (String databaseId : runScoped.getDeletedDatabaseIds()) { + swept.add("database " + databaseId + " (deleted; created by )"); + } + + CosmosDatabaseForTest.CleanupResult stale = + CosmosDatabaseForTest.cleanupStaleTestDatabases(new JanitorDatabaseManager(client)); + + return new SweepResult(swept, runScoped.isComplete() && stale.isComplete()); + }).subscribeOn(Schedulers.boundedElastic()).timeout(ACCOUNT_SWEEP_TIMEOUT).block(); + } + + private static final class SweepResult { + private final List leaks; + private final boolean complete; + + private SweepResult(List leaks, boolean complete) { + this.leaks = leaks; + this.complete = complete; + } + } + + private static String describe(CosmosTestResourceRegistry.TrackedResource resource, DeleteOutcome outcome) { + return outcome == DeleteOutcome.DELETED + ? resource + " - deleted" + : resource + " - STILL PRESENT, delete failed"; + } + + private static DeleteOutcome deleteTracked( + CosmosAsyncClient client, + CosmosTestResourceRegistry.TrackedResource resource) { + + try { + if (resource.isDatabase()) { + client.getDatabase(resource.getDatabaseId()).delete().block(); + } else { + client.getDatabase(resource.getDatabaseId()) + .getContainer(resource.getContainerId()) + .delete() + .block(); + } + + LOGGER.warn("Deleted leaked {}", resource); + return DeleteOutcome.DELETED; + } catch (CosmosException e) { + if (e.getStatusCode() == 404) { + // The test cleaned up but did not deregister - not a leak. + return DeleteOutcome.ALREADY_GONE; + } + + LOGGER.error("Failed to delete leaked {}", resource, e); + return DeleteOutcome.DELETE_FAILED; + } catch (Exception e) { + LOGGER.error("Failed to delete leaked {}", resource, e); + return DeleteOutcome.DELETE_FAILED; + } + } + + enum DeleteOutcome { + /** The resource was still present and this cleanup deleted it - the test leaked it. */ + DELETED, + /** The resource was already gone - the test deleted it but did not deregister. */ + ALREADY_GONE, + /** The resource is still present and could not be deleted here. */ + DELETE_FAILED + } + + private static final class CleanupReport { + private final List leaks; + private final boolean completed; + + private CleanupReport(List leaks, boolean completed) { + this.leaks = leaks; + this.completed = completed; + } + } + + private static void registerShutdownHook() { + synchronized (SHUTDOWN_HOOK_LOCK) { + if (shutdownHookRegistered) { + return; + } + + Runtime.getRuntime().addShutdownHook(new Thread(() -> { + if (cleanupCompleted) { + return; + } + + LOGGER.warn("JVM is shutting down before the test run finished - " + + "attempting best effort cleanup of Cosmos test resources"); + cleanup(/* failFast */ true); + }, "cosmos-test-resource-janitor")); + + shutdownHookRegistered = true; + } + } + + private static CosmosAsyncClient buildHouseKeepingClient() { + ThrottlingRetryOptions retryOptions = new ThrottlingRetryOptions(); + // Metadata operations get throttled with 429/3200 ("high rate of metadata requests") when many + // legs clean up at once; the SDK default of 9 attempts is not enough for a bulk delete. + retryOptions.setMaxRetryAttemptsOnThrottledRequests(200); + retryOptions.setMaxRetryWaitTime(Duration.ofMinutes(5)); + + return new CosmosClientBuilder() + .endpoint(TestConfigurations.HOST) + .key(TestConfigurations.MASTER_KEY) + .gatewayMode() + .throttlingRetryOptions(retryOptions) + .consistencyLevel(ConsistencyLevel.SESSION) + .buildAsyncClient(); + } + + private static void closeQuietly(CosmosAsyncClient client) { + if (client == null) { + return; + } + + try { + Mono.fromRunnable(client::close).timeout(CLIENT_CLOSE_GRACE).onErrorResume(t -> Mono.empty()).block(); + } catch (Exception e) { + LOGGER.warn("Failed to close the janitor client", e); + } + } + + private static boolean isEnabled() { + return !"false".equalsIgnoreCase(System.getProperty(ENABLED_PROPERTY)); + } + + private static boolean shouldFailOnLeak() { + return !"false".equalsIgnoreCase(System.getProperty(FAIL_ON_LEAK_PROPERTY)); + } + + /** + * The vNext emulator does not implement querying databases, so the account wide sweeps are skipped + * there - the registry based cleanup still applies. Mirrors the carve out in + * {@code TestSuiteBase.afterSuitEmulatorVNext}. + */ + private static boolean supportsDatabaseQueries() { + return !Boolean.parseBoolean(System.getProperty("COSMOS.EMULATOR_VNEXT_ENABLED", "false")); + } + + private static final class JanitorDatabaseManager implements CosmosDatabaseForTest.DatabaseManager { + private final CosmosAsyncClient client; + + private JanitorDatabaseManager(CosmosAsyncClient client) { + this.client = client; + } + + @Override + public CosmosPagedFlux queryDatabases(SqlQuerySpec query) { + return client.queryDatabases(query, null); + } + + @Override + public Mono createDatabase(CosmosDatabaseProperties databaseDefinition) { + return client.createDatabase(databaseDefinition); + } + + @Override + public CosmosAsyncDatabase getDatabase(String id) { + return client.getDatabase(id); + } + } +} diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java new file mode 100644 index 000000000000..42eaeb356966 --- /dev/null +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java @@ -0,0 +1,202 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.cosmos; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Covers the janitor's delete-ordering and leak-classification decisions. + *

+ * These are the rules that decide what gets deleted, what gets reported as a leak, and how many round + * trips cleanup spends against an account that is already throttling at the end of a long run. They are + * easy to break in a way that no other test notices - a leak silently stops being reported, or the + * container probe storm silently comes back - so they are pinned here against a fake deleter. + */ +public class CosmosTestResourceJanitorTest { + + @Test(groups = {"unit"}) + public void deletedResourcesAreReportedAsLeaks() { + FakeDeleter deleter = new FakeDeleter(); + CosmosTestResourceRegistry.TrackedResource database = database("db1"); + deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETED); + + List leaks = CosmosTestResourceJanitor.deleteTrackedResources( + Arrays.asList(database), deleter); + + assertThat(leaks).hasSize(1); + assertThat(leaks.get(0)).contains("db1").contains("deleted"); + } + + @Test(groups = {"unit"}) + public void alreadyGoneResourcesAreNotReportedAsLeaks() { + // The test deleted the resource but did not deregister. Nothing leaked, so nothing to report - + // otherwise every healthy run would fail. + FakeDeleter deleter = new FakeDeleter(); + CosmosTestResourceRegistry.TrackedResource database = database("db1"); + deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.ALREADY_GONE); + + assertThat(CosmosTestResourceJanitor.deleteTrackedResources(Arrays.asList(database), deleter)) + .isEmpty(); + } + + @Test(groups = {"unit"}) + public void failedDeletesAreReportedAsStillPresent() { + FakeDeleter deleter = new FakeDeleter(); + CosmosTestResourceRegistry.TrackedResource database = database("db1"); + deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETE_FAILED); + + List leaks = CosmosTestResourceJanitor.deleteTrackedResources( + Arrays.asList(database), deleter); + + // Must not claim it was deleted - it is still on the account. + assertThat(leaks).hasSize(1); + assertThat(leaks.get(0)).contains("STILL PRESENT"); + } + + @Test(groups = {"unit"}) + public void containersOfADeletedDatabaseAreNotProbed() { + FakeDeleter deleter = new FakeDeleter(); + CosmosTestResourceRegistry.TrackedResource database = database("db1"); + CosmosTestResourceRegistry.TrackedResource container = container("db1", "c1"); + deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETED); + + List leaks = CosmosTestResourceJanitor.deleteTrackedResources( + Arrays.asList(container, database), deleter); + + // Deleting the database removed the container; probing it would be a wasted round trip. Note the + // container is listed FIRST here - the skip must not depend on registration order. + assertThat(deleter.attempted).containsExactly(database); + // The skipped container must not be reported - a skipped resource is not a separate leak. + assertThat(leaks).hasSize(1); + } + + @Test(groups = {"unit"}) + public void containersOfAnAlreadyGoneDatabaseAreNotProbed() { + // This is the dominant real case - a test deleted its database without deregistering. If it did + // not populate the skip set, a long suite would end by 404-probing every container it ever made. + FakeDeleter deleter = new FakeDeleter(); + CosmosTestResourceRegistry.TrackedResource database = database("db1"); + deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.ALREADY_GONE); + + List leaks = CosmosTestResourceJanitor.deleteTrackedResources( + Arrays.asList(database, container("db1", "c1"), container("db1", "c2")), deleter); + + assertThat(deleter.attempted).containsExactly(database); + assertThat(leaks).isEmpty(); + } + + @Test(groups = {"unit"}) + public void containersOfADatabaseThatCouldNotBeDeletedAreStillAttempted() { + // The database is still there, so its containers are too and were never attempted. + FakeDeleter deleter = new FakeDeleter(); + CosmosTestResourceRegistry.TrackedResource database = database("db1"); + CosmosTestResourceRegistry.TrackedResource container = container("db1", "c1"); + deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETE_FAILED); + deleter.outcome(container, CosmosTestResourceJanitor.DeleteOutcome.DELETED); + + List leaks = CosmosTestResourceJanitor.deleteTrackedResources( + Arrays.asList(database, container), deleter); + + assertThat(deleter.attempted).containsExactly(database, container); + assertThat(leaks).hasSize(2); + } + + @Test(groups = {"unit"}) + public void containersOfAnUnregisteredDatabaseAreDeletedIndividually() { + FakeDeleter deleter = new FakeDeleter(); + CosmosTestResourceRegistry.TrackedResource container = container("dbNotTracked", "c1"); + deleter.outcome(container, CosmosTestResourceJanitor.DeleteOutcome.DELETED); + + List leaks = CosmosTestResourceJanitor.deleteTrackedResources( + Arrays.asList(container), deleter); + + assertThat(deleter.attempted).containsExactly(container); + assertThat(leaks).hasSize(1); + } + + @Test(groups = {"unit"}) + public void databasesAreAlwaysDeletedBeforeContainers() { + FakeDeleter deleter = new FakeDeleter(); + CosmosTestResourceRegistry.TrackedResource containerElsewhere = container("dbOther", "c1"); + CosmosTestResourceRegistry.TrackedResource database = database("db1"); + deleter.outcome(containerElsewhere, CosmosTestResourceJanitor.DeleteOutcome.DELETED); + deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETED); + + CosmosTestResourceJanitor.deleteTrackedResources( + Arrays.asList(containerElsewhere, database), deleter); + + assertThat(deleter.attempted).containsExactly(database, containerElsewhere); + } + + @BeforeMethod(groups = {"unit"}) + public void resetRegistry() { + // The registry is JVM global; reset here rather than inside a helper so that helper call order + // cannot silently wipe a resource registered earlier in the same test. + CosmosTestResourceRegistry.clear(); + } + + private static CosmosTestResourceRegistry.TrackedResource database(String databaseId) { + CosmosTestResourceRegistry.registerDatabase(databaseId); + return findTracked(databaseId, null); + } + + private static CosmosTestResourceRegistry.TrackedResource container(String databaseId, String containerId) { + CosmosTestResourceRegistry.registerContainer(databaseId, containerId); + return findTracked(databaseId, containerId); + } + + private static CosmosTestResourceRegistry.TrackedResource findTracked(String databaseId, String containerId) { + for (CosmosTestResourceRegistry.TrackedResource resource : CosmosTestResourceRegistry.leakedSnapshot()) { + if (databaseId.equals(resource.getDatabaseId()) + && (containerId == null ? resource.isDatabase() : containerId.equals(resource.getContainerId()))) { + + return resource; + } + } + + throw new AssertionError("resource was not registered"); + } + + /** + * Stands in for the delete round trip, recording what was actually attempted so the tests can assert + * on round trips avoided rather than only on the reported result. + */ + private static final class FakeDeleter + implements java.util.function.Function< + CosmosTestResourceRegistry.TrackedResource, CosmosTestResourceJanitor.DeleteOutcome> { + + private final Map outcomes = new LinkedHashMap<>(); + private final List attempted = new ArrayList<>(); + + private void outcome( + CosmosTestResourceRegistry.TrackedResource resource, + CosmosTestResourceJanitor.DeleteOutcome outcome) { + + outcomes.put(resource, outcome); + } + + @Override + public CosmosTestResourceJanitor.DeleteOutcome apply( + CosmosTestResourceRegistry.TrackedResource resource) { + + attempted.add(resource); + CosmosTestResourceJanitor.DeleteOutcome outcome = outcomes.get(resource); + if (outcome == null) { + throw new AssertionError("unexpected delete attempt for " + resource); + } + + return outcome; + } + } +} diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java new file mode 100644 index 000000000000..c5c9e2757801 --- /dev/null +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java @@ -0,0 +1,194 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.cosmos; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * Tracks the databases and containers created by the test suite so that they can be deleted even + * when the test that created them fails to do so. + *

+ * Registration is done by the sanctioned factory methods on {@code TestSuiteBase}; deletion helpers + * deregister. Whatever is still registered when the run ends is a leak, and + * {@code CosmosTestResourceJanitor} deletes it and fails the run so it gets fixed at the source. + */ +public final class CosmosTestResourceRegistry { + + private static final Logger LOGGER = LoggerFactory.getLogger(CosmosTestResourceRegistry.class); + + // LinkedHashMap keeps creation order, which makes the leak report read chronologically. + private static final Map TRACKED_RESOURCES = new LinkedHashMap<>(); + private static final ThreadLocal CURRENT_TEST = new ThreadLocal<>(); + + private CosmosTestResourceRegistry() { + } + + /** + * Records the test currently executing on this thread so leaked resources can be attributed back + * to the test that created them. + * + * @param testName fully qualified test name, or null to clear. + */ + public static void setCurrentTest(String testName) { + if (testName == null) { + CURRENT_TEST.remove(); + } else { + CURRENT_TEST.set(testName); + } + } + + public static void registerDatabase(String databaseId) { + if (databaseId == null) { + return; + } + + synchronized (TRACKED_RESOURCES) { + TRACKED_RESOURCES.put(key(databaseId, null), new TrackedResource(databaseId, null, owner())); + } + } + + public static void unregisterDatabase(String databaseId) { + if (databaseId == null) { + return; + } + + synchronized (TRACKED_RESOURCES) { + TRACKED_RESOURCES.remove(key(databaseId, null)); + // Deleting a database deletes its containers, so drop those entries too. + TRACKED_RESOURCES.values().removeIf(resource -> databaseId.equals(resource.databaseId)); + } + } + + public static void registerContainer(String databaseId, String containerId) { + if (databaseId == null || containerId == null) { + return; + } + + synchronized (TRACKED_RESOURCES) { + TRACKED_RESOURCES.put( + key(databaseId, containerId), + new TrackedResource(databaseId, containerId, owner())); + } + } + + public static void unregisterContainer(String databaseId, String containerId) { + if (databaseId == null || containerId == null) { + return; + } + + synchronized (TRACKED_RESOURCES) { + TRACKED_RESOURCES.remove(key(databaseId, containerId)); + } + } + + /** + * @return a snapshot of everything still registered, i.e. everything that has leaked so far. + */ + public static List leakedSnapshot() { + synchronized (TRACKED_RESOURCES) { + return Collections.unmodifiableList(new ArrayList<>(TRACKED_RESOURCES.values())); + } + } + + public static void clear() { + synchronized (TRACKED_RESOURCES) { + TRACKED_RESOURCES.clear(); + } + } + + private static String owner() { + String currentTest = CURRENT_TEST.get(); + if (currentTest != null) { + return currentTest; + } + + // Outside an invoked test method (for example @BeforeSuite) walk the stack for the first + // non-infrastructure frame so the leak report still names something actionable. + for (StackTraceElement frame : Thread.currentThread().getStackTrace()) { + String className = frame.getClassName(); + if (className.startsWith("com.azure.cosmos") + && !className.equals(CosmosTestResourceRegistry.class.getName()) + && !className.equals(CosmosDatabaseForTest.class.getName())) { + return className + "." + frame.getMethodName(); + } + } + + return ""; + } + + private static String key(String databaseId, String containerId) { + return databaseId + "/" + (containerId == null ? "" : containerId); + } + + /** + * A database or container created by the test suite, together with the test that created it. + */ + public static final class TrackedResource { + private final String databaseId; + private final String containerId; + private final String createdBy; + + private TrackedResource(String databaseId, String containerId, String createdBy) { + this.databaseId = databaseId; + this.containerId = containerId; + this.createdBy = createdBy; + } + + public String getDatabaseId() { + return this.databaseId; + } + + /** + * @return the container id, or null when this entry tracks a database. + */ + public String getContainerId() { + return this.containerId; + } + + public String getCreatedBy() { + return this.createdBy; + } + + public boolean isDatabase() { + return this.containerId == null; + } + + @Override + public String toString() { + return (isDatabase() ? "database " + databaseId : "container " + databaseId + "/" + containerId) + + " (created by " + createdBy + ")"; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof TrackedResource)) { + return false; + } + + TrackedResource that = (TrackedResource) other; + return Objects.equals(databaseId, that.databaseId) + && Objects.equals(containerId, that.containerId); + } + + @Override + public int hashCode() { + return Objects.hash(databaseId, containerId); + } + } + + static { + LOGGER.info("Cosmos test resource registry active for run id {}", CosmosTestRunId.get()); + } +} diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java new file mode 100644 index 000000000000..cde514011efd --- /dev/null +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java @@ -0,0 +1,134 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.cosmos; + +import org.apache.commons.lang3.StringUtils; + +import java.nio.charset.StandardCharsets; +import java.util.zip.CRC32; + +/** + * Identity of the current test run, embedded into the name of every database created by the test + * suite. + *

+ * Cleanup on the long lived shared test accounts (thin client, GSI, canary) has to be able to tell + * "resources my run created" apart from "resources another run is actively using", because several + * matrix legs and pipeline runs share a single account. The run id is that discriminator. + */ +public final class CosmosTestRunId { + + private static final int MAX_LENGTH = 20; + private static final String RUN_ID = computeRunId(); + private static final boolean IS_CI = + System.getenv("BUILD_BUILDID") != null || System.getenv("SYSTEM_JOBID") != null; + + private CosmosTestRunId() { + } + + /** + * Returns the identifier of the current test run. Stable for the lifetime of the JVM and safe to + * embed in a Cosmos resource id (lower case alphanumerics only, at most 20 characters). + * + * @return the run id. + */ + public static String get() { + return RUN_ID; + } + + /** + * @return true when running inside an Azure DevOps pipeline. + */ + public static boolean isCi() { + return IS_CI; + } + + private static String computeRunId() { + String explicit = System.getProperty("COSMOS.TEST_RUN_ID"); + if (StringUtils.isNotEmpty(explicit)) { + return sanitize(explicit); + } + + String buildId = StringUtils.defaultString(System.getenv("BUILD_BUILDID"), ""); + String attempt = StringUtils.defaultString(System.getenv("SYSTEM_JOBATTEMPT"), "1"); + + // System.JobId is a GUID that is unique per job and stable for the whole job, including its post + // steps, so hashing it makes collisions between concurrently running legs structurally impossible. + // That matters because a run scoped delete on a shared account must never match another leg's id. + String jobId = System.getenv("SYSTEM_JOBID"); + if (StringUtils.isNotEmpty(jobId)) { + // The build id is carried along purely so a stray database can be traced back to a build. + return compose(buildId + "x", shortHash(buildId + "|" + jobId + "|" + attempt)); + } + + if (StringUtils.isNotEmpty(buildId)) { + // Fallback for agents that do not expose SYSTEM_JOBID. Uniqueness between concurrent legs of + // one build then rests entirely on the hash, so the hash must survive truncation intact. + String jobName = StringUtils.defaultString(System.getenv("SYSTEM_JOBDISPLAYNAME"), + StringUtils.defaultString(System.getenv("AGENT_JOBNAME"), "")); + return compose(buildId + "x", shortHash(buildId + "|" + jobName + "|" + attempt)); + } + + String user = StringUtils.defaultString(System.getProperty("user.name"), "dev"); + return compose("l" + user, shortHash(user + "|" + ProcessHandleCompat.currentPid())); + } + + /** + * Joins a human readable prefix and a uniqueness hash within {@link #MAX_LENGTH}. + *

+ * The hash is never truncated - it is the only part that distinguishes concurrent runs, and losing + * even a few characters of it makes two runs collide and delete each other's in-flight databases. + * Only the readable prefix is trimmed, and from the end, so the leading digits of a build id (the + * most significant ones) survive. Callers hash the whole input, prefix included, so that whatever is + * trimmed here is still represented in the hash. + */ + private static String compose(String readablePrefix, String hash) { + String cleanedHash = clean(hash); + String cleanedPrefix = clean(readablePrefix); + int budget = Math.max(0, MAX_LENGTH - cleanedHash.length()); + String trimmedPrefix = cleanedPrefix.length() <= budget + ? cleanedPrefix + : cleanedPrefix.substring(0, budget); + + String composed = trimmedPrefix + cleanedHash; + return composed.isEmpty() ? "unknown" : composed; + } + + private static String shortHash(String value) { + CRC32 crc32 = new CRC32(); + crc32.update(value.getBytes(StandardCharsets.UTF_8)); + return Long.toString(crc32.getValue(), 36); + } + + private static String sanitize(String value) { + String cleaned = clean(value); + if (cleaned.isEmpty()) { + cleaned = "unknown"; + } + + return cleaned.length() <= MAX_LENGTH ? cleaned : cleaned.substring(0, MAX_LENGTH); + } + + private static String clean(String value) { + return value.toLowerCase(java.util.Locale.ROOT).replaceAll("[^a-z0-9]", ""); + } + + /** + * Java 8 baseline compatible process id lookup. + */ + private static final class ProcessHandleCompat { + private static long currentPid() { + String jvmName = java.lang.management.ManagementFactory.getRuntimeMXBean().getName(); + int atIndex = jvmName.indexOf('@'); + if (atIndex > 0) { + try { + return Long.parseLong(jvmName.substring(0, atIndex)); + } catch (NumberFormatException ignored) { + // fall through + } + } + + return 0L; + } + } +} diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/EndToEndTimeOutValidationTests.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/EndToEndTimeOutValidationTests.java index 6e47ffd4062a..8fd58dca191f 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/EndToEndTimeOutValidationTests.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/EndToEndTimeOutValidationTests.java @@ -362,7 +362,7 @@ public void clientLevelEndToEndTimeoutPolicyInOptionsShouldTimeout() { FaultInjectionRule queryItemFaultInjectionRule = null; CosmosAsyncClient setupClient = null; CosmosAsyncClient cosmosAsyncClient = null; - String dbname = "db_" + UUID.randomUUID(); + String dbname = CosmosDatabaseForTest.generateId("endToEndTimeout"); try { setupClient = copyCosmosClientBuilder(getClientBuilder()).buildAsyncClient(); diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/FaultInjectionWithAvailabilityStrategyTestsBase.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/FaultInjectionWithAvailabilityStrategyTestsBase.java index 157a1d21966f..652f8aee9323 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/FaultInjectionWithAvailabilityStrategyTestsBase.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/FaultInjectionWithAvailabilityStrategyTestsBase.java @@ -5075,7 +5075,7 @@ private static ObjectNode createTestItemAsJson(String id, String pkValue) { } private CosmosAsyncContainer createTestContainer(CosmosAsyncClient clientWithPreferredRegions) { - String dbId = UUID.randomUUID().toString(); + String dbId = CosmosDatabaseForTest.generateId("availabilityStrategy"); return createTestContainer(clientWithPreferredRegions, dbId); } diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/MaxRetryCountTests.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/MaxRetryCountTests.java index 967d3b13837d..6826877dce30 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/MaxRetryCountTests.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/MaxRetryCountTests.java @@ -2112,7 +2112,7 @@ private void logMaxCount() { private CosmosAsyncContainer createTestContainer(CosmosAsyncClient clientWithPreferredRegions) { - String dbId = UUID.randomUUID().toString(); + String dbId = CosmosDatabaseForTest.generateId("maxRetryCount"); return createTestContainer(clientWithPreferredRegions, dbId); } diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/ResourceTokenTestForV4.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/ResourceTokenTestForV4.java index 22fa0b84b7f0..b978e8417606 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/ResourceTokenTestForV4.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/ResourceTokenTestForV4.java @@ -3,7 +3,6 @@ package com.azure.cosmos; -import com.azure.cosmos.implementation.DatabaseForTest; import com.azure.cosmos.implementation.FailureValidator; import com.azure.cosmos.implementation.FeedResponseListValidator; import com.azure.cosmos.implementation.TestConfigurations; @@ -36,7 +35,7 @@ import static org.assertj.core.api.Assertions.assertThat; public class ResourceTokenTestForV4 extends TestSuiteBase { - public final String databaseId = DatabaseForTest.generateId(); + public final String databaseId = CosmosDatabaseForTest.generateId(); private CosmosAsyncDatabase createdDatabase; private CosmosAsyncContainer createdContainer; diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/TestResourceHygieneTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/TestResourceHygieneTest.java new file mode 100644 index 000000000000..45ea40b2f96c --- /dev/null +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/TestResourceHygieneTest.java @@ -0,0 +1,261 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.cosmos; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.annotations.Test; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.TreeMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Fail.fail; + +/** + * Ratchet that stops new tests from creating databases outside the sanctioned helpers. + *

+ * Databases created with an arbitrary id cannot be attributed to a run, so when a CI job is + * cancelled or times out they leak permanently on the long lived shared accounts. Tests must use + * {@code TestSuiteBase.createTestDatabase(...)} or {@link CosmosDatabaseForTest#generateId(String)} + * so cleanup can find them. + *

+ * The check is a ratchet rather than a hard ban: {@code test-resource-hygiene-baseline.properties} + * records the violations that existed when this was introduced. Adding a violation to a file - or + * introducing a new offending file - fails. Removing violations and lowering the baseline is always + * welcome. + */ +public class TestResourceHygieneTest { + + private static final Logger LOGGER = LoggerFactory.getLogger(TestResourceHygieneTest.class); + private static final String BASELINE_RESOURCE = "test-resource-hygiene-baseline.properties"; + private static final Path TEST_SOURCE_ROOT = Paths.get("src", "test", "java"); + + /** + * Files that own the sanctioned helpers, or whose entire purpose is exercising the database + * management APIs. They are not scanned at all. + */ + private static final List EXCLUDED_FILES = java.util.Arrays.asList( + "com/azure/cosmos/rx/TestSuiteBase.java", + "com/azure/cosmos/CosmosDatabaseForTest.java", + "com/azure/cosmos/CosmosTestResourceJanitor.java", + "com/azure/cosmos/CosmosTestAccountJanitor.java", + "com/azure/cosmos/CosmosTestResourceRegistry.java", + "com/azure/cosmos/TestResourceHygieneTest.java"); + + private static final Pattern METHOD_DECLARATION = Pattern.compile( + "^(?:(?:public|protected|private|static|final|abstract|default|synchronized)\\s+).*\\)\\s*\\{$"); + + private static final Pattern DATABASE_CREATION = Pattern.compile( + "(?:\\.\\s*createDatabase(?:IfNotExists)?\\s*\\()" + + "|(?:(? actual = scanResult.violations; + Map baseline = loadBaseline(); + + List regressions = new ArrayList<>(); + for (Map.Entry entry : actual.entrySet()) { + int allowed = baseline.getOrDefault(entry.getKey(), 0); + if (entry.getValue() > allowed) { + regressions.add(String.format( + "%s: %d direct database creation call(s), baseline allows %d", + entry.getKey(), entry.getValue(), allowed)); + } + } + + if (!regressions.isEmpty()) { + StringBuilder message = new StringBuilder() + .append("New direct database creation detected. Use TestSuiteBase.createTestDatabase(...) so the") + .append(" database id carries the run id and CI cleanup can delete it - see") + .append(" sdk/cosmos/AGENTS.md."); + for (String regression : regressions) { + message.append(System.lineSeparator()).append(" - ").append(regression); + } + + fail(message.toString()); + } + + List staleBaselineEntries = new ArrayList<>(); + for (Map.Entry entry : baseline.entrySet()) { + int current = actual.getOrDefault(entry.getKey(), 0); + if (current < entry.getValue()) { + staleBaselineEntries.add(String.format( + "%s: baseline allows %d but only %d remain", entry.getKey(), entry.getValue(), current)); + } + } + + // Not a failure - lowering the baseline is a manual follow up, but surface it so the ratchet + // actually tightens over time instead of drifting. + if (!staleBaselineEntries.isEmpty()) { + LOGGER.warn("Test resource hygiene baseline can be lowered:{}{}", + System.lineSeparator(), + String.join(System.lineSeparator(), staleBaselineEntries)); + } + } + + /** + * Guards the guard: if the regex stops matching, the ratchet silently passes forever. + */ + @Test(groups = {"unit"}) + public void scannerDetectsDirectDatabaseCreation() { + assertThat(countViolations("client.createDatabase(props).block();")).isEqualTo(1); + assertThat(countViolations("client.createDatabaseIfNotExists(dbId).block();")).isEqualTo(1); + assertThat(countViolations("database = createDatabase(client, dbId);")).isEqualTo(1); + assertThat(countViolations("db = createSyncDatabase(client, dbId);")).isEqualTo(1); + + assertThat(countViolations("database = createTestDatabase(client);")).isZero(); + assertThat(countViolations("String id = CosmosDatabaseForTest.generateId(\"x\");")).isZero(); + assertThat(countViolations("// client.createDatabase(props) is not allowed")).isZero(); + // A "//" inside a string literal must not hide a violation later on the line. + assertThat(countViolations("log(\"see http://x\"); client.createDatabase(props);")).isEqualTo(1); + assertThat(countViolations("container = createCollection(database, def, options);")).isZero(); + + // Declarations and interface implementations define helpers rather than call them. + assertThat(countViolations( + "public Mono createDatabase(CosmosDatabaseProperties def) {")).isZero(); + assertThat(countViolations( + "static protected CosmosAsyncDatabase createDatabase(CosmosAsyncClient c, String id) {")).isZero(); + // ... but a call that merely happens to sit on a line ending in "{" is still counted. + assertThat(countViolations("if (x) { client.createDatabase(props); }")).isEqualTo(1); + } + + private static ScanResult scan() throws IOException { + Map violations = new TreeMap<>(); + int filesScanned = 0; + try (Stream files = Files.walk(TEST_SOURCE_ROOT)) { + List javaFiles = files + .filter(Files::isRegularFile) + .filter(path -> path.toString().endsWith(".java")) + .collect(java.util.stream.Collectors.toList()); + + for (Path file : javaFiles) { + String relative = normalize(TEST_SOURCE_ROOT.relativize(file).toString()); + if (EXCLUDED_FILES.contains(relative)) { + continue; + } + + filesScanned++; + int count = 0; + for (String line : Files.readAllLines(file, StandardCharsets.UTF_8)) { + count += countViolations(line); + } + + if (count > 0) { + violations.put(relative, count); + } + } + } + + return new ScanResult(violations, filesScanned); + } + + private static final class ScanResult { + private final Map violations; + private final int filesScanned; + + private ScanResult(Map violations, int filesScanned) { + this.violations = violations; + this.filesScanned = filesScanned; + } + } + + private static int countViolations(String line) { + String code = stripComment(line); + if (code.isEmpty()) { + return 0; + } + + // Skip method declarations - those define or implement a helper rather than call one. A call site + // never both opens a body and ends the line with ") {". + if (METHOD_DECLARATION.matcher(code).matches()) { + return 0; + } + + int count = 0; + Matcher matcher = DATABASE_CREATION.matcher(code); + while (matcher.find()) { + count++; + } + + return count; + } + + private static String stripComment(String line) { + String trimmed = line.trim(); + if (trimmed.startsWith("//") || trimmed.startsWith("*") || trimmed.startsWith("/*")) { + return ""; + } + + // Only strip a trailing comment when the "//" is not inside a string literal, otherwise the rest of + // the line - which may contain a real violation - is silently dropped. + boolean inString = false; + for (int i = 0; i < trimmed.length() - 1; i++) { + char c = trimmed.charAt(i); + if (c == '\\') { + i++; + } else if (c == '"') { + inString = !inString; + } else if (!inString && c == '/' && trimmed.charAt(i + 1) == '/') { + return trimmed.substring(0, i); + } + } + + return trimmed; + } + + private static String normalize(String path) { + return path.replace('\\', '/'); + } + + private static Map loadBaseline() throws IOException { + Properties properties = new Properties(); + try (InputStream stream = + TestResourceHygieneTest.class.getClassLoader().getResourceAsStream(BASELINE_RESOURCE)) { + + if (stream == null) { + throw new IOException("Missing baseline resource " + BASELINE_RESOURCE); + } + + try (BufferedReader reader = new BufferedReader(new InputStreamReader(stream, StandardCharsets.UTF_8))) { + properties.load(reader); + } + } + + Map baseline = new TreeMap<>(); + for (String name : properties.stringPropertyNames()) { + baseline.put(normalize(name), Integer.parseInt(properties.getProperty(name).trim())); + } + + return baseline; + } +} diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/directconnectivity/MetadataRequestRetryPolicyTests.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/directconnectivity/MetadataRequestRetryPolicyTests.java index 8d7999235a66..594442a29bfa 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/directconnectivity/MetadataRequestRetryPolicyTests.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/directconnectivity/MetadataRequestRetryPolicyTests.java @@ -3,6 +3,7 @@ package com.azure.cosmos.implementation.directconnectivity; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.BridgeInternal; import com.azure.cosmos.ConsistencyLevel; import com.azure.cosmos.CosmosAsyncClient; @@ -252,7 +253,7 @@ public void forceBackgroundAddressRefresh_onConnectionTimeoutAndRequestCancellat } String faultInjectedRegion = preferredRegions.get(0); - String dbId = UUID.randomUUID().toString(); + String dbId = CosmosDatabaseForTest.generateId("metadataRetryPolicy"); String containerId = UUID.randomUUID().toString(); client.createDatabase(dbId).block(); diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/DocumentCrudTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/DocumentCrudTest.java index 016baa70b2c4..d1fc91738cdc 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/DocumentCrudTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/DocumentCrudTest.java @@ -4,12 +4,12 @@ import com.azure.cosmos.BridgeInternal; import com.azure.cosmos.CosmosAsyncClient; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.CosmosAsyncContainer; import com.azure.cosmos.CosmosAsyncDatabase; import com.azure.cosmos.CosmosClientBuilder; import com.azure.cosmos.CosmosException; import com.azure.cosmos.TestObject; -import com.azure.cosmos.implementation.DatabaseForTest; import com.azure.cosmos.implementation.FailureValidator; import com.azure.cosmos.implementation.FeedResponseListValidator; import com.azure.cosmos.implementation.HttpConstants; @@ -37,7 +37,7 @@ public class DocumentCrudTest extends TestSuiteBase { - private String databaseIdForTest = DatabaseForTest.generateId(); + private String databaseIdForTest = CosmosDatabaseForTest.generateId(); private CosmosAsyncClient client; private CosmosAsyncContainer container; diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/GatewayReadConsistencyStrategyE2ETest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/GatewayReadConsistencyStrategyE2ETest.java index 0666841e53c1..04b1b16cf9db 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/GatewayReadConsistencyStrategyE2ETest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/GatewayReadConsistencyStrategyE2ETest.java @@ -2,6 +2,7 @@ // Licensed under the MIT License. package com.azure.cosmos.rx; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.ConsistencyLevel; import com.azure.cosmos.CosmosAsyncClient; import com.azure.cosmos.CosmosAsyncContainer; @@ -84,7 +85,7 @@ public class GatewayReadConsistencyStrategyE2ETest { public void beforeClass() { System.setProperty("COSMOS.THINCLIENT_ENABLED", "true"); - databaseId = "readConsistencyStrategy-e2e-" + UUID.randomUUID().toString().substring(0, 8); + databaseId = CosmosDatabaseForTest.generateId("readConsistencyStrategyE2E"); containerId = "testcontainer"; gatewayV1Client = createGatewayV1Builder().buildAsyncClient(); diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/GatewayReadConsistencyStrategySpyWireTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/GatewayReadConsistencyStrategySpyWireTest.java index c0442706f6ec..ef31bc7a34bf 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/GatewayReadConsistencyStrategySpyWireTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/GatewayReadConsistencyStrategySpyWireTest.java @@ -2,6 +2,7 @@ // Licensed under the MIT License. package com.azure.cosmos.rx; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.ConsistencyLevel; import com.azure.cosmos.CosmosAsyncClient; import com.azure.cosmos.CosmosAsyncContainer; @@ -106,7 +107,7 @@ public void beforeClass() { .gatewayMode() .buildAsyncClient(); - databaseId = "ReadConsistencyStrategy-spy-" + UUID.randomUUID().toString().substring(0, 8); + databaseId = CosmosDatabaseForTest.generateId("readConsistencyStrategySpy"); containerId = "testcontainer"; cosmosClient.createDatabaseIfNotExists(databaseId).block(); diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/OfferQueryTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/OfferQueryTest.java index ef103d5c53c5..69b85a11372c 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/OfferQueryTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/OfferQueryTest.java @@ -3,12 +3,12 @@ package com.azure.cosmos.rx; import com.azure.cosmos.CosmosAsyncClient; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.CosmosClientBuilder; import com.azure.cosmos.implementation.AsyncDocumentClient; import com.azure.cosmos.implementation.AsyncDocumentClient.Builder; import com.azure.cosmos.implementation.CosmosPagedFluxOptions; import com.azure.cosmos.implementation.Database; -import com.azure.cosmos.implementation.DatabaseForTest; import com.azure.cosmos.implementation.DocumentCollection; import com.azure.cosmos.implementation.FeedResponseListValidator; import com.azure.cosmos.implementation.FeedResponseValidator; @@ -40,7 +40,7 @@ public class OfferQueryTest extends TestSuiteBase { public final static int SETUP_TIMEOUT = 40000; - public final String databaseId = DatabaseForTest.generateId(); + public final String databaseId = CosmosDatabaseForTest.generateId(); private List createdCollections = new ArrayList<>(); diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/OfferReadReplaceTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/OfferReadReplaceTest.java index e0936cd76b8d..b8104a30bd28 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/OfferReadReplaceTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/OfferReadReplaceTest.java @@ -2,6 +2,7 @@ // Licensed under the MIT License. package com.azure.cosmos.rx; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.implementation.OperationType; import com.azure.cosmos.implementation.QueryFeedOperationState; import com.azure.cosmos.implementation.ResourceType; @@ -10,7 +11,6 @@ import com.azure.cosmos.models.FeedResponse; import com.azure.cosmos.implementation.AsyncDocumentClient; import com.azure.cosmos.implementation.Database; -import com.azure.cosmos.implementation.DatabaseForTest; import com.azure.cosmos.implementation.DocumentCollection; import com.azure.cosmos.implementation.Offer; import com.azure.cosmos.implementation.ResourceResponse; @@ -28,7 +28,7 @@ //TODO: change to use external TestSuiteBase public class OfferReadReplaceTest extends TestSuiteBase { - public final String databaseId = DatabaseForTest.generateId(); + public final String databaseId = CosmosDatabaseForTest.generateId(); private Database createdDatabase; private DocumentCollection createdCollection; diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/PermissionQueryTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/PermissionQueryTest.java index 8a035ad43c64..4162ce89064e 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/PermissionQueryTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/PermissionQueryTest.java @@ -3,11 +3,11 @@ package com.azure.cosmos.rx; import com.azure.cosmos.CosmosAsyncClient; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.CosmosAsyncDatabase; import com.azure.cosmos.CosmosAsyncUser; import com.azure.cosmos.CosmosClientBuilder; import com.azure.cosmos.CosmosException; -import com.azure.cosmos.implementation.DatabaseForTest; import com.azure.cosmos.implementation.FailureValidator; import com.azure.cosmos.implementation.FeedResponseListValidator; import com.azure.cosmos.implementation.FeedResponseValidator; @@ -29,7 +29,7 @@ public class PermissionQueryTest extends TestSuiteBase { - public final String databaseId = DatabaseForTest.generateId(); + public final String databaseId = CosmosDatabaseForTest.generateId(); private CosmosAsyncDatabase createdDatabase; private CosmosAsyncUser createdUser; diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ReadFeedOffersTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ReadFeedOffersTest.java index f316029bbc84..179f5312abbb 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ReadFeedOffersTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ReadFeedOffersTest.java @@ -3,6 +3,7 @@ package com.azure.cosmos.rx; import com.azure.cosmos.CosmosAsyncClient; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.CosmosClientBuilder; import com.azure.cosmos.FlakyTestRetryAnalyzer; import com.azure.cosmos.implementation.CosmosPagedFluxOptions; @@ -17,7 +18,6 @@ import com.azure.cosmos.models.PartitionKeyDefinition; import com.azure.cosmos.implementation.AsyncDocumentClient; import com.azure.cosmos.implementation.Database; -import com.azure.cosmos.implementation.DatabaseForTest; import com.azure.cosmos.implementation.DocumentCollection; import com.azure.cosmos.implementation.FeedResponseListValidator; import com.azure.cosmos.implementation.FeedResponseValidator; @@ -42,7 +42,7 @@ public class ReadFeedOffersTest extends TestSuiteBase { protected static final int SETUP_TIMEOUT = 60000; protected static final int SHUTDOWN_TIMEOUT = 20000; - public final String databaseId = DatabaseForTest.generateId(); + public final String databaseId = CosmosDatabaseForTest.generateId(); private Database createdDatabase; private List allOffers = new ArrayList<>(); diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ReadFeedPermissionsTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ReadFeedPermissionsTest.java index 882cab4b4b76..c68fe7710288 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ReadFeedPermissionsTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ReadFeedPermissionsTest.java @@ -3,10 +3,10 @@ package com.azure.cosmos.rx; import com.azure.cosmos.CosmosAsyncClient; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.CosmosAsyncDatabase; import com.azure.cosmos.CosmosAsyncUser; import com.azure.cosmos.CosmosClientBuilder; -import com.azure.cosmos.implementation.DatabaseForTest; import com.azure.cosmos.implementation.FeedResponseListValidator; import com.azure.cosmos.implementation.FeedResponseValidator; import com.azure.cosmos.models.CosmosPermissionProperties; @@ -25,7 +25,7 @@ public class ReadFeedPermissionsTest extends TestSuiteBase { - public final String databaseId = DatabaseForTest.generateId(); + public final String databaseId = CosmosDatabaseForTest.generateId(); private CosmosAsyncDatabase createdDatabase; private CosmosAsyncUser createdUser; diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ResourceTokenTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ResourceTokenTest.java index 3a3354616897..0e302c38c38a 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ResourceTokenTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ResourceTokenTest.java @@ -2,6 +2,7 @@ // Licensed under the MIT License. package com.azure.cosmos.rx; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.ConnectionMode; import com.azure.cosmos.ConsistencyLevel; import com.azure.cosmos.implementation.AsyncDocumentClient; @@ -13,7 +14,6 @@ import com.azure.cosmos.models.CosmosClientTelemetryConfig; import com.azure.cosmos.implementation.ConnectionPolicy; import com.azure.cosmos.implementation.Database; -import com.azure.cosmos.implementation.DatabaseForTest; import com.azure.cosmos.implementation.Document; import com.azure.cosmos.implementation.DocumentCollection; import com.azure.cosmos.implementation.FailureValidator; @@ -53,7 +53,7 @@ // TODO change to use external TestSuiteBase public class ResourceTokenTest extends TestSuiteBase { - public final String databaseId = DatabaseForTest.generateId(); + public final String databaseId = CosmosDatabaseForTest.generateId(); private Database createdDatabase; private DocumentCollection createdCollection; diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/TestSuiteBase.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/TestSuiteBase.java index cfc11c6b9212..a66473e6f571 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/TestSuiteBase.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/TestSuiteBase.java @@ -23,6 +23,7 @@ import com.azure.cosmos.CosmosException; import com.azure.cosmos.CosmosNettyLeakDetectorFactory; import com.azure.cosmos.CosmosResponseValidator; +import com.azure.cosmos.CosmosTestResourceRegistry; import com.azure.cosmos.DirectConnectionConfig; import com.azure.cosmos.GatewayConnectionConfig; import com.azure.cosmos.Http2ConnectionConfig; @@ -935,6 +936,7 @@ private static void createCollectionIfNotExists( return Mono.empty(); }) .block(); + CosmosTestResourceRegistry.registerContainer(database.getId(), cosmosContainerProperties.getId()); } protected static void waitForCollectionToBeAvailableToRead(CosmosAsyncContainer container, CosmosAsyncClient probeClient) { @@ -1268,6 +1270,7 @@ public static CosmosAsyncContainer createCollection(CosmosAsyncDatabase database return Mono.empty(); }) .block(); + CosmosTestResourceRegistry.registerContainer(database.getId(), cosmosContainerProperties.getId()); waitForCollectionToBeAvailableToRead(database.getContainer(cosmosContainerProperties.getId()), probeClient); getFeedRangesWithRetry( getContainerForReadinessProbe(database, cosmosContainerProperties.getId(), probeClient), @@ -1395,6 +1398,7 @@ public static CosmosAsyncContainer createCollection(CosmosAsyncClient client, St public static void deleteCollection(CosmosAsyncClient client, String dbId, String collectionId) { client.getDatabase(dbId).getContainer(collectionId).delete().block(); + CosmosTestResourceRegistry.unregisterContainer(dbId, collectionId); } public static InternalObjectNode createDocument(CosmosAsyncContainer cosmosContainer, InternalObjectNode item) { @@ -1634,10 +1638,14 @@ public static void deleteCollectionIfExists(CosmosAsyncClient client, String dat public static void deleteCollection(CosmosAsyncDatabase cosmosDatabase, String collectionId) { cosmosDatabase.getContainer(collectionId).delete().block(); + CosmosTestResourceRegistry.unregisterContainer(cosmosDatabase.getId(), collectionId); } public static void deleteCollection(CosmosAsyncContainer cosmosContainer) { cosmosContainer.delete().block(); + CosmosTestResourceRegistry.unregisterContainer( + cosmosContainer.getDatabase().getId(), + cosmosContainer.getId()); } public static void deleteDocumentIfExists(CosmosAsyncClient client, String databaseId, String collectionId, String docId) { @@ -1695,23 +1703,89 @@ static private CosmosAsyncDatabase safeCreateDatabase(CosmosAsyncClient client, .filter(TestSuiteBase::isTransientCreateFailure)) .onErrorResume(e -> isConflictException(e) ? Mono.empty() : Mono.error(e)) .block(); + CosmosTestResourceRegistry.registerDatabase(databaseSettings.getId()); return client.getDatabase(databaseSettings.getId()); } + /** + * Creates a database whose id carries the current run id, and registers it for automatic cleanup. + * This is the sanctioned way for tests to create a database - it guarantees the database is + * deleted at the end of the run even if the test fails to delete it, and guarantees that cleanup + * running on a shared account can attribute it to this run instead of a concurrent one. + * + * @param client the client to create the database with. + * @return the created database. + */ + static protected CosmosAsyncDatabase createTestDatabase(CosmosAsyncClient client) { + return createTestDatabase(client, null); + } + + /** + * Overload of {@link #createTestDatabase(CosmosAsyncClient)} that embeds a human readable label in + * the generated id to make logs and portal views easier to read. The label does not affect + * uniqueness or cleanup scoping. + * + * @param client the client to create the database with. + * @param label optional label, may be null. + * @return the created database. + */ + static protected CosmosAsyncDatabase createTestDatabase(CosmosAsyncClient client, String label) { + return createDatabaseInternal(client, CosmosDatabaseForTest.generateId(label)); + } + + /** + * Synchronous counterpart of {@link #createTestDatabase(CosmosAsyncClient)}. + * + * @param client the client to create the database with. + * @param label optional label, may be null. + * @return the created database. + */ + static protected CosmosDatabase createTestSyncDatabase(CosmosClient client, String label) { + String databaseId = CosmosDatabaseForTest.generateId(label); + client.createDatabase(new CosmosDatabaseProperties(databaseId)); + CosmosTestResourceRegistry.registerDatabase(databaseId); + return client.getDatabase(databaseId); + } + + /** + * @deprecated tests should use {@link #createTestDatabase(CosmosAsyncClient, String)} so the id + * carries the run id and cleanup can attribute the database to this run. Databases created with an + * arbitrary id are still registered for cleanup, but only the in-process janitor can delete them - + * if the JVM is killed (job cancelled or timed out) they leak permanently on shared accounts. + * + * @param client the client to create the database with. + * @param databaseId the database id. + * @return the created database. + */ + @Deprecated static protected CosmosAsyncDatabase createDatabase(CosmosAsyncClient client, String databaseId) { + return createDatabaseInternal(client, databaseId); + } + + private static CosmosAsyncDatabase createDatabaseInternal(CosmosAsyncClient client, String databaseId) { CosmosDatabaseProperties databaseSettings = new CosmosDatabaseProperties(databaseId); client.createDatabase(databaseSettings) .retryWhen(Retry.fixedDelay(3, Duration.ofSeconds(5)) .filter(TestSuiteBase::isTransientCreateFailure)) .onErrorResume(e -> isConflictException(e) ? Mono.empty() : Mono.error(e)) .block(); + CosmosTestResourceRegistry.registerDatabase(databaseSettings.getId()); return client.getDatabase(databaseSettings.getId()); } + /** + * @deprecated use {@link #createTestSyncDatabase(CosmosClient, String)} instead. + * + * @param client the client to create the database with. + * @param databaseId the database id. + * @return the created database, or null when creation failed. + */ + @Deprecated static protected CosmosDatabase createSyncDatabase(CosmosClient client, String databaseId) { CosmosDatabaseProperties databaseSettings = new CosmosDatabaseProperties(databaseId); try { client.createDatabase(databaseSettings); + CosmosTestResourceRegistry.registerDatabase(databaseSettings.getId()); return client.getDatabase(databaseSettings.getId()); } catch (CosmosException e) { e.printStackTrace(); @@ -1719,6 +1793,15 @@ static protected CosmosDatabase createSyncDatabase(CosmosClient client, String d return null; } + /** + * @deprecated use {@link #createTestDatabase(CosmosAsyncClient, String)} instead. See + * {@link #createDatabase(CosmosAsyncClient, String)} for why arbitrary ids are discouraged. + * + * @param client the client to create the database with. + * @param databaseId the database id. + * @return the existing or newly created database. + */ + @Deprecated static protected CosmosAsyncDatabase createDatabaseIfNotExists(CosmosAsyncClient client, String databaseId) { List res = client.queryDatabases(String.format("SELECT * FROM r where r.id = '%s'", databaseId), null) .collectList() @@ -1726,6 +1809,7 @@ static protected CosmosAsyncDatabase createDatabaseIfNotExists(CosmosAsyncClient if (res.size() != 0) { CosmosAsyncDatabase database = client.getDatabase(databaseId); database.read().block(); + CosmosTestResourceRegistry.registerDatabase(databaseId); return database; } else { CosmosDatabaseProperties databaseSettings = new CosmosDatabaseProperties(databaseId); @@ -1733,6 +1817,7 @@ static protected CosmosAsyncDatabase createDatabaseIfNotExists(CosmosAsyncClient .retryWhen(Retry.fixedDelay(3, Duration.ofSeconds(5)) .filter(TestSuiteBase::isTransientCreateFailure)) .block(); + CosmosTestResourceRegistry.registerDatabase(databaseSettings.getId()); return client.getDatabase(databaseSettings.getId()); } } @@ -1742,6 +1827,8 @@ static protected void safeDeleteDatabase(CosmosAsyncDatabase database) { try { database.delete().block(); } catch (Exception e) { + } finally { + CosmosTestResourceRegistry.unregisterDatabase(database.getId()); } } } @@ -1754,6 +1841,8 @@ static protected void safeDeleteSyncDatabase(CosmosDatabase database) { logger.info("database deletion completed"); } catch (Exception e) { logger.error("failed to delete sync database", e); + } finally { + CosmosTestResourceRegistry.unregisterDatabase(database.getId()); } } } @@ -1806,6 +1895,9 @@ static protected void safeDeleteCollection(CosmosAsyncContainer collection) { } } finally { + CosmosTestResourceRegistry.unregisterContainer( + collection.getDatabase().getId(), + collection.getId()); try { Thread.sleep(100); } catch (InterruptedException e) { @@ -2584,7 +2676,7 @@ public static DocumentCollection createCollection(String databaseId, RequestOptions options) { AsyncDocumentClient client = createGatewayHouseKeepingDocumentClient().build(); try { - return client.createCollection("dbs/" + databaseId, collection, options).block().getResource(); + return createCollection(client, databaseId, collection, options); } finally { client.close(); } @@ -2593,21 +2685,25 @@ public static DocumentCollection createCollection(String databaseId, public static Database createDatabase(AsyncDocumentClient client, String databaseId) { Database database = new Database(); database.setId(databaseId); - return client.createDatabase(database, null).block().getResource(); + return createDatabase(client, database); } public static Database createDatabase(AsyncDocumentClient client, Database database) { - return client.createDatabase(database, null).block().getResource(); + Database created = client.createDatabase(database, null).block().getResource(); + CosmosTestResourceRegistry.registerDatabase(created.getId()); + return created; } public static DocumentCollection createCollection(AsyncDocumentClient client, String databaseId, DocumentCollection collection, RequestOptions options) { - return client.createCollection("dbs/" + databaseId, collection, options).block().getResource(); + DocumentCollection created = client.createCollection("dbs/" + databaseId, collection, options).block().getResource(); + CosmosTestResourceRegistry.registerContainer(databaseId, created.getId()); + return created; } public static DocumentCollection createCollection(AsyncDocumentClient client, String databaseId, DocumentCollection collection) { - return client.createCollection("dbs/" + databaseId, collection, null).block().getResource(); + return createCollection(client, databaseId, collection, null); } public static Document createDocument(AsyncDocumentClient client, String databaseId, String collectionId, Document document) { @@ -2661,6 +2757,8 @@ protected static void safeDeleteDatabase(AsyncDocumentClient client, Database da client.deleteDatabase(database.getSelfLink(), null).block(); } catch (Exception e) { // Ignore deletion errors + } finally { + CosmosTestResourceRegistry.unregisterDatabase(database.getId()); } } } @@ -2671,6 +2769,8 @@ protected static void safeDeleteDatabase(AsyncDocumentClient client, String data client.deleteDatabase(TestUtils.getDatabaseNameLink(databaseId), null).block(); } catch (Exception e) { System.err.println("Failed to delete database '" + databaseId + "': " + e.getMessage()); + } finally { + CosmosTestResourceRegistry.unregisterDatabase(databaseId); } } } @@ -2691,6 +2791,8 @@ protected static void safeDeleteCollection(AsyncDocumentClient client, String da client.deleteCollection("/dbs/" + databaseId + "/colls/" + collectionId, null).block(); } catch (Exception e) { // Ignore deletion errors + } finally { + CosmosTestResourceRegistry.unregisterContainer(databaseId, collectionId); } } } diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/WorkloadIdDirectInterceptorTests.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/WorkloadIdDirectInterceptorTests.java index b49f476ff399..410bb068fe92 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/WorkloadIdDirectInterceptorTests.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/WorkloadIdDirectInterceptorTests.java @@ -2,6 +2,7 @@ // Licensed under the MIT License. package com.azure.cosmos.rx; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.CosmosAsyncClient; import com.azure.cosmos.CosmosAsyncContainer; import com.azure.cosmos.CosmosAsyncDatabase; @@ -52,7 +53,7 @@ */ public class WorkloadIdDirectInterceptorTests extends TestSuiteBase { - private static final String DATABASE_ID = "workloadIdDirectTestDb-" + UUID.randomUUID(); + private static final String DATABASE_ID = CosmosDatabaseForTest.generateId("workloadIdDirect"); private static final String CONTAINER_ID = "workloadIdDirectTestContainer-" + UUID.randomUUID(); private CosmosAsyncClient clientWithWorkloadId; diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/WorkloadIdE2ETests.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/WorkloadIdE2ETests.java index 1e00a25cf8e4..f808c1d69629 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/WorkloadIdE2ETests.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/WorkloadIdE2ETests.java @@ -2,6 +2,7 @@ // Licensed under the MIT License. package com.azure.cosmos.rx; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.CosmosAsyncClient; import com.azure.cosmos.CosmosAsyncContainer; import com.azure.cosmos.CosmosAsyncDatabase; @@ -45,7 +46,7 @@ */ public class WorkloadIdE2ETests extends TestSuiteBase { - private static final String DATABASE_ID = "workloadIdTestDb-" + UUID.randomUUID(); + private static final String DATABASE_ID = CosmosDatabaseForTest.generateId("workloadIdE2E"); private static final String CONTAINER_ID = "workloadIdTestContainer-" + UUID.randomUUID(); private CosmosAsyncClient clientWithWorkloadId; diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/changefeed/epkversion/IncrementalChangeFeedProcessorTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/changefeed/epkversion/IncrementalChangeFeedProcessorTest.java index 4f0c58acf135..e94dc37f4205 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/changefeed/epkversion/IncrementalChangeFeedProcessorTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/changefeed/epkversion/IncrementalChangeFeedProcessorTest.java @@ -3,6 +3,7 @@ package com.azure.cosmos.rx.changefeed.epkversion; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.BridgeInternal; import com.azure.cosmos.ChangeFeedProcessor; import com.azure.cosmos.ChangeFeedProcessorBuilder; @@ -111,7 +112,7 @@ public class IncrementalChangeFeedProcessorTest extends TestSuiteBase { private final int FEED_COLLECTION_THROUGHPUT = 400; private final int FEED_COLLECTION_THROUGHPUT_FOR_SPLIT = 10100; private final int LEASE_COLLECTION_THROUGHPUT = 400; - private final String MULTI_WRITE_DATABASE_NAME = "multi-write-test-database" + UUID.randomUUID(); + private final String MULTI_WRITE_DATABASE_NAME = CosmosDatabaseForTest.generateId("cfpMultiWrite"); private final String MULTI_WRITE_MONITORED_COLLECTION_NAME = "multi-write-test-monitored-container" + UUID.randomUUID(); private final String MULTI_WRITE_LEASE_COLLECTION_NAME = "multi-write-test-lease-container" + UUID.randomUUID(); @@ -510,7 +511,7 @@ public void readFeedDocumentsStartFromCustomDateForMultiWrite_WithCFPReadFromSat CosmosAsyncContainer createdLeaseCollectionSatelliteRegion = null; CosmosAsyncDatabase cosmosAsyncDatabaseRegionOne = null; - String dbId = UUID.randomUUID().toString(); + String dbId = CosmosDatabaseForTest.generateId("cfp"); String feedCollectionId = UUID.randomUUID().toString(); String leaseCollectionId = UUID.randomUUID().toString(); @@ -644,7 +645,7 @@ public void readFeedDocumentsStartFromCustomDateForMultiWrite_WithCFPReadSwitchT CosmosAsyncContainer createdLeaseCollectionSatelliteRegion = null; CosmosAsyncDatabase cosmosAsyncDatabaseRegionOne = null; - String dbId = UUID.randomUUID().toString(); + String dbId = CosmosDatabaseForTest.generateId("cfp"); String feedContainerId = UUID.randomUUID().toString(); String leaseContainerId = UUID.randomUUID().toString(); diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/changefeed/pkversion/IncrementalChangeFeedProcessorTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/changefeed/pkversion/IncrementalChangeFeedProcessorTest.java index 52e01924ab29..d440d21e1354 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/changefeed/pkversion/IncrementalChangeFeedProcessorTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/changefeed/pkversion/IncrementalChangeFeedProcessorTest.java @@ -2,6 +2,7 @@ // Licensed under the MIT License. package com.azure.cosmos.rx.changefeed.pkversion; +import com.azure.cosmos.CosmosDatabaseForTest; import com.azure.cosmos.ChangeFeedProcessor; import com.azure.cosmos.ChangeFeedProcessorBuilder; import com.azure.cosmos.ConsistencyLevel; @@ -95,7 +96,7 @@ public class IncrementalChangeFeedProcessorTest extends TestSuiteBase { private final int FEED_COLLECTION_THROUGHPUT = 400; private final int FEED_COLLECTION_THROUGHPUT_FOR_SPLIT = 10100; private final int LEASE_COLLECTION_THROUGHPUT = 400; - private final String MULTI_WRITE_DATABASE_NAME = "multi-write-test-database"+ UUID.randomUUID(); + private final String MULTI_WRITE_DATABASE_NAME = CosmosDatabaseForTest.generateId("cfpMultiWrite"); private final String MULTI_WRITE_MONITORED_COLLECTION_NAME = "multi-write-test-monitored-container"+ UUID.randomUUID(); private final String MULTI_WRITE_LEASE_COLLECTION_NAME = "multi-write-test-lease-container"+ UUID.randomUUID(); @@ -525,7 +526,7 @@ public void readFeedDocumentsStartFromCustomDateForMultiWrite_WithCFPReadSwitchT CosmosAsyncContainer createdLeaseCollectionSatelliteRegion = null; CosmosAsyncDatabase cosmosAsyncDatabaseRegionOne = null; - String dbId = UUID.randomUUID().toString(); + String dbId = CosmosDatabaseForTest.generateId("cfp"); String feedContainerId = UUID.randomUUID().toString(); String leaseContainerId = UUID.randomUUID().toString(); diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/cfp-split-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/cfp-split-testng.xml index d39a972d2aec..63c104b0751f 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/cfp-split-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/cfp-split-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-misc-direct-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-misc-direct-testng.xml index 2ac6db27163d..569e634a2e95 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-misc-direct-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-misc-direct-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-misc-gateway-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-misc-gateway-testng.xml index abdf3c0e1e22..d5b6aee8975a 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-misc-gateway-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-misc-gateway-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-read-all-read-many-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-read-all-read-many-testng.xml index 1fccc58fc151..102b1af63428 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-read-all-read-many-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/circuit-breaker-read-all-read-many-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/consistency-overrides-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/consistency-overrides-testng.xml index 5d0a5dc59100..aee57bce1530 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/consistency-overrides-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/consistency-overrides-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/direct-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/direct-testng.xml index 64951b687986..6962c8425fc3 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/direct-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/direct-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/e2e-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/e2e-testng.xml index 98325c1bd9ae..8b7a6c780872 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/e2e-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/e2e-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/emulator-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/emulator-testng.xml index 066030582262..ecdda2b166e2 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/emulator-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/emulator-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/emulator-vnext-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/emulator-vnext-testng.xml index 8b0ed83728e9..84dfb6e94f5e 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/emulator-vnext-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/emulator-vnext-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/examples-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/examples-testng.xml index 0dbd85186569..cd25a73a99cc 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/examples-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/examples-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fast-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fast-testng.xml index 684fd2e99dad..6d27a02c6e7c 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fast-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fast-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-customer-workflows-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-customer-workflows-testng.xml index edfa8a57770f..95a21389f420 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-customer-workflows-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-customer-workflows-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-multi-master-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-multi-master-testng.xml index 95720e19ec27..040a09baa882 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-multi-master-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-multi-master-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-sm-customer-workflows-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-sm-customer-workflows-testng.xml index 976b8fbdc204..45a75050964b 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-sm-customer-workflows-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-sm-customer-workflows-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-thinclient-multi-master-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-thinclient-multi-master-testng.xml index 3f5cc579a414..a30c81bc2abd 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-thinclient-multi-master-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-thinclient-multi-master-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-thinclient-multi-region-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-thinclient-multi-region-testng.xml index c175292f121e..cd49b848f54b 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-thinclient-multi-region-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/fi-thinclient-multi-region-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/flaky-multi-master-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/flaky-multi-master-testng.xml index 174bfcec036f..4ce2db55a09a 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/flaky-multi-master-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/flaky-multi-master-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/gsi-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/gsi-testng.xml index c1b658e62aff..61a7fa0b7a45 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/gsi-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/gsi-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/long-emulator-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/long-emulator-testng.xml index 9359b0a8abbb..e82e1c87c7f1 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/long-emulator-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/long-emulator-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/long-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/long-testng.xml index d9b096c96eae..eb205ef43d22 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/long-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/long-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/manual-http-network-fault-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/manual-http-network-fault-testng.xml index b6127237e422..e17cd3104f66 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/manual-http-network-fault-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/manual-http-network-fault-testng.xml @@ -2,6 +2,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-master-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-master-testng.xml index 1bc9acb85918..7b8c44057c45 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-master-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-master-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-region-strong.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-region-strong.xml index 9b8607cbf971..7b3d97963c83 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-region-strong.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-region-strong.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-region-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-region-testng.xml index f9dfd7eba045..61d0b6b818aa 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-region-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/multi-region-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/query-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/query-testng.xml index 4422c770175d..98173c1d0156 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/query-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/query-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/split-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/split-testng.xml index 65c6dec92ee6..096778d4bbf8 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/split-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/split-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/test-resource-hygiene-baseline.properties b/sdk/cosmos/azure-cosmos-tests/src/test/resources/test-resource-hygiene-baseline.properties new file mode 100644 index 000000000000..22bdae8da638 --- /dev/null +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/test-resource-hygiene-baseline.properties @@ -0,0 +1,64 @@ +# Baseline for TestResourceHygieneTest. +# +# Each entry is "=" that existed when the +# test resource hygiene ratchet was introduced. The build fails when a file exceeds its entry, or when +# a file not listed here creates a database directly. +# +# Do not add entries for new tests. Use TestSuiteBase.createTestDatabase(...) instead - see +# sdk/cosmos/AGENTS.md. Lowering an entry after migrating a file is always welcome. +com/azure/cosmos/AzureKeyCredentialTest.java=4 +com/azure/cosmos/ContainerPropertiesTest.java=1 +com/azure/cosmos/CosmosClientEncryptionKeyTest.java=1 +com/azure/cosmos/CosmosContainerChangeFeedTest.java=1 +com/azure/cosmos/CosmosContainerContentResponseOnWriteTest.java=1 +com/azure/cosmos/CosmosContainerTest.java=1 +com/azure/cosmos/CosmosDatabaseContentResponseOnWriteTest.java=2 +com/azure/cosmos/CosmosDatabaseTest.java=11 +com/azure/cosmos/CosmosDiagnosticsTest.java=4 +com/azure/cosmos/CosmosMultiHashTest.java=1 +com/azure/cosmos/CosmosTracerTest.java=2 +com/azure/cosmos/CosmosUserTest.java=1 +com/azure/cosmos/EmulatorVNextWithHttpTest.java=1 +com/azure/cosmos/EndToEndTimeOutValidationTests.java=1 +com/azure/cosmos/FaultInjectionWithAvailabilityStrategyTestsBase.java=1 +com/azure/cosmos/FeedRangeTest.java=1 +com/azure/cosmos/InvalidHostnameTest.java=2 +com/azure/cosmos/MaxRetryCountTests.java=1 +com/azure/cosmos/ReadManyByPartitionKeyTest.java=1 +com/azure/cosmos/ResourceTokenTestForV4.java=1 +com/azure/cosmos/implementation/batch/BulkExecutorTest.java=1 +com/azure/cosmos/implementation/batch/TransactionalBulkExecutorTest.java=1 +com/azure/cosmos/implementation/directconnectivity/MetadataRequestRetryPolicyTests.java=1 +com/azure/cosmos/rx/AadAuthorizationTests.java=1 +com/azure/cosmos/rx/CollectionCrudTest.java=2 +com/azure/cosmos/rx/ContainerCreateDeleteWithSameNameTest.java=1 +com/azure/cosmos/rx/ContainerQueryTest.java=1 +com/azure/cosmos/rx/DatabaseCrudTest.java=6 +com/azure/cosmos/rx/DatabaseQueryTest.java=2 +com/azure/cosmos/rx/DocumentCrudTest.java=1 +com/azure/cosmos/rx/FullTextIndexTest.java=1 +com/azure/cosmos/rx/GatewayReadConsistencyStrategyE2ETest.java=1 +com/azure/cosmos/rx/GatewayReadConsistencyStrategySpyWireTest.java=1 +com/azure/cosmos/rx/GlobalSecondaryIndexContainerCrudTest.java=1 +com/azure/cosmos/rx/HybridSearchQueryTest.java=1 +com/azure/cosmos/rx/MultiMasterConflictResolutionTest.java=1 +com/azure/cosmos/rx/NonStreamingOrderByQueryVectorSearchTest.java=1 +com/azure/cosmos/rx/OfferQueryTest.java=1 +com/azure/cosmos/rx/OfferReadReplaceTest.java=1 +com/azure/cosmos/rx/PermissionCrudTest.java=1 +com/azure/cosmos/rx/PermissionQueryTest.java=1 +com/azure/cosmos/rx/ReadFeedCollectionsTest.java=1 +com/azure/cosmos/rx/ReadFeedDatabasesTest.java=2 +com/azure/cosmos/rx/ReadFeedOffersTest.java=1 +com/azure/cosmos/rx/ReadFeedPermissionsTest.java=1 +com/azure/cosmos/rx/ReadFeedUsersTest.java=1 +com/azure/cosmos/rx/ResourceTokenTest.java=1 +com/azure/cosmos/rx/ThroughputTests.java=4 +com/azure/cosmos/rx/UniqueIndexTest.java=1 +com/azure/cosmos/rx/UserCrudTest.java=1 +com/azure/cosmos/rx/UserQueryTest.java=1 +com/azure/cosmos/rx/VectorIndexTest.java=1 +com/azure/cosmos/rx/WorkloadIdDirectInterceptorTests.java=1 +com/azure/cosmos/rx/WorkloadIdE2ETests.java=1 +com/azure/cosmos/rx/changefeed/epkversion/IncrementalChangeFeedProcessorTest.java=3 +com/azure/cosmos/rx/changefeed/pkversion/IncrementalChangeFeedProcessorTest.java=3 diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/thinclient-endpoint-probe-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/thinclient-endpoint-probe-testng.xml index 3645644e2d79..7a698b722b41 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/thinclient-endpoint-probe-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/thinclient-endpoint-probe-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/resources/thinclient-testng.xml b/sdk/cosmos/azure-cosmos-tests/src/test/resources/thinclient-testng.xml index 7e9ec9f26db8..cd1642a5b433 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/resources/thinclient-testng.xml +++ b/sdk/cosmos/azure-cosmos-tests/src/test/resources/thinclient-testng.xml @@ -24,6 +24,7 @@ + diff --git a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DatabaseForTest.java b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DatabaseForTest.java index aeeb61da40c9..c5b26ca5fcfb 100644 --- a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DatabaseForTest.java +++ b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DatabaseForTest.java @@ -15,14 +15,27 @@ import java.time.Duration; import java.time.LocalDateTime; +import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.util.Collections; import java.util.List; +/** + * Test-only helper for creating and reaping the shared databases used by the SDK's own test and + * benchmark suites. It lives in {@code src/main} purely for module visibility (azure-cosmos-benchmark + * consumes it) and is not part of the public or supported surface. + *

+ * {@link #cleanupStaleTestDatabases} performs destructive deletes against whatever account it is pointed + * at, so the age threshold below must stay well above the longest test run. The equivalent helper used by + * azure-cosmos-tests is {@code com.azure.cosmos.CosmosDatabaseForTest}; keep the two id formats + * compatible. + */ public class DatabaseForTest { private static final Logger logger = LoggerFactory.getLogger(DatabaseForTest.class); public static final String SHARED_DB_ID_PREFIX = "RxJava.SDKTest.SharedDatabase"; - private static final Duration CLEANUP_THRESHOLD_DURATION = Duration.ofHours(2); + // Must stay comfortably above the longest live test stage timeout (currently 210 minutes), otherwise + // this sweep can delete a database that a concurrently running test job is still using. + private static final Duration CLEANUP_THRESHOLD_DURATION = Duration.ofHours(8); private static final String DELIMITER = "_"; private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss"); @@ -39,11 +52,16 @@ private boolean isStale() { } private boolean isOlderThan(Duration dur) { - return createdTime.isBefore(LocalDateTime.now().minus(dur)); + return createdTime.isBefore(nowUtc().minus(dur)); + } + + // Ids are written on one machine and compared on another, so both sides must use UTC. + private static LocalDateTime nowUtc() { + return LocalDateTime.now(ZoneOffset.UTC); } public static String generateId() { - return SHARED_DB_ID_PREFIX + DELIMITER + TIME_FORMATTER.format(LocalDateTime.now()) + DELIMITER + RandomStringUtils.randomAlphabetic(3); + return SHARED_DB_ID_PREFIX + DELIMITER + TIME_FORMATTER.format(nowUtc()) + DELIMITER + RandomStringUtils.randomAlphabetic(3); } private static DatabaseForTest from(Database db) { @@ -57,7 +75,9 @@ private static DatabaseForTest from(Database db) { } String[] parts = StringUtils.split(id, DELIMITER); - if (parts.length != 3) { + // 3 parts: __. 4 parts adds a run id and is produced by + // azure-cosmos-tests' CosmosDatabaseForTest; both forms appear on the shared test accounts. + if (parts == null || parts.length < 3 || parts.length > 4) { return null; } if (!StringUtils.equals(parts[0], SHARED_DB_ID_PREFIX)) { @@ -100,7 +120,8 @@ public static void cleanupStaleTestDatabases(DatabaseManager client) { DatabaseForTest dbForTest = DatabaseForTest.from(db); - if (dbForTest.isStale()) { + // A null dbForTest means the id does not follow the test convention - leave it alone. + if (dbForTest != null && dbForTest.isStale()) { logger.info("Deleting database {}", db.getId()); client.deleteDatabase(db.getId()).block(); } diff --git a/sdk/cosmos/cleanup-test-resources.yml b/sdk/cosmos/cleanup-test-resources.yml new file mode 100644 index 000000000000..0ee8997ad222 --- /dev/null +++ b/sdk/cosmos/cleanup-test-resources.yml @@ -0,0 +1,39 @@ +# Deletes the Cosmos databases created by the current job. +# +# The in-process janitor (CosmosTestResourceJanitor) already cleans up at the end of a test run, but it +# cannot help when the JVM is killed before any listener or shutdown hook runs. This step runs at the end +# of the job and deletes exactly the databases tagged with this job's run id, so it is safe on the long +# lived shared accounts where several legs execute concurrently. +# +# It is a bonus layer, not the backstop: on cancellation it only gets cancelTimeoutInMinutes (5 by +# default) to finish, which a cold Maven start plus a metadata sweep can exceed. Cancelled and +# timed-out jobs are covered by the scheduled janitor pipeline (sdk/cosmos/janitor.yml). + +parameters: + - name: AccountHost + type: string + - name: AccountKey + type: string + - name: DisplayName + type: string + default: 'Clean up Cosmos test resources for this run' + +steps: + - task: Maven@4 + displayName: ${{ parameters.DisplayName }} + # Cleanup must run even when the tests failed - that is exactly when resources are most likely to + # have been left behind. always() does also run on cancellation, but everything after a cancel has + # to finish inside cancelTimeoutInMinutes (5 by default), which a cold Maven start plus a metadata + # sweep can exceed. Cancelled and timed-out jobs are therefore covered by janitor.yml, not here. + condition: always() + continueOnError: true + inputs: + mavenPomFile: sdk/cosmos/azure-cosmos-tests/pom.xml + goals: 'exec:java' + options: >- + -Dexec.args="--account-host ${{ parameters.AccountHost }} --account-key ${{ parameters.AccountKey }}" + mavenOptions: '$(MemoryOptions) $(LoggingOptions)' + javaHomeOption: 'JDKVersion' + jdkVersionOption: $(JavaTestVersion) + jdkArchitectureOption: 'x64' + publishJUnitResults: false diff --git a/sdk/cosmos/dev.md b/sdk/cosmos/dev.md index f8ac22cb9dae..90ef17ae2dfb 100644 --- a/sdk/cosmos/dev.md +++ b/sdk/cosmos/dev.md @@ -27,6 +27,11 @@ Running tests require Azure Cosmos DB Endpoint credentials: mvn test -DACCOUNT_HOST="https://REPLACE_ME_WITH_YOURS.documents.azure.com:443/" -DACCOUNT_KEY="REPLACE_ME_WITH_YOURS" ``` +Tests that create databases must use `TestSuiteBase.createTestDatabase(...)` so CI cleanup can delete +them - see [test resource hygiene](AGENTS.md#test-resource-hygiene-required-for-any-test-that-touches-an-account). +Creating a database directly fails +the build. + ## Import into Intellij or Eclipse - Load the main parent project pom file in Intellij/Eclipse (That should automatically load examples). diff --git a/sdk/cosmos/janitor.yml b/sdk/cosmos/janitor.yml new file mode 100644 index 000000000000..d7f78a10c2e4 --- /dev/null +++ b/sdk/cosmos/janitor.yml @@ -0,0 +1,100 @@ +# Scheduled janitor for the long lived shared Cosmos test accounts. +# +# The in-process janitor (CosmosTestResourceJanitor) and the always-run post steps in tests.yml cover +# the cases where the test JVM or the job gets a chance to clean up. This pipeline is the backstop for +# the cases where they do not - a cancelled job, an agent that dies, or a stage that hits its timeout - +# by sweeping test databases older than any plausible in-flight run. +# +# Only databases whose id follows the test naming convention +# (RxJava.SDKTest.SharedDatabase___) are deleted, so hand created fixtures on +# these accounts are never touched. OlderThanHours must stay comfortably above the longest test stage +# timeout (currently 210 minutes) so that a running job's databases are never deleted. +# +# The pipeline definition must be linked to the same variable group as the Cosmos live test pipeline so +# that the account endpoint/key variables below resolve. + +trigger: none +pr: none + +schedules: + - cron: '0 */6 * * *' + displayName: 'Every six hours' + branches: + include: + - main + always: true + +parameters: + - name: OlderThanHours + type: number + default: 8 + - name: Accounts + type: object + default: + - Name: ThinClient + HostVariable: thinclient-test-endpoint + KeyVariable: thinclient-test-key + - Name: ThinClientCanaryMultiRegion + HostVariable: thin-client-canary-multi-region-session-endpoint + KeyVariable: thin-client-canary-multi-region-session-key + - Name: ThinClientCanaryMultiWriter + HostVariable: thin-client-canary-multi-writer-session-endpoint + KeyVariable: thin-client-canary-multi-writer-session-key + - Name: Gsi + HostVariable: gsi-pipeline-uri + KeyVariable: gsi-pipeline-key + +extends: + template: /eng/pipelines/templates/stages/1es-redirect.yml + parameters: + UseOfficial: false + stages: + - stage: CleanupCosmosTestAccounts + displayName: 'Clean up stale Cosmos test resources' + variables: + - template: /eng/pipelines/templates/variables/globals.yml + - template: /eng/pipelines/templates/variables/image.yml + jobs: + - job: Cleanup + timeoutInMinutes: 90 + pool: + name: $(LINUXPOOL) + image: $(LINUXVMIMAGE) + os: linux + steps: + - template: /eng/pipelines/templates/steps/maven-authenticate.yml + + - task: Maven@4 + displayName: 'Build azure-cosmos-tests' + inputs: + mavenPomFile: pom.xml + goals: 'install' + options: >- + --batch-mode --fail-at-end -DskipTests $(DefaultSkipOptions) -Djacoco.skip=true + -pl com.azure:azure-cosmos-tests -am + mavenOptions: '$(MemoryOptions) $(LoggingOptions)' + javaHomeOption: 'JDKVersion' + jdkVersionOption: $(JavaTestVersion) + jdkArchitectureOption: 'x64' + publishJUnitResults: false + + - ${{ each account in parameters.Accounts }}: + - task: Maven@4 + displayName: 'Sweep ${{ account.Name }}' + # One unreachable account must not stop the others from being swept. An account whose + # sweep fails or is incomplete surfaces as SucceededWithIssues on this step - the stage + # stays green, so check the step annotations rather than the stage result. + condition: always() + continueOnError: true + inputs: + mavenPomFile: sdk/cosmos/azure-cosmos-tests/pom.xml + goals: 'exec:java' + options: >- + -Dexec.args="--account-host $(${{ account.HostVariable }}) + --account-key $(${{ account.KeyVariable }}) + --older-than ${{ parameters.OlderThanHours }}" + mavenOptions: '$(MemoryOptions) $(LoggingOptions)' + javaHomeOption: 'JDKVersion' + jdkVersionOption: $(JavaTestVersion) + jdkArchitectureOption: 'x64' + publishJUnitResults: false diff --git a/sdk/cosmos/tests.yml b/sdk/cosmos/tests.yml index 1fee90f5c46b..e767cd634cdb 100644 --- a/sdk/cosmos/tests.yml +++ b/sdk/cosmos/tests.yml @@ -70,6 +70,13 @@ extends: - template: /eng/pipelines/templates/stages/archetype-sdk-tests-isolated.yml parameters: TestName: 'Cosmos_Live_Test_Http2NetworkFault' + # Long lived shared account: nothing tears it down at the end of the run, so leaked + # databases stay forever. Delete this job's databases even if the job failed. + PostSteps: + - template: /sdk/cosmos/cleanup-test-resources.yml + parameters: + AccountHost: $(thinclient-test-endpoint) + AccountKey: $(thinclient-test-key) PreSteps: - script: | sudo apt-get update -qq && sudo apt-get install -y -qq iptables @@ -108,6 +115,13 @@ extends: - template: /eng/pipelines/templates/stages/archetype-sdk-tests-isolated.yml parameters: TestName: 'Cosmos_Live_Test_ThinClient' + # Long lived shared account: nothing tears it down at the end of the run, so leaked + # databases stay forever. Delete this job's databases even if the job failed. + PostSteps: + - template: /sdk/cosmos/cleanup-test-resources.yml + parameters: + AccountHost: $(thinclient-test-endpoint) + AccountKey: $(thinclient-test-key) CloudConfig: Public: ServiceConnection: azure-sdk-tests-cosmos @@ -140,6 +154,13 @@ extends: - template: /eng/pipelines/templates/stages/archetype-sdk-tests-isolated.yml parameters: TestName: 'Cosmos_Live_Test_ThinClient_EndpointProbe' + # Long lived shared account: nothing tears it down at the end of the run, so leaked + # databases stay forever. Delete this job's databases even if the job failed. + PostSteps: + - template: /sdk/cosmos/cleanup-test-resources.yml + parameters: + AccountHost: $(thinclient-test-endpoint) + AccountKey: $(thinclient-test-key) CloudConfig: Public: ServiceConnection: azure-sdk-tests-cosmos @@ -176,6 +197,13 @@ extends: - template: /eng/pipelines/templates/stages/archetype-sdk-tests-isolated.yml parameters: TestName: 'Cosmos_Live_Test_ThinClient_MultiRegion' + # Long lived shared account: nothing tears it down at the end of the run, so leaked + # databases stay forever. Delete this job's databases even if the job failed. + PostSteps: + - template: /sdk/cosmos/cleanup-test-resources.yml + parameters: + AccountHost: $(thin-client-canary-multi-region-session-endpoint) + AccountKey: $(thin-client-canary-multi-region-session-key) CloudConfig: Public: ServiceConnection: azure-sdk-tests-cosmos @@ -208,6 +236,13 @@ extends: - template: /eng/pipelines/templates/stages/archetype-sdk-tests-isolated.yml parameters: TestName: 'Cosmos_Live_Test_ThinClient_MultiMaster' + # Long lived shared account: nothing tears it down at the end of the run, so leaked + # databases stay forever. Delete this job's databases even if the job failed. + PostSteps: + - template: /sdk/cosmos/cleanup-test-resources.yml + parameters: + AccountHost: $(thin-client-canary-multi-writer-session-endpoint) + AccountKey: $(thin-client-canary-multi-writer-session-key) CloudConfig: Public: ServiceConnection: azure-sdk-tests-cosmos @@ -240,6 +275,13 @@ extends: - template: /eng/pipelines/templates/stages/archetype-sdk-tests-isolated.yml parameters: TestName: 'Cosmos_Live_Test_GSI' + # Long lived shared account: nothing tears it down at the end of the run, so leaked + # databases stay forever. Delete this job's databases even if the job failed. + PostSteps: + - template: /sdk/cosmos/cleanup-test-resources.yml + parameters: + AccountHost: $(gsi-pipeline-uri) + AccountKey: $(gsi-pipeline-key) CloudConfig: Public: ServiceConnection: azure-sdk-tests-cosmos From 67828b869e170d0c2190efbbd616637a30adc1a6 Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Thu, 30 Jul 2026 13:32:42 -0700 Subject: [PATCH 02/15] Declare the Cosmos test secrets variable group in janitor.yml Linking a variable group on the pipeline definition is how sdk/cosmos/tests.yml works, but that linkage is applied by the engineering system's pipeline generator. janitor.yml needs a hand created definition, so declare the group in YAML instead - creating the pipeline then only requires authorizing the group for it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- sdk/cosmos/janitor.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/cosmos/janitor.yml b/sdk/cosmos/janitor.yml index d7f78a10c2e4..71e70c7b9c14 100644 --- a/sdk/cosmos/janitor.yml +++ b/sdk/cosmos/janitor.yml @@ -52,6 +52,10 @@ extends: - stage: CleanupCosmosTestAccounts displayName: 'Clean up stale Cosmos test resources' variables: + # Declared here rather than linked on the pipeline definition (as sdk/cosmos/tests.yml does) so + # that creating the pipeline needs no extra configuration - the group only has to be authorized + # for it. Supplies the account endpoint/key pairs referenced below. + - group: 'Test Secrets for Cosmos Live Tests - user administered' - template: /eng/pipelines/templates/variables/globals.yml - template: /eng/pipelines/templates/variables/image.yml jobs: From ced4ee3b8ce8eeadb2d5c6160860b4a4ce6b8a71 Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Thu, 30 Jul 2026 17:11:49 -0700 Subject: [PATCH 03/15] Validate test database ids at creation, and fix cspell The static ratchet in TestResourceHygieneTest counts creation call sites per file, so it cannot see three ways a database can still end up with a name CI cleanup will never find: - an id built at runtime rather than written literally, - an id swapped inside one of the 56 files that already carry a baseline allowance, with the call count unchanged, - creation through an API the scanner does not match on. All three matter specifically when the JVM is killed - a cancelled or timed out job - because the pipeline post step and the scheduled janitor locate databases by name, so a wrongly named one is invisible to both and leaks permanently on a shared account. CosmosTestResourceRegistry now rejects, at registration time, any database id that does not parse as a test id, failing the test immediately and naming the offender. Containers are validated on their parent database, since deleting a database reclaims them. Verified by reproducing all three vectors: the swapped id inside a baselined file passes the static ratchet and is caught here. Also add a cspell override for the two changed markdown files. The flagged tokens are JVM system property flags (-DACCOUNT_HOST, -Dcodesnippet.skip, -DCOSMOS...); those in dev.md are pre-existing and only surfaced because the file is now part of the diff. Verified with the same command CI runs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .vscode/cspell.json | 11 ++++++ sdk/cosmos/AGENTS.md | 22 +++++++---- .../cosmos/CosmosDatabaseForTestTest.java | 38 ++++++++++++++++++ .../cosmos/CosmosTestResourceJanitorTest.java | 32 ++++++++------- .../cosmos/CosmosTestResourceRegistry.java | 39 +++++++++++++++++++ 5 files changed, 122 insertions(+), 20 deletions(-) diff --git a/.vscode/cspell.json b/.vscode/cspell.json index a09099e36c8a..7fcc0914506d 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -1019,6 +1019,17 @@ "kevinb" ] }, + { + "filename": [ + "**/sdk/cosmos/AGENTS.md", + "**/sdk/cosmos/dev.md" + ], + "words": [ + "DACCOUNT", + "Dcodesnippet", + "DCOSMOS" + ] + }, { "filename": "**/sdk/cosmos/live-platform-matrix.json", "words": [ diff --git a/sdk/cosmos/AGENTS.md b/sdk/cosmos/AGENTS.md index de7103cc6b92..fb33c7f6b88d 100644 --- a/sdk/cosmos/AGENTS.md +++ b/sdk/cosmos/AGENTS.md @@ -47,13 +47,21 @@ mvn -f sdk/cosmos/azure-cosmos-tests/pom.xml exec:java \ Omit `--older-than` to delete only the current job's databases. -### Guardrail - -`TestResourceHygieneTest` is a ratchet: -`azure-cosmos-tests/src/test/resources/test-resource-hygiene-baseline.properties` records the -violations that existed when the check was introduced, and the build fails when a file exceeds its -entry or a new offending file appears. Do not add entries for new tests; when you migrate a file, -lower or remove its entry. +### Guardrails + +Two layers, because a static scan alone is not enough: + +- **At runtime**, `CosmosTestResourceRegistry` rejects any database id that CI cleanup could not + attribute to this run, failing the test immediately and naming it. This is the layer that actually + holds: it catches ids built at runtime, ids swapped inside a file that already has a ratchet + allowance, and creation through APIs the scanner does not know about. Disable only with + `-DCOSMOS.TEST_RESOURCE_ID_VALIDATION_ENABLED=false`. +- **Statically**, `TestResourceHygieneTest` ratchets against direct database creation. + `azure-cosmos-tests/src/test/resources/test-resource-hygiene-baseline.properties` records the + violations that existed when the check was introduced, and the build fails when a file exceeds its + entry or a new offending file appears. It runs in the `unit` group, so it gives fast feedback on + every PR without needing an account. Do not add entries for new tests; when you migrate a file, + lower or remove its entry. ### Escape hatches diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java index 55e34aa220c5..43ce883d735e 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java @@ -60,6 +60,44 @@ public void generatedIdIsRecognizedAndCarriesTheRunId() { assertThat(id).doesNotContain("/").doesNotContain("\\").doesNotContain("#").doesNotContain("?"); } + @Test(groups = {"unit"}) + public void registeringAnUnattributableDatabaseIdFailsTheTest() { + // The static ratchet cannot see an id built at runtime, or swapped inside a file that already + // has a baseline allowance. This runtime check is what actually closes those gaps: an id that CI + // cleanup could not find by name must never reach a shared account unnoticed. + try { + CosmosTestResourceRegistry.registerDatabase("myHardcodedLeakyDb"); + org.testng.Assert.fail("Expected an AssertionError for an unattributable database id"); + } catch (AssertionError expected) { + assertThat(expected).hasMessageContaining("myHardcodedLeakyDb"); + assertThat(expected).hasMessageContaining("createTestDatabase"); + } finally { + CosmosTestResourceRegistry.clear(); + } + } + + @Test(groups = {"unit"}) + public void registeringAGeneratedDatabaseIdIsAccepted() { + try { + CosmosTestResourceRegistry.registerDatabase(CosmosDatabaseForTest.generateId("ok")); + assertThat(CosmosTestResourceRegistry.leakedSnapshot()).hasSize(1); + } finally { + CosmosTestResourceRegistry.clear(); + } + } + + @Test(groups = {"unit"}) + public void legacyIdsRemainRegisterableDuringRollout() { + // Builds predating the run id still create three segment ids; those parse, so they must not trip + // the new check while both formats are in flight. + try { + CosmosTestResourceRegistry.registerDatabase("RxJava.SDKTest.SharedDatabase_20240101T101010_abc"); + assertThat(CosmosTestResourceRegistry.leakedSnapshot()).hasSize(1); + } finally { + CosmosTestResourceRegistry.clear(); + } + } + @Test(groups = {"unit"}) public void runIdNeverContainsTheIdDelimiter() { // parse() splits on "_". A run id containing one would give every generated id five segments, diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java index 42eaeb356966..91c43adc70e0 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java @@ -24,17 +24,23 @@ */ public class CosmosTestResourceJanitorTest { + // Real generated ids: registration now rejects ids that CI cleanup could not attribute to a run, + // so the fixtures have to be as valid as production's. + private static final String DB_ONE = CosmosDatabaseForTest.generateId("janitorOne"); + private static final String DB_OTHER = CosmosDatabaseForTest.generateId("janitorOther"); + private static final String DB_UNTRACKED = CosmosDatabaseForTest.generateId("janitorUntracked"); + @Test(groups = {"unit"}) public void deletedResourcesAreReportedAsLeaks() { FakeDeleter deleter = new FakeDeleter(); - CosmosTestResourceRegistry.TrackedResource database = database("db1"); + CosmosTestResourceRegistry.TrackedResource database = database(DB_ONE); deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETED); List leaks = CosmosTestResourceJanitor.deleteTrackedResources( Arrays.asList(database), deleter); assertThat(leaks).hasSize(1); - assertThat(leaks.get(0)).contains("db1").contains("deleted"); + assertThat(leaks.get(0)).contains(DB_ONE).contains("deleted"); } @Test(groups = {"unit"}) @@ -42,7 +48,7 @@ public void alreadyGoneResourcesAreNotReportedAsLeaks() { // The test deleted the resource but did not deregister. Nothing leaked, so nothing to report - // otherwise every healthy run would fail. FakeDeleter deleter = new FakeDeleter(); - CosmosTestResourceRegistry.TrackedResource database = database("db1"); + CosmosTestResourceRegistry.TrackedResource database = database(DB_ONE); deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.ALREADY_GONE); assertThat(CosmosTestResourceJanitor.deleteTrackedResources(Arrays.asList(database), deleter)) @@ -52,7 +58,7 @@ public void alreadyGoneResourcesAreNotReportedAsLeaks() { @Test(groups = {"unit"}) public void failedDeletesAreReportedAsStillPresent() { FakeDeleter deleter = new FakeDeleter(); - CosmosTestResourceRegistry.TrackedResource database = database("db1"); + CosmosTestResourceRegistry.TrackedResource database = database(DB_ONE); deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETE_FAILED); List leaks = CosmosTestResourceJanitor.deleteTrackedResources( @@ -66,8 +72,8 @@ public void failedDeletesAreReportedAsStillPresent() { @Test(groups = {"unit"}) public void containersOfADeletedDatabaseAreNotProbed() { FakeDeleter deleter = new FakeDeleter(); - CosmosTestResourceRegistry.TrackedResource database = database("db1"); - CosmosTestResourceRegistry.TrackedResource container = container("db1", "c1"); + CosmosTestResourceRegistry.TrackedResource database = database(DB_ONE); + CosmosTestResourceRegistry.TrackedResource container = container(DB_ONE, "c1"); deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETED); List leaks = CosmosTestResourceJanitor.deleteTrackedResources( @@ -85,11 +91,11 @@ public void containersOfAnAlreadyGoneDatabaseAreNotProbed() { // This is the dominant real case - a test deleted its database without deregistering. If it did // not populate the skip set, a long suite would end by 404-probing every container it ever made. FakeDeleter deleter = new FakeDeleter(); - CosmosTestResourceRegistry.TrackedResource database = database("db1"); + CosmosTestResourceRegistry.TrackedResource database = database(DB_ONE); deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.ALREADY_GONE); List leaks = CosmosTestResourceJanitor.deleteTrackedResources( - Arrays.asList(database, container("db1", "c1"), container("db1", "c2")), deleter); + Arrays.asList(database, container(DB_ONE, "c1"), container(DB_ONE, "c2")), deleter); assertThat(deleter.attempted).containsExactly(database); assertThat(leaks).isEmpty(); @@ -99,8 +105,8 @@ public void containersOfAnAlreadyGoneDatabaseAreNotProbed() { public void containersOfADatabaseThatCouldNotBeDeletedAreStillAttempted() { // The database is still there, so its containers are too and were never attempted. FakeDeleter deleter = new FakeDeleter(); - CosmosTestResourceRegistry.TrackedResource database = database("db1"); - CosmosTestResourceRegistry.TrackedResource container = container("db1", "c1"); + CosmosTestResourceRegistry.TrackedResource database = database(DB_ONE); + CosmosTestResourceRegistry.TrackedResource container = container(DB_ONE, "c1"); deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETE_FAILED); deleter.outcome(container, CosmosTestResourceJanitor.DeleteOutcome.DELETED); @@ -114,7 +120,7 @@ public void containersOfADatabaseThatCouldNotBeDeletedAreStillAttempted() { @Test(groups = {"unit"}) public void containersOfAnUnregisteredDatabaseAreDeletedIndividually() { FakeDeleter deleter = new FakeDeleter(); - CosmosTestResourceRegistry.TrackedResource container = container("dbNotTracked", "c1"); + CosmosTestResourceRegistry.TrackedResource container = container(DB_UNTRACKED, "c1"); deleter.outcome(container, CosmosTestResourceJanitor.DeleteOutcome.DELETED); List leaks = CosmosTestResourceJanitor.deleteTrackedResources( @@ -127,8 +133,8 @@ public void containersOfAnUnregisteredDatabaseAreDeletedIndividually() { @Test(groups = {"unit"}) public void databasesAreAlwaysDeletedBeforeContainers() { FakeDeleter deleter = new FakeDeleter(); - CosmosTestResourceRegistry.TrackedResource containerElsewhere = container("dbOther", "c1"); - CosmosTestResourceRegistry.TrackedResource database = database("db1"); + CosmosTestResourceRegistry.TrackedResource containerElsewhere = container(DB_OTHER, "c1"); + CosmosTestResourceRegistry.TrackedResource database = database(DB_ONE); deleter.outcome(containerElsewhere, CosmosTestResourceJanitor.DeleteOutcome.DELETED); deleter.outcome(database, CosmosTestResourceJanitor.DeleteOutcome.DELETED); diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java index c5c9e2757801..0353003ed263 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java @@ -24,6 +24,7 @@ public final class CosmosTestResourceRegistry { private static final Logger LOGGER = LoggerFactory.getLogger(CosmosTestResourceRegistry.class); + private static final String ID_VALIDATION_PROPERTY = "COSMOS.TEST_RESOURCE_ID_VALIDATION_ENABLED"; // LinkedHashMap keeps creation order, which makes the leak report read chronologically. private static final Map TRACKED_RESOURCES = new LinkedHashMap<>(); @@ -51,11 +52,45 @@ public static void registerDatabase(String databaseId) { return; } + requireCleanableId(databaseId); synchronized (TRACKED_RESOURCES) { TRACKED_RESOURCES.put(key(databaseId, null), new TrackedResource(databaseId, null, owner())); } } + /** + * Fails the test when a database id cannot be attributed to this run. + *

+ * The in-process janitor can clean up any database it was told about, but the pipeline post step and + * the scheduled janitor find databases by name. A database whose id does not carry the run + * id is therefore invisible to them, and if the JVM is killed - a cancelled or timed out job, which + * is exactly when cleanup matters most - it leaks permanently on a shared account. + *

+ * This check is the backstop for the static ratchet in {@code TestResourceHygieneTest}: the ratchet + * counts creation call sites per file and so cannot see an id that is built at runtime, swapped + * inside a file that already has an allowance, or produced through an API it does not know about. + * Checking the id itself catches all of those. + * + * @param databaseId the id to validate. + */ + private static void requireCleanableId(String databaseId) { + if (isValidationDisabled() || CosmosDatabaseForTest.isTestDatabaseId(databaseId)) { + return; + } + + throw new AssertionError(String.format( + "Test database id '%s' created by %s does not follow the required naming convention, so CI" + + " cleanup cannot attribute it to this run and it would leak permanently on the shared" + + " test accounts. Create databases with TestSuiteBase.createTestDatabase(client, label)" + + " or name them with CosmosDatabaseForTest.generateId(label) - see sdk/cosmos/AGENTS.md.", + databaseId, + owner())); + } + + private static boolean isValidationDisabled() { + return "false".equalsIgnoreCase(System.getProperty(ID_VALIDATION_PROPERTY)); + } + public static void unregisterDatabase(String databaseId) { if (databaseId == null) { return; @@ -73,6 +108,10 @@ public static void registerContainer(String databaseId, String containerId) { return; } + // Containers are reclaimed transitively - deleting a database removes them - so the requirement + // is on the parent database's id, not the container's. + requireCleanableId(databaseId); + synchronized (TRACKED_RESOURCES) { TRACKED_RESOURCES.put( key(databaseId, containerId), From 7e7f2a3808bb51c31cf39b9bb21b8cda51446033 Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Thu, 30 Jul 2026 22:24:34 -0700 Subject: [PATCH 04/15] Address review feedback on the test resource registry and guard - registerDatabase / registerContainer used put, so re-registering an existing resource (createDatabaseIfNotExists, or a container recreated with the same id) reattributed it to whichever test touched it last, defeating the point of naming the creator in a leak report. Use putIfAbsent; a genuine delete-then-recreate still records the new owner because unregister removes the entry first. - The run id hash was CRC32, and the comment claimed collisions were "structurally impossible". CRC32 is 32 bits, so that was wrong. Use a truncated SHA-256 (56 bits) and state the real guarantee: the job id makes the hash input unique per leg, and a collision is negligible rather than impossible. Id shape is unchanged - 20 chars, build id still readable. - METHOD_DECLARATION required a leading modifier, so package-private declarations were counted as violations. Key the match on the (...) { shape instead, which a call never has. Regenerated violation counts are identical to the baseline, so nothing is newly skipped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../cosmos/CosmosTestResourceRegistry.java | 8 +++-- .../com/azure/cosmos/CosmosTestRunId.java | 33 +++++++++++++++---- .../azure/cosmos/TestResourceHygieneTest.java | 12 ++++++- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java index 0353003ed263..48653cf6b339 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java @@ -54,7 +54,10 @@ public static void registerDatabase(String databaseId) { requireCleanableId(databaseId); synchronized (TRACKED_RESOURCES) { - TRACKED_RESOURCES.put(key(databaseId, null), new TrackedResource(databaseId, null, owner())); + // putIfAbsent, not put: re-registration (createDatabaseIfNotExists on an existing database) + // must not reattribute the resource to a later test. A genuine delete-then-recreate still + // records the new owner, because unregisterDatabase removes the entry first. + TRACKED_RESOURCES.putIfAbsent(key(databaseId, null), new TrackedResource(databaseId, null, owner())); } } @@ -113,7 +116,8 @@ public static void registerContainer(String databaseId, String containerId) { requireCleanableId(databaseId); synchronized (TRACKED_RESOURCES) { - TRACKED_RESOURCES.put( + // putIfAbsent for the same reason as registerDatabase - see the comment there. + TRACKED_RESOURCES.putIfAbsent( key(databaseId, containerId), new TrackedResource(databaseId, containerId, owner())); } diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java index cde514011efd..f3cbdda3a265 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java @@ -6,7 +6,8 @@ import org.apache.commons.lang3.StringUtils; import java.nio.charset.StandardCharsets; -import java.util.zip.CRC32; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; /** * Identity of the current test run, embedded into the name of every database created by the test @@ -19,6 +20,7 @@ public final class CosmosTestRunId { private static final int MAX_LENGTH = 20; + private static final int HASH_BYTES = 7; private static final String RUN_ID = computeRunId(); private static final boolean IS_CI = System.getenv("BUILD_BUILDID") != null || System.getenv("SYSTEM_JOBID") != null; @@ -53,8 +55,10 @@ private static String computeRunId() { String attempt = StringUtils.defaultString(System.getenv("SYSTEM_JOBATTEMPT"), "1"); // System.JobId is a GUID that is unique per job and stable for the whole job, including its post - // steps, so hashing it makes collisions between concurrently running legs structurally impossible. - // That matters because a run scoped delete on a shared account must never match another leg's id. + // steps, so the hash input differs for every concurrently running leg. The id is a truncated + // digest, so a collision is not impossible - it is ~2^-56 per pair - but uniqueness no longer + // depends on job display names happening to differ. That matters because a run scoped delete on a + // shared account must never match another leg's id. String jobId = System.getenv("SYSTEM_JOBID"); if (StringUtils.isNotEmpty(jobId)) { // The build id is carried along purely so a stray database can be traced back to a build. @@ -94,10 +98,27 @@ private static String compose(String readablePrefix, String hash) { return composed.isEmpty() ? "unknown" : composed; } + /** + * Truncated SHA-256, rendered base36. Deliberately not CRC32: this hash is the only thing keeping two + * concurrently running jobs from sharing a run id, and a run scoped delete that matched another job + * would delete its in-flight databases. 56 bits keeps a collision negligible while staying short + * enough to leave room for the readable build id. + */ private static String shortHash(String value) { - CRC32 crc32 = new CRC32(); - crc32.update(value.getBytes(StandardCharsets.UTF_8)); - return Long.toString(crc32.getValue(), 36); + try { + byte[] digest = MessageDigest.getInstance("SHA-256") + .digest(value.getBytes(StandardCharsets.UTF_8)); + + long truncated = 0L; + for (int i = 0; i < HASH_BYTES; i++) { + truncated = (truncated << 8) | (digest[i] & 0xFFL); + } + + return Long.toString(truncated, 36); + } catch (NoSuchAlgorithmException e) { + // SHA-256 is required of every JRE, so this cannot happen. + throw new IllegalStateException("SHA-256 is unavailable", e); + } } private static String sanitize(String value) { diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/TestResourceHygieneTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/TestResourceHygieneTest.java index 45ea40b2f96c..d3832127c979 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/TestResourceHygieneTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/TestResourceHygieneTest.java @@ -58,8 +58,15 @@ public class TestResourceHygieneTest { "com/azure/cosmos/CosmosTestResourceRegistry.java", "com/azure/cosmos/TestResourceHygieneTest.java"); + /** + * A method declaration: optional modifiers, a return type, a name, a parameter list, and an opening + * brace at end of line. The modifiers are optional so package-private declarations are recognized + * too; what actually distinguishes a declaration from a call is the {@code (} shape, + * which a call ({@code client.createDatabase(} or {@code = createDatabase(}) never has. + */ private static final Pattern METHOD_DECLARATION = Pattern.compile( - "^(?:(?:public|protected|private|static|final|abstract|default|synchronized)\\s+).*\\)\\s*\\{$"); + "^(?:(?:public|protected|private|static|final|abstract|default|synchronized)\\s+)*" + + "[\\w.<>\\[\\],\\s]+\\s+\\w+\\s*\\([^;]*\\)\\s*\\{$"); private static final Pattern DATABASE_CREATION = Pattern.compile( "(?:\\.\\s*createDatabase(?:IfNotExists)?\\s*\\()" @@ -145,6 +152,9 @@ public void scannerDetectsDirectDatabaseCreation() { "public Mono createDatabase(CosmosDatabaseProperties def) {")).isZero(); assertThat(countViolations( "static protected CosmosAsyncDatabase createDatabase(CosmosAsyncClient c, String id) {")).isZero(); + // Package-private declarations have no modifier at all. + assertThat(countViolations( + "CosmosAsyncDatabase createDatabase(CosmosAsyncClient c, String id) {")).isZero(); // ... but a call that merely happens to sit on a line ending in "{" is still counted. assertThat(countViolations("if (x) { client.createDatabase(props); }")).isEqualTo(1); } From 4266d9b6e84092eae25a7e7ca7fb6d8273df1cf1 Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Thu, 30 Jul 2026 22:45:59 -0700 Subject: [PATCH 05/15] Isolate registry state between unit test classes CosmosDatabaseForTestTest asserted on the size of the JVM-global registry snapshot, so it failed whenever CosmosTestResourceJanitorTest ran first and left an entry behind. That is ordering dependent, which is why it passed locally and on the macOS leg but failed on ubuntu2404_18. Clear the registry before and after every method in both classes, and assert that the specific database id is present rather than counting global entries. Verified in both class orderings and against the full unit suite (2659 tests). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../cosmos/CosmosDatabaseForTestTest.java | 45 +++++++++++++------ .../cosmos/CosmosTestResourceJanitorTest.java | 7 ++- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java index 43ce883d735e..c4a898a8ed08 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java @@ -10,6 +10,8 @@ import com.azure.cosmos.models.ModelBridgeInternal; import com.azure.cosmos.util.UtilBridgeInternal; import org.mockito.Mockito; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -38,6 +40,14 @@ public class CosmosDatabaseForTestTest { private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss"); + @BeforeMethod(groups = {"unit"}) + @AfterMethod(groups = {"unit"}) + public void resetRegistry() { + // CosmosTestResourceRegistry is JVM global and shared with the other test classes in this suite, + // so isolate on both sides rather than relying on suite ordering. + CosmosTestResourceRegistry.clear(); + } + /** * A hand created database that matches the query prefix but does not parse as a generated id. This is * the fixture that only the production code protects - it reaches deleteMatching and survives solely @@ -71,31 +81,27 @@ public void registeringAnUnattributableDatabaseIdFailsTheTest() { } catch (AssertionError expected) { assertThat(expected).hasMessageContaining("myHardcodedLeakyDb"); assertThat(expected).hasMessageContaining("createTestDatabase"); - } finally { - CosmosTestResourceRegistry.clear(); } + + assertThat(registeredDatabaseIds()).doesNotContain("myHardcodedLeakyDb"); } @Test(groups = {"unit"}) public void registeringAGeneratedDatabaseIdIsAccepted() { - try { - CosmosTestResourceRegistry.registerDatabase(CosmosDatabaseForTest.generateId("ok")); - assertThat(CosmosTestResourceRegistry.leakedSnapshot()).hasSize(1); - } finally { - CosmosTestResourceRegistry.clear(); - } + String databaseId = CosmosDatabaseForTest.generateId("ok"); + CosmosTestResourceRegistry.registerDatabase(databaseId); + + assertThat(registeredDatabaseIds()).contains(databaseId); } @Test(groups = {"unit"}) public void legacyIdsRemainRegisterableDuringRollout() { // Builds predating the run id still create three segment ids; those parse, so they must not trip // the new check while both formats are in flight. - try { - CosmosTestResourceRegistry.registerDatabase("RxJava.SDKTest.SharedDatabase_20240101T101010_abc"); - assertThat(CosmosTestResourceRegistry.leakedSnapshot()).hasSize(1); - } finally { - CosmosTestResourceRegistry.clear(); - } + String legacyId = "RxJava.SDKTest.SharedDatabase_20240101T101010_abc"; + CosmosTestResourceRegistry.registerDatabase(legacyId); + + assertThat(registeredDatabaseIds()).contains(legacyId); } @Test(groups = {"unit"}) @@ -225,6 +231,17 @@ public void ageBasedCleanupSparesYoungDatabasesAndNonTestIds() { assertThat(deleted).containsExactlyInAnyOrder(old, oldLegacy); } + private static List registeredDatabaseIds() { + List ids = new ArrayList<>(); + for (CosmosTestResourceRegistry.TrackedResource resource : CosmosTestResourceRegistry.leakedSnapshot()) { + if (resource.isDatabase()) { + ids.add(resource.getDatabaseId()); + } + } + + return ids; + } + private static String idFor(LocalDateTime createdAt, String runId) { return idFor(createdAt, runId, "abcdefghij"); } diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java index 91c43adc70e0..96cb061687ac 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceJanitorTest.java @@ -3,6 +3,7 @@ package com.azure.cosmos; +import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -145,9 +146,11 @@ public void databasesAreAlwaysDeletedBeforeContainers() { } @BeforeMethod(groups = {"unit"}) + @AfterMethod(groups = {"unit"}) public void resetRegistry() { - // The registry is JVM global; reset here rather than inside a helper so that helper call order - // cannot silently wipe a resource registered earlier in the same test. + // The registry is JVM global. Reset on both sides: before, so helper call order cannot wipe a + // resource registered earlier in the same test; after, so this class leaves no residue for + // whatever test class the suite runs next. CosmosTestResourceRegistry.clear(); } From bff52826410c6b3cec05aa20e82f4f041142ae0b Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Thu, 30 Jul 2026 22:58:31 -0700 Subject: [PATCH 06/15] Name the test, not the helper, in leak reports The first CI run with the janitor enabled reported leaks as "created by com.azure.cosmos.rx.TestSuiteBase.createDatabaseInternal", which identifies the shared helper rather than the test that leaked - useless for acting on the report, and the report naming the offender is the whole point. Skip the shared test infrastructure when walking the stack for an owner, and fall back to an infrastructure frame (marked as such) only when no test frame is on the stack at all. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../cosmos/CosmosTestResourceRegistry.java | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java index 48653cf6b339..6e9bfc29620f 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestResourceRegistry.java @@ -154,18 +154,35 @@ private static String owner() { return currentTest; } - // Outside an invoked test method (for example @BeforeSuite) walk the stack for the first - // non-infrastructure frame so the leak report still names something actionable. + // Outside an invoked test method (for example @BeforeSuite) walk the stack for the first frame + // that is not test infrastructure. Skipping the shared helpers matters: naming + // TestSuiteBase.createDatabaseInternal tells nobody which test leaked, which is the whole point + // of the report. Fall back to the first infrastructure frame only if nothing better exists. + String infrastructureFrame = null; for (StackTraceElement frame : Thread.currentThread().getStackTrace()) { String className = frame.getClassName(); - if (className.startsWith("com.azure.cosmos") - && !className.equals(CosmosTestResourceRegistry.class.getName()) - && !className.equals(CosmosDatabaseForTest.class.getName())) { - return className + "." + frame.getMethodName(); + if (!className.startsWith("com.azure.cosmos")) { + continue; } + + if (isInfrastructure(className)) { + if (infrastructureFrame == null) { + infrastructureFrame = className + "." + frame.getMethodName(); + } + continue; + } + + return className + "." + frame.getMethodName(); } - return ""; + return infrastructureFrame != null ? infrastructureFrame + " (no test frame on stack)" : ""; + } + + private static boolean isInfrastructure(String className) { + return className.equals(CosmosTestResourceRegistry.class.getName()) + || className.equals(CosmosDatabaseForTest.class.getName()) + || className.equals(CosmosTestResourceJanitor.class.getName()) + || className.equals("com.azure.cosmos.rx.TestSuiteBase"); } private static String key(String databaseId, String containerId) { From fce3289a9043e3495733836e62697059ae841573 Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Fri, 31 Jul 2026 07:38:23 -0700 Subject: [PATCH 07/15] Use an absolute link to the Cosmos AGENTS.md Verify Links rejects relative links, and the anchored form was additionally flagged as invalid format: DO NOT use relative link AGENTS.md#test-resource-hygiene-... 'sdk/cosmos/dev.md' has 1 broken link(s) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- sdk/cosmos/dev.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/cosmos/dev.md b/sdk/cosmos/dev.md index 90ef17ae2dfb..86447f24aa13 100644 --- a/sdk/cosmos/dev.md +++ b/sdk/cosmos/dev.md @@ -28,9 +28,8 @@ mvn test -DACCOUNT_HOST="https://REPLACE_ME_WITH_YOURS.documents.azure.com:443/" ``` Tests that create databases must use `TestSuiteBase.createTestDatabase(...)` so CI cleanup can delete -them - see [test resource hygiene](AGENTS.md#test-resource-hygiene-required-for-any-test-that-touches-an-account). -Creating a database directly fails -the build. +them - see [test resource hygiene](https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/cosmos/AGENTS.md). +Creating a database directly fails the build. ## Import into Intellij or Eclipse From 6b26417f8f675ef4b59b192b03d180d80fe7c2fb Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Fri, 31 Jul 2026 07:41:59 -0700 Subject: [PATCH 08/15] Delete the databases three CosmosDiagnosticsTest tests created The janitor's first CI run found these, which is what it is for: negativeE2ETimeoutWithPointOperation negativeE2ETimeoutWithQueryOperation responseStatisticRequestStartTimeUTCForDirectCall Each creates a database and its finally block only closes the client, so the database survives the run. Harmless on the emulator, but these tests also run against the shared fixed accounts, where it is a permanent leak. Delete the database before closing the client that owns it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java index 4c863508754f..6c77e93a4836 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java @@ -1655,6 +1655,7 @@ public void responseStatisticRequestStartTimeUTCForDirectCall() { if (faultInjectionRule != null) { faultInjectionRule.disable(); } + safeDeleteDatabase(client == null ? null : client.getDatabase(databaseId)); safeClose(client); } } @@ -1689,6 +1690,7 @@ public void negativeE2ETimeoutWithPointOperation() { logger.info("Expected request timeout: ", cancelledException); } finally { + safeDeleteDatabase(client == null ? null : client.getDatabase(databaseId)); safeClose(client); } } @@ -1728,6 +1730,7 @@ public void negativeE2ETimeoutWithQueryOperation() { logger.info("Expected request timeout: ", cancelledException); } finally { + safeDeleteDatabase(client == null ? null : client.getDatabase(databaseId)); safeClose(client); } } From a5bcca33fd86f8a390a4ecba19d7d5704f8394eb Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Fri, 31 Jul 2026 09:46:27 -0700 Subject: [PATCH 09/15] Shorten generated test database ids ThroughputControlTests fails on this branch with ThroughputControlInitializationException caused by CosmosException 400, body "

Bad Request - Invalid URL

" and the failure count tracked the length of the shared database id: 1 failure at a 103 character id, 12 at 108. The same tests pass on main, where the id is 82 characters (build 6635933, 124 successful invocations), so this is caused by this branch, not pre-existing. The database id is not used only as a database name. Throughput control derives a group id of ///, base64 encodes it and appends a UUID to form a control item id, so every character added here is amplified. I could not pin the exact limit from the logs - the arithmetic says main should already exceed 255 characters, yet main passes - so rather than guess at a threshold, bring the id back well under the length that is known to work. shared database id: 109 -> 71 characters worst case with a label: 81 characters Achieved by replacing the UUID random suffix with 8 alphanumerics, capping the run id at 16 characters and labels at 10, and dropping the label from the shared database. Uniqueness is unaffected: the timestamp and run id already scope the id, and 36^8 random values sit underneath. Both limits are pinned by assertions so this cannot silently regrow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../azure/cosmos/CosmosDatabaseForTest.java | 20 +++++++++++++------ .../cosmos/CosmosDatabaseForTestTest.java | 9 +++++++++ .../com/azure/cosmos/CosmosTestRunId.java | 4 ++-- .../com/azure/cosmos/rx/TestSuiteBase.java | 2 +- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTest.java index 89bc6b622b20..463b10e3861c 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTest.java @@ -7,6 +7,7 @@ import com.azure.cosmos.models.SqlParameter; import com.azure.cosmos.models.SqlQuerySpec; import com.azure.cosmos.util.CosmosPagedFlux; +import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,7 +19,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.UUID; import java.util.function.Predicate; /** @@ -37,6 +37,8 @@ public final class CosmosDatabaseForTest { public static final String SHARED_DB_ID_PREFIX = "RxJava.SDKTest.SharedDatabase"; private static final String DELIMITER = "_"; private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss"); + private static final int RANDOM_SUFFIX_LENGTH = 8; + private static final int MAX_LABEL_LENGTH = 10; private CosmosDatabaseForTest() { } @@ -51,15 +53,21 @@ public static String generateId() { /** * Generates a run tagged database id. The optional label only makes logs and portal views readable - - * uniqueness comes from the UUID and cleanup scoping from the run id. + * uniqueness comes from the timestamp plus the random suffix, and cleanup scoping from the run id. + *

+ * Length matters and is deliberately kept short. The database id is embedded in derived resource + * names elsewhere in the SDK - notably the throughput control group id, which is + * {@code ///}, base64 encoded and then extended with a UUID to + * form a control item id. A long database id overflows that and the emulator rejects the request + * with "400 Bad Request - Invalid URL", so keep this comfortably shorter than the ~82 character ids + * that are known to work. * * @param label optional human readable label, may be null. * @return a database id that cleanup is able to attribute to the current run. */ public static String generateId(String label) { - String suffix = StringUtils.isEmpty(label) - ? UUID.randomUUID().toString() - : sanitizeLabel(label) + UUID.randomUUID(); + String random = RandomStringUtils.randomAlphanumeric(RANDOM_SUFFIX_LENGTH); + String suffix = StringUtils.isEmpty(label) ? random : sanitizeLabel(label) + random; return SHARED_DB_ID_PREFIX + DELIMITER + TIME_FORMATTER.format(nowUtc()) @@ -69,7 +77,7 @@ public static String generateId(String label) { private static String sanitizeLabel(String label) { String sanitized = label.replaceAll("[^A-Za-z0-9]", ""); - return sanitized.length() <= 40 ? sanitized : sanitized.substring(0, 40); + return sanitized.length() <= MAX_LABEL_LENGTH ? sanitized : sanitized.substring(0, MAX_LABEL_LENGTH); } /** diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java index c4a898a8ed08..f2f7f2940b43 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDatabaseForTestTest.java @@ -67,7 +67,16 @@ public void generatedIdIsRecognizedAndCarriesTheRunId() { assertThat(CosmosDatabaseForTest.isTestDatabaseId(id)).isTrue(); // Cosmos ids are capped at 255 characters and may not contain / \ # ? assertThat(id.length()).isLessThan(255); + // Much tighter in practice: the database id is embedded in the throughput control group id, + // which is base64 encoded and extended with a UUID. Ids around 100 characters made the emulator + // reject those requests with "400 Bad Request - Invalid URL"; ~82 characters is known to work. + assertThat(id.length()).isLessThanOrEqualTo(82); assertThat(id).doesNotContain("/").doesNotContain("\\").doesNotContain("#").doesNotContain("?"); + + // The budget has to hold for the longest label a caller can pass, not just a short one. + String longLabelId = CosmosDatabaseForTest.generateId("aVeryLongDescriptiveLabelIndeed"); + assertThat(longLabelId.length()).isLessThanOrEqualTo(82); + assertThat(CosmosDatabaseForTest.isTestDatabaseId(longLabelId)).isTrue(); } @Test(groups = {"unit"}) diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java index f3cbdda3a265..07f72bd228b5 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosTestRunId.java @@ -19,7 +19,7 @@ */ public final class CosmosTestRunId { - private static final int MAX_LENGTH = 20; + private static final int MAX_LENGTH = 16; private static final int HASH_BYTES = 7; private static final String RUN_ID = computeRunId(); private static final boolean IS_CI = @@ -30,7 +30,7 @@ private CosmosTestRunId() { /** * Returns the identifier of the current test run. Stable for the lifetime of the JVM and safe to - * embed in a Cosmos resource id (lower case alphanumerics only, at most 20 characters). + * embed in a Cosmos resource id (lower case alphanumerics only, at most 16 characters). * * @return the run id. */ diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/TestSuiteBase.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/TestSuiteBase.java index 4aa82ae20bc9..62a787d46278 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/TestSuiteBase.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/TestSuiteBase.java @@ -712,7 +712,7 @@ public void beforeSuite() { logger.info("beforeSuite Started"); try (CosmosAsyncClient houseKeepingClient = createGatewayHouseKeepingDocumentClient(true).buildAsyncClient()) { - SHARED_DATABASE = createTestDatabase(houseKeepingClient, "shared"); + SHARED_DATABASE = createTestDatabase(houseKeepingClient); CosmosContainerRequestOptions options = new CosmosContainerRequestOptions(); SHARED_MULTI_PARTITION_COLLECTION = createCollection(SHARED_DATABASE, getCollectionDefinitionWithRangeRangeIndex(), options, 10100); SHARED_MULTI_PARTITION_COLLECTION_WITH_ID_AS_PARTITION_KEY = createCollection(SHARED_DATABASE, getCollectionDefinitionWithRangeRangeIndexWithIdAsPartitionKey(), options, 10100); From 5b5a0f3a118533cd7fa20bd37a492968b9508e62 Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Fri, 31 Jul 2026 16:22:06 -0700 Subject: [PATCH 10/15] Fix the cleanup post step, and the leak it found on the live accounts First live run of this change (build 6646150): 68 of 76 legs green, including every thin client, GSI, Spring and Spark leg. Two real problems in the other 8. 1. The always-run cleanup post step never deleted anything. It failed with Could not find artifact com.azure:azure-cosmos:jar:4.82.0-beta.1 because it did not pass DefaultOptions, so Maven resolved against the agent's default ~/.m2 instead of $(MAVEN_CACHE_FOLDER) where the built jar lives. continueOnError masked it as SucceededWithIssues. Pass DefaultOptions in both cleanup-test-resources.yml and janitor.yml, and align janitor.yml's build step so install and exec:java share one local repository. This is the layer that covers a dead JVM, so it was the least visible and the most important to fix. 2. PermissionCrudTest created a database in @BeforeClass and only closed the client in @AfterClass, leaking it on every "fast" leg - 6 of the 8 failures, all attributed to before_PermissionCrudTest by the janitor. Delete it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../test/java/com/azure/cosmos/rx/PermissionCrudTest.java | 1 + sdk/cosmos/cleanup-test-resources.yml | 4 ++++ sdk/cosmos/janitor.yml | 5 ++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/PermissionCrudTest.java b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/PermissionCrudTest.java index 2cbafba1648a..024c452baef8 100644 --- a/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/PermissionCrudTest.java +++ b/sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/PermissionCrudTest.java @@ -206,6 +206,7 @@ public void before_PermissionCrudTest() { @AfterClass(groups = { "fast" }, timeOut = SHUTDOWN_TIMEOUT, alwaysRun = true) public void afterClass() { + safeDeleteDatabase(createdDatabase); safeClose(client); } diff --git a/sdk/cosmos/cleanup-test-resources.yml b/sdk/cosmos/cleanup-test-resources.yml index 0ee8997ad222..bb33707f5bea 100644 --- a/sdk/cosmos/cleanup-test-resources.yml +++ b/sdk/cosmos/cleanup-test-resources.yml @@ -30,7 +30,11 @@ steps: inputs: mavenPomFile: sdk/cosmos/azure-cosmos-tests/pom.xml goals: 'exec:java' + # DefaultOptions carries -Dmaven.repo.local=$(MAVEN_CACHE_FOLDER). Without it Maven resolves against + # the agent's default ~/.m2, which does not contain the locally built azure-cosmos jar, and the step + # dies with "Could not find artifact com.azure:azure-cosmos" before deleting anything. options: >- + $(DefaultOptions) -Dexec.args="--account-host ${{ parameters.AccountHost }} --account-key ${{ parameters.AccountKey }}" mavenOptions: '$(MemoryOptions) $(LoggingOptions)' javaHomeOption: 'JDKVersion' diff --git a/sdk/cosmos/janitor.yml b/sdk/cosmos/janitor.yml index 8d197dffec9a..a67b3c7ca2c3 100644 --- a/sdk/cosmos/janitor.yml +++ b/sdk/cosmos/janitor.yml @@ -75,8 +75,10 @@ extends: inputs: mavenPomFile: pom.xml goals: 'install' + # DefaultOptions pins maven.repo.local so the install here and the exec:java sweep + # below resolve against the same local repository. options: >- - --batch-mode --fail-at-end -DskipTests $(DefaultSkipOptions) -Djacoco.skip=true + $(DefaultOptions) -DskipTests $(DefaultSkipOptions) -Djacoco.skip=true -pl com.azure:azure-cosmos-tests -am mavenOptions: '$(MemoryOptions) $(LoggingOptions)' javaHomeOption: 'JDKVersion' @@ -96,6 +98,7 @@ extends: mavenPomFile: sdk/cosmos/azure-cosmos-tests/pom.xml goals: 'exec:java' options: >- + $(DefaultOptions) -Dexec.args="--account-host $(${{ account.HostVariable }}) --account-key $(${{ account.KeyVariable }}) --older-than ${{ parameters.OlderThanHours }}" From 351f1644158a5f72349d1ce54f62b39679c3491f Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Mon, 3 Aug 2026 10:39:15 -0700 Subject: [PATCH 11/15] Never let cleanup turn a live test job red Build 6651706 failed ~50 of 76 legs, up from 8. The underlying cause is infrastructure: the agents cannot resolve the fixed Cosmos account hosts. java.net.UnknownHostException: sdkci-multiregion-strong.documents.azure.com [ERROR] Tests run: 78, Failures: 1, Errors: 0, Skipped: 77 The 77 skips are the tell - the account is unreachable, so the tests never run. The cleanup post step then added a second error to every one of those legs. That part is mine. Making it resolve artifacts correctly (previous commit) meant it finally reached the account, where it hit the same DNS failure, and a failing Maven task reports ##[error] even with continueOnError. Cleanup failing says nothing about the code under test, and it fails precisely when the tests were already broken, so it must not be able to fail a job. Run it through a script that reports problems as warnings and always exits 0. Anything it cannot delete is left to the scheduled janitor, which is the actual backstop. Also passes the account host and key as environment variables rather than on the command line, so the key is not echoed into the log. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- sdk/cosmos/cleanup-test-resources.yml | 47 ++++++++++++++------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/sdk/cosmos/cleanup-test-resources.yml b/sdk/cosmos/cleanup-test-resources.yml index bb33707f5bea..09a7bb9e1905 100644 --- a/sdk/cosmos/cleanup-test-resources.yml +++ b/sdk/cosmos/cleanup-test-resources.yml @@ -5,9 +5,13 @@ # of the job and deletes exactly the databases tagged with this job's run id, so it is safe on the long # lived shared accounts where several legs execute concurrently. # -# It is a bonus layer, not the backstop: on cancellation it only gets cancelTimeoutInMinutes (5 by -# default) to finish, which a cold Maven start plus a metadata sweep can exceed. Cancelled and -# timed-out jobs are covered by the scheduled janitor pipeline (sdk/cosmos/janitor.yml). +# It is a bonus layer, not the backstop, and it must never turn a job red: cleanup failing tells us +# nothing about the code under test, and the accounts are unreachable in exactly the situations where the +# tests were already failing. Anything it cannot delete is reported as a warning and left to the +# scheduled janitor pipeline (sdk/cosmos/janitor.yml), which is the real backstop. +# +# On cancellation this only gets cancelTimeoutInMinutes (5 by default) to finish, which a cold Maven +# start plus a metadata sweep can exceed - another reason janitor.yml, not this step, is the backstop. parameters: - name: AccountHost @@ -19,25 +23,22 @@ parameters: default: 'Clean up Cosmos test resources for this run' steps: - - task: Maven@4 + - pwsh: | + # Deliberately never fails: see the header comment. Errors are surfaced as warnings instead. + mvn -f sdk/cosmos/azure-cosmos-tests/pom.xml ` + "-Dmaven.repo.local=$env:MAVEN_CACHE_FOLDER" ` + --batch-mode ` + "-Dexec.args=--account-host $env:CLEANUP_ACCOUNT_HOST --account-key $env:CLEANUP_ACCOUNT_KEY" ` + exec:java + + if ($LASTEXITCODE -ne 0) { + Write-Host "##vso[task.logissue type=warning]Cosmos test resource cleanup did not complete (mvn exit $LASTEXITCODE). Databases created by this job may still exist; the scheduled janitor pipeline will reclaim them." + } + + exit 0 displayName: ${{ parameters.DisplayName }} - # Cleanup must run even when the tests failed - that is exactly when resources are most likely to - # have been left behind. always() does also run on cancellation, but everything after a cancel has - # to finish inside cancelTimeoutInMinutes (5 by default), which a cold Maven start plus a metadata - # sweep can exceed. Cancelled and timed-out jobs are therefore covered by janitor.yml, not here. condition: always() - continueOnError: true - inputs: - mavenPomFile: sdk/cosmos/azure-cosmos-tests/pom.xml - goals: 'exec:java' - # DefaultOptions carries -Dmaven.repo.local=$(MAVEN_CACHE_FOLDER). Without it Maven resolves against - # the agent's default ~/.m2, which does not contain the locally built azure-cosmos jar, and the step - # dies with "Could not find artifact com.azure:azure-cosmos" before deleting anything. - options: >- - $(DefaultOptions) - -Dexec.args="--account-host ${{ parameters.AccountHost }} --account-key ${{ parameters.AccountKey }}" - mavenOptions: '$(MemoryOptions) $(LoggingOptions)' - javaHomeOption: 'JDKVersion' - jdkVersionOption: $(JavaTestVersion) - jdkArchitectureOption: 'x64' - publishJUnitResults: false + env: + # Passed as environment variables rather than on the command line so the key is not echoed. + CLEANUP_ACCOUNT_HOST: ${{ parameters.AccountHost }} + CLEANUP_ACCOUNT_KEY: ${{ parameters.AccountKey }} From cefea423ae572829b9aa46d1e65fb3d429a2b8d3 Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Mon, 3 Aug 2026 14:39:43 -0700 Subject: [PATCH 12/15] Apply Cosmos account capabilities on create, not just on reconcile Provisioning a fresh tenant produced 18 accounts with no capabilities at all, so every vector-search test failed with A Container Vector Policy has been provided, but the capability has not been enabled on your account New-AzCosmosDBAccount -Capabilities is silently ignored by some Az.CosmosDB versions (seen on Az 12.2.0). The script only reconciled capabilities on its already-exists path, so the fix was to run the whole script a second time - easy to miss, and it makes every 90 day tenant rotation a two pass job. Stop passing capabilities to New-AzCosmosDBAccount and always reconcile through ARM PATCH after the account exists, on both the create and the exists path. The outcome no longer depends on module behaviour, and the PATCH is now verified (polled for up to 5 minutes) so a run cannot report success while leaving accounts the tests will fail against. Verified against the live sdk-ci accounts: -WhatIf still PATCHes nothing and emits only stubbed keys, and a real run is idempotent - 15 accounts report "capabilities up to date" with zero unnecessary PATCH calls. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../New-CosmosLiveTestAccounts.ps1 | 110 +++++++++++++----- .../pipeline/account-provisioning/README.md | 5 +- 2 files changed, 86 insertions(+), 29 deletions(-) diff --git a/sdk/cosmos/pipeline/account-provisioning/New-CosmosLiveTestAccounts.ps1 b/sdk/cosmos/pipeline/account-provisioning/New-CosmosLiveTestAccounts.ps1 index 9875e94be927..007f96bbab8d 100644 --- a/sdk/cosmos/pipeline/account-provisioning/New-CosmosLiveTestAccounts.ps1 +++ b/sdk/cosmos/pipeline/account-provisioning/New-CosmosLiveTestAccounts.ps1 @@ -134,6 +134,77 @@ function New-LocationObjects([string[]] $regionList) { return ,$locations } +# Adds any missing capabilities to an existing account via ARM PATCH. +# +# Capabilities are handled here, never via New-/Update-AzCosmosDBAccount: +# - Update-AzCosmosDBAccount cannot set them at all. +# - New-AzCosmosDBAccount -Capabilities is silently ignored by some Az.CosmosDB versions +# (observed on Az 12.2.0: accounts came up with no capabilities at all, which failed +# every vector-search test until the script was run a second time). +# Reconciling after the account exists makes the outcome independent of module behaviour, +# so a fresh tenant rotation is a single pass. +# +# Cosmos capabilities are additive and cannot be removed, so we only ever add, and always +# send the full merged list. +function Sync-AccountCapability { + # SupportsShouldProcess so -WhatIf propagates from the caller and this never PATCHes on a dry run. + [CmdletBinding(SupportsShouldProcess = $true)] + param( + [Parameter(Mandatory)] [string] $AccountName, + [Parameter(Mandatory)] [AllowEmptyCollection()] [string[]] $DesiredCapabilities, + [Parameter(Mandatory)] [string] $ResourceGroupName, + [Parameter(Mandatory)] [string] $SubscriptionId, + [string] $Selector + ) + + if ($DesiredCapabilities.Count -eq 0) { return } + + $account = Get-AzCosmosDBAccount -ResourceGroupName $ResourceGroupName -Name $AccountName -ErrorAction SilentlyContinue + if (-not $account) { + # Only reachable under -WhatIf, where the account was never actually created. + return + } + + $existingCaps = @() + if ($account.Capabilities) { $existingCaps = @($account.Capabilities | ForEach-Object { $_.Name }) } + + $missingCaps = @($DesiredCapabilities | Where-Object { $existingCaps -notcontains $_ }) + if ($missingCaps.Count -eq 0) { + Write-Info "Cosmos account '$AccountName' (selector=$Selector); capabilities up to date" + return + } + + $mergedCaps = @($existingCaps + $missingCaps | Select-Object -Unique) + if (-not $PSCmdlet.ShouldProcess($AccountName, "Add capabilities [$($missingCaps -join ', ')]")) { return } + + Write-Info "Account '$AccountName' (selector=$Selector); adding missing capabilities: $($missingCaps -join ', ')" + $resourceId = "/subscriptions/$SubscriptionId/resourceGroups/$ResourceGroupName/providers/Microsoft.DocumentDB/databaseAccounts/$AccountName" + $body = @{ + properties = @{ + capabilities = @($mergedCaps | ForEach-Object { @{ name = $_ } }) + } + } | ConvertTo-Json -Depth 6 + + $resp = Invoke-AzRestMethod -Method PATCH -Path "$($resourceId)?api-version=2024-11-15" -Payload $body + if ($resp.StatusCode -ge 300) { + throw "Failed to add capabilities to '$AccountName' (HTTP $($resp.StatusCode)): $($resp.Content)" + } + + # PATCH returns before the capability is durably applied; confirm it landed so a fresh + # provisioning run cannot silently produce accounts the tests then fail against. + $deadline = (Get-Date).AddMinutes(5) + while ((Get-Date) -lt $deadline) { + Start-Sleep -Seconds 10 + $check = Get-AzCosmosDBAccount -ResourceGroupName $ResourceGroupName -Name $AccountName -ErrorAction SilentlyContinue + $nowCaps = @() + if ($check -and $check.Capabilities) { $nowCaps = @($check.Capabilities | ForEach-Object { $_.Name }) } + $stillMissing = @($DesiredCapabilities | Where-Object { $nowCaps -notcontains $_ }) + if ($stillMissing.Count -eq 0) { return } + } + + throw "Capabilities [$($missingCaps -join ', ')] did not apply to '$AccountName' within 5 minutes" +} + # --- Create / update each account, then collect endpoint + keys -------------- $secret = [ordered]@{ version = 1 @@ -178,39 +249,22 @@ foreach ($acct in $definition.accounts) { if ($acct.PSObject.Properties.Name -contains 'enablePartitionMerge' -and $acct.enablePartitionMerge) { $params['EnablePartitionMerge'] = $true } - if ($capabilities.Count -gt 0) { $params['Capabilities'] = $capabilities } $null = New-AzCosmosDBAccount @params } } else { - # Account already exists. Reconcile capabilities: Cosmos capabilities are additive - # and cannot be removed, so we only add any desired capability that is missing - # (e.g. EnableNoSQLVectorSearch). This makes the script idempotent for capability - # changes on already-provisioned accounts. - $existingCaps = @() - if ($existing.Capabilities) { $existingCaps = @($existing.Capabilities | ForEach-Object { $_.Name }) } - $missingCaps = @($capabilities | Where-Object { $existingCaps -notcontains $_ }) - if ($missingCaps.Count -gt 0) { - $mergedCaps = @($existingCaps + $missingCaps | Select-Object -Unique) - if ($PSCmdlet.ShouldProcess($accountName, "Add capabilities [$($missingCaps -join ', ')]")) { - Write-Info "Account '$accountName' exists (selector=$selector); adding missing capabilities: $($missingCaps -join ', ')" - # Capabilities cannot be set via Update-AzCosmosDBAccount, so PATCH the account - # through ARM. Capabilities are additive; send the full merged list. - $resourceId = "/subscriptions/$SubscriptionId/resourceGroups/$ResourceGroupName/providers/Microsoft.DocumentDB/databaseAccounts/$accountName" - $body = @{ - properties = @{ - capabilities = @($mergedCaps | ForEach-Object { @{ name = $_ } }) - } - } | ConvertTo-Json -Depth 6 - $resp = Invoke-AzRestMethod -Method PATCH -Path "$($resourceId)?api-version=2024-11-15" -Payload $body - if ($resp.StatusCode -ge 300) { - throw "Failed to add capabilities to '$accountName' (HTTP $($resp.StatusCode)): $($resp.Content)" - } - } - } else { - Write-Info "Cosmos account '$accountName' already exists (selector=$selector); capabilities up to date" - } + Write-Info "Cosmos account '$accountName' already exists (selector=$selector)" } + # Reconcile capabilities on both paths. Deliberately not passed to New-AzCosmosDBAccount: + # some Az.CosmosDB versions ignore -Capabilities silently, which produced accounts with no + # capabilities at all and needed a second run of this script to fix. + Sync-AccountCapability ` + -AccountName $accountName ` + -DesiredCapabilities $capabilities ` + -ResourceGroupName $ResourceGroupName ` + -SubscriptionId $SubscriptionId ` + -Selector $selector + # Read endpoint + keys. Under -WhatIf (dry run) never read real keys — stub them so a # preview never emits secrets, even for already-provisioned accounts. if ($WhatIfPreference) { diff --git a/sdk/cosmos/pipeline/account-provisioning/README.md b/sdk/cosmos/pipeline/account-provisioning/README.md index 4aeff30e401e..44a31662f1f2 100644 --- a/sdk/cosmos/pipeline/account-provisioning/README.md +++ b/sdk/cosmos/pipeline/account-provisioning/README.md @@ -46,7 +46,10 @@ The emitted JSON conforms to the schema at ``` Idempotent: existing accounts are left in place and missing capabilities are added; the -JSON is regenerated with current endpoints/keys. +JSON is regenerated with current endpoints/keys. A single run is enough for a fresh tenant - +capabilities are applied by ARM PATCH after the account exists, and verified, rather than being +passed to `New-AzCosmosDBAccount` (some Az.CosmosDB versions ignore `-Capabilities` silently, +which previously produced accounts with no capabilities and required a second run). The multi-master accounts are separated by contention domain: From 26bce45b16a57b15ed4e05aad6b6ccd16cc1eefd Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Tue, 4 Aug 2026 12:11:06 -0700 Subject: [PATCH 13/15] Provision the GSI account in the region its tests prefer The GSI live stage was failing with java.net.UnknownHostException: gsi-pipeline.documents.azure.com java.net.UnknownHostException: gsi-pipeline-eastus2.documents.azure.com Tests run: 22, Failures: 1, Errors: 0, Skipped: 21 because those accounts went away with the old ephemeral tenant. The definition already had a gsi-single-session entry, but it inherited regionDefaults.singleRegion (Central US), while live-gsi-platform-matrix.json runs GSI single-region with PREFERRED_LOCATIONS=["East US 2"]. An account without the preferred region leaves the client with nothing to prefer, which is presumably why the original account lived in East US 2. Let a definition entry pin its own regions, and use it for gsi-single-session. Recreated sdkci-gsi-single-session in East US 2 (the Central US one was empty and unreferenced - the stage still points at the old gsi-pipeline-* secrets). The stage reads $(gsi-pipeline-uri)/$(gsi-pipeline-key) rather than the account resolver, so those two secrets still have to be repointed at the new account by hand; that is a separate change from this script. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../New-CosmosLiveTestAccounts.ps1 | 12 +++++++++++- sdk/cosmos/pipeline/account-provisioning/README.md | 5 +++++ .../cosmos-live-test-accounts.definition.json | 3 +++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/sdk/cosmos/pipeline/account-provisioning/New-CosmosLiveTestAccounts.ps1 b/sdk/cosmos/pipeline/account-provisioning/New-CosmosLiveTestAccounts.ps1 index 007f96bbab8d..ecd8035e89a1 100644 --- a/sdk/cosmos/pipeline/account-provisioning/New-CosmosLiveTestAccounts.ps1 +++ b/sdk/cosmos/pipeline/account-provisioning/New-CosmosLiveTestAccounts.ps1 @@ -220,7 +220,17 @@ foreach ($acct in $definition.accounts) { $multiRegion = [bool]$acct.enableMultipleRegions $multiWrite = [bool]$acct.enableMultipleWriteLocations - $regionList = if ($multiRegion) { $multiRegionList } else { $singleRegionList } + # An account may pin its own regions when the defaults do not suit it - the GSI account, for + # example, must live in East US 2 because live-gsi-platform-matrix.json sets + # PREFERRED_LOCATIONS=["East US 2"] on a single-region account, and a preferred region the account + # does not have leaves the client with nothing to prefer. + $regionList = if ($acct.PSObject.Properties.Name -contains 'regions' -and $acct.regions) { + @($acct.regions) + } elseif ($multiRegion) { + $multiRegionList + } else { + $singleRegionList + } $locations = New-LocationObjects $regionList $capabilities = @() diff --git a/sdk/cosmos/pipeline/account-provisioning/README.md b/sdk/cosmos/pipeline/account-provisioning/README.md index 44a31662f1f2..817393d64e2c 100644 --- a/sdk/cosmos/pipeline/account-provisioning/README.md +++ b/sdk/cosmos/pipeline/account-provisioning/README.md @@ -23,6 +23,11 @@ regenerate the fresh endpoints/keys. | `New-CosmosLiveTestAccounts.ps1` | Creates `sdk-ci` RG (if missing) + accounts; outputs accounts JSON. | | `cosmos-live-test-accounts.definition.json` | Desired accounts (logical selector + config). | +An account entry may set `"regions": [...]` to pin its own regions when `regionDefaults` does not +suit it. `gsi-single-session` uses this to sit in **East US 2**, because +`live-gsi-platform-matrix.json` runs it single-region with `PREFERRED_LOCATIONS=["East US 2"]`, and a +preferred region the account does not have leaves the client with nothing to prefer. + The emitted JSON conforms to the schema at `../live-test-accounts.schema.json`, which the pipeline pre-step `../resolve-cosmos-test-account.sh` parses. diff --git a/sdk/cosmos/pipeline/account-provisioning/cosmos-live-test-accounts.definition.json b/sdk/cosmos/pipeline/account-provisioning/cosmos-live-test-accounts.definition.json index 3d2cd411407f..afbc21721197 100644 --- a/sdk/cosmos/pipeline/account-provisioning/cosmos-live-test-accounts.definition.json +++ b/sdk/cosmos/pipeline/account-provisioning/cosmos-live-test-accounts.definition.json @@ -216,6 +216,9 @@ "defaultConsistencyLevel": "Session", "enableMultipleWriteLocations": false, "enableMultipleRegions": false, + "regions": [ + "East US 2" + ], "enablePartitionMerge": false, "thinClient": false, "includeSecondaryKey": true From 513fcb5b2db802c91f1cef0c7dc05fcaf2bad69b Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Tue, 4 Aug 2026 14:57:50 -0700 Subject: [PATCH 14/15] Give each Scala module its own compiler-bridge cache java - cosmos - ci has been red for over a week on main with [ERROR] ## Exception when compiling 134 sources to .../azure-cosmos-spark_3-3_2-12/target/classes java.lang.ClassNotFoundException: xsbt.CompilerInterface at sbt.internal.inc.AnalyzingCompiler.getBridgeClass(AnalyzingCompiler.scala:372) with the module and the missing class varying between runs (main build 6631046 failed on azure-cosmos-spark_3-4_2-12 with xsbt/WeakLog instead). scala-maven-plugin's secondaryCacheDir defaults to the shared, global ~/.sbt/1.0/zinc/org.scala-sbt. sbt_inc.CompilerBridgeFactory installs the bridge by checking targetJar.exists(), compiling into a temp dir, then calling sbt.io.IO.jar() straight onto the final path - no lock, no temp-then-rename. cosmos-sdk-client.yml builds with -T 2, so two Scala modules of the same Scala version both see "not installed yet", both compile, and both write the same jar. Whichever module reads it mid-write gets a truncated zip, which surfaces as whichever xsbt class happened to be missing. ~/.sbt is not covered by the Maven pipeline cache, so the bridge is cold on every run and the race fires nearly every time. Point secondaryCacheDir at ${project.build.directory} so no two modules share a bridge jar. This also makes concurrent Maven processes on one agent safe. Costs one bridge compile per Scala module (~7-8s) instead of one per Scala version. Upgrading doesn't help - 4.9.10 still has no lock and no atomic rename. Verified with -T 2 and a cold cache on the two modules that raced: each used its own target/scala-compiler-bridge, ~/.sbt/1.0/zinc was never created, and help:effective-pom confirms all nine Scala-compiling modules resolve to their own target dir (including the grandchildren under azure-cosmos-spark_3-5 and azure-cosmos-spark_4, which redefine the build-scala profile). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../azure-cosmos-spark-account-data-resolver-sample/pom.xml | 3 +++ sdk/cosmos/azure-cosmos-spark_3/pom.xml | 3 +++ sdk/cosmos/fabric-cosmos-spark-auth_3/pom.xml | 3 +++ sdk/cosmos/fabric-cosmos-spark-auth_4-0_2-13/pom.xml | 3 +++ 4 files changed, 12 insertions(+) diff --git a/sdk/cosmos/azure-cosmos-spark-account-data-resolver-sample/pom.xml b/sdk/cosmos/azure-cosmos-spark-account-data-resolver-sample/pom.xml index 3c877c0b39ea..b129931eebcb 100644 --- a/sdk/cosmos/azure-cosmos-spark-account-data-resolver-sample/pom.xml +++ b/sdk/cosmos/azure-cosmos-spark-account-data-resolver-sample/pom.xml @@ -375,6 +375,9 @@ 1.8 1.8 2.12.19 + + ${project.build.directory}/scala-compiler-bridge diff --git a/sdk/cosmos/azure-cosmos-spark_3/pom.xml b/sdk/cosmos/azure-cosmos-spark_3/pom.xml index 2405150dc171..81e5e817941e 100644 --- a/sdk/cosmos/azure-cosmos-spark_3/pom.xml +++ b/sdk/cosmos/azure-cosmos-spark_3/pom.xml @@ -429,6 +429,9 @@ ${maven.compiler.source} ${maven.compiler.target} ${scala.version} + + ${project.build.directory}/scala-compiler-bridge diff --git a/sdk/cosmos/fabric-cosmos-spark-auth_3/pom.xml b/sdk/cosmos/fabric-cosmos-spark-auth_3/pom.xml index c20cf8c5f228..273439f54e77 100644 --- a/sdk/cosmos/fabric-cosmos-spark-auth_3/pom.xml +++ b/sdk/cosmos/fabric-cosmos-spark-auth_3/pom.xml @@ -269,6 +269,9 @@ 1.8 1.8 2.12.19 + + ${project.build.directory}/scala-compiler-bridge diff --git a/sdk/cosmos/fabric-cosmos-spark-auth_4-0_2-13/pom.xml b/sdk/cosmos/fabric-cosmos-spark-auth_4-0_2-13/pom.xml index b8eeb2b18612..f58c54ca30c1 100644 --- a/sdk/cosmos/fabric-cosmos-spark-auth_4-0_2-13/pom.xml +++ b/sdk/cosmos/fabric-cosmos-spark-auth_4-0_2-13/pom.xml @@ -269,6 +269,9 @@ 17 17 2.13.17 + + ${project.build.directory}/scala-compiler-bridge From d98b46af7a222a4ddf57998114d52e9f9d8ffdff Mon Sep 17 00:00:00 2001 From: tvaron3 Date: Tue, 4 Aug 2026 15:59:17 -0700 Subject: [PATCH 15/15] Keep the new cosmos spellings in the cosmos cspell config The Maven flag fragments picked up from AGENTS.md and dev.md (DACCOUNT, Dcodesnippet, DCOSMOS) belong with the other cosmos-only words rather than in the repo-wide dictionary, so fold them into the existing "**/sdk/cosmos/*" override and leave .vscode/cspell.json untouched. check-spelling.yml passes CspellConfigPath: .vscode/cspell.json, but cspell still discovers sdk/cosmos/cspell.yaml for files under sdk/cosmos and that file imports ../../.vscode/cspell.json, so the repo-wide dictionary is not lost. Verified with the pinned cspell 10.0.1 using the same invocation Invoke-Cspell.ps1 makes (lint --config .vscode/cspell.json --no-must-find-files --root --file-list stdin): 0 issues with the words here, and 8 unknown-word errors with them removed from both configs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .vscode/cspell.json | 11 ----------- sdk/cosmos/cspell.yaml | 3 +++ 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/.vscode/cspell.json b/.vscode/cspell.json index 7fcc0914506d..a09099e36c8a 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -1019,17 +1019,6 @@ "kevinb" ] }, - { - "filename": [ - "**/sdk/cosmos/AGENTS.md", - "**/sdk/cosmos/dev.md" - ], - "words": [ - "DACCOUNT", - "Dcodesnippet", - "DCOSMOS" - ] - }, { "filename": "**/sdk/cosmos/live-platform-matrix.json", "words": [ diff --git a/sdk/cosmos/cspell.yaml b/sdk/cosmos/cspell.yaml index 2237242789e8..755dd5762d92 100644 --- a/sdk/cosmos/cspell.yaml +++ b/sdk/cosmos/cspell.yaml @@ -6,3 +6,6 @@ overrides: - DCOUNT - dedupe - colls + - DACCOUNT + - Dcodesnippet + - DCOSMOS