diff --git a/.github/workflows/code-check.yml b/.github/workflows/code-check.yml index 5ef487bc7..ad5f54c9f 100644 --- a/.github/workflows/code-check.yml +++ b/.github/workflows/code-check.yml @@ -8,6 +8,12 @@ on: workflow_dispatch: +# No `concurrency`/`cancel-in-progress`, deliberately. GitHub force-terminates a +# cancelled job's remaining steps -- `if: always()` included -- after a 5-minute +# cancellation timeout, and an S-00 cluster refuses DELETE until it is ACTIVE +# (~460s). A run cancelled in its first few minutes would die holding a cluster +# it is not yet allowed to delete, with nothing scheduled to reap it. Letting +# both runs finish costs clusters; cancelling them costs stranded clusters. jobs: test-coverage: runs-on: ubuntu-latest @@ -16,6 +22,12 @@ jobs: contents: read actions: write + # One ledger for the whole job, so the cleanup step at the end can reap + # what any of the pytest steps created. See the matching block in + # coverage.yml. + env: + SINGLESTOREDB_TEST_DEPLOYMENT_LOG: ${{ github.workspace }}/deployments.jsonl + services: singlestore: image: ghcr.io/singlestore-labs/singlestoredb-dev:latest @@ -29,14 +41,22 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: - fetch-depth: 2 - + # Full history, because the change detector below diffs against + # origin/main, a ref a shallow clone does not create. With + # fetch-depth: 2 every diff died on `fatal: bad revision + # 'origin/main'`, which the detector read as "nothing changed", so the + # management step never ran on a PR. + fetch-depth: 0 + + # One version, not a matrix: the supported range is covered by + # smoke-test.yml and pre-commit.yml, so this tracks the newest final + # release rather than the floor in pyproject.toml. - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v7 with: - python-version: "3.10" + python-version: "3.14" cache: "pip" - name: Install dependencies @@ -78,8 +98,8 @@ jobs: COMMIT_MSG=$(git log -1 --format='%s' HEAD) if [[ "$COMMIT_MSG" =~ ^Prepare\ for\ v[0-9]+\.[0-9]+\.[0-9]+\ release$ ]]; then echo "🚀 Release preparation commit detected: $COMMIT_MSG" - echo "changes-detected=true" >> $GITHUB_OUTPUT - echo "changed-directories=release" >> $GITHUB_OUTPUT + echo "changes-detected=true" >> "$GITHUB_OUTPUT" + echo "changed-directories=release" >> "$GITHUB_OUTPUT" echo "" echo "🎯 RESULT: Full test suite will run for release preparation" exit 0 @@ -94,10 +114,15 @@ jobs: for DIR in $MONITORED_DIRS; do if [ -d "$DIR" ]; then - CHANGED_FILES=$(git diff --name-only $BASE_COMMIT HEAD -- "$DIR" || true) + # No `|| true` here: a git failure means the comparison did not + # happen, and swallowing it silently downgrades the run to the + # no-management path instead of reporting the breakage. + CHANGED_FILES=$(git diff --name-only "$BASE_COMMIT" HEAD -- "$DIR") if [ -n "$CHANGED_FILES" ]; then echo "✅ Changes detected in: $DIR" echo "Files changed:" + # shellcheck disable=SC2001 # prefixing every line, which + # ${var//search/replace} cannot do echo "$CHANGED_FILES" | sed 's/^/ - /' CHANGES_FOUND=true if [ -z "$CHANGED_DIRS" ]; then @@ -115,13 +140,13 @@ jobs: # Set outputs if [ "$CHANGES_FOUND" = true ]; then - echo "changes-detected=true" >> $GITHUB_OUTPUT - echo "changed-directories=$CHANGED_DIRS" >> $GITHUB_OUTPUT + echo "changes-detected=true" >> "$GITHUB_OUTPUT" + echo "changed-directories=$CHANGED_DIRS" >> "$GITHUB_OUTPUT" echo "" echo "🎯 RESULT: Changes detected in monitored directories" else - echo "changes-detected=false" >> $GITHUB_OUTPUT - echo "changed-directories=" >> $GITHUB_OUTPUT + echo "changes-detected=false" >> "$GITHUB_OUTPUT" + echo "changed-directories=" >> "$GITHUB_OUTPUT" echo "" echo "🎯 RESULT: No changes in monitored directories" fi @@ -137,8 +162,12 @@ jobs: - name: Run MySQL protocol tests (with management API) if: steps.check-changes.outputs.changes-detected == 'true' + # -m 'not management_v1' keeps the v2 management coverage while dropping + # the deprecated v1 suite, which coverage.yml runs nightly instead. The + # -m 'not management' steps below need no second term: they already + # exclude everything v1 deploys. run: | - pytest -v --cov=singlestoredb --pyargs singlestoredb.tests + pytest -v -m 'not management_v1' --cov=singlestoredb --pyargs singlestoredb.tests env: COVERAGE_FILE: "coverage-mysql.cov" SINGLESTOREDB_URL: "root:root@127.0.0.1:3307" @@ -171,7 +200,7 @@ jobs: SINGLESTOREDB_FUSION_ENABLE_HIDDEN: "1" - name: Run HTTP protocol tests - # -n 0 overrides the -n 3 in pyproject.toml's addopts: the HTTP/Data API + # -n 0 overrides the -n 2 in pyproject.toml's addopts: the HTTP/Data API # run must be serial. Setup goes over SINGLESTOREDB_INIT_DB_URL (MySQL), # so load_sql takes its `SET GLOBAL HTTP_PROXY_PORT` + `RESTART PROXY` # branch (singlestoredb/tests/utils.py:227) once per worker, and a proxy @@ -195,3 +224,27 @@ jobs: coverage report coverage xml coverage html + + # if: always() is the whole point -- this has to run when the job fails or + # is cancelled, which is what left three clusters billing in run + # 35631802648 (see the matching step in coverage.yml). On a PR the + # management step above only runs when the change detector fires, so most + # runs reach this with an empty ledger and report nothing. + # + # Best effort, not a guarantee: a cancelled job's remaining steps are + # force-terminated after GitHub's 5-minute cancellation timeout, so this + # covers a cancel whose clusters are already ACTIVE but not one in the + # first few minutes, where DELETE is still refused. That remainder needs + # `cleanup_deployments.py --older-than` run by hand. + # + # The secret sweep alongside it is the same rolling janitor coverage.yml + # runs; see the comment on that step. + - name: Clean up what the tests left behind + if: always() + run: | + python -m singlestoredb.tests.cleanup_deployments --secrets --yes \ + || true + python -m singlestoredb.tests.cleanup_deployments \ + --ledger "$SINGLESTOREDB_TEST_DEPLOYMENT_LOG" --yes + env: + SINGLESTOREDB_MANAGEMENT_TOKEN: ${{ secrets.CLUSTER_API_KEY }} diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 6c9546fa9..da4188472 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -10,6 +10,12 @@ jobs: runs-on: ubuntu-latest environment: Base + # One ledger for the whole job, so the cleanup step below can reap what any + # of the pytest steps created. Per job, not shared: a job's sweep can then + # only reach records it wrote itself. + env: + SINGLESTOREDB_TEST_DEPLOYMENT_LOG: ${{ github.workspace }}/deployments.jsonl + services: singlestore: image: ghcr.io/singlestore-labs/singlestoredb-dev:latest @@ -22,12 +28,15 @@ jobs: ROOT_PASSWORD: "root" steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 + # One version, not a matrix: the supported range is covered by + # smoke-test.yml and pre-commit.yml, so this tracks the newest final + # release rather than the floor in pyproject.toml. - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v7 with: - python-version: "3.10" + python-version: "3.14" cache: "pip" - name: Install dependencies @@ -36,8 +45,11 @@ jobs: pip install -e ".[dev]" - name: Run MySQL protocol tests + # -m 'not management_v1' keeps the v2 management coverage while dropping + # the deprecated v1 suite; the management-v1-tests job below is where + # that runs. run: | - pytest -v --cov=singlestoredb --pyargs singlestoredb.tests + pytest -v -m 'not management_v1' --cov=singlestoredb --pyargs singlestoredb.tests env: COVERAGE_FILE: "coverage-mysql.cov" SINGLESTOREDB_URL: "root:root@127.0.0.1:3307" @@ -58,7 +70,7 @@ jobs: SINGLESTOREDB_FUSION_ENABLE_HIDDEN: "1" - name: Run HTTP protocol tests - # -n 0 overrides the -n 3 in pyproject.toml's addopts: the HTTP/Data API + # -n 0 overrides the -n 2 in pyproject.toml's addopts: the HTTP/Data API # run must be serial. Setup goes over SINGLESTOREDB_INIT_DB_URL (MySQL), # so load_sql takes its `SET GLOBAL HTTP_PROXY_PORT` + `RESTART PROXY` # branch (singlestoredb/tests/utils.py:227) once per worker, and a proxy @@ -82,3 +94,117 @@ jobs: coverage report coverage xml coverage html + + # if: always() is the whole point -- this has to run when the job is + # cancelled, the case that produced the leak. Run 35631802648 was + # cancelled 19 minutes into TestClusterFusion.setUpClass's + # create_cluster(wait_on_active=True, wait_timeout=1200): the log ends at + # '##[error]The operation was canceled.' with no pytest summary and no + # sweep output, leaving three clusters billing with nothing in the process + # having recorded them. The ledger is that record. + # + # Last step in the job, so it covers every pytest step above it. + # + # Best effort, not a guarantee: a cancelled job's remaining steps are + # force-terminated after GitHub's 5-minute cancellation timeout, and an + # S-00 cluster refuses DELETE until it is ACTIVE (~460s). The leak above is + # covered, being 19 minutes in; a cancel in the first few minutes would be + # killed here still getting 400/409, and needs `cleanup_deployments.py + # --older-than` run by hand. + - name: Clean up what the tests left behind + if: always() + run: | + # Secrets first, and its status discarded. TestSecrets creates an + # org-scoped secret that only its own test body deletes, so a killed + # run strands one for good; --secrets is age-guarded, which keeps it + # off this run's and off a concurrent job's, so what it removes is + # what *earlier* runs stranded. A secret bills nothing, and the + # ledger sweep is what this step's status should report. + python -m singlestoredb.tests.cleanup_deployments --secrets --yes \ + || true + python -m singlestoredb.tests.cleanup_deployments \ + --ledger "$SINGLESTOREDB_TEST_DEPLOYMENT_LOG" --yes + env: + SINGLESTOREDB_MANAGEMENT_TOKEN: ${{ secrets.CLUSTER_API_KEY }} + + # The deprecated v1 management API. management.version defaults to v2, so this + # is a legacy gate: it runs here nightly rather than on every PR, and it is + # what gets deleted along with management/v1/. Selects both the mocked v1 + # units and the live v1 deployments, plus test_fusion's v1 WORKSPACE grammar. + management-v1-tests: + runs-on: ubuntu-latest + environment: Base + + # Waits for test-coverage rather than running alongside it, so the v1 + # workspace groups are never in flight at the same time as the v2 suite's + # cluster pool. This is a nightly cron, so the extra wall clock is free. + # + # Runs even when test-coverage fails: a v2 failure above says nothing about + # the v1 endpoints, and skipping v1 for it would hide a v1 regression behind + # an unrelated one. + # + # !cancelled() rather than always(), which stays true through cancellation + # too. A cancelled run must not start provisioning workspace groups here: + # remaining steps are force-terminated 5 minutes into a cancel, so the sweep + # below would be killed while the new deployments were still pre-ACTIVE and + # refusing DELETE -- stranding exactly what it exists to clean up. + needs: test-coverage + if: ${{ !cancelled() }} + + # A ledger of its own: separate runner, separate workspace, and neither + # job's sweep can reach the other's records. + env: + SINGLESTOREDB_TEST_DEPLOYMENT_LOG: ${{ github.workspace }}/deployments.jsonl + + services: + singlestore: + image: ghcr.io/singlestore-labs/singlestoredb-dev:latest + ports: + - 3307:3306 + - 8081:8080 + - 9081:9081 + env: + SINGLESTORE_LICENSE: ${{ secrets.SINGLESTORE_LICENSE }} + ROOT_PASSWORD: "root" + + steps: + - uses: actions/checkout@v7 + + # As in test-coverage above: newest final release, not the floor. + - name: Set up Python + uses: actions/setup-python@v7 + with: + python-version: "3.14" + cache: "pip" + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -e ".[dev]" + + - name: Run v1 management API tests + # -n 0 overrides the -n 2 in pyproject.toml's addopts, which is tuned for + # the v2 suite's shared cluster pool. The v1 classes deploy workspace + # groups of their own, so two workers would put twice that in flight. + # Serial keeps this job to one deployment at a time. + run: | + pytest -v -n 0 -m 'management_v1' --pyargs singlestoredb.tests + env: + SINGLESTOREDB_URL: "root:root@127.0.0.1:3307" + SINGLESTOREDB_PURE_PYTHON: 0 + SINGLESTORE_LICENSE: ${{ secrets.SINGLESTORE_LICENSE }} + SINGLESTOREDB_MANAGEMENT_TOKEN: ${{ secrets.CLUSTER_API_KEY }} + SINGLESTOREDB_FUSION_ENABLE_HIDDEN: "1" + + # See the matching step in test-coverage for why this is if: always(), + # and for what the secret sweep is doing here. The v1 suite deploys + # workspace groups, which are the kind that force exists for. + - name: Clean up what the tests left behind + if: always() + run: | + python -m singlestoredb.tests.cleanup_deployments --secrets --yes \ + || true + python -m singlestoredb.tests.cleanup_deployments \ + --ledger "$SINGLESTOREDB_TEST_DEPLOYMENT_LOG" --yes + env: + SINGLESTOREDB_MANAGEMENT_TOKEN: ${{ secrets.CLUSTER_API_KEY }} diff --git a/.github/workflows/fusion-docs.yml b/.github/workflows/fusion-docs.yml index 75a74ffa6..c7b8ac5c8 100644 --- a/.github/workflows/fusion-docs.yml +++ b/.github/workflows/fusion-docs.yml @@ -14,12 +14,14 @@ jobs: actions: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - - name: Set up Python 3.11 - uses: actions/setup-python@v5 + # Only drives resources/gen_fusion_handlers_doc.py, so this tracks the + # newest final release rather than anything the package supports. + - name: Set up Python 3.14 + uses: actions/setup-python@v7 with: - python-version: 3.11 + python-version: "3.14" cache: "pip" - name: Install dependencies diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index a9217a93f..ae1917e24 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -14,12 +14,13 @@ jobs: - "3.11" - "3.12" - "3.13" + - "3.14" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 + uses: actions/setup-python@v7 with: python-version: ${{ matrix.python-version }} diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d3a669c1e..465cd4e4b 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -39,7 +39,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: Install dependencies run: | @@ -49,8 +49,16 @@ jobs: - name: Initialize database id: initialize-database + # A new cluster has exactly one user, admin, hence the fixed name below. + # The project is named here, not in a repo variable, so the deployment + # target is visible in the workflow. + # + # POST /v2/clusters generates its own admin password and ignores any sent + # to it, so the script resets it to CLUSTER_PASSWORD over SQL once the + # cluster is up. A generated password could not reach the other jobs + # anyway: the runner refuses to write a masked value as a job output. run: | - python resources/create_test_cluster.py --password="${{ secrets.CLUSTER_PASSWORD }}" --token="${{ secrets.CLUSTER_API_KEY }}" --init-sql singlestoredb/tests/test.sql --output=github --expires=2h "python - $GITHUB_WORKFLOW - $GITHUB_RUN_NUMBER" + python resources/create_test_cluster.py --password="${{ secrets.CLUSTER_PASSWORD }}" --token="${{ secrets.CLUSTER_API_KEY }}" --project="Standard Project" --init-sql singlestoredb/tests/test.sql --output=github --expires=2h "python - $GITHUB_WORKFLOW - $GITHUB_RUN_NUMBER" env: PYTHONPATH: ${{ github.workspace }} @@ -73,12 +81,15 @@ jobs: - windows-2022 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 + # This job's matrix varies only over os; cibuildwheel supplies its own + # interpreters, so 3.14 here is just the host Python that drives it. + # Host-only pins track the newest final release rather than the floor. + - name: Set up Python + uses: actions/setup-python@v7 with: - python-version: "3.10" + python-version: "3.14" cache: "pip" - name: Install dependencies @@ -100,7 +111,7 @@ jobs: - name: Set up QEMU if: runner.os == 'Linux' - uses: docker/setup-qemu-action@v2 + uses: docker/setup-qemu-action@v4 with: platforms: all @@ -118,7 +129,10 @@ jobs: # points --pyargs at the workspace, so that pyproject is the inifile # here. Without the plugin pytest exits on the unknown arguments. CIBW_TEST_REQUIRES: "pytest pytest-xdist" - CIBW_ENVIRONMENT: "SINGLESTOREDB_URL='mysql://${{ secrets.CLUSTER_USER }}:${{ secrets.CLUSTER_PASSWORD }}@${{ needs.setup-database.outputs.cluster-host }}:3306/${{ needs.setup-database.outputs.cluster-database }}?pure_python=0'" + # CLUSTER_PASSWORD has to survive both the userinfo half of the URL + # and the single-quoted shell word cibuildwheel evaluates, so keep the + # secret alphanumeric: no ':', '@', '/', '%' or quote characters. + CIBW_ENVIRONMENT: "SINGLESTOREDB_URL='mysql://admin:${{ secrets.CLUSTER_PASSWORD }}@${{ needs.setup-database.outputs.cluster-host }}:3306/${{ needs.setup-database.outputs.cluster-database }}?pure_python=0'" PYTHONPATH: ${{ github.workspace }} # - name: Build conda @@ -147,14 +161,14 @@ jobs: mv ./wheelhouse/*.whl ./dist/. - name: Archive source dist and wheel - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: artifacts-${{ runner.os }} path: dist retention-days: 2 # - name: Archive conda -# uses: actions/upload-artifact@v4 +# uses: actions/upload-artifact@v7 # with: # name: conda-${{ matrix.os }} # path: ./conda-bld @@ -175,22 +189,22 @@ jobs: url: https://pypi.org/p/singlestoredb steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: Download Linux wheels and sdist - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 with: name: artifacts-Linux path: dist - name: Download Windows wheels and sdist - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 with: name: artifacts-Windows path: dist - name: Download Mac wheels and sdist - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 with: name: artifacts-macOS path: dist @@ -228,7 +242,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: Install dependencies run: | @@ -238,14 +252,28 @@ jobs: - name: Drop database if: ${{ always() }} + # The password reaches the script through the environment rather than + # being interpolated into the command, so the shell never sees its + # characters. run: | - python resources/drop_db.py --user "${{ secrets.CLUSTER_USER }}" --password "${{ secrets.CLUSTER_PASSWORD }}" --host "${{ needs.setup-database.outputs.cluster-host }}" --port 3306 --database "${{ needs.setup-database.outputs.cluster-database }}" + python resources/drop_db.py --user admin --password "$CLUSTER_PASSWORD" --host "$CLUSTER_HOST" --port 3306 --database "$CLUSTER_DATABASE" env: PYTHONPATH: ${{ github.workspace }} + CLUSTER_PASSWORD: ${{ secrets.CLUSTER_PASSWORD }} + CLUSTER_HOST: ${{ needs.setup-database.outputs.cluster-host }} + CLUSTER_DATABASE: ${{ needs.setup-database.outputs.cluster-database }} - - name: Shutdown workspace + - name: Shutdown cluster if: ${{ always() }} + # An empty ID would send the DELETE to /v2/clusters/ and leave a live + # cluster behind, so fail loudly instead; the ID then has to be recovered + # by hand. --fail-with-body makes a refused DELETE fail this step rather + # than print the error and exit 0. run: | - curl -H "Accept: application/json" -H "Authorization: Bearer ${{ secrets.CLUSTER_API_KEY }}" -X DELETE "https://api.singlestore.com/v1/workspaces/${{ env.CLUSTER_ID }}" + if [ -z "$CLUSTER_ID" ]; then + echo "::error::No cluster ID from setup-database; the cluster (if any) must be terminated by hand" + exit 1 + fi + curl --fail-with-body -H "Accept: application/json" -H "Authorization: Bearer ${{ secrets.CLUSTER_API_KEY }}" -X DELETE "https://api.singlestore.com/v2/clusters/$CLUSTER_ID?force=true" env: CLUSTER_ID: ${{ needs.setup-database.outputs.cluster-id }} diff --git a/.github/workflows/smoke-test.yml b/.github/workflows/smoke-test.yml index 688a2dc18..87140ffe2 100644 --- a/.github/workflows/smoke-test.yml +++ b/.github/workflows/smoke-test.yml @@ -12,12 +12,15 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - - name: Set up Python 3.11 - uses: actions/setup-python@v5 + # This job only drives resources/create_test_cluster.py; the versions the + # package is tested against are the smoke-test matrix below. So it tracks + # the newest final release. + - name: Set up Python 3.14 + uses: actions/setup-python@v7 with: - python-version: 3.11 + python-version: "3.14" cache: "pip" - name: Install dependencies @@ -28,8 +31,16 @@ jobs: - name: Initialize database id: initialize-database + # A new cluster has exactly one user, admin, hence the fixed name below. + # The project is named here, not in a repo variable, so the deployment + # target is visible in the workflow. + # + # POST /v2/clusters generates its own admin password and ignores any sent + # to it, so the script resets it to CLUSTER_PASSWORD over SQL once the + # cluster is up. A generated password could not reach the other jobs + # anyway: the runner refuses to write a masked value as a job output. run: | - python resources/create_test_cluster.py --password="${{ secrets.CLUSTER_PASSWORD }}" --token="${{ secrets.CLUSTER_API_KEY }}" --init-sql singlestoredb/tests/test.sql --output=github --expires=2h "python - $GITHUB_WORKFLOW - $GITHUB_RUN_NUMBER" + python resources/create_test_cluster.py --password="${{ secrets.CLUSTER_PASSWORD }}" --token="${{ secrets.CLUSTER_API_KEY }}" --project="Standard Project" --init-sql singlestoredb/tests/test.sql --output=github --expires=2h "python - $GITHUB_WORKFLOW - $GITHUB_RUN_NUMBER" env: PYTHONPATH: ${{ github.workspace }} @@ -48,12 +59,17 @@ jobs: matrix: os: - ubuntu-24.04 + # The floor in pyproject.toml (requires-python >=3.9) up to the newest + # final release. 3.15 is absent deliberately: as of 2026-09-18 it is at + # rc2 (GA 2026-10-01), and setup-python needs allow-prereleases plus an + # explicit "3.15.0-rc.2" to install it. Add a bare "3.15" once it ships. python-version: - "3.9" - "3.10" - "3.11" - "3.12" - "3.13" + - "3.14" driver: - mysql - https @@ -100,10 +116,10 @@ jobs: buffered: 1 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 + uses: actions/setup-python@v7 with: python-version: ${{ matrix.python-version }} cache: "pip" @@ -118,11 +134,14 @@ jobs: run: pytest -v --pyargs singlestoredb.tests.test_basics env: PYTHONPATH: ${{ github.workspace }} - SINGLESTOREDB_URL: "${{ matrix.driver }}://${{ secrets.CLUSTER_USER }}:${{ secrets.CLUSTER_PASSWORD }}@${{ needs.setup-database.outputs.cluster-host }}:3306/${{ needs.setup-database.outputs.cluster-database }}?pure_python=${{ matrix.pure-python }}&buffered=${{ matrix.buffered }}" + # CLUSTER_PASSWORD goes into the userinfo half of a URL here, so it + # has to be free of characters that would need percent-encoding -- + # ':', '@', '/', '%' and the like. Keep the secret alphanumeric. + SINGLESTOREDB_URL: "${{ matrix.driver }}://admin:${{ secrets.CLUSTER_PASSWORD }}@${{ needs.setup-database.outputs.cluster-host }}:3306/${{ needs.setup-database.outputs.cluster-database }}?pure_python=${{ matrix.pure-python }}&buffered=${{ matrix.buffered }}" - name: Run tests if: ${{ matrix.driver == 'https' }} - # -n 0 overrides the -n 3 in pyproject.toml's addopts: the Data API is + # -n 0 overrides the -n 2 in pyproject.toml's addopts: the Data API is # not run in parallel. This job avoids the `RESTART PROXY` hazard the # code-check/coverage HTTP steps hit -- no SINGLESTOREDB_INIT_DB_URL # here, so load_sql's setup connection is itself HTTP and skips that @@ -131,7 +150,7 @@ jobs: run: pytest -v -n 0 --pyargs singlestoredb.tests.test_basics env: PYTHONPATH: ${{ github.workspace }} - SINGLESTOREDB_URL: "${{ matrix.driver }}://${{ secrets.CLUSTER_USER }}:${{ secrets.CLUSTER_PASSWORD }}@${{ needs.setup-database.outputs.cluster-host }}:443/${{ needs.setup-database.outputs.cluster-database }}?pure_python=${{ matrix.pure-python }}&buffered=${{ matrix.buffered }}" + SINGLESTOREDB_URL: "${{ matrix.driver }}://admin:${{ secrets.CLUSTER_PASSWORD }}@${{ needs.setup-database.outputs.cluster-host }}:443/${{ needs.setup-database.outputs.cluster-database }}?pure_python=${{ matrix.pure-python }}&buffered=${{ matrix.buffered }}" shutdown-database: @@ -140,12 +159,13 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - - name: Set up Python 3.11 - uses: actions/setup-python@v5 + # As in setup-database: this only drives resources/drop_db.py. + - name: Set up Python 3.14 + uses: actions/setup-python@v7 with: - python-version: 3.11 + python-version: "3.14" cache: "pip" - name: Install dependencies @@ -156,14 +176,28 @@ jobs: - name: Drop database if: ${{ always() }} + # The password reaches the script through the environment rather than + # being interpolated into the command, so the shell never sees its + # characters. run: | - python resources/drop_db.py --user "${{ secrets.CLUSTER_USER }}" --password "${{ secrets.CLUSTER_PASSWORD }}" --host "${{ needs.setup-database.outputs.cluster-host }}" --port 3306 --database "${{ needs.setup-database.outputs.cluster-database }}" + python resources/drop_db.py --user admin --password "$CLUSTER_PASSWORD" --host "$CLUSTER_HOST" --port 3306 --database "$CLUSTER_DATABASE" env: PYTHONPATH: ${{ github.workspace }} + CLUSTER_PASSWORD: ${{ secrets.CLUSTER_PASSWORD }} + CLUSTER_HOST: ${{ needs.setup-database.outputs.cluster-host }} + CLUSTER_DATABASE: ${{ needs.setup-database.outputs.cluster-database }} - - name: Shutdown workspace + - name: Shutdown cluster if: ${{ always() }} + # An empty ID would send the DELETE to /v2/clusters/ and leave a live + # cluster behind, so fail loudly instead; the ID then has to be recovered + # by hand. --fail-with-body makes a refused DELETE fail this step rather + # than print the error and exit 0. run: | - curl -H "Accept: application/json" -H "Authorization: Bearer ${{ secrets.CLUSTER_API_KEY }}" -X DELETE "https://api.singlestore.com/v1/workspaces/${{ env.CLUSTER_ID }}" + if [ -z "$CLUSTER_ID" ]; then + echo "::error::No cluster ID from setup-database; the cluster (if any) must be terminated by hand" + exit 1 + fi + curl --fail-with-body -H "Accept: application/json" -H "Authorization: Bearer ${{ secrets.CLUSTER_API_KEY }}" -X DELETE "https://api.singlestore.com/v2/clusters/$CLUSTER_ID?force=true" env: CLUSTER_ID: ${{ needs.setup-database.outputs.cluster-id }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9d4c60017..b190dd101 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -41,3 +41,7 @@ repos: hooks: - id: mypy additional_dependencies: [types-requests] +- repo: https://github.com/Mateusz-Grzelinski/actionlint-py + rev: v1.7.7.23 + hooks: + - id: actionlint diff --git a/pyproject.toml b/pyproject.toml index 77ff8ca60..25750c421 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,13 +101,15 @@ exclude = ["docs*", "resources*", "examples*", "licenses*"] # and honours the xdist_group marks below. It is here rather than in the # invocation because forgetting it costs real money. # -# 3 workers, not `auto`: the ceiling is the management API's tolerance for +# 2 workers, not `auto`: the ceiling is the management API's tolerance for # concurrent provisioning and the org's cluster quota, not this host's CPUs. +# 3 was too many in practice -- the management suite had more clusters in +# flight at once than the org wanted to carry. # # Note that xdist must be installed for pytest to start at all with these set # (`pip install -e ".[test]"`), that SINGLESTOREDB_MANAGEMENT_TRACE's terminal # summary needs -n 0, and that USE_DATA_API=1 in parallel is unverified. -addopts = ["-n", "3", "--dist", "loadgroup"] +addopts = ["-n", "2", "--dist", "loadgroup"] markers = [ "management", "management_v1: exercises the v1 management API, which v2 has replaced. Deselect with -m 'not management_v1'; the v1 endpoints only need a nightly gate now that v2 is the default.", diff --git a/resources/build_docs.py b/resources/build_docs.py index 628f3a1fd..5e54b184c 100755 --- a/resources/build_docs.py +++ b/resources/build_docs.py @@ -384,6 +384,11 @@ def apply_content_transformations(self, content: str, links: Dict[str, str]) -> # Change workspace.Stage to workspace.stage content = re.sub(r'>workspace\.Stage\.', r'>workspace.stage.', content) + # Change cluster.Stage to cluster.stage. Stage is re-exported from the + # v2 cluster module, so it is documented under both names for as long + # as management/workspace.py is still documented. + content = re.sub(r'>cluster\.Stage\.', r'>cluster.stage.', content) + # Fix class/method links content = re.sub( r'(]+>)?(\s*]*>\s*\s*)([\w\.]+)(\s*\s*)', diff --git a/resources/create_test_cluster.py b/resources/create_test_cluster.py index 186fadfa8..02f26ab6b 100755 --- a/resources/create_test_cluster.py +++ b/resources/create_test_cluster.py @@ -5,10 +5,8 @@ import os import random import re -import secrets import subprocess import sys -import time import uuid from optparse import OptionParser @@ -16,31 +14,39 @@ # Handle command-line options -usage = 'usage: %prog [options] workspace-name' +usage = 'usage: %prog [options] cluster-name' parser = OptionParser(usage=usage) parser.add_option( '-r', '--region', default='AWS::*US East 1*', - help='region pattern or ID', -) -parser.add_option( - '-p', '--password', - default=secrets.token_urlsafe(20) + '-x&$', - help='admin password', + help='region pattern to deploy into, as provider::name ' + '(AWS::*US East 1*); * is a wildcard', ) parser.add_option( '-e', '--expires', default='4h', - help='timestamp when workspace should expire (4h)', + help='when the cluster should expire, as a timestamp or a ' + 'duration such as 4h (4h)', ) parser.add_option( '-s', '--size', default='S-00', - help='size of the workspace (S-00)', + help='size of the cluster (S-00)', +) +parser.add_option( + '-p', '--password', + help='password to give the admin user once the cluster is up; required, ' + 'because the password the API generates cannot be reported to a ' + 'caller that masks it', ) parser.add_option( '-t', '--token', - help='API key for the workspace management API', + help='API key for the management API', +) +parser.add_option( + '--project', + help='ID or name of the project to deploy into; defaults to the ' + 'organization\'s STANDARD-edition project', ) parser.add_option( '--http-port', type='int', @@ -53,7 +59,7 @@ parser.add_option( '-o', '--output', default='env', choices=['env', 'github', 'json'], - help='report workspace information in the requested format: github, env, json', + help='report cluster information in the requested format: github, env, json', ) parser.add_option( '-d', '--database', @@ -66,120 +72,166 @@ parser.print_help() sys.exit(1) +if not options.password: + print('ERROR: --password is required', file=sys.stderr) + sys.exit(1) + if options.init_sql and not os.path.isfile(options.init_sql): - print('ERROR: Could not locate SQL file: {options.init_sql}', file=sys.stderr) + print(f'ERROR: Could not locate SQL file: {options.init_sql}', file=sys.stderr) sys.exit(1) -# Connect to workspace. This is still the deprecated v1 workspace-group -# grammar because the v1 test suite it sets up needs workspace groups; -# it gets ported to manage_clusters() when that suite goes. Pinned to v1 -# because manage_workspaces() otherwise follows the management.version option. -wm = s2.manage_workspaces(options.token or None, version='v1') +# Pin v2 explicitly rather than following the ambient management.version +# option: this script provisions clusters, which only exist in v2. +mgr = s2.manage_clusters(options.token or None, version='v2') -# Find matching region -if '::' in options.region: - pattern = options.region.replace('*', '.*') - regions = wm.regions - for item in random.sample(regions, k=len(regions)): - region_name = '{}::{}'.format(item.provider, item.name) - if re.match(pattern, region_name): - options.region = item.id - break -if '::' in options.region: +# Find a matching region. A v2 region is identified by the +# (provider, region_name) pair rather than an ID, so the matched Region object +# itself is handed to create_cluster. Candidates are shuffled to spread +# deployments across whichever regions match, and the pattern is tried against +# both the display name and the provider region name -- 'US East 1' and +# 'us-east-1' -- since either may land in Region.name. +pattern = options.region.replace('*', '.*') +regions = list(mgr.regions) + + +def candidates(item): + """Return the names ``item`` can be matched by, most specific first.""" + for label in (item.name, item.region_name): + if label: + yield f'{item.provider}::{label}' if '::' in options.region else label + + +region = None +for item in random.sample(regions, k=len(regions)): + if any(re.match(pattern, x) for x in candidates(item)): + region = item + break + +if region is None: print( - 'ERROR: Could not find a region mating the pattern: ' - '{options.region}', file=sys.stderr, + 'ERROR: Could not find a region matching the pattern ' + f'{options.region}; the API reports: ' + + ', '.join(sorted(f'{x.provider}::{x.name}' for x in regions)), + file=sys.stderr, ) sys.exit(1) -# Create workspace group -wg_name = 'Python Client Testing' - -wgs = [x for x in wm.workspace_groups if x.name == wg_name] -if len(wgs) > 1: - print('ERROR: There is more than one workspace group with the specified name.') - sys.exit(1) -elif len(wgs) == 1: - wg = wgs[0] +# Choose a project. projectID is required by POST /v2/clusters and only +# auto-resolves for an organization with a single project, so pick the +# STANDARD-edition one when it was not named explicitly. +if options.project: + project_id = options.project else: - wg = wm.create_workspace_group( - wg_name, - region=options.region, - admin_password=options.password, - # firewall_ranges=requests.get('https://api.github.com/meta').json()['actions'], - firewall_ranges=['0.0.0.0/0'], - allow_all_traffic=True, - ) - -# Make sure the workspace group exists before continuing -timeout = 300 -while timeout > 0 and not [x for x in wm.workspace_groups if x.name == wg_name]: - time.sleep(10) - timeout -= 10 + projects = list(mgr.projects) + standard = [x for x in projects if x.edition == 'STANDARD'] + if not standard: + print( + 'ERROR: No STANDARD-edition project in this organization; pass ' + '--project with one of: ' + + ', '.join(f'{x.name} ({x.id}, {x.edition})' for x in projects), + file=sys.stderr, + ) + sys.exit(1) + project_id = standard[0].id + + +# A cluster name must match [a-z0-9]([a-z0-9-]*[a-z0-9])? and be 1-32 +# characters: fold everything outside that alphabet to a hyphen, truncate, and +# trim any hyphen the cut exposes. +name = re.sub(r'[^a-z0-9]+', '-', args[0].lower()).strip('-')[:32].rstrip('-') +if not name: + print(f'ERROR: Cluster name is empty after cleaning: {args[0]}', file=sys.stderr) + sys.exit(1) -ws_name = re.sub(r'^-|-$', r'', re.sub(r'-+', r'-', re.sub(r'\s+', '-', args[0].lower()))) -ws = wg.create_workspace( - ws_name, +# wait_on_active covers ACTIVE, then the endpoint, then the firewall, so the +# cluster is actually reachable by the time this returns. +cluster = mgr.create_cluster( + name, + region=region, size=options.size, + firewall_ranges=['0.0.0.0/0'], + expires_at=options.expires, + project=project_id, wait_on_active=True, + wait_timeout=1200, ) -# Make sure the endpoint exists before continuing -timeout = 300 -while timeout > 0 and not ws.endpoint: - time.sleep(10) - ws.refresh() - timeout -= 10 - -if not ws.endpoint: - print('ERROR: Endpoint was never activated.') - sys.exit(1) - - -# Extra pause for server to become available -time.sleep(10) - -database = options.database -if not database: - database = 'TEMP_{}'.format(uuid.uuid4()).replace('-', '_') - -host = ws.endpoint +host = cluster.endpoint if ':' in host: host, port = host.split(':', 1) port = int(port) else: port = 3306 -# Print workspace information +database = options.database +if not database: + database = 'TEMP_{}'.format(uuid.uuid4()).replace('-', '_') + +# Report before touching the cluster any further. Everything below can fail +# against a cluster that is already billing, and the ID reported here is the +# caller's only handle on it -- a CI teardown job with an empty cluster-id output +# would DELETE /v2/clusters/ and leak the cluster it meant to remove. +# +# No password is reported: the caller passed it in, so it already has it. if options.output == 'env': - print(f'CLUSTER_ID={ws.id}') + print(f'CLUSTER_ID={cluster.id}') print(f'CLUSTER_HOST={host}') print(f'CLUSTER_PORT={port}') print(f'CLUSTER_DATABASE={database}') elif options.output == 'github': with open(os.environ['GITHUB_OUTPUT'], 'a') as output: - print(f'cluster-id={ws.id}', file=output) + print(f'cluster-id={cluster.id}', file=output) print(f'cluster-host={host}', file=output) print(f'cluster-port={port}', file=output) print(f'cluster-database={database}', file=output) elif options.output == 'json': print('{') - print(f' "cluster-id": "{ws.id}",') + print(f' "cluster-id": "{cluster.id}",') print(f' "cluster-host": "{host}",') - print(f' "cluster-port": {port}') - print(f' "cluster-database": {database}') + print(f' "cluster-port": {port},') + print(f' "cluster-database": "{database}"') print('}') +# The API generates the admin password and reports it only on the create +# response: no route hands it back later, and it is None after any refresh(). +# It is read back rather than set because the API accepts an adminPassword on +# both POST and PATCH and ignores both -- item 9 of +# docs/management-api-audit.md. +generated = cluster.admin_password +if not generated: + print( + 'ERROR: cluster was created without a readable admin password', + file=sys.stderr, + ) + sys.exit(1) + +# Trade the generated password for the caller's, because the generated one +# cannot leave this process: a GitHub Actions runner drops any output whose value +# is masked -- "Skip output 'cluster-password' since it may contain secret" -- so +# masking it and passing it to another job are mutually exclusive. +# +# ALTER USER, not SET PASSWORD, which wants a pre-hashed value and rejects a +# literal with '1372: Password hash should be a 41-digit hexadecimal number'. +password = options.password +escaped = password.replace('\\', '\\\\').replace("'", "\\'") + +with s2.connect( + host=host, port=port, user='admin', + password=generated, connect_timeout=30, +) as conn: + with conn.cursor() as cur: + cur.execute(f"ALTER USER 'admin'@'%' IDENTIFIED BY '{escaped}'") + # Initialize the database if options.init_sql: init_db = [ os.path.join(os.path.dirname(__file__), 'init_db.py'), '--host', str(host), '--port', str(port), - '--user', 'admin', '--password', options.password, + '--user', 'admin', '--password', password, '--database', database, ] diff --git a/resources/drop_test_cluster.py b/resources/drop_test_cluster.py index 16ed7539d..6a7105dd7 100755 --- a/resources/drop_test_cluster.py +++ b/resources/drop_test_cluster.py @@ -2,7 +2,6 @@ # type: ignore from __future__ import annotations -import re import sys from optparse import OptionParser @@ -10,11 +9,11 @@ # Handle command-line options -usage = 'usage: %prog [options] workspace-id' +usage = 'usage: %prog [options] cluster-id' parser = OptionParser(usage=usage) parser.add_option( '-t', '--token', - help='API key for the workspace management API', + help='API key for the management API', ) (options, args) = parser.parse_args() @@ -23,33 +22,10 @@ sys.exit(1) -# Connect to workspace. This is still the deprecated v1 workspace-group -# grammar because the v1 test suite it sets up needs workspace groups; -# it gets ported to manage_clusters() when that suite goes. Pinned to v1 -# because manage_workspaces() otherwise follows the management.version option. -wm = s2.manage_workspaces(options.token or None, version='v1') +# Pin v2 explicitly rather than following the ambient management.version +# option: clusters only exist in v2. +mgr = s2.manage_clusters(options.token or None, version='v2') -wg_name = 'Python Client Testing' - -wgs = [x for x in wm.workspace_groups if x.name == wg_name] -if len(wgs) > 1: - print('ERROR: There is more than one workspace group with the specified name.') - sys.exit(1) -elif len(wgs) == 0: - print('ERROR: There is no workspace group with the specified name.') - sys.exit(1) -wg = wgs[0] - -ws_name = re.sub(r'^-|-$', r'', re.sub(r'-+', r'-', re.sub(r'\s+', '-', args[0].lower()))) - -wss = [x for x in wg.workspaces if x.name == ws_name] -if len(wss) > 1: - print('ERROR: There is more than one workspace with the specified name.') - sys.exit(1) -elif len(wss) == 0: - print('ERROR: There is no workspace with the specified name.') - sys.exit(1) -ws = wss[0] - -# Terminate workspace -ws.terminate() +# force=True so a cluster with connections still open goes away; this only +# ever runs against clusters this repo's CI created. +mgr.get_cluster(args[0]).terminate(force=True, wait_on_terminated=True) diff --git a/singlestoredb/functions/decorator.py b/singlestoredb/functions/decorator.py index 3da98ff46..5239f27f6 100644 --- a/singlestoredb/functions/decorator.py +++ b/singlestoredb/functions/decorator.py @@ -1,4 +1,3 @@ -import asyncio import functools import inspect from typing import Any @@ -122,7 +121,7 @@ def _func( if func is None: def decorate(func: UDFType) -> UDFType: - if asyncio.iscoroutinefunction(func): + if inspect.iscoroutinefunction(func): async def async_wrapper(*args: Any, **kwargs: Any) -> UDFType: return await func(*args, **kwargs) # type: ignore async_wrapper._singlestoredb_attrs = _singlestoredb_attrs # type: ignore @@ -136,7 +135,7 @@ def wrapper(*args: Any, **kwargs: Any) -> UDFType: return decorate - if asyncio.iscoroutinefunction(func): + if inspect.iscoroutinefunction(func): async def async_wrapper(*args: Any, **kwargs: Any) -> UDFType: return await func(*args, **kwargs) # type: ignore async_wrapper._singlestoredb_attrs = _singlestoredb_attrs # type: ignore diff --git a/singlestoredb/functions/ext/asgi.py b/singlestoredb/functions/ext/asgi.py index af3dbd385..876cf9eab 100755 --- a/singlestoredb/functions/ext/asgi.py +++ b/singlestoredb/functions/ext/asgi.py @@ -522,7 +522,7 @@ def build_udf_endpoint( """ if returns_data_format in ['scalar', 'list']: - is_async = asyncio.iscoroutinefunction(func) + is_async = inspect.iscoroutinefunction(func) async def do_func( cancel_event: threading.Event, @@ -568,7 +568,7 @@ def build_vector_udf_endpoint( """ masks = get_masked_params(func) array_cls = get_array_class(returns_data_format) - is_async = asyncio.iscoroutinefunction(func) + is_async = inspect.iscoroutinefunction(func) async def do_func( cancel_event: threading.Event, @@ -633,7 +633,7 @@ def build_tvf_endpoint( """ if returns_data_format in ['scalar', 'list']: - is_async = asyncio.iscoroutinefunction(func) + is_async = inspect.iscoroutinefunction(func) async def do_func( cancel_event: threading.Event, @@ -698,7 +698,7 @@ async def do_func( # each result row, so we just have to use the same # row ID for all rows in the result. - is_async = asyncio.iscoroutinefunction(func) + is_async = inspect.iscoroutinefunction(func) # Call function on each column of data async with timer('call_function'): @@ -787,7 +787,7 @@ def make_func( info['timeout'] = max(timeout, 1) # Set async flag - info['is_async'] = asyncio.iscoroutinefunction(func) + info['is_async'] = inspect.iscoroutinefunction(func) # Setup argument types for rowdat_1 parser colspec = [] diff --git a/singlestoredb/functions/signature.py b/singlestoredb/functions/signature.py index 657611a65..346116b4f 100644 --- a/singlestoredb/functions/signature.py +++ b/singlestoredb/functions/signature.py @@ -276,6 +276,7 @@ def simplify_dtype(dtype: Any) -> List[Any]: list of dtype strings, TupleCollections, and ArrayCollections """ + dtype = utils.resolve_type_alias(dtype) origin = typing.get_origin(dtype) atype = type(dtype) args = [] @@ -889,7 +890,12 @@ def get_schema( function_type = 'udf' udf_parameter = '`returns=`' if mode == 'return' else '`args=`' + spec = utils.resolve_type_alias(spec) spec, is_optional = unwrap_optional(spec) + # Again: the first pass only sees the outermost layer. An Optional is a + # Union, not an alias, so `Optional[NDArray[...]]` -- every typed numpy + # annotation marked nullable, on numpy 2.5 -- reaches here unexpanded. + spec = utils.resolve_type_alias(spec) origin = typing.get_origin(spec) args = typing.get_args(spec) args_origins = [typing.get_origin(x) if x is not None else None for x in args] @@ -1151,6 +1157,8 @@ def vector_check(obj: Any) -> Tuple[Any, str]: 'scalar', 'list', 'numpy', 'pandas', or 'polars' """ + obj = utils.resolve_type_alias(obj) + if utils.is_numpy(obj): if len(typing.get_args(obj)) < 2: return None, 'numpy' diff --git a/singlestoredb/functions/utils.py b/singlestoredb/functions/utils.py index 5b948e2c4..7aecd0a34 100644 --- a/singlestoredb/functions/utils.py +++ b/singlestoredb/functions/utils.py @@ -36,6 +36,62 @@ def is_union(x: Any) -> bool: return typing.get_origin(x) in _UNION_TYPES +def _is_type_alias(obj: Any) -> bool: + """Check if an object is a PEP 695 type alias.""" + # Duck-typed rather than isinstance(obj, typing.TypeAliasType): the class + # only exists in 3.12+, and a library supporting older versions may be + # using the typing_extensions backport instead. + return hasattr(obj, '__value__') and hasattr(obj, '__type_params__') + + +def resolve_type_alias(obj: Any) -> Any: + """ + Expand a PEP 695 type alias to the type it stands for. + + numpy 2.5 redefined ``npt.NDArray`` as an alias -- + ``type NDArray[ScalarT] = ndarray[_AnyShape, dtype[ScalarT]]`` -- rather + than a subscripted generic. For ``NDArray[np.str_]`` that makes + ``typing.get_origin`` return the alias object instead of ``numpy.ndarray`` + and ``typing.get_args`` return ``(np.str_,)`` instead of the + ``(shape, dtype[...])`` pair the type checks here read. Expanding the alias + puts the annotation back into the subscripted-generic form, so the rest of + the introspection works the same on every numpy version. + + Parameters + ---------- + obj : Any + Python type annotation + + Returns + ------- + Any + The annotation with any type aliases expanded + + """ + while True: + origin = typing.get_origin(obj) + + # A subscripted alias: `NDArray[np.str_]`. Subscripting the alias's own + # value substitutes the arguments for its type parameters. This case is + # tested before the bare one below because a subscripted alias forwards + # attribute lookups to the alias it came from, so it answers to + # __value__ as well -- reading that here would drop the arguments. + if _is_type_alias(origin): + try: + obj = origin.__value__[typing.get_args(obj)] + except TypeError: + # Not substitutable; leave it for the caller to reject + return obj + continue + + # A bare alias used directly as an annotation: `type Vec = NDArray[f64]` + if origin is None and _is_type_alias(obj): + obj = obj.__value__ + continue + + return obj + + def get_annotations(obj: Any) -> Dict[str, Any]: """Get the annotations of an object.""" return typing.get_type_hints(obj) @@ -60,6 +116,8 @@ def get_type_name(obj: Any) -> str: def is_numpy(obj: Any) -> bool: """Check if an object is a numpy array.""" + obj = resolve_type_alias(obj) + if str(obj).startswith('numpy.ndarray['): return True diff --git a/singlestoredb/management/manager.py b/singlestoredb/management/manager.py index 37ba35708..c20f571de 100644 --- a/singlestoredb/management/manager.py +++ b/singlestoredb/management/manager.py @@ -1,9 +1,14 @@ #!/usr/bin/env python """SingleStoreDB Base Manager.""" +import functools +import logging import os +import random +import re import sys import time from typing import Any +from typing import Callable from typing import Dict from typing import List from typing import Optional @@ -23,6 +28,9 @@ from .utils import get_token +logger = logging.getLogger(__name__) + + def set_organization(kwargs: Dict[str, Any]) -> None: """Set the organization ID in the dictionary.""" if kwargs.get('params', {}).get('organizationID', None): @@ -35,13 +43,15 @@ def set_organization(kwargs: Dict[str, Any]) -> None: kwargs['params']['organizationID'] = org -#: Methods that may be replayed after a transport-level failure. POST is -#: absent on purpose: a dropped connection does not say whether the server -#: acted on the request, and replaying ``POST /clusters`` would deploy twice. -#: Everything the long ``wait_on_*`` loops issue is a GET, so the retries -#: cover the failure mode that actually shows up -- a keep-alive connection -#: the far end closed while the client was sleeping between polls, which -#: surfaces as ``RemoteDisconnected`` on the next request. +#: Methods that may be replayed after a transport-level failure. POST is absent +#: on purpose: a dropped connection does not say whether the server acted, and +#: replaying ``POST /clusters`` would deploy twice. Everything the long +#: ``wait_on_*`` loops issue is a GET, so this covers the failure mode that shows +#: up -- a keep-alive connection the far end closed while the client slept +#: between polls, surfacing as ``RemoteDisconnected`` on the next request. +#: +#: :func:`retry_on_lock` is the one POST replay, keyed on an error message this +#: policy never sees. RETRY_METHODS = frozenset(['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']) #: Status codes worth retrying. These are the transient ones; a 4xx other @@ -75,6 +85,97 @@ def build_retry( ) +#: "could not acquire lock within duration", the API's refusal to start a +#: creation while another one in the organization holds the lock. Both +#: ``POST /workspaceGroups`` and ``POST /clusters`` say "error creating +#: workspace", so the match cannot key on the noun. +#: +#: Matched on the message, not the status: a name collision is a 500 too, and +#: only the wording says nothing was created, which is what makes replaying the +#: POST safe. +LOCK_ERROR_RE = re.compile(r'acquire[^.]{0,40}lock', re.I) + +#: Ceiling on the wait between lock retries, and the random extra added to each +#: one. Capped because what is being waited out is another creation's POST +#: returning, not a deployment coming up. Jittered because two clients that +#: collided back off identically from the same moment -- two xdist workers, say +#: -- and would otherwise retry in step indefinitely. +LOCK_RETRY_MAX_INTERVAL = 60.0 +LOCK_RETRY_JITTER = 5.0 + + +def lock_retry_policy() -> Tuple[int, float]: + """ + Return the (retries, interval) applied to an organization lock conflict. + + ``retries`` counts attempts *after* the first, so the defaults wait 20, 40, + 60, 60 and 60 seconds -- four minutes at worst, small enough that a stuck + organization fails rather than idling out a CI job's timeout. + + Set ``SINGLESTOREDB_MANAGEMENT_LOCK_RETRIES=0`` to raise the conflict at + once instead. + """ + return ( + int(os.environ.get('SINGLESTOREDB_MANAGEMENT_LOCK_RETRIES', '5')), + float( + os.environ.get('SINGLESTOREDB_MANAGEMENT_LOCK_RETRY_INTERVAL', '20'), + ), + ) + + +def is_lock_error(exc: BaseException) -> bool: + """Is this error the organization refusing to take the lock?""" + return bool(LOCK_ERROR_RE.search(str(exc))) + + +def lock_retry_wait(attempt: int, interval: float) -> float: + """Seconds to wait before replaying a creation that lost the lock.""" + return min(interval * attempt, LOCK_RETRY_MAX_INTERVAL) + \ + random.uniform(0, LOCK_RETRY_JITTER) + + +def retry_on_lock(func: Callable[..., Any]) -> Callable[..., Any]: + """ + Wait out an organization lock conflict on a deployment creation. + + Replaying this POST is safe where widening :data:`RETRY_METHODS` would not + be: the lock message says the creation never started. A creation that made + something and *then* failed reports something else, and would surface on the + replay as a name conflict rather than being swallowed. + + Worn by ``WorkspaceManager.create_workspace_group`` and + ``ClusterManager.create_cluster`` only -- the two calls that contend for the + lock, and the ones Fusion's ``CREATE WORKSPACE GROUP``/``CREATE CLUSTER`` + go through. Any other ``ManagementError`` is raised at once, as is the + conflict itself once :func:`lock_retry_policy`'s budget runs out. + """ + @functools.wraps(func) + def wrapper(self: Any, *args: Any, **kwargs: Any) -> Any: + retries, interval = lock_retry_policy() + attempt = 0 + while True: + try: + return func(self, *args, **kwargs) + except ManagementError as exc: + attempt += 1 + if attempt > retries or not is_lock_error(exc): + raise + wait = lock_retry_wait(attempt, interval) + logger.info( + f'{func.__name__} could not take the organization lock ' + f'({exc}); attempt {attempt} of {retries + 1}, retrying ' + f'in {wait:.1f}s', + ) + timing.sleep(wait, f'{func.__name__} organization lock') + + # Says which methods wear this, for a test to assert against. On the + # wrapper's ``__dict__``, so ``functools.wraps`` carries it out through any + # later decorator -- the test suite wraps these methods again. + wrapper.__retry_on_lock__ = True # type: ignore[attr-defined] + + return wrapper + + def default_timeout() -> Tuple[float, float]: """ Return the (connect, read) timeout applied when a caller gives none. @@ -103,12 +204,11 @@ class Manager: #: Management API version if none is specified. The shared #: :data:`~singlestoredb.management._version_import.DEFAULT_VERSION`, which - #: also supplies the ``management.version`` option default, so the two - #: cannot drift. Deliberately not a reading of that option: it is read by - #: the ``manage_*`` factories at call time, and reading it here would let a - #: version-specific class declare itself to be whatever the option happened - #: to say. A class that implements one specific version pins that version - #: as a literal instead of inheriting this. + #: also supplies the ``management.version`` option default, so the two cannot + #: drift. Deliberately not a reading of that option, which the ``manage_*`` + #: factories read at call time: reading it here would let a version-specific + #: class declare itself to be whatever the option happened to say. Such a + #: class pins its version as a literal instead of inheriting this. default_version = DEFAULT_VERSION #: Base URL if none is specified. diff --git a/singlestoredb/management/utils.py b/singlestoredb/management/utils.py index bfdcc8658..c596cd8e3 100644 --- a/singlestoredb/management/utils.py +++ b/singlestoredb/management/utils.py @@ -408,6 +408,113 @@ def enable_http_tracing() -> None: requests_log.propagate = True +#: Both timestamp shapes the API returns: RFC 3339, and a Go ``time.Time`` +#: rendered by ``String()`` -- ``2026-09-17 14:42:41.445984 +0000 UTC``, which is +#: how ``GET /v2/clusters/{id}`` reports ``expiresAt``. The trailing zone name is +#: not ISO 8601, so that value used to fail to parse and read as unset. It and +#: Go's monotonic reading are both optional. +#: +#: ``Z`` counts as an offset so RFC 3339 gets the same fraction padding: +#: ``...20.43888Z`` otherwise reached the converter with five digits, which only +#: 3.11 and later parse. +_GO_DATETIME_RE = re.compile( + r'^(?P\d{4}-\d{2}-\d{2}[ T]\d{2}:\d{2}:\d{2}(?:\.\d+)?)' + r'(?:\s*(?P[Zz]|[+-]\d{2}:?\d{2}))?' + r'(?:\s+(?P[A-Za-z]\S*))?' + r'(?:\s+m=\S+)?$', +) + + +def _normalize_datetime(obj: str) -> str: + """ + Return ``obj`` as something :func:`converters.datetime_fromisoformat` reads. + + Reduces both shapes :data:`_GO_DATETIME_RE` matches to a bare ISO 8601 + timestamp plus an optional numeric offset. Fractional seconds are padded to + microseconds: Go trims trailing zeros, and ``datetime.fromisoformat`` + accepts only 3 or 6 digits before Python 3.11. + + Parameters + ---------- + obj : str + Timestamp as reported by the API + + Returns + ------- + str + + """ + match = _GO_DATETIME_RE.match(obj.strip()) + if match is None: + # Not a shape this recognizes; hand it over untouched so the converter + # gets its usual chance to make sense of it. + return obj.replace('Z', '') + + stamp = match.group('stamp') + + # Fix datetimes with truncated zeros + if '.' in stamp: + stamp, micros = stamp.split('.', 1) + micros = micros[:6] + '0' * (6 - len(micros)) + stamp = stamp + '.' + micros + + # Go writes +0000; 3.9 and 3.10 want +00:00, so always emit the colon. Z is + # spelled out for the same reason -- nothing before 3.11 reads it. + offset = match.group('offset') or '' + if offset in ('Z', 'z'): + offset = '+00:00' + elif offset and ':' not in offset: + offset = offset[:3] + ':' + offset[3:] + + return stamp + offset + + +def _is_go_zero_time(obj: Union[datetime.date, datetime.datetime]) -> bool: + """ + Return whether ``obj`` is Go's zero time, which means "unset". + + An unassigned Go ``time.Time`` renders as January 1 of year 1, and the API + returns that for a field it has no value for -- most visibly an ``expiresAt`` + on a resource that does not expire. Testing the parsed value rather than the + string covers both spellings (``0001-01-01T00:00:00Z`` and + ``0001-01-01 00:00:00 +0000 UTC``) and any trimmings they carry. + + Parameters + ---------- + obj : datetime.date or datetime.datetime + Parsed timestamp + + Returns + ------- + bool + + """ + return (obj.year, obj.month, obj.day) == (1, 1, 1) + + +def _as_naive_utc(obj: datetime.datetime) -> datetime.datetime: + """ + Return ``obj`` as a naive UTC datetime. + + An aware value is shifted onto UTC and stripped; a naive one already means + UTC and is left alone. One convention either way, or two timestamps read off + the same object could not be compared. + + Parameters + ---------- + obj : datetime.datetime + Parsed timestamp, with or without a timezone + + Returns + ------- + datetime.datetime + + """ + if obj.tzinfo is None: + return obj + return obj.astimezone(datetime.timezone.utc).replace(tzinfo=None) + + def to_datetime( obj: Optional[Union[str, datetime.datetime]], ) -> Optional[datetime.datetime]: @@ -416,20 +523,18 @@ def to_datetime( return None if isinstance(obj, datetime.datetime): return obj - if obj == '0001-01-01T00:00:00Z': - return None - obj = obj.replace('Z', '') - # Fix datetimes with truncated zeros - if '.' in obj: - obj, micros = obj.split('.', 1) - micros = micros + '0' * (6 - len(micros)) - obj = obj + '.' + micros - out = converters.datetime_fromisoformat(obj) + out = converters.datetime_fromisoformat(_normalize_datetime(obj)) if isinstance(out, str): return None + if out is None: + return None + # Before _as_naive_utc: shifting an aware year-1 value onto UTC can carry it + # below datetime.MINYEAR, which raises instead of returning None. + if _is_go_zero_time(out): + return None if isinstance(out, datetime.date) and not isinstance(out, datetime.datetime): return datetime.datetime(out.year, out.month, out.day) - return out + return _as_naive_utc(out) def to_datetime_strict( @@ -440,22 +545,18 @@ def to_datetime_strict( raise TypeError('not possible to convert None to datetime') if isinstance(obj, datetime.datetime): return obj - if obj == '0001-01-01T00:00:00Z': - raise ValueError('not possible to convert 0001-01-01T00:00:00Z to datetime') - obj = obj.replace('Z', '') - # Fix datetimes with truncated zeros - if '.' in obj: - obj, micros = obj.split('.', 1) - micros = micros + '0' * (6 - len(micros)) - obj = obj + '.' + micros - out = converters.datetime_fromisoformat(obj) + out = converters.datetime_fromisoformat(_normalize_datetime(obj)) if not out: raise TypeError('not possible to convert None to datetime') if isinstance(out, str): raise ValueError('value cannot be str') + # See to_datetime: checked here rather than after the UTC shift, which can + # raise on a year-1 value. + if _is_go_zero_time(out): + raise ValueError(f'not possible to convert {obj} to datetime') if isinstance(out, datetime.date) and not isinstance(out, datetime.datetime): return datetime.datetime(out.year, out.month, out.day) - return out + return _as_naive_utc(out) def from_datetime( diff --git a/singlestoredb/management/v1/workspace.py b/singlestoredb/management/v1/workspace.py index 718b292f9..7fda33e56 100644 --- a/singlestoredb/management/v1/workspace.py +++ b/singlestoredb/management/v1/workspace.py @@ -47,6 +47,7 @@ from ...exceptions import ManagementError from ..billing import Billing as Billing from ..manager import Manager +from ..manager import retry_on_lock from ..region import Region from ..stage import StageObject as StageObject from ..utils import camel_to_snake_dict @@ -850,7 +851,14 @@ def terminate( raise ManagementError( msg='No workspace manager is associated with this object.', ) - self._manager._delete(f'workspaceGroups/{self.id}', params=dict(force=force)) + # 'true'/'false', not the bool: requests renders a bool param with + # str(), so force=True went out as force=True. force is what makes a + # group with live workspaces in it go away, so the value has to be read. + # Workspace.terminate above spells it out by hand for the same reason. + self._manager._delete( + f'workspaceGroups/{self.id}', + params=dict(force='true' if force else 'false'), + ) if wait_on_terminated: remaining = float(wait_timeout) while True: @@ -1267,6 +1275,7 @@ def shared_tier_regions(self) -> NamedList[Region]: [Region.from_dict(item, self) for item in res.json()], ) + @retry_on_lock def create_workspace_group( self, name: str, diff --git a/singlestoredb/management/v2/cluster.py b/singlestoredb/management/v2/cluster.py index 511b7cace..145d21253 100644 --- a/singlestoredb/management/v2/cluster.py +++ b/singlestoredb/management/v2/cluster.py @@ -24,6 +24,7 @@ from ...exceptions import ManagementError from ..billing import Billing as Billing from ..manager import Manager +from ..manager import retry_on_lock from ..organization import Organization from ..organization import Organizations as Organizations from ..region import Region @@ -611,7 +612,16 @@ def update( allow_all_traffic : bool, optional Allow all traffic to the cluster admin_password : str, optional - Admin password for the cluster + Admin password for the cluster. + + .. warning:: Ignored, exactly as on ``POST /v2/clusters``. ``PATCH`` + accepts the field and does not honor it: a live probe found the + patched value refused with ``1045: Access denied`` while the + password the create generated kept working. The only value that + authenticates is that generated one, carried on the create + response as :attr:`Cluster.admin_password`. Still sent in case + the API starts honoring it. See item 9 of + ``docs/management-api-audit.md``. expires_at : str, optional Timestamp of when the cluster will expire. Expiration time can be specified as a timestamp or a duration. @@ -704,7 +714,12 @@ def terminate( """ manager = self._require_manager() - manager._delete(f'clusters/{self.id}', params=dict(force=force)) + # 'true'/'false', not the bool: requests renders a bool param with + # str(), so force=True went out as force=True. + manager._delete( + f'clusters/{self.id}', + params=dict(force='true' if force else 'false'), + ) if wait_on_terminated: remaining = float(wait_timeout) while True: @@ -1393,6 +1408,7 @@ def _resolve_project_id( ', '.join(f'{x.name} ({x.id})' for x in projects) + '.', ) + @retry_on_lock def create_cluster( self, name: str, diff --git a/singlestoredb/mysql/tests/thirdparty/test_MySQLdb/test_MySQLdb_capabilities.py b/singlestoredb/mysql/tests/thirdparty/test_MySQLdb/test_MySQLdb_capabilities.py index c1daf6b44..c197ef9a8 100644 --- a/singlestoredb/mysql/tests/thirdparty/test_MySQLdb/test_MySQLdb_capabilities.py +++ b/singlestoredb/mysql/tests/thirdparty/test_MySQLdb/test_MySQLdb_capabilities.py @@ -5,8 +5,6 @@ from . import capabilities from singlestoredb.mysql.tests import base -warnings.filterwarnings('error') - class test_MySQLdb(capabilities.DatabaseTest): @@ -25,6 +23,30 @@ class test_MySQLdb(capabilities.DatabaseTest): leak_test = False + # These tests want warnings raised as exceptions -- test_truncation asks the + # server for an over-long column and expects the driver to complain. Scoped + # to the test rather than set at module import: this package's __init__ + # imports this module, so a module-level warnings.filterwarnings('error') + # was installed process-wide the moment anything imported one of these + # classes (singlestoredb/tests/test_dbapi.py does), and every later warning + # in the session -- a DeprecationWarning from a dependency, say -- became a + # fatal error in an unrelated test. + def setUp(self): + self._warnings = warnings.catch_warnings() + self._warnings.__enter__() + warnings.simplefilter('error') + try: + super().setUp() + except Exception: + self._warnings.__exit__(None, None, None) + raise + + def tearDown(self): + try: + super().tearDown() + finally: + self._warnings.__exit__(None, None, None) + def quote_identifier(self, ident): return '`%s`' % ident diff --git a/singlestoredb/tests/cleanup_deployments.py b/singlestoredb/tests/cleanup_deployments.py index 7b8d745c2..478e07e8b 100644 --- a/singlestoredb/tests/cleanup_deployments.py +++ b/singlestoredb/tests/cleanup_deployments.py @@ -42,20 +42,41 @@ are tracked as they are created and swept per test class by ``conftest.py``, which cannot see -- or touch -- another run's deployments. -Why strays keep appearing: that tracking, the per-class sweep and this script -all live on the ``versioned-management-api`` branch and nowhere else. A run -from ``main`` has only ``tearDownClass``, so a killed run or a ``setUpClass`` -that raises leaks a workspace group permanently, and ``main`` still uses names -this script only knows through :data:`LEGACY_PATTERNS`. Until the sweep is on -the default branch, expect to run this by hand. +The exception, and the reason this is wired into CI, is ``--ledger``. A run +with ``SINGLESTOREDB_TEST_DEPLOYMENT_LOG`` set records every creation to a +JSONL file as it happens (``utils.ledger_pending``/``ledger_live``/ +``ledger_gone``), so a run that was killed outright leaves an exact list of +what it made:: + + python -m singlestoredb.tests.cleanup_deployments --ledger deployments.jsonl + +That mode replaces *both* guards above. The ledger names deployments rather +than guessing at them, so the patterns are unnecessary; and its entries are +minutes old by construction, so the age filter would spare every one of them. +What keeps it off other people's deployments instead is that it touches only +ids and names the ledger records, and that each CI job writes its own ledger. + +Finally, ``--secrets`` sweeps a different subject: the org-scoped secrets +``TestSecrets.test_get_secret`` creates. They bill nothing, but they are +permanent, and the test deletes its own only if it is not killed mid-test:: + + python -m singlestoredb.tests.cleanup_deployments --secrets --yes + +That is a rolling janitor rather than a run-scoped cleanup -- a secret is named +per-run but a name still says nothing about *which* run, so the age guard is +what keeps this off a live one. It cannot reap the run it is called from; what +it removes is what earlier runs stranded. """ import argparse import datetime +import json +import os import re import sys import warnings from collections.abc import Container from typing import Any +from typing import Dict from typing import List from typing import Optional from typing import Tuple @@ -78,31 +99,44 @@ #: anything younger could belong to a run in progress. DEFAULT_MIN_AGE_HOURS = 6.0 +#: How long to keep retrying a deployment the API will not delete yet. Longer +#: than ``utils.TERMINATE_RETRY_TIMEOUT``, which is short so the between-class +#: sweep cannot stall the suite: nothing runs after this tool, the deployment may +#: still be coming up, ``DELETE`` is refused until it is, and an S-00 cluster +#: reaching ACTIVE is ~460s at worst. The only cost is the CI step's wall clock. +#: +#: An upper bound, not a promise: a *cancelled* job's steps are force-terminated +#: after GitHub's 5-minute cancellation timeout, so a cancel early in a provision +#: gets killed here whatever this says. +TERMINATE_TIMEOUT = 600.0 + #: Names the suite generates. Anchored, because these run against a real #: organization: a pattern that matched a name someone chose by hand would #: terminate a deployment that is not ours. PATTERNS = [ # test_management_v1.py / test_management_v2.py fixtures re.compile(r'^(wg|ws|cl)-test-[A-Za-z0-9_-]+$'), + # TestWorkspace.test_update renames its live group to wg-foo- and + # never renames it back, so it carries that name for the rest of the class. + # Unmatched, a group stranded after that test was invisible here. + re.compile(r'^wg-foo-[A-Za-z0-9_-]+$'), re.compile(r'^starter-(ws|cl)-test-[A-Za-z0-9_-]+$'), # test_fusion.py fixtures re.compile(r'^[A-C] Fusion Testing [0-9a-f]+$'), re.compile(r'^[a-z]-fusion-cluster-[0-9a-f]+$'), re.compile(r'^jobs-fusion-[0-9a-f]+$'), re.compile(r'^stage-fusion-\d-[0-9a-f]+$'), - # test_create_drop_workspace_group's subject. Hex covers the decimal - # id(self) the test used to name it with, so groups stranded by older - # runs -- which this pattern did not match, and which therefore piled up - # invisibly -- are reaped too. + # test_create_drop_workspace_group's subject. Hex also covers the decimal + # id(self) the test used to name it with, so groups stranded by older runs + # are reaped too. re.compile(r'^Create WG Test [0-9a-f]+$'), ] -#: Names the suite used to generate. Kept separate so it is obvious what is -#: only here for cleanup, and matched all the same: a stranded deployment is -#: billed regardless of which revision made it, and ``main`` still creates -#: these -- it carries none of ``utils.track()``, the per-class sweep or this -#: script, so a run there leaks with nothing to reap it. Retire an entry once -#: no branch produces the name and the organization is clean of it. +#: Names the suite used to generate. Kept separate so it is obvious what is only +#: here for cleanup, and matched all the same: a stranded deployment bills +#: whichever revision made it, and ``main`` still creates these with nothing to +#: reap them. Retire an entry once no branch produces the name and the +#: organization is clean of it. LEGACY_PATTERNS = [ # TestStageFusion's two workspace groups, before it moved to v2 clusters # named stage-fusion-- and then to the shared cluster pool @@ -110,15 +144,39 @@ # TestFilesFusion's workspace group, which nothing in the class ever # read; it creates no deployment at all now re.compile(r'^Files Fusion Testing [0-9a-f]+$'), - # 'Group '. No revision of this repo generates this, so it is here - # on the owner's say-so rather than by attribution. Eight hex characters - # minimum, which is what the ones in the organization have: the bare - # 'Group 1' / 'Group 2' that a person or the portal produces is a real - # deployment someone is using, and a plain [0-9a-f]+ would match it. + # 'Group '. No revision of this repo generates this, so it is here on + # the owner's say-so. Eight hex characters minimum, which is what the ones + # in the organization have: a plain [0-9a-f]+ would also match the bare + # 'Group 1' a person or the portal produces. re.compile(r'^Group [0-9a-f]{8,}$'), ] +#: Secret names the suite generates. A secret is not a deployment -- it bills +#: nothing and lives on its own route -- so these are swept only when +#: ``--secrets`` asks for it, and never alongside the deployment patterns. +SECRET_PATTERNS = [ + # TestSecrets.test_get_secret, v1 and v2 + re.compile(r'^secret_v[12]_test_[0-9a-f]+$'), +] + +#: Secret names earlier revisions generated. Both are fixed rather than +#: per-run, which is what let two concurrent runs delete each other's secret; +#: ``main`` still creates them, so they are still reaped. +LEGACY_SECRET_PATTERNS = [ + re.compile(r'^secret_name$'), + re.compile(r'^secret_v2_test$'), +] + +#: Hours a secret must have existed before it is treated as stranded. Far lower +#: than :data:`DEFAULT_MIN_AGE_HOURS`, because the window it guards is far +#: shorter: the test creates a secret and deletes it in the same test body, a +#: second or two apart, so no secret a live run owns is even minutes old. Not +#: zero, because a run killed between the POST and the DELETE looks exactly +#: like one that is still between them. +DEFAULT_SECRET_MIN_AGE_HOURS = 1.0 + + def is_test_deployment(name: Optional[str]) -> bool: """Was this name generated by the test suite, now or in the past?""" if not name: @@ -126,6 +184,13 @@ def is_test_deployment(name: Optional[str]) -> bool: return any(x.match(name) for x in PATTERNS + LEGACY_PATTERNS) +def is_test_secret(name: Optional[str]) -> bool: + """Was this secret name generated by the test suite, now or in the past?""" + if not name: + return False + return any(x.match(name) for x in SECRET_PATTERNS + LEGACY_SECRET_PATTERNS) + + def _created_at(obj: Any) -> Optional[datetime.datetime]: """When this deployment was created, or None if the API did not say.""" created = getattr(obj, 'created_at', None) @@ -254,7 +319,7 @@ def keep(obj: Any) -> bool: if 'cluster' in kinds or 'starter-cluster' in kinds: try: - clusters = s2.manage_clusters(version='v2') + clusters = _manager('v2') except Exception as exc: print(f'! Could not reach management API v2: {exc}', file=sys.stderr) else: @@ -274,14 +339,7 @@ def keep(obj: Any) -> bool: if 'workspace-group' in kinds or 'starter-workspace' in kinds: try: - # v1 is deprecated, and asking for it here is the point: workspace - # groups exist nowhere else, so the warning is noise on every run. - with warnings.catch_warnings(): - warnings.filterwarnings( - 'ignore', category=DeprecationWarning, - message='.*manage_workspaces.*', - ) - workspaces = s2.manage_workspaces(version='v1') + workspaces = _manager('v1') except Exception as exc: print(f'! Could not reach management API v1: {exc}', file=sys.stderr) else: @@ -304,12 +362,430 @@ def keep(obj: Any) -> bool: return found, spared, unmatched +# +# Secret mode +# +# Why this is separate from everything above: a secret is org-scoped and +# permanent, it costs nothing to leave lying around, and it is reached through +# ``secrets`` rather than through any deployment listing. It is here because +# ``TestSecrets.test_get_secret`` is the one test that creates an org-scoped +# named object, and nothing in the suite sweeps one as it is created -- a run +# killed between its POST and its DELETE strands a secret for good. +# + + +def find_stranded_secrets( + mgr: Any, + older_than: float = DEFAULT_SECRET_MIN_AGE_HOURS, + include_unknown_age: bool = False, +) -> Tuple[List[Tuple[str, Any]], List[str], List[str]]: + """ + List the organization's secrets that the test suite stranded. + + Returns the same three lists as :func:`find_leftovers` -- the secrets to + delete, labels for the ones the age guard held back, and labels for the + ones whose names :data:`SECRET_PATTERNS` does not recognize. + + Only one version's manager is needed: ``secrets`` is identical at v1 and + v2 (see ``management/v2/organization.py``). + """ + from singlestoredb.management.organization import Secret + + found: List[Tuple[str, Any]] = [] + spared: List[str] = [] + unmatched: List[str] = [] + + # Verified live only this far: ``GET secrets`` with no parameters is + # accepted and answers with a ``secrets`` array -- the ``?name=`` form is + # all ``Organization.get_secret`` ever sends. UNVERIFIED: that the array is + # *every* secret in the organization rather than a page of them. It could + # not be shown against an organization that has none; if the route turns + # out to paginate, a sweep here is incomplete rather than wrong. + res = mgr._get('secrets') + for item in res.json().get('secrets') or []: + secret = Secret.from_dict(item) + + if secret.deleted_at is not None: + continue + + age = _age_hours(secret) + + if not is_test_secret(secret.name): + unmatched.append( + '{}{}'.format( + secret.name or '', + '' if age is None else f' ({age:.1f}h old)', + ), + ) + continue + + if age is None: + if not include_unknown_age: + spared.append(f'{secret.name} (creation time not reported)') + continue + elif older_than > 0 and age < older_than: + spared.append(f'{secret.name} ({age:.1f}h old, too new)') + continue + + found.append((f'secret {secret.name} ({secret.id})', secret)) + + return found, spared, unmatched + + +def _run_secret_sweep( + older_than: float, + include_unknown_age: bool, + yes: bool, + show_unmatched: bool, +) -> int: + """Report, and with ``yes`` delete, the secrets the suite stranded.""" + mgr = _manager('v2') + + try: + leftovers, spared, unmatched = find_stranded_secrets( + mgr, older_than, include_unknown_age, + ) + except Exception as exc: + # Reported, not raised: this runs as a cleanup step, and a secret bills + # nothing, so failing the job over one is the wrong trade. + print(f'! Could not list secrets: {exc}', file=sys.stderr) + return 1 + + if show_unmatched: + if unmatched: + print( + f'{len(unmatched)} secret(s) not recognized as the suite\'s, ' + 'and so never swept:', + ) + for label in sorted(unmatched): + print(f' ? {label}') + print() + else: + print('Every secret is recognized by SECRET_PATTERNS.\n') + + if spared: + print(f'{len(spared)} match(es) left alone by the age filter:') + for label in spared: + print(f' - {label}') + print() + + if not leftovers: + print('No stranded test secrets found.') + return 0 + + print(f'{len(leftovers)} stranded test secret(s):') + for label, _ in leftovers: + print(f' - {label}') + + if not yes: + print('\nDry run; pass --yes to delete these.') + return 0 + + failed = 0 + for label, secret in leftovers: + try: + mgr._delete(f'secrets/{secret.id}') + except Exception as exc: + failed += 1 + print(f'✗ {label}: {exc}') + else: + print(f'✓ deleted {label}') + + return 1 if failed else 0 + + +# +# Ledger mode +# +# Why: GH Actions run 35631802648, job ``test-coverage``, was cancelled 19 +# minutes into ``create_cluster(wait_on_active=True)``. The log ends at +# ``##[error]The operation was canceled.`` with no sweep output -- three clusters +# live, no in-process handler ever run. A file written as they are created is the +# only way another process can learn their names. +# + +#: How each ledger kind is resolved back to a live object: the management API +#: version that owns it, the point lookup for a record that has an id, and the +#: listing to search by name for a ``pending`` record that never got one. +#: +#: The kinds are the values of ``utils._KIND_BY_CLASS``. An unknown kind is +#: reported rather than skipped, the alternative being to silently not reap it. +LEDGER_KINDS = { + 'cluster': ( + 'v2', 'get_cluster', lambda mgr: mgr.clusters, + ), + 'starter_cluster': ( + 'v2', 'get_starter_cluster', lambda mgr: mgr.starter_clusters, + ), + 'workspace_group': ( + 'v1', 'get_workspace_group', lambda mgr: mgr.workspace_groups, + ), + 'workspace': ( + # WorkspaceManager has no `workspaces` of its own, so the search goes + # group by group -- the same walk utils._CREATORS uses. + 'v1', 'get_workspace', + lambda mgr: [w for g in mgr.workspace_groups for w in g.workspaces], + ), + 'starter_workspace': ( + 'v1', 'get_starter_workspace', lambda mgr: mgr.starter_workspaces, + ), +} + + +def _manager(version: str) -> Any: + """Management API manager for ``'v1'`` or ``'v2'``.""" + if version == 'v2': + return s2.manage_clusters(version='v2') + # v1 is deprecated, and asking for it here is the point: workspace groups + # exist nowhere else, so the warning is noise on every run. + with warnings.catch_warnings(): + warnings.filterwarnings( + 'ignore', category=DeprecationWarning, + message='.*manage_workspaces.*', + ) + return s2.manage_workspaces(version='v1') + + +def fold_ledger(lines: Any) -> List[Dict[str, Any]]: + """ + Reduce ledger records to the deployments that should still be live. + + The ledger is an append-only history, not a state: a deployment shows up as + ``pending``, then ``live`` once it has an id, then ``gone`` once terminated. + Folding keeps every deployment whose last event was not ``gone``. + + A ``pending`` is keyed by ``(kind, name)`` because that is all it has; the + matching ``live`` retires it and re-keys on the id, so a normal creation's + two records collapse to one entry. A ``pending`` left standing means the + creator was interrupted before returning -- the cancelled-mid-wait case, + resolvable only by name. + + Order is creation order, since dicts preserve insertion order. The caller + reverses it, so a workspace goes before the group that holds it, matching + ``utils.cleanup_tracked()``. + + Malformed lines are skipped with a warning rather than aborting: this is the + last step of a CI job, and one truncated line must not stop the rest from + being reaped. + """ + live: Dict[Any, Dict[str, Any]] = {} + + for lineno, line in enumerate(lines, start=1): + line = line.strip() + if not line: + continue + try: + record = json.loads(line) + except ValueError as exc: + print( + f'! ledger line {lineno} is not JSON, skipping it: {exc}', + file=sys.stderr, + ) + continue + if not isinstance(record, dict): + continue + + event = record.get('event') + kind = record.get('kind') + name = record.get('name') + ident = record.get('id') + + by_name = ('name', kind, name) + by_id = ('id', kind, ident) + + if event == 'pending': + if name is not None: + live.setdefault(by_name, record) + elif event == 'live': + live.pop(by_name, None) + if ident is not None: + live[by_id] = record + elif name is not None: + # No id in the record: keep it findable by name rather than + # dropping it. Should not happen, but losing the deployment is + # the expensive direction. + live[by_name] = record + elif event == 'gone': + if ident is not None: + live.pop(by_id, None) + live.pop(by_name, None) + + return list(live.values()) + + +def read_ledger(path: str) -> List[Dict[str, Any]]: + """ + Fold the ledger at ``path``, newest first. + + A missing file is not an error: the variable can be set on a job whose + tests created nothing, and a CI cleanup step that failed in that case would + turn every such run red. + """ + if not os.path.exists(path): + print(f'No ledger at {path}; nothing this run created was recorded.') + return [] + with open(path, encoding='utf-8') as file: + records = fold_ledger(file) + # Newest first, so a workspace is terminated before its group. + records.reverse() + return records + + +def find_ledger_leftovers( + path: str, +) -> Tuple[List[Tuple[str, Any]], List[str], List[str]]: + """ + Resolve the ledger's still-live records to live deployment objects. + + Returns + ------- + (List[Tuple[str, Any]], List[str], List[str]) + The deployments to terminate, labels for the records that resolved to + nothing -- already gone, so nothing to do -- and labels for the ones + that could not be resolved *and* could still be live, which is what + makes the run exit non-zero. + + A 404 from the point lookup means the deployment is already gone, the common + case for a run that finished normally. Anything else -- a transport failure, + an unknown kind -- goes in the third list: "could not tell" and "not there" + must not read the same when the difference is a cluster billing. + """ + from singlestoredb.exceptions import ManagementError + + found: List[Tuple[str, Any]] = [] + gone: List[str] = [] + unresolved: List[str] = [] + + managers: Dict[str, Any] = {} + + def manager_for(version: str) -> Any: + if version not in managers: + managers[version] = _manager(version) + return managers[version] + + for record in read_ledger(path): + kind = record.get('kind') + name = record.get('name') + ident = record.get('id') + label = '{} {} ({})'.format( + str(kind).replace('_', ' '), name or '', ident or 'no id', + ) + + if kind not in LEDGER_KINDS: + unresolved.append(f'{label}: unknown kind {kind!r}') + continue + version, lookup_name, listing = LEDGER_KINDS[kind] + + try: + mgr = manager_for(version) + except Exception as exc: + unresolved.append( + f'{label}: could not reach management API ' + f'{version}: {exc}', + ) + continue + + obj = None + try: + if ident is not None: + obj = getattr(mgr, lookup_name)(ident) + else: + # A `pending` record: the creator never returned an id, so the + # only handle on it is the name. Matched over the listing + # exactly as utils._recover_orphan does. + for candidate in listing(mgr): + if getattr(candidate, 'name', None) == name: + obj = candidate + break + except ManagementError as exc: + if exc.errno == 404: + gone.append(label) + continue + unresolved.append(f'{label}: {exc}') + continue + except Exception as exc: + unresolved.append(f'{label}: {exc}') + continue + + if obj is None: + gone.append(label) + elif getattr(obj, 'terminated_at', None) is not None: + gone.append(f'{label} (already terminated)') + else: + found.append((label, obj)) + + return found, gone, unresolved + + +def _run_ledger_sweep(path: str, yes: bool) -> int: + """Report, and with ``yes`` terminate, everything the ledger still lists.""" + leftovers, gone, unresolved = find_ledger_leftovers(path) + + print( + f'Ledger {path}: {len(leftovers)} still live, {len(gone)} already ' + f'gone, {len(unresolved)} unresolved.\n', + ) + + if unresolved: + print( + f'{len(unresolved)} ledger record(s) could not be resolved, so ' + 'they may still be live:', + ) + for label in unresolved: + print(f' ? {label}') + print() + + if not leftovers: + # Non-zero only for the records whose state is unknown: a clean run + # whose sweep already terminated everything must not fail the job. + print('Nothing left behind by this run.') + return 1 if unresolved else 0 + + print(f'{len(leftovers)} deployment(s) left behind by this run:') + for label, _ in leftovers: + print(f' - {label}') + + if not yes: + print('\nDry run; pass --yes to terminate these.') + return 1 if unresolved else 0 + + from singlestoredb.tests import utils + + failed = 0 + for label, obj in leftovers: + try: + utils.terminate(obj, timeout=TERMINATE_TIMEOUT) + except Exception as exc: + failed += 1 + print(f'✗ {label}: {exc}') + else: + print(f'✓ terminated {label}') + + return 1 if (failed or unresolved) else 0 + + def main(argv: Optional[List[str]] = None) -> int: parser = argparse.ArgumentParser(description=__doc__.split('\n\n')[1]) parser.add_argument( '--yes', action='store_true', help='actually terminate; without this the run only reports', ) + parser.add_argument( + '--ledger', metavar='PATH', + help='sweep exactly what the run that wrote this JSONL ledger created ' + '(see SINGLESTOREDB_TEST_DEPLOYMENT_LOG). Replaces the name ' + 'patterns and the age filter, which would spare everything in it ' + 'for being minutes old. CI runs this as an if: always() step', + ) + parser.add_argument( + '--secrets', action='store_true', + help='sweep stranded org-scoped secrets instead of deployments (see ' + 'SECRET_PATTERNS). Honours --yes, --older-than, ' + f'--include-unknown-age and --show-unmatched; --older-than ' + f'defaults to {DEFAULT_SECRET_MIN_AGE_HOURS} here rather than ' + f'{DEFAULT_MIN_AGE_HOURS}, since a secret a live run owns is ' + 'seconds old, not hours', + ) parser.add_argument( '--older-than', type=float, default=DEFAULT_MIN_AGE_HOURS, metavar='HOURS', @@ -357,6 +833,46 @@ def main(argv: Optional[List[str]] = None) -> int: ) args = parser.parse_args(argv) + # --ledger asks "what did *this* run make?", not "what looks stranded?", so + # it does not compose with the name and age guards. Erroring beats silently + # ignoring them. + if args.ledger: + for flag, value in ( + ('--older-than', args.older_than != DEFAULT_MIN_AGE_HOURS), + ('--since', args.since is not None), + ('--any-name', args.any_name), + ('--kind', bool(args.kinds)), + ('--show-unmatched', args.show_unmatched), + ('--secrets', args.secrets), + ): + if value: + parser.error(f'{flag} does not apply with --ledger') + return _run_ledger_sweep(args.ledger, args.yes) + + # A different subject, not a different filter: --secrets sweeps secrets + # *instead of* deployments, so the flags that select deployments do not + # compose with it either. + if args.secrets: + for flag, value in ( + ('--since', args.since is not None), + ('--any-name', args.any_name), + ('--kind', bool(args.kinds)), + ): + if value: + parser.error(f'{flag} does not apply with --secrets') + # An explicit --older-than 6 is indistinguishable from the default + # here, which costs nothing: it is the value the caller asked for + # either way. + older_than = ( + DEFAULT_SECRET_MIN_AGE_HOURS + if args.older_than == DEFAULT_MIN_AGE_HOURS + else args.older_than + ) + return _run_secret_sweep( + older_than, args.include_unknown_age, args.yes, + args.show_unmatched, + ) + kinds = args.kinds or list(KINDS) leftovers, spared, unmatched = find_leftovers( @@ -415,7 +931,10 @@ def main(argv: Optional[List[str]] = None) -> int: failed = 0 for label, obj in leftovers: try: - utils.terminate(obj) + # Same budget as the ledger sweep: --since or --older-than 0 can + # select a deployment that is still provisioning, and nothing runs + # after this either. + utils.terminate(obj, timeout=TERMINATE_TIMEOUT) except Exception as exc: failed += 1 print(f'✗ {label}: {exc}') diff --git a/singlestoredb/tests/conftest.py b/singlestoredb/tests/conftest.py index 9d426a647..0de122150 100644 --- a/singlestoredb/tests/conftest.py +++ b/singlestoredb/tests/conftest.py @@ -297,6 +297,67 @@ def on_sigterm(signum: int, frame: Any) -> None: logger.debug('Not the main thread; no SIGTERM sweep installed') +#: Key the workers stash their stranded deployment labels under in +#: ``config.workeroutput``. +_STRANDED_KEY = 'singlestoredb_stranded_deployments' + + +def pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None: + """ + Sweep in an xdist worker, and hand what survived to the controller. + + Under the parallel default the sweep and its ``STILL LIVE`` banner run in a + worker, whose stdout the controller discards -- so a leak was silent even + when the sweep ran and failed, the one case the banner exists for. + ``config.workeroutput`` is xdist's channel for this; its absence means + ``-n 0``, where ``pytest_unconfigure`` already prints to a real terminal. + + The sweep has to happen here, not in ``pytest_unconfigure``, to have + anything to report: xdist's ``pytest_sessionfinish`` hookwrapper sends + ``workeroutput`` after yielding, which is before ``pytest_unconfigure`` + runs. The sweep is idempotent -- a successful one empties ``_tracked`` -- + so the later call finds nothing to do. + """ + workeroutput = getattr(session.config, 'workeroutput', None) + if workeroutput is None: + return + + _sweep_live_deployments() + + try: + workeroutput[_STRANDED_KEY] = _test_utils().tracked_labels() + except Exception: # pragma: no cover - shutdown path + pass + + +def pytest_testnodedown(node: Any, error: Any) -> None: + """ + Report, on the controller, what a worker could not terminate. + + The worker's own banner went to a captured stream; this is the copy anyone + actually sees. + """ + stranded = getattr(node, 'workeroutput', {}).get(_STRANDED_KEY) or [] + if not stranded: + return + + print('\n' + '!' * 70) + print( + f'STILL LIVE on {node.gateway.id} -- these deployments could not be ' + 'terminated and are costing money:', + ) + for label in stranded: + print(f' - {label}') + print( + 'Reap them with: python -m singlestoredb.tests.cleanup_deployments ' + '--yes', + ) + print('!' * 70) + logger.error( + f'{len(stranded)} deployment(s) left live by {node.gateway.id}', + ) + + def pytest_unconfigure(config: pytest.Config) -> None: """ Pytest hook that runs after all tests complete. @@ -328,10 +389,10 @@ def pytest_unconfigure(config: pytest.Config) -> None: #: ``pytest_terminal_summary`` can read it without a fixture. #: #: Every test that ran under an active trace is in here, including the ones that -#: made no management call at all: ``trace_management_api_class`` subtracts this -#: list from the class total to get the fixture share, so a test missing from it -#: has its wall clock charged to ``setUpClass``. The event-less ones are -#: filtered out at report time by :func:`_traced` instead. +#: made no management call: ``trace_management_api_class`` subtracts this list +#: from the class total to get the fixture share, so a test missing from it would +#: have its wall clock charged to ``setUpClass``. :func:`_traced` filters the +#: event-less ones out at report time instead. _management_traces: List[Tuple[str, Any]] = [] #: The same, for the class fixtures rather than the tests. Separate because the diff --git a/singlestoredb/tests/test_dbapi.py b/singlestoredb/tests/test_dbapi.py index 8bacde56d..3242e3e56 100644 --- a/singlestoredb/tests/test_dbapi.py +++ b/singlestoredb/tests/test_dbapi.py @@ -1,8 +1,12 @@ # type: ignore +import importlib import os +import unittest +import warnings import singlestoredb as s2 from . import utils +from singlestoredb.mysql.tests.thirdparty.test_MySQLdb import test_MySQLdb_capabilities from singlestoredb.mysql.tests.thirdparty.test_MySQLdb import test_MySQLdb_dbapi20 @@ -25,3 +29,19 @@ def tearDownClass(cls): def _connect(self): return s2.connect(database=type(self).dbname) + + +class TestWarningFilters(unittest.TestCase): + """Importing the vendored MySQLdb tests must not escalate warnings.""" + + def test_capabilities_import_leaves_filters_alone(self): + # The capabilities module wants warnings raised as errors, but it must + # scope that to its own tests. It used to call + # warnings.filterwarnings('error') at module level, and since this + # module imports that package, the filter was installed process-wide + # and turned any later warning -- a DeprecationWarning from a + # dependency, say -- into a failure in an unrelated test. Reload to + # re-run the module body against a known set of filters. + before = list(warnings.filters) + importlib.reload(test_MySQLdb_capabilities) + self.assertEqual(warnings.filters, before) diff --git a/singlestoredb/tests/test_fusion.py b/singlestoredb/tests/test_fusion.py index 248259dc0..f4337e513 100644 --- a/singlestoredb/tests/test_fusion.py +++ b/singlestoredb/tests/test_fusion.py @@ -969,31 +969,35 @@ def setUpClass(cls): # US-only: no test here asserts anything about these groups' regions, # and creation in some non-US regions fails with a control-plane 500. us_regions = [x for x in mgr.regions if x.name.startswith('US')] - wg = mgr.create_workspace_group( - f'A Fusion Testing {cls.id}', - region=random.choice(us_regions), - firewall_ranges=[], - ) - cls.workspace_groups.append(wg) - wg = mgr.create_workspace_group( - f'B Fusion Testing {cls.id}', - region=random.choice(us_regions), - firewall_ranges=[], - ) - cls.workspace_groups.append(wg) - wg = mgr.create_workspace_group( - f'C Fusion Testing {cls.id}', - region=random.choice(us_regions), - firewall_ranges=[], - ) - cls.workspace_groups.append(wg) + for letter in ('A', 'B', 'C'): + cls.workspace_groups.append( + mgr.create_workspace_group( + f'{letter} Fusion Testing {cls.id}', + region=random.choice(us_regions), + firewall_ranges=[], + expires_at=utils.DEPLOYMENT_EXPIRES_AT, + ), + ) @classmethod def tearDownClass(cls): - if not cls.dbexisted: - utils.drop_database(cls.dbname) + # Deployments first, and each one guarded. Dropping the database first + # meant a database error aborted the teardown before a single group was + # terminated; an unguarded loop meant a failure on the first group + # abandoned the other two. while cls.workspace_groups: - cls.workspace_groups.pop().terminate(force=True) + group = cls.workspace_groups.pop() + try: + group.terminate(force=True) + except Exception: + # Left to utils.cleanup_tracked, which retries and then reports + # it; raising here would replace the test's own failure. + pass + try: + if not cls.dbexisted: + utils.drop_database(cls.dbname) + except Exception: + pass def setUp(self): self.enabled = os.environ.get('SINGLESTOREDB_FUSION_ENABLED') @@ -1156,14 +1160,11 @@ def test_show_workspaces(self): f'"B Fusion Testing {self.id}" with size S-00', ) - # Wait for the three to be listed, not for them to be ACTIVE. Nothing + # Wait for the three to be listed, not for them to be ACTIVE: nothing # below asserts a state value -- 'State' is checked as a column name, - # never for its contents -- so all this test needs is that SHOW - # WORKSPACES can see them. Requiring ACTIVE cost around 450 seconds a - # run for no assertion, and at a 30 second interval most of that was - # overshoot. Polled through timing.sleep so a traced run accounts for - # it; a bare time.sleep here was invisible to the tracer and landed in - # the unlabelled 'other' bucket. + # never for its contents. Requiring ACTIVE cost around 450 seconds a run + # for no assertion. Polled through timing.sleep so a traced run accounts + # for it; a bare time.sleep landed in the unlabelled 'other' bucket. wanted = ('show-ws-1', 'show-ws-2', 'show-ws-3') deadline = time.time() + 600 while True: @@ -1396,20 +1397,19 @@ class _ClusterFusionMixin: """ Plumbing shared by the CLUSTER fusion suites. - These are the v2 mirror of :class:`TestWorkspaceFusion`, flat rather than - nested. A cluster is created in one statement where a workspace needed - two, so there is no group fixture and no ``IN GROUP`` clause anywhere. - Names are lowercase and hyphenated because ``POST /v2/clusters`` enforces - ``[a-z0-9]([a-z0-9-]*[a-z0-9])?`` at 1-32 characters (audit item 7) -- - the spaced names the v1 suite uses are rejected. - - This was one class deploying three clusters in ``setUpClass``, which every - test then waited out whether or not it touched a cluster: the two - lifecycle tests deploy their own and the region, project and grammar - tests need none at all, yet all of them paid for three. The classes below - declare what they need in :attr:`fixture_prefixes` instead, so the - cluster-less ones start immediately and no class deploys more than it - reads. + A cluster is created in one statement, so there is no group fixture and no + ``IN GROUP`` clause anywhere. Names are lowercase and hyphenated because + ``POST /v2/clusters`` enforces ``[a-z0-9]([a-z0-9-]*[a-z0-9])?`` at 1-32 + characters (audit item 7). + + Each class declares what it needs in :attr:`fixture_prefixes` rather than + the whole file sharing one three-cluster ``setUpClass``, which every test + used to wait out whether or not it touched a cluster. Only + :class:`TestClusterFusionSuspendResume` still names a prefix, being the one + class that needs a cluster up front *and* mutates it: + :class:`TestClusterFusion` reads without mutating and borrows from + ``utils.shared_clusters``, and the lifecycle suites create their own clusters + in the test bodies, those creates being the subject under test. Not a ``TestCase``, and named with a leading underscore: pytest collects any ``Test``-prefixed ``TestCase`` subclass it can reach, so a base that @@ -1481,6 +1481,7 @@ def setUpClass(cls): f'{prefix}-fusion-cluster-{cls.id}', region=region, size='S-00', + expires_at=utils.DEPLOYMENT_EXPIRES_AT, project=cls.project_id, wait_on_active=True, wait_timeout=1200, @@ -1489,8 +1490,9 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): - if not cls.dbexisted: - utils.drop_database(cls.dbname) + # Clusters before the database: a drop_database failure used to abort + # the teardown before anything was terminated, leaving three clusters + # to the sweep. while cls.clusters: cluster = cls.clusters.pop() try: @@ -1503,6 +1505,11 @@ def tearDownClass(cls): cluster.terminate(force=True) except Exception: pass + try: + if not cls.dbexisted: + utils.drop_database(cls.dbname) + except Exception: + pass def setUp(self): self.enabled = os.environ.get('SINGLESTOREDB_FUSION_ENABLED') @@ -1530,24 +1537,41 @@ def tearDown(self): @pytest.mark.management +@pytest.mark.xdist_group(utils.SHARED_CLUSTER_STAGE_GROUP) class TestClusterFusion(_ClusterFusionMixin, unittest.TestCase): """ - ``SHOW CLUSTERS`` against three deployed clusters. + ``SHOW CLUSTERS`` against the shared cluster pool. + + Borrows rather than deploying: nothing here mutates a cluster -- these are + four ``SHOW`` statements -- which is what ``utils.shared_clusters`` asks of a + consumer. ``SUSPEND``/``RESUME`` cannot borrow and deploys its own in + :class:`TestClusterFusionSuspendResume`. - Three of them so the ``LIKE``/``ORDER BY``/``LIMIT`` assertions have - something to sort. Nothing here mutates a cluster, which is what makes the - fixture shareable -- ``SUSPEND``/``RESUME`` cannot share it and deploys its - own in :class:`TestClusterFusionSuspendResume`. + Three of them, so the ``LIKE``/``ORDER BY``/``LIMIT`` assertions have + something to sort. In the Stage group rather than the Jobs one because Stage + already asks for two and the pool grows to the largest request: one extra + cluster there instead of three, and Jobs stays at one. + + Assertions are scoped to ``utils.shared_cluster_pattern()`` and counted + against ``utils.shared_cluster_names()``, never a literal, since the pool + grows to whatever the largest request in the process turns out to be. """ - fixture_prefixes = ('a', 'b', 'c') + #: Borrowed, so kept out of ``clusters``, which ``tearDownClass`` + #: terminates. A pool cluster must outlive the class that used it. + pool_clusters: List[Any] = [] + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.pool_clusters = utils.shared_clusters(3) def test_show_clusters(self): self.cur.execute('show clusters') names = [x[0] for x in self.cur.fetchall()] assert self.cur.description[0][0] == 'Name' - for prefix in ('a', 'b', 'c'): - assert f'{prefix}-fusion-cluster-{self.id}' in names, names + for cluster in type(self).pool_clusters: + assert cluster.name in names, names def test_show_clusters_columns(self): self.cur.execute('show clusters') @@ -1562,42 +1586,44 @@ def test_show_clusters_columns(self): 'TerminatedAt', ], cols + cluster = type(self).pool_clusters[0] rows = {x[0]: x for x in self.cur.fetchall()} - row = rows[f'a-fusion-cluster-{self.id}'] + row = rows[cluster.name] # Region is the provider slug; Cluster has no region object at v2. assert row[2], row assert row[5], row # ProjectName, not the ID: the column reports the name the project - # listing gives for the ID the cluster was deployed into. - project = type(self).manager.projects[type(self).project_id] - assert row[9] == project.name, row + # listing gives for the ID the cluster was deployed into. Read back from + # the cluster, not from this class's project_id, which the pool resolved + # independently. + expected = type(self).manager.get_cluster(cluster.id).project + assert row[9] == expected.name, row def test_show_clusters_like(self): - self.cur.execute(f'show clusters like "a-fusion-cluster-{self.id}"') + one = type(self).pool_clusters[0].name + self.cur.execute(f'show clusters like "{one}"') names = [x[0] for x in self.cur.fetchall()] - assert names == [f'a-fusion-cluster-{self.id}'], names + assert names == [one], names - self.cur.execute(f'show clusters like "%-fusion-cluster-{self.id}"') + self.cur.execute( + f'show clusters like "{utils.shared_cluster_pattern()}"', + ) names = [x[0] for x in self.cur.fetchall()] - assert len(names) == 3, names + assert sorted(names) == sorted(utils.shared_cluster_names()), names def test_show_clusters_order_by_and_limit(self): - self.cur.execute( - f'show clusters like "%-fusion-cluster-{self.id}" order by name', - ) + pattern = utils.shared_cluster_pattern() + + self.cur.execute(f'show clusters like "{pattern}" order by name') names = [x[0] for x in self.cur.fetchall()] assert names == sorted(names), names - self.cur.execute( - f'show clusters like "%-fusion-cluster-{self.id}" ' - 'order by name desc', - ) + self.cur.execute(f'show clusters like "{pattern}" order by name desc') names = [x[0] for x in self.cur.fetchall()] assert names == sorted(names, reverse=True), names self.cur.execute( - f'show clusters like "%-fusion-cluster-{self.id}" ' - 'order by name limit 2', + f'show clusters like "{pattern}" order by name limit 2', ) names = [x[0] for x in self.cur.fetchall()] assert len(names) == 2, names @@ -1833,16 +1859,31 @@ def test_create_cluster_without_project(self): 'this test is for', ) - with self.assertRaises(Exception): - self.cur.execute( - f'create cluster "{name}" in region "{region.region_name}"', - ) + try: + with self.assertRaises(Exception): + self.cur.execute( + f'create cluster "{name}" in region ' + f'"{region.region_name}"', + ) + finally: + # One listing, serving both purposes: the assertion that nothing was + # created, and the cleanup for when something was. The terminate + # below therefore fires only when the assertion is about to fail. + # + # Belt and braces: the handler goes through + # ClusterManager.create_cluster, which utils._CREATORS wraps, so a + # cluster created here is tracked and the sweep would find it + # anyway. This just means not waiting for the sweep. + live = [ + x for x in mgr.clusters + if x.name == name and x.terminated_at is None + ] + for cluster in live: + try: + utils.terminate(cluster) + except Exception: + pass - # Nothing should have been created - live = [ - x for x in mgr.clusters - if x.name == name and x.terminated_at is None - ] assert not live, live def test_create_cluster_named_project(self): @@ -1880,18 +1921,21 @@ def test_create_cluster_named_project(self): assert mgr.get_cluster(cluster_id).project.id == project.id finally: - # force=True: the cluster is still PENDING, having never been - # waited out, and a termination request is refused otherwise. + # utils.terminate, not a bare terminate(force=True): the cluster is + # PENDING, never having been waited out, and force does not make a + # pre-ACTIVE deployment deletable -- the API refuses it with a 400 or + # 409 either way. utils.terminate retries until it lands, where a + # single DELETE would be swallowed by the except below. if cluster_id is not None: try: - mgr.get_cluster(cluster_id).terminate(force=True) + utils.terminate(mgr.get_cluster(cluster_id)) except Exception: pass else: for cluster in mgr.clusters: if cluster.name == name and cluster.terminated_at is None: try: - cluster.terminate(force=True) + utils.terminate(cluster) except Exception: pass diff --git a/singlestoredb/tests/test_management_utils.py b/singlestoredb/tests/test_management_utils.py index b7b25f1e5..de9560239 100644 --- a/singlestoredb/tests/test_management_utils.py +++ b/singlestoredb/tests/test_management_utils.py @@ -8,8 +8,10 @@ only because that is where the bugs were found. """ import datetime +import json import os import pathlib +import shutil import tempfile import unittest from types import SimpleNamespace @@ -17,7 +19,11 @@ from unittest.mock import patch from singlestoredb.exceptions import ManagementError +from singlestoredb.management.utils import _normalize_datetime from singlestoredb.management.utils import normalize_remote_path +from singlestoredb.management.utils import to_datetime +from singlestoredb.management.utils import to_datetime_strict +from singlestoredb.tests.utils import admin_password from singlestoredb.tests.utils import counting_file_space from singlestoredb.tests.utils import counting_stage @@ -870,6 +876,200 @@ def test_a_transport_failure_names_the_route(self): self.assertIn('clusters/abc', msg) +class TestLockRetry(unittest.TestCase): + """ + ``manager.retry_on_lock``, the wait for a creation the organization lock + blocks. + + The API refuses the creation with "could not acquire lock" while another one + in the organization holds it, and nothing else retries that: POST is out of + ``RETRY_METHODS``. In a ``setUpClass`` one conflict fails every test in the + class -- a whole ``management_v1`` run went that way, and the v2 shared + cluster pool went the same way an hour later. + """ + + #: Verbatim from the two runs that failed. The ``/clusters`` one says + #: "error creating workspace", so the match cannot key on the noun. + LOCK_MESSAGES = ( + 'error creating workspace group (wg-test-8jtfylajdmax-vast7ln): ' + 'could not acquire lock within duration [0, 2026/09/23 16:45:04]', + 'error creating workspace (cl-test-shared-1-b0dad293): could not ' + 'acquire lock within duration [0, 2026-09-23T17:37:16Z]', + ) + + #: The default budget, per lock_retry_policy. + WAITS = [20.0, 40.0, 60.0, 60.0, 60.0] + + def setUp(self): + from singlestoredb.management import manager as manager_mod + self.manager_mod = manager_mod + self.slept = [] + + # The waits go through timing.sleep, so a trace reports them as waits. + patcher = patch( + 'singlestoredb.management.timing.time.sleep', self.slept.append, + ) + patcher.start() + self.addCleanup(patcher.stop) + + # Jitter off, so the waits asserted below are the spacing itself. + patcher = patch.object(manager_mod, 'LOCK_RETRY_JITTER', 0.0) + patcher.start() + self.addCleanup(patcher.stop) + + def _creator(self, *msgs, errno=500): + """A decorated creator that fails with these messages, then succeeds.""" + calls = [] + + class Mgr: + @self.manager_mod.retry_on_lock + def create(self, name, **kwargs): + calls.append(name) + if len(calls) <= len(msgs): + raise ManagementError(errno=errno, msg=msgs[len(calls) - 1]) + return f'deployment {name}' + + mgr = Mgr() + mgr.calls = calls + return mgr + + def test_a_lock_conflict_is_retried_until_it_succeeds(self): + mgr = self._creator(*self.LOCK_MESSAGES) + self.assertEqual(mgr.create('wg-test-a', region='x'), 'deployment wg-test-a') + self.assertEqual(len(mgr.calls), 3) + self.assertEqual(self.slept, [20.0, 40.0]) + + def test_the_retry_reuses_the_name(self): + """The conflict is a refusal to start, so nothing was created and there + is no deployment for the next attempt to collide with.""" + mgr = self._creator(self.LOCK_MESSAGES[0]) + mgr.create('wg-test-a') + self.assertEqual(mgr.calls, ['wg-test-a', 'wg-test-a']) + + def test_another_error_is_not_retried(self): + """A rejected request is a real failure; retrying only delays it.""" + mgr = self._creator('region is not available') + with self.assertRaises(ManagementError): + mgr.create('wg-test-a') + self.assertEqual(len(mgr.calls), 1) + self.assertEqual(self.slept, []) + + def test_an_unrelated_500_is_not_retried(self): + """500 is also what a name collision comes back as, so the message is + the only thing that says nothing was created.""" + mgr = self._creator('error creating workspace group (x): already exists') + with self.assertRaises(ManagementError): + mgr.create('wg-test-a') + self.assertEqual(len(mgr.calls), 1) + + def test_the_budget_is_bounded_and_the_error_is_raised(self): + """A stuck organization has to fail rather than idle out the job.""" + mgr = self._creator(*([self.LOCK_MESSAGES[0]] * 100)) + with self.assertRaises(ManagementError) as cm: + mgr.create('wg-test-a') + self.assertIn('acquire lock', str(cm.exception)) + self.assertEqual(len(mgr.calls), len(self.WAITS) + 1) + self.assertEqual(len(self.slept), len(self.WAITS)) + + def test_the_wait_is_capped(self): + mgr = self._creator(*([self.LOCK_MESSAGES[0]] * 100)) + with self.assertRaises(ManagementError): + mgr.create('wg-test-a') + self.assertLessEqual( + max(self.slept), self.manager_mod.LOCK_RETRY_MAX_INTERVAL, + ) + self.assertEqual(self.slept, self.WAITS) + + def test_the_waits_are_jittered(self): + """Two clients that collide back off by the same amounts from the same + moment, so without jitter they retry in step forever.""" + patcher = patch.object(self.manager_mod, 'LOCK_RETRY_JITTER', 5.0) + patcher.start() + self.addCleanup(patcher.stop) + + mgr = self._creator(*([self.LOCK_MESSAGES[0]] * 100)) + with self.assertRaises(ManagementError): + mgr.create('wg-test-a') + self.assertNotEqual(self.slept, self.WAITS) + for wait, base in zip(self.slept, self.WAITS): + self.assertGreaterEqual(wait, base) + self.assertLess(wait, base + 5.0) + + def test_both_observed_wordings_match(self): + for msg in self.LOCK_MESSAGES: + mgr = self._creator(msg) + mgr.create('wg-test-a') + self.assertEqual(len(mgr.calls), 2, msg) + + def test_the_match_does_not_depend_on_the_status(self): + """The status a conflict arrives as is not documented, and it cannot + tell a conflict from a rejection either way.""" + mgr = self._creator(self.LOCK_MESSAGES[0], errno=409) + mgr.create('wg-test-a') + self.assertEqual(len(mgr.calls), 2) + + def test_a_success_is_not_delayed(self): + mgr = self._creator() + mgr.create('wg-test-a') + self.assertEqual(len(mgr.calls), 1) + self.assertEqual(self.slept, []) + + def test_the_retry_can_be_turned_off(self): + mgr = self._creator(self.LOCK_MESSAGES[0]) + with patch.dict( + os.environ, {'SINGLESTOREDB_MANAGEMENT_LOCK_RETRIES': '0'}, + ): + with self.assertRaises(ManagementError): + mgr.create('wg-test-a') + self.assertEqual(len(mgr.calls), 1) + self.assertEqual(self.slept, []) + + def test_the_budget_is_configurable(self): + mgr = self._creator(*([self.LOCK_MESSAGES[0]] * 100)) + with patch.dict( + os.environ, { + 'SINGLESTOREDB_MANAGEMENT_LOCK_RETRIES': '2', + 'SINGLESTOREDB_MANAGEMENT_LOCK_RETRY_INTERVAL': '3', + }, + ): + with self.assertRaises(ManagementError): + mgr.create('wg-test-a') + self.assertEqual(len(mgr.calls), 3) + self.assertEqual(self.slept, [3.0, 6.0]) + + +class TestLockRetryIsApplied(unittest.TestCase): + """Which creations wear ``retry_on_lock``: the two that take the lock.""" + + def _wears_it(self, method): + return getattr(method, '__retry_on_lock__', False) + + def test_the_two_creations_that_take_the_lock(self): + from singlestoredb.management.v1.workspace import WorkspaceManager + from singlestoredb.management.v2.cluster import ClusterManager + + self.assertTrue(self._wears_it(WorkspaceManager.create_workspace_group)) + self.assertTrue(self._wears_it(ClusterManager.create_cluster)) + + def test_and_nothing_else(self): + """Deliberately narrow: a workspace inside an existing group and the + starter deployments do not contend for this lock.""" + from singlestoredb.management.v1.workspace import WorkspaceGroup + from singlestoredb.management.v1.workspace import WorkspaceManager + from singlestoredb.management.v2.cluster import ClusterManager + + for klass, name in ( + (WorkspaceManager, 'create_workspace'), + (WorkspaceManager, 'create_starter_workspace'), + (WorkspaceGroup, 'create_workspace'), + (ClusterManager, 'create_starter_cluster'), + ): + self.assertFalse( + self._wears_it(getattr(klass, name)), + f'{klass.__name__}.{name}', + ) + + class TestWaitOnEndpoint(unittest.TestCase): """ ``Manager._wait_on_endpoint`` polls a new deployment by connecting to it. @@ -949,8 +1149,18 @@ def _restore(self): self.utils._in_flight.clear() self.utils._in_flight.extend(self.saved_in_flight) - def _deployment(self, name, terminated_at=None, state='ACTIVE'): - """A stand-in that is not a Mock, so tracking does not skip it.""" + def _deployment( + self, name, terminated_at=None, state='ACTIVE', classname=None, + ): + """ + A stand-in that is not a Mock, so tracking does not skip it. + + ``classname`` renames the class, which is how the ledger decides a + kind (``utils._KIND_BY_CLASS`` is keyed by class name). The default + ``Deployment`` is deliberately *not* a ledger kind, so the tests that + only care about tracking write no ledger records even when one is + configured. + """ class Deployment: def __init__(self): self.name = name @@ -966,6 +1176,8 @@ def refresh(self): def terminate(self, force=False): self.terminated_with = force + if classname: + Deployment.__name__ = classname return Deployment() def test_mocked_deployments_are_not_tracked(self): @@ -1071,7 +1283,7 @@ def create_then_fail_waiting(recv, name, **kwargs): raise ManagementError(msg=f'Exceeded waiting time for {name}') wrapped = self.utils._tracking_wrapper( - create_then_fail_waiting, lambda recv: recv.clusters, + create_then_fail_waiting, 'cluster', lambda recv: recv.clusters, ) with self.assertRaises(ManagementError): wrapped(receiver, 'cl-test-shared-0-abc', wait_on_active=True) @@ -1096,7 +1308,7 @@ def interrupted(recv, name, **kwargs): raise KeyboardInterrupt wrapped = self.utils._tracking_wrapper( - interrupted, lambda recv: recv.clusters, + interrupted, 'cluster', lambda recv: recv.clusters, ) with self.assertRaises(KeyboardInterrupt): wrapped(receiver, 'cl-1') @@ -1118,7 +1330,7 @@ def test_a_mocked_receiver_does_not_track_what_it_returns(self): for value in (returned, 'sentinel'): wrapped = self.utils._tracking_wrapper( lambda recv, name, value=value, **kwargs: value, - lambda recv: [], + 'cluster', lambda recv: [], ) self.assertIs(wrapped(mgr, 'my-cluster'), value) @@ -1133,7 +1345,7 @@ def test_a_real_receiver_still_tracks_what_it_returns(self): returned._manager = None wrapped = self.utils._tracking_wrapper( - lambda recv, name, **kwargs: returned, lambda recv: [], + lambda recv, name, **kwargs: returned, 'cluster', lambda recv: [], ) wrapped(mgr, 'cl-1') self.assertEqual(self.utils.tracked_labels(), ["Deployment 'cl-1'"]) @@ -1144,7 +1356,9 @@ def test_a_mocked_receiver_is_not_searched_for_orphans(self): def boom(recv, name, **kwargs): raise ManagementError(msg='boom') - wrapped = self.utils._tracking_wrapper(boom, lambda recv: recv.clusters) + wrapped = self.utils._tracking_wrapper( + boom, 'cluster', lambda recv: recv.clusters, + ) with self.assertRaises(ManagementError): wrapped(MagicMock(), 'cl-1') self.assertEqual(self.utils._tracked, []) @@ -1162,7 +1376,7 @@ def boom(recv, name, **kwargs): def finder(recv): raise AssertionError('recovery called the live API') - wrapped = self.utils._tracking_wrapper(boom, finder) + wrapped = self.utils._tracking_wrapper(boom, 'cluster', finder) with self.assertRaises(ManagementError): wrapped(receiver, 'cl-1') self.assertEqual(self.utils._tracked, []) @@ -1184,7 +1398,7 @@ def create_then_wait(recv, name, **kwargs): raise AssertionError('the process would have been killed here') wrapped = self.utils._tracking_wrapper( - create_then_wait, lambda recv: recv.clusters, + create_then_wait, 'cluster', lambda recv: recv.clusters, ) with self.assertRaises(AssertionError): wrapped(receiver, 'cl-1', wait_on_active=True) @@ -1205,7 +1419,7 @@ def finder(recv): made = self._deployment('cl-1') wrapped = self.utils._tracking_wrapper( - lambda recv, name, **kwargs: made, finder, + lambda recv, name, **kwargs: made, 'cluster', finder, ) self.assertIs(wrapped(receiver, 'cl-1'), made) self.assertEqual(self.utils._in_flight, []) @@ -1216,7 +1430,9 @@ def boom(recv, name, **kwargs): receiver.clusters = [self._deployment('cl-2')] with self.assertRaises(ManagementError): - self.utils._tracking_wrapper(boom, finder)(receiver, 'cl-2') + self.utils._tracking_wrapper( + boom, 'cluster', finder, + )(receiver, 'cl-2') self.assertEqual(self.utils._in_flight, []) # The orphan was recovered once, not once per code path. self.assertEqual(len(self.utils._tracked), 2) @@ -1226,7 +1442,7 @@ def create(recv, name, **kwargs): raise AssertionError(str(self.utils._in_flight)) wrapped = self.utils._tracking_wrapper( - create, lambda recv: recv.clusters, + create, 'cluster', lambda recv: recv.clusters, ) with self.assertRaises(AssertionError) as raised: wrapped(MagicMock(), 'cl-1') @@ -1296,7 +1512,7 @@ def test_every_creation_method_is_wrapped(self): import importlib self.utils.install_deployment_tracking() - for module_name, class_name, method_name, _ in self.utils._CREATORS: + for module_name, class_name, method_name, _, _ in self.utils._CREATORS: klass = getattr(importlib.import_module(module_name), class_name) method = getattr(klass, method_name, None) self.assertIsNotNone( @@ -1313,7 +1529,9 @@ def test_every_creator_takes_name_first_and_has_a_finder(self): import importlib import inspect - for module_name, class_name, method_name, finder in \ + from singlestoredb.tests import cleanup_deployments + + for module_name, class_name, method_name, kind, finder in \ self.utils._CREATORS: klass = getattr(importlib.import_module(module_name), class_name) method = getattr(klass, method_name) @@ -1328,6 +1546,590 @@ def test_every_creator_takes_name_first_and_has_a_finder(self): 'a failed create would not be recoverable', ) self.assertTrue(callable(finder)) + # The ledger's `pending` record carries this kind, and the reaper + # resolves it through cleanup_deployments.LEDGER_KINDS. A kind + # neither side knows would make a cancelled create unreapable, + # which is the whole point of the ledger. + self.assertIn( + kind, set(self.utils._KIND_BY_CLASS.values()), + f'{class_name}.{method_name} has an unknown ledger kind', + ) + self.assertIn(kind, cleanup_deployments.LEDGER_KINDS) + + +class TestDeploymentLedger(TestDeploymentTracking): + """ + The on-disk ledger that makes a killed run's deployments reapable. + + Inherits ``TestDeploymentTracking``'s fixtures for the module globals and + the non-Mock deployment stand-in. It re-runs that class's tests with a + ledger configured, which is worth having: those tests all use the default + ``Deployment`` classname, so they also pin that a ledger being configured + changes nothing about the in-memory behaviour. + """ + + def setUp(self): + super().setUp() + self.dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.dir, True) + self.ledger = os.path.join(self.dir, 'deployments.jsonl') + patcher = patch.dict( + os.environ, {self.utils.LEDGER_ENV_VAR: self.ledger}, + ) + patcher.start() + self.addCleanup(patcher.stop) + + def records(self): + """Every record in the ledger, in the order it was written.""" + if not os.path.exists(self.ledger): + return [] + with open(self.ledger) as file: + return [json.loads(x) for x in file if x.strip()] + + def events(self): + return [(x['event'], x.get('kind'), x.get('name')) for x in + self.records()] + + # + # Writing + # + + def test_no_ledger_is_written_without_the_environment_variable(self): + """Opt-in is the whole contract: a local run must behave exactly as it + did before, with no file appearing anywhere.""" + with patch.dict(os.environ, {}, clear=False): + del os.environ[self.utils.LEDGER_ENV_VAR] + self.utils.track(self._deployment('cl-1', classname='Cluster')) + self.assertFalse(os.path.exists(self.ledger)) + + def test_a_mocked_creation_writes_nothing(self): + """The unit tests drive the creators with patched transports. Recording + those would have the reaper chasing ids that never existed, and -- worse + -- exit non-zero on every one it could not resolve.""" + wrapped = self.utils._tracking_wrapper( + lambda recv, name, **kwargs: MagicMock(), + 'cluster', lambda recv: [], + ) + wrapped(MagicMock(), 'cl-1') + self.utils.track(MagicMock()) + self.assertEqual(self.records(), []) + + def test_a_real_creation_writes_pending_then_live(self): + """In that order, and with the pending written before the creator is + even called: the window this closes is the one where the POST has landed + and nothing in the process knows an id yet.""" + made = self._deployment('cl-1', classname='Cluster') + seen = [] + + def create(recv, name, **kwargs): + # What the ledger holds *during* the wait, which is where the + # cancelled job died. + seen.extend(self.events()) + return made + + receiver = SimpleNamespace( + _get=object(), _post=object(), _delete=object(), + ) + wrapped = self.utils._tracking_wrapper( + create, 'cluster', lambda recv: [], + ) + wrapped(receiver, 'cl-1') + + self.assertEqual(seen, [('pending', 'cluster', 'cl-1')]) + self.assertEqual( + self.events(), [ + ('pending', 'cluster', 'cl-1'), + ('live', 'cluster', 'cl-1'), + ], + ) + self.assertEqual(self.records()[1]['id'], 'cl-1') + + def test_the_pending_name_comes_from_the_keyword_too(self): + receiver = SimpleNamespace( + _get=object(), _post=object(), _delete=object(), + ) + self.utils._tracking_wrapper( + lambda recv, name, **kwargs: None, 'workspace_group', + lambda recv: [], + )(receiver, name='wg-1') + self.assertEqual( + self.events(), [('pending', 'workspace_group', 'wg-1')], + ) + + def test_a_create_that_dies_mid_wait_leaves_pending_with_no_gone(self): + """The reported failure, as the ledger sees it. The creator raises and + the orphan is not in the listing yet, so nothing else is ever written -- + and that lone `pending` is what the reaper resolves by name.""" + receiver = SimpleNamespace( + _get=object(), _post=object(), _delete=object(), clusters=[], + ) + + def create_then_fail_waiting(recv, name, **kwargs): + raise ManagementError(msg=f'Exceeded waiting time for {name}') + + wrapped = self.utils._tracking_wrapper( + create_then_fail_waiting, 'cluster', lambda recv: recv.clusters, + ) + with self.assertRaises(ManagementError): + wrapped(receiver, 'a-fusion-cluster-1f2e', wait_on_active=True) + + self.assertEqual( + self.events(), + [('pending', 'cluster', 'a-fusion-cluster-1f2e')], + ) + + def test_a_recovered_orphan_is_recorded_live(self): + """``_recover_orphan`` goes through ``track()``, so the id it digs out + of the listing reaches the ledger and the reaper can use the point + lookup instead of searching by name.""" + orphan = self._deployment('cl-1', classname='Cluster') + receiver = SimpleNamespace( + _get=object(), _post=object(), _delete=object(), + clusters=[orphan], + ) + + def boom(recv, name, **kwargs): + raise ManagementError(msg='boom') + + with self.assertRaises(ManagementError): + self.utils._tracking_wrapper( + boom, 'cluster', lambda recv: recv.clusters, + )(receiver, 'cl-1') + + self.assertEqual( + self.events(), [ + ('pending', 'cluster', 'cl-1'), + ('live', 'cluster', 'cl-1'), + ], + ) + + def test_a_successful_sweep_appends_gone(self): + obj = self._deployment('cl-1', classname='Cluster') + self.utils.track(obj) + self.assertEqual(len(self.utils.cleanup_tracked()), 1) + self.assertEqual( + self.events(), [ + ('live', 'cluster', 'cl-1'), + ('gone', 'cluster', 'cl-1'), + ], + ) + + def test_a_deployment_already_gone_is_recorded_gone(self): + """A test that terminated in its own teardown: the sweep finds it gone + rather than terminating it, and the record still has to be closed or + the reaper spends a lookup on it and reports it unresolved.""" + obj = self._deployment( + 'cl-1', terminated_at='now', classname='Cluster', + ) + self.utils.track(obj) + self.assertEqual(self.utils.cleanup_tracked(), []) + self.assertEqual( + [x['event'] for x in self.records()], ['live', 'gone'], + ) + + def test_a_failed_terminate_writes_no_gone(self): + """The deployment is still live and still billing, so the reaper must + still see it.""" + obj = self._deployment('cl-1', classname='Cluster') + + def boom(force=False): + raise ManagementError(errno=500, msg='boom') + + obj.terminate = boom + self.utils.track(obj) + self.assertEqual(self.utils.cleanup_tracked(), []) + self.assertEqual([x['event'] for x in self.records()], ['live']) + + def test_untrack_records_gone_only_for_something_tracked(self): + obj = self._deployment('cl-1', classname='Cluster') + self.utils.untrack(obj) + self.assertEqual(self.records(), []) + + self.utils.track(obj) + self.utils.untrack(obj) + self.assertEqual([x['event'] for x in self.records()], ['live', 'gone']) + + def test_a_write_failure_is_logged_and_not_raised(self): + """This sits on the creation path of every management test: an + unwritable ledger must cost a warning, not a failed test run.""" + with patch.dict( + os.environ, + {self.utils.LEDGER_ENV_VAR: os.path.join(self.dir, 'no', 'such')}, + ): + with self.assertLogs(self.utils.logger, 'WARNING') as logs: + self.utils.track(self._deployment('cl-1', classname='Cluster')) + self.assertIn('deployment ledger', logs.output[0]) + + # + # Folding + # + + def fold(self, *lines): + from singlestoredb.tests import cleanup_deployments + return cleanup_deployments.fold_ledger(lines) + + def test_folding_keeps_only_what_is_not_gone(self): + kept = self.fold( + json.dumps(dict(event='pending', kind='cluster', name='cl-1')), + json.dumps( + dict(event='live', kind='cluster', name='cl-1', id='id-1'), + ), + json.dumps( + dict(event='gone', kind='cluster', name='cl-1', id='id-1'), + ), + # Created and never terminated. + json.dumps(dict(event='pending', kind='cluster', name='cl-2')), + json.dumps( + dict(event='live', kind='cluster', name='cl-2', id='id-2'), + ), + # Interrupted before it returned: pending only. + json.dumps(dict(event='pending', kind='cluster', name='cl-3')), + ) + self.assertEqual( + [(x['event'], x.get('id'), x['name']) for x in kept], + [('live', 'id-2', 'cl-2'), ('pending', None, 'cl-3')], + ) + + def test_a_live_record_retires_its_pending(self): + """Otherwise the reaper resolves the same cluster twice -- once by id + and once by name -- and reports two.""" + kept = self.fold( + json.dumps(dict(event='pending', kind='cluster', name='cl-1')), + json.dumps( + dict(event='live', kind='cluster', name='cl-1', id='id-1'), + ), + ) + self.assertEqual(len(kept), 1) + self.assertEqual(kept[0]['id'], 'id-1') + + def test_gone_cancels_a_pending_that_never_went_live(self): + kept = self.fold( + json.dumps(dict(event='pending', kind='cluster', name='cl-1')), + json.dumps(dict(event='gone', kind='cluster', name='cl-1')), + ) + self.assertEqual(kept, []) + + def test_the_same_name_in_two_kinds_is_two_deployments(self): + """`cl-test-abc` as a cluster and as a workspace are different things, + and a `gone` for one must not clear the other.""" + kept = self.fold( + json.dumps(dict(event='pending', kind='cluster', name='x')), + json.dumps(dict(event='pending', kind='workspace', name='x')), + json.dumps(dict(event='gone', kind='cluster', name='x')), + ) + self.assertEqual([x['kind'] for x in kept], ['workspace']) + + def test_a_malformed_line_is_skipped_rather_than_fatal(self): + """A truncated last line -- a process killed between the write and the + fsync -- must not cost the reaper every other record.""" + kept = self.fold( + json.dumps(dict(event='pending', kind='cluster', name='cl-1')), + '{"event": "pending", "kin', + '', + '[]', + ) + self.assertEqual([x['name'] for x in kept], ['cl-1']) + + def test_reading_reverses_into_newest_first(self): + """A workspace has to be terminated before the group that holds it, the + same ordering ``cleanup_tracked`` uses.""" + from singlestoredb.tests import cleanup_deployments + with open(self.ledger, 'w') as file: + for kind, name in ( + ('workspace_group', 'wg-1'), ('workspace', 'ws-1'), + ): + file.write( + json.dumps(dict(event='pending', kind=kind, name=name)) + + '\n', + ) + self.assertEqual( + [x['name'] for x in cleanup_deployments.read_ledger(self.ledger)], + ['ws-1', 'wg-1'], + ) + + # + # Resolving, with the management API stubbed out + # + + def stub_managers(self, **attrs): + """Patch the reaper's manager lookup with a namespace.""" + from singlestoredb.tests import cleanup_deployments + mgr = SimpleNamespace(**attrs) + patcher = patch.object( + cleanup_deployments, '_manager', lambda version: mgr, + ) + patcher.start() + self.addCleanup(patcher.stop) + return cleanup_deployments, mgr + + def write_ledger(self, *records): + with open(self.ledger, 'w') as file: + for record in records: + file.write(json.dumps(record) + '\n') + + def test_an_id_that_404s_is_treated_as_already_gone(self): + """The common case by far: the ledger records every creation, and a run + that ended normally terminated all of them. A clean sweep must exit 0 + and terminate nothing.""" + def get_cluster(ident): + raise ManagementError(errno=404, msg='not found') + + mod, _ = self.stub_managers(get_cluster=get_cluster) + self.write_ledger( + dict(event='live', kind='cluster', name='cl-1', id='id-1'), + ) + found, gone, unresolved = mod.find_ledger_leftovers(self.ledger) + self.assertEqual(found, []) + self.assertEqual(len(gone), 1) + self.assertEqual(unresolved, []) + self.assertEqual(mod.main(['--ledger', self.ledger, '--yes']), 0) + + def test_a_live_id_is_resolved_and_terminated(self): + obj = self._deployment('cl-1', classname='Cluster') + mod, _ = self.stub_managers(get_cluster=lambda ident: obj) + self.write_ledger( + dict(event='live', kind='cluster', name='cl-1', id='id-1'), + ) + self.assertEqual(mod.main(['--ledger', self.ledger, '--yes']), 0) + self.assertTrue(obj.terminated_with) + + def test_a_dry_run_terminates_nothing(self): + obj = self._deployment('cl-1', classname='Cluster') + mod, _ = self.stub_managers(get_cluster=lambda ident: obj) + self.write_ledger( + dict(event='live', kind='cluster', name='cl-1', id='id-1'), + ) + self.assertEqual(mod.main(['--ledger', self.ledger]), 0) + self.assertIsNone(obj.terminated_with) + + def test_a_pending_record_is_resolved_by_name_over_the_listing(self): + """No id was ever returned, so the listing is the only handle -- the + same match ``_recover_orphan`` makes, and the case a cancelled + ``wait_on_active`` leaves.""" + wanted = self._deployment('a-fusion-cluster-1f2e', classname='Cluster') + other = self._deployment('someone-elses', classname='Cluster') + mod, _ = self.stub_managers(clusters=[other, wanted]) + self.write_ledger( + dict(event='pending', kind='cluster', name='a-fusion-cluster-1f2e'), + ) + self.assertEqual(mod.main(['--ledger', self.ledger, '--yes']), 0) + self.assertTrue(wanted.terminated_with) + self.assertIsNone(other.terminated_with) + + def test_a_pending_name_absent_from_the_listing_is_gone(self): + """The POST never landed, so there is nothing to reap and nothing to + complain about.""" + mod, _ = self.stub_managers(clusters=[]) + self.write_ledger(dict(event='pending', kind='cluster', name='cl-1')) + found, gone, unresolved = mod.find_ledger_leftovers(self.ledger) + self.assertEqual((found, len(gone), unresolved), ([], 1, [])) + + def test_an_already_terminated_deployment_is_not_terminated_again(self): + obj = self._deployment( + 'cl-1', terminated_at='now', classname='Cluster', + ) + mod, _ = self.stub_managers(get_cluster=lambda ident: obj) + self.write_ledger( + dict(event='live', kind='cluster', name='cl-1', id='id-1'), + ) + self.assertEqual(mod.main(['--ledger', self.ledger, '--yes']), 0) + self.assertIsNone(obj.terminated_with) + + def test_a_lookup_failure_that_is_not_a_404_exits_non_zero(self): + """"Could not tell" and "not there" must not read the same when the + difference is a cluster billing.""" + def get_cluster(ident): + raise ManagementError(errno=500, msg='gateway sulked') + + mod, _ = self.stub_managers(get_cluster=get_cluster) + self.write_ledger( + dict(event='live', kind='cluster', name='cl-1', id='id-1'), + ) + found, gone, unresolved = mod.find_ledger_leftovers(self.ledger) + self.assertEqual((found, gone), ([], [])) + self.assertEqual(len(unresolved), 1) + self.assertEqual(mod.main(['--ledger', self.ledger, '--yes']), 1) + + def test_an_unknown_kind_is_reported_rather_than_skipped(self): + mod, _ = self.stub_managers() + self.write_ledger(dict(event='live', kind='mystery', name='x', id='1')) + _, _, unresolved = mod.find_ledger_leftovers(self.ledger) + self.assertEqual(len(unresolved), 1) + self.assertIn('unknown kind', unresolved[0]) + + def test_a_missing_ledger_is_not_an_error(self): + """The variable is set for a whole job, including steps whose tests + create nothing. Failing there would turn those runs red.""" + from singlestoredb.tests import cleanup_deployments + missing = os.path.join(self.dir, 'never-written.jsonl') + self.assertEqual(cleanup_deployments.read_ledger(missing), []) + self.assertEqual( + cleanup_deployments.main(['--ledger', missing, '--yes']), 0, + ) + + def test_the_sweep_waits_out_a_provision_rather_than_the_class_budget(self): + """The whole point of the ledger is a job cancelled inside + ``wait_on_active``, whose cluster is minutes from deletable. Borrowing + ``utils.TERMINATE_RETRY_TIMEOUT`` -- short so the per-class sweep cannot + stall the suite -- would exhaust the budget and leave it billing, and + nothing runs after this to try again.""" + from singlestoredb.tests import utils + obj = self._deployment('cl-1', classname='Cluster') + mod, _ = self.stub_managers(get_cluster=lambda ident: obj) + self.write_ledger( + dict(event='live', kind='cluster', name='cl-1', id='id-1'), + ) + + calls = [] + patcher = patch.object( + utils, 'terminate', + lambda obj, **kwargs: calls.append(kwargs), + ) + patcher.start() + self.addCleanup(patcher.stop) + + self.assertEqual(mod.main(['--ledger', self.ledger, '--yes']), 0) + self.assertEqual(len(calls), 1) + self.assertEqual(calls[0]['timeout'], mod.TERMINATE_TIMEOUT) + self.assertGreater(calls[0]['timeout'], utils.TERMINATE_RETRY_TIMEOUT) + + def test_ledger_mode_refuses_the_guards_it_replaces(self): + """Silently ignoring --older-than would read as a safety guard that is + not there.""" + from singlestoredb.tests import cleanup_deployments + for extra in ( + ['--older-than', '0'], ['--any-name'], + ['--kind', 'cluster'], ['--show-unmatched'], + ): + with self.assertRaises(SystemExit): + cleanup_deployments.main( + ['--ledger', self.ledger] + extra, + ) + + +class TestTerminateRetry(unittest.TestCase): + """ + ``utils.terminate()``'s bounded retry for a deployment the API will not + delete yet. + + A deployment killed mid-provision is PENDING/TRANSITIONING and the DELETE + comes back 400 or 409. Nothing retried that: Manager.RETRY_STATUSES is + {429, 500, 502, 503, 504}, so the per-class sweep warned, the session-end + sweep tried once more and the cluster stayed up. + """ + + def setUp(self): + from singlestoredb.tests import utils + self.utils = utils + self.slept = [] + # A fake clock, not just a stubbed sleep: the retry budget is measured + # with time.monotonic(), so a sleep that does not advance it makes the + # deadline unreachable and the loop only ends when the stub runs out of + # refusals. That is the opposite of what the budget test asserts. + self.now = 0.0 + + def sleep(seconds): + self.slept.append(seconds) + self.now += seconds + + for name, value in ( + ('sleep', sleep), ('monotonic', lambda: self.now), + ): + patcher = patch(f'time.{name}', value) + patcher.start() + self.addCleanup(patcher.stop) + + def _refuser(self, *errnos): + """A deployment whose terminate raises these in turn, then succeeds.""" + class Deployment: + attempts = 0 + terminated_with = None + + def terminate(inner, force=False): + inner.attempts += 1 + if inner.attempts <= len(errnos): + raise ManagementError( + errno=errnos[inner.attempts - 1], + msg='still provisioning', + ) + inner.terminated_with = force + + return Deployment() + + def test_a_400_is_retried_until_it_succeeds(self): + obj = self._refuser(400, 409) + self.utils.terminate(obj) + self.assertEqual(obj.attempts, 3) + self.assertTrue(obj.terminated_with) + self.assertEqual(self.slept, [15.0, 15.0]) + + def test_a_404_is_not_retried(self): + """It is already gone; retrying would burn the whole budget waiting for + something that is not coming back.""" + obj = self._refuser(404) + with self.assertRaises(ManagementError): + self.utils.terminate(obj) + self.assertEqual(obj.attempts, 1) + self.assertEqual(self.slept, []) + + def test_a_5xx_is_not_retried_here(self): + """The transport already retried it; another round trip from this layer + is not what fixes it.""" + obj = self._refuser(503) + with self.assertRaises(ManagementError): + self.utils.terminate(obj) + self.assertEqual(obj.attempts, 1) + + def test_the_budget_is_bounded_and_the_error_is_re_raised(self): + """Raising is what keeps the deployment in ``_tracked``, so the + end-of-session sweep gets another go at it.""" + obj = self._refuser(*([409] * 100)) + with self.assertRaises(ManagementError): + self.utils.terminate(obj, timeout=45.0, interval=15.0) + self.assertEqual(obj.attempts, 3) + self.assertEqual(self.slept, [15.0, 15.0]) + + def test_a_starter_kind_is_terminated_without_force(self): + """StarterWorkspace.terminate / StarterCluster.terminate take no + arguments at all.""" + class Starter: + called = False + + def terminate(inner): + inner.called = True + + obj = Starter() + self.utils.terminate(obj) + self.assertTrue(obj.called) + + def test_force_is_passed_when_the_signature_accepts_it(self): + """``force`` is what makes a workspace group with live workspaces in it + go away, so this is not cosmetic.""" + seen = [] + + class Group: + def terminate(inner, force=False): + seen.append(force) + + self.utils.terminate(Group()) + self.assertEqual(seen, [True]) + + def test_a_type_error_from_inside_terminate_is_not_a_second_delete(self): + """The signature is inspected rather than discovered by catching + TypeError from the call. The old ``except TypeError`` also caught one + raised *inside* a terminate that did accept force, and retried without + it -- two DELETEs, the second unforced, which is exactly the shape that + leaves a workspace group behind.""" + calls = [] + + class Group: + def terminate(inner, force=False): + calls.append(force) + raise TypeError('something inside went wrong') + + with self.assertRaises(TypeError): + self.utils.terminate(Group()) + self.assertEqual(calls, [True]) class TestSharedClusterPool(unittest.TestCase): @@ -1340,6 +2142,21 @@ def setUp(self): from singlestoredb.tests import utils self.utils = utils + # Redirected before anything can create a cluster: the stand-in + # manager's create_cluster calls the real utils.track, which ledgers, + # and _pool_id is the live one, so under CI these mocked units used to + # append `id-of-cl-test-shared-N-` to the job's real + # ledger. The cleanup step then could not resolve those ids and exited + # non-zero on every run, burying any genuine unresolved record. + tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmp, True) + patcher = patch.dict( + os.environ, + {utils.LEDGER_ENV_VAR: os.path.join(tmp, 'deployments.jsonl')}, + ) + patcher.start() + self.addCleanup(patcher.stop) + self.saved_pool = list(utils._pool) self.saved_skip = utils._pool_skip self.saved_tracked = list(utils._tracked) @@ -1424,6 +2241,10 @@ def test_the_pool_is_built_once(self): self.assertEqual([x.id for x in first], [x.id for x in second]) self.assertEqual(len(self.created), 2) + # A lock conflict during the pool build is waited out by the + # @retry_on_lock on create_cluster, which a stand-in manager does not + # have: see TestLockRetry. + def test_the_pool_grows_to_the_largest_request(self): with self._patched(): one = self.utils.shared_clusters(1) @@ -1504,6 +2325,53 @@ def test_an_explicit_project_does_not_need_a_standard_one(self): self.assertEqual(self.created[0][2]['project'], 'chosen-project') + def test_pool_clusters_are_given_an_expiry(self): + # The only cleanup that survives the process being killed, so it has to + # be on the POST rather than left to the sweep. + with self._patched(): + self.utils.shared_clusters(2) + + self.assertEqual( + [x[2].get('expires_at') for x in self.created], + [self.utils.DEPLOYMENT_EXPIRES_AT] * 2, + ) + + def test_the_pattern_matches_the_pool_and_is_scoped_to_this_process(self): + with self._patched(): + self.utils.shared_clusters(2) + + pattern = self.utils.shared_cluster_pattern() + prefix, _, suffix = pattern.partition('%') + + # A LIKE pattern, so assert it the way the server would read it: + # every pool name matches, and the suffix is the per-process id that + # keeps another run's pool from matching. + for name in self.utils.shared_cluster_names(): + self.assertTrue(name.startswith(prefix), (name, pattern)) + self.assertTrue(name.endswith(suffix), (name, pattern)) + + self.assertEqual(suffix, f'-{self.utils._pool_id}') + + # And another process's pool does not: same prefix, different id. + other = f'cl-test-shared-0-{"f" * 8}' + self.assertTrue(other.startswith(prefix), (other, pattern)) + self.assertFalse(other.endswith(suffix), (other, pattern)) + + def test_the_names_follow_the_pool_as_it_grows(self): + # Read at assertion time rather than cached, so a class that asks for + # more clusters later cannot leave an exact-count expectation stale. + with self._patched(): + self.utils.shared_clusters(1) + self.assertEqual(len(self.utils.shared_cluster_names()), 1) + + self.utils.shared_clusters(3) + self.assertEqual(len(self.utils.shared_cluster_names()), 3) + + self.assertEqual( + self.utils.shared_cluster_names(), + [x[0] for x in self.created], + ) + class TestClearStage(unittest.TestCase): """ @@ -1676,6 +2544,14 @@ def test_the_default_spares_anything_a_run_could_still_own(self): # Not zero: a default that swept every match would make running this # during a test run destructive. self.assertGreaterEqual(self.mod.DEFAULT_MIN_AGE_HOURS, 1) + # Nothing runs after this tool, so its terminate budget has to cover a + # full provision (~460s for an S-00 cluster reaching ACTIVE) rather than + # the per-class budget, which is short on purpose. + from singlestoredb.tests import utils + self.assertGreater( + self.mod.TERMINATE_TIMEOUT, utils.TERMINATE_RETRY_TIMEOUT, + ) + self.assertGreaterEqual(self.mod.TERMINATE_TIMEOUT, 460) names, spared = self._find([ self._cluster('cl-test-mid-run', hours=1), ]) @@ -1815,5 +2691,368 @@ def test_a_since_that_is_not_a_date_is_rejected(self): self.mod.parse_since('last tuesday') +class TestStrandedSecretPatterns(unittest.TestCase): + """ + ``--secrets`` deletes org-scoped objects in a real organization, and a + secret nobody can read back is not recoverable, so the name gate and the + age gate both matter more here than they do for a deployment. + """ + + def setUp(self): + from singlestoredb.tests import cleanup_deployments + self.mod = cleanup_deployments + + def test_generated_names_match(self): + for name in ( + 'secret_v1_test_deadbeef', + 'secret_v2_test_deadbeef', + ): + self.assertTrue(self.mod.is_test_secret(name), name) + + def test_retired_names_still_match(self): + # The fixed names, which main still creates. Reaping them is the only + # thing that removes one a killed run stranded. + for name in ('secret_name', 'secret_v2_test'): + self.assertTrue(self.mod.is_test_secret(name), name) + + def test_names_a_person_chose_do_not_match(self): + for name in ( + None, + '', + 'secret', + 'openai_api_key', + 'secret_v3_test_deadbeef', + 'prod_secret_name', + 'secret_name_prod', + ): + self.assertFalse(self.mod.is_test_secret(name), name) + + def _secret(self, name, hours=None, deleted=False): + """A secret as the API reports one in the listing.""" + created = None + if hours is not None: + created = ( + datetime.datetime.now(tz=datetime.timezone.utc) + - datetime.timedelta(hours=hours) + ).isoformat() + return dict( + secretID=f'id-{name}', + name=name, + createdBy='someone', + createdAt=created, + lastUpdatedBy='someone', + lastUpdatedAt=created, + deletedAt=created if deleted else None, + ) + + def _manager(self, *items): + mgr = MagicMock() + mgr._get.return_value.json.return_value = dict(secrets=list(items)) + return mgr + + def _find(self, *items, **kwargs): + mgr = self._manager(*items) + found, spared, self.unmatched = self.mod.find_stranded_secrets( + mgr, **kwargs, + ) + self.listed = mgr._get.call_args[0][0] + return [x[1].name for x in found], spared + + def test_the_listing_asks_for_every_secret(self): + # Not ?name=: the point is to find names this process never chose. + self._find() + self.assertEqual(self.listed, 'secrets') + + def test_the_age_filter_spares_a_secret_a_live_run_may_own(self): + names, spared = self._find( + self._secret('secret_v2_test_deadbeef', hours=5), + self._secret('secret_v2_test_beefcafe', hours=0.01), + ) + self.assertEqual(names, ['secret_v2_test_deadbeef']) + self.assertEqual(len(spared), 1) + self.assertIn('secret_v2_test_beefcafe', spared[0]) + + def test_the_default_age_is_shorter_than_the_deployment_one(self): + # The window guarded is a test body, not a suite: a secret is created + # and deleted seconds apart. Still not zero -- a run killed between the + # POST and the DELETE looks like one still between them. + self.assertGreater(self.mod.DEFAULT_SECRET_MIN_AGE_HOURS, 0) + self.assertLess( + self.mod.DEFAULT_SECRET_MIN_AGE_HOURS, + self.mod.DEFAULT_MIN_AGE_HOURS, + ) + + def test_an_unreported_creation_time_is_spared_by_default(self): + names, spared = self._find(self._secret('secret_v2_test_ace0')) + self.assertEqual(names, []) + self.assertIn('secret_v2_test_ace0', spared[0]) + + names, _ = self._find( + self._secret('secret_v2_test_ace0'), include_unknown_age=True, + ) + self.assertEqual(names, ['secret_v2_test_ace0']) + + def test_an_unrecognized_name_is_reported_not_swept(self): + names, _ = self._find( + self._secret('secret_v2_test_cafe', hours=10), + self._secret('openai_api_key', hours=10), + ) + self.assertEqual(names, ['secret_v2_test_cafe']) + self.assertEqual(len(self.unmatched), 1) + self.assertIn('openai_api_key', self.unmatched[0]) + + def test_an_already_deleted_secret_is_ignored_entirely(self): + # Neither swept nor reported as unrecognized: it is already gone, so + # there is nothing for a reader of the output to act on. + names, spared = self._find( + self._secret('secret_v2_test_0ff0', hours=10, deleted=True), + self._secret('someones_deleted_key', hours=10, deleted=True), + ) + self.assertEqual((names, spared, self.unmatched), ([], [], [])) + + +class TestStrandedSecretSweep(unittest.TestCase): + """``--secrets`` end to end, with the management API stubbed out.""" + + def setUp(self): + from singlestoredb.tests import cleanup_deployments + self.mod = cleanup_deployments + self.mgr = MagicMock() + self.mgr._get.return_value.json.return_value = dict( + secrets=[ + dict( + secretID='id-old', name='secret_v2_test_deadbeef', + createdBy='x', lastUpdatedBy='x', lastUpdatedAt=None, + createdAt=( + datetime.datetime.now(tz=datetime.timezone.utc) + - datetime.timedelta(hours=10) + ).isoformat(), + ), + dict( + secretID='id-new', name='secret_v2_test_beefcafe', + createdBy='x', lastUpdatedBy='x', lastUpdatedAt=None, + createdAt=datetime.datetime.now( + tz=datetime.timezone.utc, + ).isoformat(), + ), + ], + ) + patcher = patch.object( + self.mod, '_manager', lambda version: self.mgr, + ) + patcher.start() + self.addCleanup(patcher.stop) + + def deleted(self): + return [x[0][0] for x in self.mgr._delete.call_args_list] + + def test_the_old_secret_is_deleted_and_the_new_one_is_not(self): + self.assertEqual(self.mod.main(['--secrets', '--yes']), 0) + self.assertEqual(self.deleted(), ['secrets/id-old']) + + def test_a_dry_run_deletes_nothing(self): + self.assertEqual(self.mod.main(['--secrets']), 0) + self.assertEqual(self.deleted(), []) + + def test_older_than_is_honoured(self): + self.assertEqual( + self.mod.main(['--secrets', '--older-than', '20', '--yes']), 0, + ) + self.assertEqual(self.deleted(), []) + + def test_no_deployment_listing_is_touched(self): + # --secrets is a different subject, not an extra filter: asking for it + # must not walk the clusters or the workspace groups. + self.mod.main(['--secrets', '--yes']) + self.mgr.clusters.__iter__.assert_not_called() + + def test_a_listing_failure_is_reported_rather_than_raised(self): + # A cleanup step, and a secret bills nothing: failing the job over one + # is the wrong trade. Non-zero, so the log still says something went + # wrong. + self.mgr._get.side_effect = ManagementError(msg='no such route') + self.assertEqual(self.mod.main(['--secrets', '--yes']), 1) + + def test_a_failed_delete_exits_non_zero(self): + self.mgr._delete.side_effect = ManagementError(msg='nope') + self.assertEqual(self.mod.main(['--secrets', '--yes']), 1) + + def test_the_deployment_selectors_do_not_compose_with_it(self): + import contextlib + import io + for argv in ( + ['--secrets', '--kind', 'cluster'], + ['--secrets', '--any-name'], + ['--secrets', '--since', 'today'], + ['--secrets', '--ledger', 'x.jsonl'], + ): + with self.assertRaises(SystemExit, msg=argv), \ + contextlib.redirect_stderr(io.StringIO()): + self.mod.main(argv) + + +class TestToDatetime(unittest.TestCase): + """ + ``to_datetime`` has to read both timestamp shapes the API returns. + + Most fields come back as RFC 3339, but ``GET /v2/clusters/{id}`` reports + ``expiresAt`` as a Go ``time.Time.String()`` rendering -- verified live: + ``2026-09-17 14:42:41.445984 +0000 UTC`` against a ``createdAt`` of + ``2026-09-17T13:42:41.493848Z`` on the same cluster. The trailing zone name + is not ISO 8601, and parsing it used to fail into ``None``, which reads as + "this cluster never expires". + """ + + def test_rfc_3339(self): + out = to_datetime('2026-09-17T13:42:41.493848Z') + self.assertEqual(out, datetime.datetime(2026, 9, 17, 13, 42, 41, 493848)) + + def test_go_time_string(self): + out = to_datetime('2026-09-17 14:42:41.445984 +0000 UTC') + self.assertEqual(out, datetime.datetime(2026, 9, 17, 14, 42, 41, 445984)) + + def test_offset_is_normalized_to_include_a_colon(self): + # Go writes +0000; datetime.fromisoformat only accepts that spelling on + # 3.11 and later, so the normalizer has to insert the colon itself. This + # asserts on the normalized string rather than on a parsed result + # because the parsed result is only wrong on 3.9 and 3.10, which would + # leave the failure invisible to anyone testing on a newer interpreter. + self.assertEqual( + _normalize_datetime('2026-09-17 14:42:41.445984 +0000 UTC'), + '2026-09-17 14:42:41.445984+00:00', + ) + self.assertEqual( + _normalize_datetime('2026-09-17 09:42:41 +0530 IST'), + '2026-09-17 09:42:41+05:30', + ) + # An offset that already carries a colon is left as it is. + self.assertEqual( + _normalize_datetime('2026-09-17 09:42:41 +05:30 IST'), + '2026-09-17 09:42:41+05:30', + ) + + def test_rfc_3339_fraction_is_padded(self): + # The API trims trailing zeros here too: a job's createdAt came back as + # '2026-09-18T12:39:20.43888Z'. Only 3.11 and later read a fraction that + # is neither 3 nor 6 digits, so before Z was recognized as an offset this + # value skipped the padding and to_datetime_strict raised on 3.10. + self.assertEqual( + _normalize_datetime('2026-09-18T12:39:20.43888Z'), + '2026-09-18T12:39:20.438880+00:00', + ) + self.assertEqual( + to_datetime_strict('2026-09-18T12:39:20.43888Z'), + datetime.datetime(2026, 9, 18, 12, 39, 20, 438880), + ) + + def test_rfc_3339_nanoseconds_are_truncated(self): + # Nine digits does not fit a datetime; the extra ones are dropped. + self.assertEqual( + _normalize_datetime('2026-09-18T12:39:20.438880123Z'), + '2026-09-18T12:39:20.438880+00:00', + ) + + def test_go_time_string_with_truncated_fraction(self): + # Go trims trailing zeros, so the fraction is not always 6 digits. + out = to_datetime('2026-09-17 14:42:41.4 +0000 UTC') + self.assertEqual(out, datetime.datetime(2026, 9, 17, 14, 42, 41, 400000)) + + def test_go_time_string_with_monotonic_reading(self): + out = to_datetime( + '2026-09-17 14:42:41.445984 +0000 UTC m=+0.000000001', + ) + self.assertEqual(out, datetime.datetime(2026, 9, 17, 14, 42, 41, 445984)) + + def test_offset_is_applied_and_dropped(self): + # Shifted onto UTC and left naive, matching the RFC 3339 values, so two + # timestamps read off one object can be compared. + out = to_datetime('2026-09-17 09:42:41 -0500 EST') + self.assertEqual(out, datetime.datetime(2026, 9, 17, 14, 42, 41)) + self.assertIsNone(out.tzinfo) + + def test_both_shapes_subtract(self): + created = to_datetime('2026-09-17T13:42:41.493848Z') + expires = to_datetime('2026-09-17 14:42:41.445984 +0000 UTC') + self.assertAlmostEqual( + (expires - created).total_seconds(), 3600, delta=1, + ) + + def test_date_only(self): + out = to_datetime('2026-09-17') + self.assertEqual(out, datetime.datetime(2026, 9, 17)) + + def test_zero_sentinel_and_unparseable_are_none(self): + self.assertIsNone(to_datetime('0001-01-01T00:00:00Z')) + self.assertIsNone(to_datetime(None)) + self.assertIsNone(to_datetime('')) + self.assertIsNone(to_datetime('not a date')) + + def test_the_go_spelling_of_the_zero_sentinel_is_none_too(self): + # Go's zero time means "unset" -- an expiresAt on a resource that does + # not expire -- and arrives in whichever shape the field uses. Reading + # the Go spelling as a real timestamp reported year 1 as an expiry. + self.assertIsNone(to_datetime('0001-01-01 00:00:00 +0000 UTC')) + # Recognized from the parsed value, so the trimmings Go may add do not + # each need their own literal. + self.assertIsNone( + to_datetime('0001-01-01 00:00:00 +0000 UTC m=+0.000000001'), + ) + self.assertIsNone(to_datetime('0001-01-01 00:00:00 +0000 GMT')) + self.assertIsNone(to_datetime('0001-01-01')) + + def test_datetime_passes_through(self): + given = datetime.datetime(2026, 9, 17, 13, 42, 41) + self.assertIs(to_datetime(given), given) + + def test_strict_reads_the_go_shape_too(self): + out = to_datetime_strict('2026-09-17 14:42:41.445984 +0000 UTC') + self.assertEqual(out, datetime.datetime(2026, 9, 17, 14, 42, 41, 445984)) + + def test_strict_still_raises_on_nothing(self): + with self.assertRaises(TypeError): + to_datetime_strict(None) + with self.assertRaises(ValueError): + to_datetime_strict('0001-01-01T00:00:00Z') + + def test_strict_raises_on_the_go_spelling_of_the_sentinel(self): + with self.assertRaises(ValueError): + to_datetime_strict('0001-01-01 00:00:00 +0000 UTC') + + +class TestAdminPassword(unittest.TestCase): + """The generated admin password must satisfy the API's policy on every + draw, not merely most of them: a ``secrets.token_urlsafe`` password + containing ``abc`` or ``321``, or one whose only punctuation is the ``&`` + the API does not count as special, is rejected with a 400 -- which the old + generator hit at a low enough rate to look like an API flake.""" + + #: Enough draws that a per-character rule would have to be enforced, not + #: just usually satisfied, to pass. A 24-character password holds 22 + #: three-character windows. + DRAWS = 2000 + + def test_the_policy_holds_on_every_draw(self): + for _ in range(self.DRAWS): + password = admin_password() + self.assertEqual(len(password), 24) + self.assertTrue(any(x.islower() for x in password), password) + self.assertTrue(any(x.isupper() for x in password), password) + self.assertTrue(any(x.isdigit() for x in password), password) + # A special character the API actually counts as one: `&` is + # accepted in a password but does not satisfy the rule. + self.assertTrue(any(x in '-_$' for x in password), password) + self.assertFalse('&' in password, password) + for i in range(len(password) - 2): + a, b, c = (ord(x) for x in password[i:i+3]) + # No three characters a step of -1, 0 or 1 apart in a row. + self.assertFalse(b - a == c - b and abs(b - a) <= 1, password) + + def test_the_length_is_honoured(self): + self.assertEqual(len(admin_password(32)), 32) + + def test_the_draws_differ(self): + self.assertEqual(len({admin_password() for _ in range(100)}), 100) + + if __name__ == '__main__': unittest.main() diff --git a/singlestoredb/tests/test_management_v1.py b/singlestoredb/tests/test_management_v1.py index 3328969a5..7a6533d47 100755 --- a/singlestoredb/tests/test_management_v1.py +++ b/singlestoredb/tests/test_management_v1.py @@ -35,6 +35,7 @@ from singlestoredb.management.job import TargetType from singlestoredb.management.region import Region from singlestoredb.management.utils import NamedList +from singlestoredb.tests import utils TEST_DIR = pathlib.Path(os.path.dirname(__file__)) @@ -68,7 +69,7 @@ def setUpClass(cls): cls.manager = s2.manage_workspaces(version='v1') us_regions = [x for x in cls.manager.regions if 'US' in x.name] - cls.password = secrets.token_urlsafe(20) + '-x&$' + cls.password = utils.admin_password() name = clean_name(secrets.token_urlsafe(20)[:20]) @@ -77,15 +78,24 @@ def setUpClass(cls): region=random.choice(us_regions).id, admin_password=cls.password, firewall_ranges=['0.0.0.0/0'], + expires_at=utils.DEPLOYMENT_EXPIRES_AT, ) try: + # No expiry of its own: only the group has an expiresAt, and it + # takes its workspaces with it. See utils.DEPLOYMENT_EXPIRES_AT. cls.workspace = cls.workspace_group.create_workspace( f'ws-test-{name}-x', wait_on_active=True, ) except Exception: - cls.workspace_group.terminate(force=True) + # Guarded: an unguarded terminate here would replace the create + # failure with whatever the DELETE raised. utils.cleanup_tracked + # retries it and reports it. + try: + cls.workspace_group.terminate(force=True) + except Exception: + pass raise @classmethod @@ -252,15 +262,13 @@ def setUpClass(cls): name = shared_database_name(secrets.token_urlsafe(20)[:20]) # The starter-tier user name has to be unique across every starter - # deployment in the project, not just within this one: creating the - # same name in a second starter deployment fails while the first is - # live. So it is namespaced like the deployment and the database are, - # or this class collides with TestStarterCluster in test_management_v2 - # -- they run on different xdist workers -- and with any starter - # deployment an earlier failed run leaked. The API answers the - # collision with a bare 500, which names nothing. + # deployment in the project, not just within this one, so it is + # namespaced like the deployment and the database are. Otherwise this + # class collides with TestStarterCluster in test_management_v2 -- they + # run on different xdist workers -- and with anything an earlier failed + # run leaked. The API answers the collision with a bare 500. cls.starter_username = f'starter_user_{name[:8]}' - cls.password = secrets.token_urlsafe(20) + cls.password = utils.admin_password() cls.database_name = f'starter_db_{name}' @@ -366,7 +374,7 @@ def setUpClass(cls): cls.manager = s2.manage_workspaces(version='v1') us_regions = [x for x in cls.manager.regions if 'US' in x.name] - cls.password = secrets.token_urlsafe(20) + '-x&$' + cls.password = utils.admin_password() name = clean_name(secrets.token_urlsafe(20)[:20]) @@ -375,6 +383,7 @@ def setUpClass(cls): region=random.choice(us_regions).id, admin_password=cls.password, firewall_ranges=['0.0.0.0/0'], + expires_at=utils.DEPLOYMENT_EXPIRES_AT, ) @classmethod @@ -926,29 +935,33 @@ def tearDownClass(cls): cls.manager = None def test_get_secret(self): - # manually create secret and then get secret - # try to delete the secret if it exists - try: - secret = self.manager.organizations.current.get_secret('secret_name') - - secret_id = secret.id - - self.manager._delete(f'secrets/{secret_id}') - except s2.ManagementError: - pass + # Per-run name; see the twin in test_management_v2.py for why the fixed + # 'secret_name' this used to carry -- and the leftover-clearing delete + # that a fixed name required -- had two concurrent runs deleting each + # other's secret. + name = f'secret_v1_test_{secrets.token_hex(4)}' - self.manager._post( + created = self.manager._post( 'secrets', json=dict( - name='secret_name', + name=name, value='secret_value', ), - ) - - secret = self.manager.organizations.current.get_secret('secret_name') + ).json() + + # The ID comes from the create response, not from the lookup under + # test: binding it inside the try would leave the cleanup raising + # UnboundLocalError over whatever the lookup failed with. This delete is + # the only thing that removes the secret now -- nothing else sweeps one + # as it is made. test_management_v2.py's twin does it this way. + secret_id = created['secret']['secretID'] + try: + secret = self.manager.organizations.current.get_secret(name) - assert secret.name == 'secret_name' - assert secret.value == 'secret_value' + assert secret.name == name + assert secret.value == 'secret_value' + finally: + self.manager._delete(f'secrets/{secret_id}') @pytest.mark.management @@ -965,7 +978,7 @@ def setUpClass(cls): cls.manager = s2.manage_workspaces(version='v1') us_regions = [x for x in cls.manager.regions if 'US' in x.name] - cls.password = secrets.token_urlsafe(20) + '-x&$' + cls.password = utils.admin_password() name = clean_name(secrets.token_urlsafe(20)[:20]) @@ -974,15 +987,24 @@ def setUpClass(cls): region=random.choice(us_regions).id, admin_password=cls.password, firewall_ranges=['0.0.0.0/0'], + expires_at=utils.DEPLOYMENT_EXPIRES_AT, ) try: + # No expiry of its own: only the group has an expiresAt, and it + # takes its workspaces with it. See utils.DEPLOYMENT_EXPIRES_AT. cls.workspace = cls.workspace_group.create_workspace( f'ws-test-{name}-x', wait_on_active=True, ) except Exception: - cls.workspace_group.terminate(force=True) + # Guarded: an unguarded terminate here would replace the create + # failure with whatever the DELETE raised. utils.cleanup_tracked + # retries it and reports it. + try: + cls.workspace_group.terminate(force=True) + except Exception: + pass raise @classmethod diff --git a/singlestoredb/tests/test_management_v2.py b/singlestoredb/tests/test_management_v2.py index 5842bdc56..ce11fefa3 100644 --- a/singlestoredb/tests/test_management_v2.py +++ b/singlestoredb/tests/test_management_v2.py @@ -1332,6 +1332,7 @@ def setUpClass(cls): region=region, size='S-00', firewall_ranges=['0.0.0.0/0'], + expires_at=utils.DEPLOYMENT_EXPIRES_AT, project=_project_id(cls.manager), wait_on_active=True, ) @@ -1516,7 +1517,7 @@ def setUpClass(cls): # xdist worker -- and with anything an earlier failed run leaked. The # API reports the collision as a bare 500. cls.starter_username = f'starter_user_{name[:8]}' - cls.password = secrets.token_urlsafe(20) + cls.password = utils.admin_password() cls.database_name = f'starter_db_{name}' @@ -1764,20 +1765,20 @@ def tearDownClass(cls): cls.manager = None def test_get_secret(self): - # A fixed name, deliberately not one built from id(self): that is a - # process-local address, so a name built from it can never match what - # an interrupted run left behind, which makes the cleanup below dead - # code. A secret is org-scoped and permanent and nothing sweeps them, - # so a leaked one is leaked for good. Distinct from the v1 suite's - # 'secret_name' so the two suites do not delete each other's. - name = 'secret_v2_test' - - # Clear a leftover secret from a previous run - try: - leftover = self.manager.organizations.current.get_secret(name) - self.manager._delete(f'secrets/{leftover.id}') - except s2.ManagementError: - pass + # Per-run name. A secret is org-scoped, so a fixed one is shared with + # every other run in the organization -- and this test used to open by + # deleting any leftover of that fixed name, which is a concurrent run's + # live secret as often as a stranded one. Two runs at once then raced: + # each deleted what the other had just created, and the loser's + # get_secret() failed or read the wrong value. + # + # Not id(self) either: that is a process-local address, so two + # processes can mint the same name. + # + # Nothing sweeps secrets as they are created, so the delete below is + # the cleanup; cleanup_deployments.py --secrets reaps what a run killed + # between the POST and the DELETE strands. + name = f'secret_v2_test_{secrets.token_hex(4)}' created = self.manager._post( 'secrets', diff --git a/singlestoredb/tests/test_udf_type_aliases.py b/singlestoredb/tests/test_udf_type_aliases.py new file mode 100644 index 000000000..60c6b721c --- /dev/null +++ b/singlestoredb/tests/test_udf_type_aliases.py @@ -0,0 +1,99 @@ +# mypy: disable-error-code="attr-defined,type-arg,valid-type,var-annotated" +""" +UDF annotations written with a PEP 695 type alias. + +numpy 2.5 defines ``npt.NDArray`` as an alias of +``ndarray[_AnyShape, dtype[ScalarT]]`` rather than as a subscripted generic, so +``typing.get_origin`` of an NDArray annotation returns the alias object instead +of ``numpy.ndarray``, and ``typing.get_args`` returns the scalar type instead of +the ``(shape, dtype)`` pair the signature machinery reads. The aliases below are +built with ``typing.TypeAliasType`` directly rather than taken from ``npt``, so +the alias form is covered whatever numpy is installed. + +These live in their own module because the type checker cannot follow an alias +built at runtime -- the file-level suppressions above would otherwise apply to +the hand-written annotations in ``test_udf_returns.py``. + +""" +import sys +import typing +import unittest +from typing import Any +from typing import Callable +from typing import Optional + +import numpy as np +import numpy.typing as npt + +from singlestoredb.functions import udf +from singlestoredb.functions.signature import get_signature +from singlestoredb.functions.signature import signature_to_sql + + +def to_sql(func: Callable[..., Any]) -> str: + """Convert a function signature to SQL.""" + out = signature_to_sql(get_signature(func)) + return out.split('EXTERNAL FUNCTION ')[1].split('AS REMOTE')[0].strip() + + +@unittest.skipIf( + sys.version_info < (3, 12), + 'PEP 695 type aliases require Python 3.12+', +) +class TypeAliasTest(unittest.TestCase): + + def test_subscripted_alias(self) -> None: + ScalarT = typing.TypeVar('ScalarT') + NDArrayAlias = typing.TypeAliasType( # noqa: TYP006 + 'NDArrayAlias', + np.ndarray[Any, np.dtype[ScalarT]], + type_params=(ScalarT,), + ) + + @udf + def foo_a(x: NDArrayAlias[np.str_]) -> NDArrayAlias[np.str_]: + return np.array([f'{i}: {v}' for i, v in enumerate(x)]) + + assert to_sql(foo_a) == '`foo_a`(`x` TEXT NOT NULL) RETURNS TEXT NOT NULL' + + def test_bare_alias(self) -> None: + Vec = typing.TypeAliasType('Vec', npt.NDArray[np.float64]) # noqa: TYP006 + + @udf + def foo_b(x: Vec) -> Vec: + return x * 2 + + assert to_sql(foo_b) == '`foo_b`(`x` DOUBLE NOT NULL) RETURNS DOUBLE NOT NULL' + + def test_optional_subscripted_alias(self) -> None: + ScalarT = typing.TypeVar('ScalarT') + NDArrayAlias = typing.TypeAliasType( # noqa: TYP006 + 'NDArrayAlias', + np.ndarray[Any, np.dtype[ScalarT]], + type_params=(ScalarT,), + ) + + @udf + def foo_c( + x: Optional[NDArrayAlias[np.str_]], + ) -> Optional[NDArrayAlias[np.str_]]: + return x + + # NOT NULL despite the Optional: the numpy branch of `get_schema` does + # not thread `is_optional` into its ParamSpec. Pre-existing on every + # numpy version, and asserted here so the alias expansion above is + # provably nullability-neutral. + assert to_sql(foo_c) == '`foo_c`(`x` TEXT NOT NULL) RETURNS TEXT NOT NULL' + + def test_optional_bare_alias(self) -> None: + Vec = typing.TypeAliasType('Vec', npt.NDArray[np.float64]) # noqa: TYP006 + + @udf + def foo_d(x: Optional[Vec]) -> Optional[Vec]: + return x + + assert to_sql(foo_d) == '`foo_d`(`x` DOUBLE NOT NULL) RETURNS DOUBLE NOT NULL' + + +if __name__ == '__main__': + unittest.main() diff --git a/singlestoredb/tests/utils.py b/singlestoredb/tests/utils.py index 94d754c47..f0b22e91f 100644 --- a/singlestoredb/tests/utils.py +++ b/singlestoredb/tests/utils.py @@ -2,11 +2,13 @@ # type: ignore """Utilities for testing.""" import glob +import json import logging import os import random import re import secrets +import string import unittest import uuid from types import SimpleNamespace @@ -286,39 +288,105 @@ def drop_user(name: str) -> None: cur.execute(f'DROP USER IF EXISTS {name};') +#: The characters the API counts towards `password must contain at least 1 +#: special characters`. Probed one character at a time against +#: `POST /v1/workspaceGroups`, which checks the password before it looks the +#: region up: `-`, `_`, `$`, `!`, `@`, `#`, `%` and `*` all satisfy the rule, +#: and `&` does not -- so a password whose only punctuation is an `&` is a +#: 400. The old hand-appended `-x&$` suffix passed on its `-` and `$`, not on +#: its `&`. These three are the ones that are also URL-unreserved or a +#: sub-delimiter, in case a password ever reaches a connection string. +_PASSWORD_SPECIALS = '-_$' + +#: Characters an admin password is drawn from. +_PASSWORD_ALPHABET = string.ascii_letters + string.digits + _PASSWORD_SPECIALS + + +def _runs_on(a: str, b: str, c: str) -> bool: + """Whether ``a b c`` is three characters in a row of the same step.""" + first, second = ord(b) - ord(a), ord(c) - ord(b) + return first == second and abs(first) <= 1 + + +def admin_password(length: int = 24) -> str: + """ + Return a password the management API will accept. + + The API enforces a policy the obvious ``secrets.token_urlsafe(20)`` does + not satisfy: the password must mix cases, digits and at least one special + character (see :data:`_PASSWORD_SPECIALS`), and it must not contain more + than two consecutive sequential characters -- a + token holding ``abc`` or ``321`` anywhere in it is rejected with a 400, + which made the old generator fail a small fraction of runs rather than + never. Identical runs (``aaa``) are excluded on the same terms, being the + same shape of rule and no loss of entropy worth keeping. + + Sequential is read on code points here, which is stricter than the letter + and digit sequences the API means but simpler, and it costs nothing: a + character that would close a run is redrawn, not the whole password. + + """ + while True: + chars: List[str] = [] + while len(chars) < length: + char = secrets.choice(_PASSWORD_ALPHABET) + if len(chars) >= 2 and _runs_on(chars[-2], chars[-1], char): + continue + chars.append(char) + password = ''.join(chars) + if ( + any(x.islower() for x in password) + and any(x.isupper() for x in password) + and any(x.isdigit() for x in password) + and any(x in _PASSWORD_SPECIALS for x in password) + ): + return password + + # # Live deployment tracking # -# Every workspace group, workspace, cluster and starter cluster a test creates -# costs money until it is terminated, and the usual `tearDownClass` is not -# enough on its own: +# Every deployment a test creates costs money until it is terminated, and +# `tearDownClass` is not enough on its own: unittest skips it entirely if +# `setUpClass` raises, so a fixture that dies partway through leaks what it had +# already made, and a test that fails before its own cleanup line leaks too. # -# * unittest does not call `tearDownClass` at all if `setUpClass` raises, so -# a fixture that dies partway through -- two of three clusters created, -# then a dropped connection -- leaks everything it had made so far; -# * a test that creates a deployment in its body and then fails before its -# own cleanup line leaks it too. +# So creations are registered here as well, and `cleanup_tracked()` sweeps what +# is left: per test class as the run moves on, and again for everything at the +# end of the session (see conftest.py). Terminating twice is harmless, so a test +# that cleans up after itself need not untrack. # -# So creations are registered here as well, and `cleanup_tracked()` sweeps -# whatever is left: per test class as the run moves on to the next one, and -# again for everything at the end of the session (see conftest.py). -# Terminating twice is harmless -- the second attempt finds it gone and is -# ignored -- so tracked objects do not have to be untracked by the tests that -# clean up after themselves. +# All of that is in-process. A job killed mid-provision leaves a PENDING cluster +# nothing here gets another chance to delete; `expires_at` is the answer to that +# and only that, being honoured by the control plane either way. # +#: Expiry to request on every deployment a test creates, as the duration string +#: `POST` accepts. A backstop under the sweep and the ledger, not a replacement: +#: a test still terminates what it created and nothing waits for an expiry. +#: +#: Two hours, against a `wait_timeout` of 1200s and a longest test of about +#: twenty minutes (Fusion `CREATE`/`DROP`, which provisions twice in sequence): +#: headroom enough that an expiry cannot land on a deployment still in use and +#: read as an unrelated API flake. +#: +#: Only `ClusterManager.create_cluster` (v2) and +#: `WorkspaceManager.create_workspace_group` (v1) take it, which is also where +#: the cost is. A v1 workspace needs none -- `expiresAt` belongs to the group -- +#: and the starter deployments accept no such argument. +DEPLOYMENT_EXPIRES_AT = '2h' + #: (owner, label, object) for every deployment created so far and not yet #: swept. The owner is the test class that was running at creation time, so #: a class's leftovers can be dropped when the run leaves that class rather #: than idling -- and billing -- until the session ends. _tracked: List[Tuple[str, str, Any]] = [] -#: (receiver, finder, args, kwargs) for every creation call currently -#: executing. A creator POSTs and only then waits for the deployment to come -#: up, so for the whole ``wait_on_active`` window -- twenty minutes for a -#: cluster -- something billable exists that nothing has registered yet: -#: ``_tracking_wrapper`` tracks on return and recovers in its ``except``, and -#: neither runs if the process is killed. See :func:`recover_in_flight`. +#: (receiver, finder, args, kwargs) for every creation call currently executing. +#: A creator POSTs and only then waits for the deployment to come up, so for the +#: whole ``wait_on_active`` window -- twenty minutes for a cluster -- something +#: billable exists that nothing has registered yet. See +#: :func:`recover_in_flight`. _in_flight: List[Tuple[Any, Any, Tuple[Any, ...], Dict[str, Any]]] = [] #: Test class currently running, as set by conftest. @@ -336,6 +404,138 @@ def set_owner(owner: str) -> None: _owner = owner +# +# Durable deployment ledger +# +# The leak the in-memory sweeps cannot cover, proven by GH Actions run +# 35631802648: job ``test-coverage`` was cancelled 19 minutes into +# ``TestClusterFusion.setUpClass``'s ``create_cluster(wait_on_active=True)``. The +# log ends at ``##[error]The operation was canceled.`` -- no pytest summary, no +# sweep, no ``STILL LIVE`` banner. After the SIGKILL, ``_tracked`` and +# ``_in_flight`` went with the process and nothing on disk named the three +# clusters. +# +# So every creation is also appended to a JSONL file, flushed and fsync'd per +# line, which ``cleanup_deployments.py --ledger`` reads from a separate process +# in an ``if: always()`` CI step. The in-memory sweeps remain the fast path; +# this is the record of last resort. +# +# Three events per deployment: ``pending`` before the POST (by name, there being +# no id yet), ``live`` once there is an id, ``gone`` once terminated. The reaper +# folds the file and takes anything whose last event is not ``gone``. +# +# Opt-in via SINGLESTOREDB_TEST_DEPLOYMENT_LOG: unset, nothing is written. +# + +#: Environment variable naming the ledger file. Read per write rather than +#: cached at import so a test can point it at a tmp_path. +LEDGER_ENV_VAR = 'SINGLESTOREDB_TEST_DEPLOYMENT_LOG' + +#: Deployment kind for each created object's class, so the reaper knows which +#: manager and point lookup to resolve a record against rather than guessing from +#: the name, which is convention only. +#: +#: Keyed by class name, not the class, to avoid importing v1 and v2 management +#: just to write a log line. +_KIND_BY_CLASS = { + 'WorkspaceGroup': 'workspace_group', + 'Workspace': 'workspace', + 'StarterWorkspace': 'starter_workspace', + 'Cluster': 'cluster', + 'StarterCluster': 'starter_cluster', +} + + +def ledger_path() -> Optional[str]: + """Path of the deployment ledger, or None if none was configured.""" + return os.environ.get(LEDGER_ENV_VAR) or None + + +def _ledger_write(**record: Any) -> None: + """ + Append one record to the deployment ledger. + + Opened, written and fsync'd per record: surviving SIGKILL is the whole + purpose, and a line still in a buffer records nothing. One open per creation + is nothing against a creation that takes minutes. + + That also makes it safe for the parallel default without locking. The xdist + workers are separate processes sharing the file, but each record is one short + ``write()`` to an ``O_APPEND`` handle, which Linux will not interleave, so + the reaper never sees a partial line. + + Never raises: this sits on the creation path of every management test, so an + unwritable ledger must cost a warning, not a failed run. + """ + path = ledger_path() + if not path: + return + try: + # default=str so an unexpected value (a datetime, an enum) degrades to + # its repr instead of raising and losing the whole record. + line = json.dumps(record, default=str, sort_keys=True) + '\n' + with open(path, 'a', encoding='utf-8') as file: + file.write(line) + file.flush() + os.fsync(file.fileno()) + except Exception as exc: + logger.warning( + f'Could not append {record!r} to the deployment ledger at ' + f'{path!r}; a deployment this run creates may not be reaped: ' + f'{exc}', + ) + + +def _ledger_kind(obj: Any) -> Optional[str]: + """Ledger kind for a created object, or None if it is not a deployment.""" + return _KIND_BY_CLASS.get(type(obj).__name__) + + +def ledger_pending(kind: str, args: Tuple[Any, ...], kwargs: Any) -> None: + """ + Record that a deployment of this kind is about to be created. + + The name is taken the same way :func:`_recover_orphan` takes it -- keyword + first, else the first positional, which every creator's signature makes the + name (pinned by ``test_management_utils.py``). Without a usable name there + is nothing for the reaper to resolve, so no record is written. + """ + name = kwargs.get('name') or (args[0] if args else None) + if not isinstance(name, str): + return + _ledger_write(event='pending', kind=kind, name=name) + + +def ledger_live(obj: Any) -> None: + """Record that a created deployment exists, now that it has an id.""" + kind = _ledger_kind(obj) + if kind is None: + return + _ledger_write( + event='live', kind=kind, + id=getattr(obj, 'id', None), + name=getattr(obj, 'name', None), + ) + + +def ledger_gone(obj: Any) -> None: + """ + Record that a deployment has been terminated. + + Carries the name as well as the id so it also cancels a ``pending`` + record: an orphan recovered by name and then swept in-process would + otherwise still be listed as live by the reaper. + """ + kind = _ledger_kind(obj) + if kind is None: + return + _ledger_write( + event='gone', kind=kind, + id=getattr(obj, 'id', None), + name=getattr(obj, 'name', None), + ) + + def _is_mocked(obj: Any) -> bool: """ Did this object come out of a mocked manager? @@ -378,6 +578,9 @@ def track(obj: Any, label: str = '') -> Any: ), obj, )) + # Here rather than in the wrapper, so an orphan `_recover_orphan` digs + # out of a listing gets its id into the ledger too. + ledger_live(obj) return obj @@ -427,24 +630,99 @@ def _recover_orphan( def untrack(obj: Any) -> None: """Forget a deployment that has been terminated.""" + found = False for i, entry in reversed(list(enumerate(_tracked))): if entry[2] is obj: _tracked.pop(i) - - -def terminate(obj: Any) -> None: + found = True + # Only for something actually tracked. Untracking an object that was never + # registered -- a mocked one, or one already swept -- says nothing about a + # real deployment, and a spurious ``gone`` hides a live cluster. + if found: + ledger_gone(obj) + + +#: How long :func:`terminate` keeps retrying a deployment the API will not +#: delete yet, and the spacing between attempts. Three minutes is deliberately +#: less than a full provision (~460s for an S-00 cluster), because waiting one +#: out here would stall the sweep between every test class. It buys the common +#: case -- a deployment most of the way up -- and leaves the rest to the +#: session-end sweep and then to ``cleanup_deployments.TERMINATE_TIMEOUT``, +#: which is the end of the line and can afford the wait. +TERMINATE_RETRY_TIMEOUT = 180.0 +TERMINATE_RETRY_INTERVAL = 15.0 + + +def _terminate_once(obj: Any) -> None: """ - Terminate a deployment, whatever kind it is. + Issue one terminate, whatever this kind's signature looks like. ``force=True`` is what makes a workspace group with live workspaces in it - go away; the starter variants take no arguments at all. + go away; the starter variants (``StarterWorkspace.terminate``, + ``StarterCluster.terminate``) take no arguments at all. + + The signature is inspected rather than discovered by catching ``TypeError`` + from the call: that also caught a ``TypeError`` raised from *inside* a + terminate which did accept ``force``, and retried without it -- two DELETEs, + the second unforced, which is what leaves a workspace group behind. """ + import inspect + try: + params = inspect.signature(obj.terminate).parameters + except (TypeError, ValueError): # pragma: no cover - unintrospectable + # A builtin or a C-level callable. Fall back to the old behaviour. + params = {} + + if 'force' in params: obj.terminate(force=True) - except TypeError: + else: obj.terminate() +def terminate( + obj: Any, + timeout: float = TERMINATE_RETRY_TIMEOUT, + interval: float = TERMINATE_RETRY_INTERVAL, +) -> None: + """ + Terminate a deployment, whatever kind it is, retrying a 4xx refusal. + + A deployment killed mid-provision is ``PENDING``/``TRANSITIONING`` and the + API refuses to delete it, with a 400 or a 409. Nothing else retries that -- + ``Manager.RETRY_STATUSES`` covers only ``{429, 500, 502, 503, 504}`` -- so + the per-class sweep warned, the session-end sweep tried once more, usually + still too early, and the deployment stayed up. Hence the bounded retry. + + Only 4xx other than 404 is retried. A 404 means it is already gone, so + retrying would burn the budget on something that is not coming back; a 5xx + or 429 has already been retried by the transport, and another round trip + from this layer is not what fixes it. + + Raises the last error if the budget runs out, which keeps the deployment in + ``_tracked`` so the session-end sweep gets another go. + """ + import time + + deadline = time.monotonic() + timeout + while True: + try: + _terminate_once(obj) + return + except ManagementError as exc: + errno = exc.errno + if errno is None or errno == 404 or not 400 <= errno < 500: + raise + # No budget left for another attempt *plus* the wait before it. + if time.monotonic() + interval >= deadline: + raise + logger.info( + f'{obj!r} is not deletable yet ({exc}); retrying the ' + f'terminate in {interval:g}s', + ) + time.sleep(interval) + + def _creator_is_mocked(target: Any) -> bool: """ Is this creation call going through a mocked manager? @@ -473,9 +751,14 @@ def _creator_is_mocked(target: Any) -> bool: ) -#: (module, class, method, finder) tuples for the calls that bring a billable -#: deployment into existence. Wrapping them is what makes tracking automatic, -#: so a new test cannot leak a cluster by forgetting to register it. +#: (module, class, method, kind, finder) tuples for the calls that bring a +#: billable deployment into existence. Wrapping them is what makes tracking +#: automatic, so a new test cannot leak a cluster by forgetting to register it. +#: +#: ``kind`` is the ledger kind the call produces, and must be a value of +#: :data:`_KIND_BY_CLASS`: it tells the reaper which manager to search for a +#: ``pending`` record, which has no id. Stated here rather than derived from +#: ``method_name``, which ``create_workspace`` shares across two receivers. #: #: ``finder`` takes the receiver -- the manager, or the group for #: ``WorkspaceGroup.create_workspace`` -- and returns the collection to search @@ -484,34 +767,34 @@ def _creator_is_mocked(target: Any) -> bool: _CREATORS = [ ( 'singlestoredb.management.v1.workspace', 'WorkspaceManager', - 'create_workspace_group', + 'create_workspace_group', 'workspace_group', lambda recv: recv.workspace_groups, ), ( 'singlestoredb.management.v1.workspace', 'WorkspaceManager', - 'create_workspace', + 'create_workspace', 'workspace', # WorkspaceManager has no `workspaces` of its own, so the search goes # group by group. Only ever walked on the failure path. lambda recv: [w for g in recv.workspace_groups for w in g.workspaces], ), ( 'singlestoredb.management.v1.workspace', 'WorkspaceManager', - 'create_starter_workspace', + 'create_starter_workspace', 'starter_workspace', lambda recv: recv.starter_workspaces, ), ( 'singlestoredb.management.v1.workspace', 'WorkspaceGroup', - 'create_workspace', + 'create_workspace', 'workspace', lambda recv: recv.workspaces, ), ( 'singlestoredb.management.v2.cluster', 'ClusterManager', - 'create_cluster', + 'create_cluster', 'cluster', lambda recv: recv.clusters, ), ( 'singlestoredb.management.v2.cluster', 'ClusterManager', - 'create_starter_cluster', + 'create_starter_cluster', 'starter_cluster', lambda recv: recv.starter_clusters, ), ] @@ -519,7 +802,7 @@ def _creator_is_mocked(target: Any) -> bool: _tracking_installed = False -def _tracking_wrapper(func: Any, finder: Any) -> Any: +def _tracking_wrapper(func: Any, kind: str, finder: Any) -> Any: """ Wrap a creation method so its result -- or its orphan -- gets tracked. @@ -533,19 +816,20 @@ def _tracking_wrapper(func: Any, finder: Any) -> Any: (see :func:`recover_in_flight`). ``_creator_is_mocked``, not ``_is_mocked``: the receiver is the manager (or - the workspace group), and ``_is_mocked`` looks for a ``_manager`` - attribute, which a manager does not have -- so a real manager with a - patched ``_post`` would read as live and the recovery would fire a real - API call from a unit test. ``_creator_is_mocked`` inspects the receiver's - own transport and handles both receiver shapes. - - That same verdict also decides whether the *result* is tracked, rather than - leaving it to ``track()``. ``track()`` can only judge what it is handed, - and it is deliberately biased toward "real" for anything it cannot place -- - including an object whose ``_manager`` is ``None``, which is exactly what a - unit test's stubbed ``get_cluster`` returns. Nothing a mocked creator - returns names a deployment that exists, so the receiver's verdict is the - authoritative one and it is the one used here. + the workspace group), and ``_is_mocked`` looks for a ``_manager`` attribute, + which a manager does not have -- so a real manager with a patched ``_post`` + would read as live and the recovery would fire a real API call from a unit + test. ``_creator_is_mocked`` inspects the receiver's own transport. + + That same verdict decides whether the *result* is tracked, rather than + leaving it to ``track()``, which can only judge what it is handed and is + biased toward "real" for anything it cannot place -- including the + ``_manager is None`` object a stubbed ``get_cluster`` returns. + + The ``pending`` ledger record is written *before* ``func`` is called. From + the POST onward something is billable, and everything else that could record + it -- ``track()`` on return, ``_recover_orphan()``, ``recover_in_flight()`` + -- runs after the wait a cancelled CI job never survives. """ import functools @@ -555,6 +839,7 @@ def wrapper(receiver: Any, *args: Any, **kwargs: Any) -> Any: entry = (receiver, finder, args, kwargs) if not mocked: _in_flight.append(entry) + ledger_pending(kind, args, kwargs) try: out = func(receiver, *args, **kwargs) return out if mocked else track(out) @@ -628,12 +913,12 @@ def install_deployment_tracking() -> None: import importlib - for module_name, class_name, method_name, finder in _CREATORS: + for module_name, class_name, method_name, kind, finder in _CREATORS: try: klass = getattr(importlib.import_module(module_name), class_name) setattr( klass, method_name, - _tracking_wrapper(getattr(klass, method_name), finder), + _tracking_wrapper(getattr(klass, method_name), kind, finder), ) except AttributeError as exc: # A renamed method must not silently stop being tracked. @@ -708,6 +993,9 @@ def cleanup_tracked(owner: Optional[str] = None) -> List[str]: _, label, obj = entry if _is_gone(obj): _tracked.remove(entry) + # A test that terminated in its own teardown: close the record here + # rather than leaving the reaper to look up an id that 404s. + ledger_gone(obj) continue try: terminate(obj) @@ -719,6 +1007,7 @@ def cleanup_tracked(owner: Optional[str] = None) -> List[str]: logger.warning(f'Could not terminate {label}: {exc}') else: _tracked.remove(entry) + ledger_gone(obj) removed.append(label) return removed @@ -737,44 +1026,47 @@ def tracked_labels() -> List[str]: # # Shared deployment pool # -# Several classes need nothing from a deployment but that it is live: the -# Stage and Job suites read and write through the management API against -# whatever cluster they are handed. Deploying one apiece cost 2190s of the -# 8915s a traced run took, and an S-00 cluster reaching ACTIVE is ~460s that -# cannot be made faster -- so the only lever is deploying fewer of them. +# Several classes need nothing from a deployment but that it is live: the Stage +# and Job suites read and write through the management API against whatever +# cluster they are handed. Deploying one apiece cost 2190s of the 8915s a traced +# run took, and an S-00 cluster reaching ACTIVE is ~460s that cannot be made +# faster -- so the only lever is deploying fewer of them. # # The pool is built on first use and reused for the rest of the process. A -# class must not mutate what it borrows, so anything whose subject *is* the +# borrower must not mutate what it borrows, so anything whose subject *is* the # deployment keeps deploying its own: ``TestCluster`` and ``TestWorkspace`` -# (``test_update`` PATCHes the cluster and cycles it back through PENDING), -# ``TestClusterFusionCreateDrop`` and ``TestClusterFusionSuspendResume``. So -# does ``TestWorkspaceFusion``, whose workspace groups are the subject of its -# ``SHOW WORKSPACE GROUPS`` assertions and cost 40s to deploy unwaited anyway. +# (``test_update`` PATCHes the cluster back through PENDING), +# ``TestClusterFusionCreateDrop``, ``TestClusterFusionSuspendResume`` and +# ``TestWorkspaceFusion``. # -# What makes the four borrowers safe is that each scopes its assertions to -# itself: every Stage path is namespaced with the class's ``cls.id``, job -# listings filter by job id rather than listing a deployment's jobs, and none -# of them asserts a row count over an org-wide listing. +# Each borrower also scopes its assertions to itself -- Stage paths namespaced +# with ``cls.id``, job listings filtered by job id -- so none of them asserts a +# row count over an org-wide listing. ``TestClusterFusion`` does count rows, +# ``SHOW CLUSTERS ... LIKE`` being what it tests, and stays inside that rule by +# counting :func:`shared_cluster_pattern` against :func:`shared_cluster_names`. # -# The pool is process-wide, so under ``pytest-xdist`` every worker that gets a -# borrowing class builds a pool of its own. The ``xdist_group`` marks below -# keep the borrowers together on a worker; see ``SHARED_CLUSTER_*_GROUP``. +# The pool is process-wide, so under ``pytest-xdist`` every worker with a +# borrowing class builds one of its own. The ``xdist_group`` marks below keep the +# borrowers together on a worker. # #: ``xdist_group`` names for the classes that borrow from the pool, so -#: ``--dist loadgroup`` puts each set on one worker and each set builds one -#: pool. Two groups rather than one: a single group serialises all four classes -#: behind one pool build, and the groups run concurrently on separate workers, -#: so splitting costs one extra cluster and halves that chain. -#: -#: Stage wants two clusters (``TestStageFusion`` names a second one in -#: ``IN GROUP``) and jobs want one, so the split follows what they borrow: +#: ``--dist loadgroup`` puts each set on one worker and each set builds one pool. +#: Two groups rather than one: a single group serialises every borrower behind one +#: pool build, where these two run concurrently on separate workers for the cost +#: of one extra cluster. #: -#: * ``SHARED_CLUSTER_STAGE_GROUP`` -- ``TestStageFusion``, v2 ``TestStage`` +#: * ``SHARED_CLUSTER_STAGE_GROUP`` -- ``TestStageFusion`` (two; it names a +#: second in ``IN GROUP``), v2 ``TestStage`` (one), ``TestClusterFusion`` +#: (three, for its ``LIKE``/``ORDER BY``/``LIMIT`` rows) #: * ``SHARED_CLUSTER_JOBS_GROUP`` -- ``TestJobsFusion``, v2 ``TestJob`` #: -#: Without ``-n``/``--dist loadgroup`` the marks do nothing: one process, one -#: pool of two, which is the serial behaviour they were added on top of. +#: ``TestClusterFusion`` sits with Stage deliberately: the pool grows to the +#: largest request, so the class that wants three costs Stage's pool one extra +#: cluster, against two if it joined Jobs. +#: +#: Without ``-n``/``--dist loadgroup`` the marks do nothing: one process, one pool +#: of three. SHARED_CLUSTER_STAGE_GROUP = 'shared-cluster-stage' SHARED_CLUSTER_JOBS_GROUP = 'shared-cluster-jobs' @@ -860,6 +1152,7 @@ def setUpClass(cls): # pool cluster stands in for those, so it has to be at # least as reachable as what it replaces. firewall_ranges=['0.0.0.0/0'], + expires_at=DEPLOYMENT_EXPIRES_AT, project=project_id, wait_on_active=True, wait_timeout=1200, @@ -871,6 +1164,29 @@ def setUpClass(cls): return _pool[:count] +def shared_cluster_pattern() -> str: + """ + ``LIKE`` pattern matching this process's pool clusters and nothing else. + + The suffix scopes it: ``_pool_id`` is minted per process, so another xdist + worker's pool -- or any other ``cl-test-*`` deployment -- does not match. + + Pair it with :func:`shared_cluster_names`, not a literal count: the pool + grows to the largest request any class makes. + """ + return f'cl-test-shared-%-{_pool_id}' + + +def shared_cluster_names() -> List[str]: + """ + Names of every cluster in the pool as it stands right now. + + Read at assertion time, not cached: a later class asking for more clusters + grows the pool, which would leave a cached expectation stale. + """ + return [x.name for x in _pool] + + class CountingManager: """ Stand-in for a :class:`Manager` that records every request.