Visitar URL original
Read the CI DB read URL from SSM instead of hardcoding play.clickhouse.com by leshikus · Pull Request #129096 · ClickHouse/ClickHouse · GitHub
Skip to content

Read the CI DB read URL from SSM instead of hardcoding play.clickhouse.com - #129096

Merged
leshikus merged 3 commits into
ClickHouse:masterfrom
leshikus:cidb-read-url-from-ssm
Nov 6, 2026
Merged

Read the CI DB read URL from SSM instead of hardcoding play.clickhouse.com#129096
leshikus merged 3 commits into
ClickHouse:masterfrom
leshikus:cidb-read-url-from-ssm

Conversation

@leshikus

@leshikus leshikus commented Nov 3, 2026 •
edited by robot-ch-test-poll
Loading

Copy link
Copy Markdown
Collaborator

CI data ingestion moved from play.clickhouse.com to a staging instance (https://github.com/ClickHouse/clickhouse-private/issues/36465). Read links and queries in ci/ still pointed at the old host, which showed stale data from 2026-10-29 until it was changed to forward to the new one. This change reads from the new host directly. A failed test case in this PR's report links to the new host.

  1. Single source. CIDB.get_read_url returns the url of the SSM connection secret, which the write path already uses. Without it the two paths drift again at the next move.
  2. Hardcoded hosts removed. Settings.CI_DB_READ_URL, the PLAY_CIDB_URL constant and the literal play.clickhouse.com in cidb_cluster.py and sqlancer_notify.py are gone; performance_tests.py uses read_cidb(). Only CI_DB_READ_USER stays.
  3. Report links. runner.py stops passing a URL, so get_link_to_test_case_statistics uses the URL of the CIDB object.
  4. Lookup on the host. The runner reads the URL once before docker run and passes it as CI_DB_READ_URL, so job containers never call SSM and need no boto3. Without it, local runs skip the query history label.
  5. Local tools. check_ci.py now needs access to the SSM secret. Without it the flaky-test issue helper fails on the lookup.

Changelog category (leave one):

  • CI Fix or Improvement (changelog entry is not required)

Changelog entry:

Workflow [PR]
Sync PR [sync-upstream/pr/129096]

Version info

  • Merged into: 26.11.1.1537-master (included in 26.11 and later)

@leshikus leshikus added the can be tested Allows running workflows for external contributors label Nov 3, 2026
@clickhouse-gh clickhouse-gh Bot added the comp-ci-infrastructure CI/CD pipelines (GitHub Actions, CI scripts, runners). label Nov 3, 2026
@leshikus @claude
Read the CI DB read URL from the SSM connection secret
3dce45a 
Read links and queries used a hardcoded play.clickhouse.com, which no longer receives CI data after the ingestion moved to the staging instance.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@leshikus
leshikus force-pushed the cidb-read-url-from-ssm branch from 805f88b to 0cefdb8 Compare November 4, 2026 14:13
@clickhouse-gh

clickhouse-gh Bot commented Nov 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [a5a32e7]

Summary: ✅

AI Review

Summary

Reviewed #129096 at a5a32e77adc28a1baf61c79172f96d79ebac25e6. The change derives the CI database read endpoint from the connection secret and passes it into job containers for history queries and report links. Both previous findings are fixed, and no new actionable findings remain.

Final Verdict

✅ Approve.

@clickhouse-gh clickhouse-gh Bot added the pr-ci label Nov 4, 2026
@clickhouse-gh
clickhouse-gh Bot requested a review from maxknv November 4, 2026 14:14
@leshikus
leshikus force-pushed the cidb-read-url-from-ssm branch from 0cefdb8 to 3dce45a Compare November 4, 2026 15:08
clickhouse-gh[bot]
clickhouse-gh Bot reviewed Nov 4, 2026
View reviewed changes
Comment thread ci/jobs/scripts/sqlancer_notify.py Resolved
Comment thread ci/jobs/performance_tests.py Resolved
@leshikus

leshikus commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Evidence that the cidb link in a CI report uses the new host.

Report Code Host in the failed test cidb link
PR 129198, 2026-11-04 12:31 UTC master without this change https://play.clickhouse.com/play?user=play&run=1 (7 links)
This PR, commit 0cefdb878c3 this change plus a temporary failing test, 09999_cidb_link_probe https://xy8c38q4lw.us-east-2.aws.clickhouse-staging.com/play?user=play&run=1 (2 links)

The temporary test is no longer in the branch. Not run in a job: the read_cidb() paths of the performance job and check_ci.py.

@leshikus
leshikus marked this pull request as ready for review November 4, 2026 15:44
@leshikus
leshikus requested a review from Felixoid November 4, 2026 15:44
leshikus and others added 2 commits November 4, 2026 16:52
@leshikus @claude
Resolve the CI DB read URL on the host and pass it into the job conta…
cf2fee4 
…iner

The review pointed out that `sqlancer_notify.py` runs in `clickhouse/sqlancer-test`, which has neither `requests` nor `boto3`. Importing `CIDB` there made the notifier raise, so alerts for new SQLancer failures were skipped silently.

The runner now reads the `url` of the connection secret on the host, before `docker run`, and passes it as `CI_DB_READ_URL`. The lookup is skipped for local runs, and a failure only prints a warning. The URL is checked to carry no credentials. `sqlancer_notify.py` reads the variable and no longer imports `CIDB`. `CIDB.get_read_url` returns the variable first and falls back to the lookup for tools that run outside a container.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@leshikus @claude
Pass the CI DB read URL into the performance query history links
a5a32e7 
The review pointed out that `build_perf_query_history_link` looked up the SSM secret for every slower or unstable query, so 100 rows made 100 lookups and a credentials error in a local run escaped the report's exception handler.

`main` now reads `CI_DB_READ_URL` once, as set by the runner on the host, and passes it to `build_check_results_children`. Without the variable, as in local runs, the rows keep no `query history` label.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
maxknv
maxknv reviewed Nov 4, 2026
View reviewed changes

@maxknv maxknv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get the idea behind CI_DB_READ_URL. Can you elaborate a bit on that?
Why CIDB_CONNECTING_STRING is not enough?

@maxknv maxknv self-assigned this Nov 4, 2026
@leshikus

leshikus commented Nov 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

CIDB_CONNECTING_STRING contains secrets that we don't want inside the Docker container. CI_DB_READ_URL is only for read operations and contains no credentials. @maxknv

I asked @Felixoid whether we could keep these parameters outside SSM, for example in S3, but he insisted that they should be in SSM. Thus we have to pass the string to Docker, because we don't want to pass AWS credentials to it.

@maxknv
maxknv self-requested a review November 6, 2026 13:37
@leshikus
leshikus added this pull request to the merge queue Nov 6, 2026
Merged via the queue into ClickHouse:master with commit 8b39a16 Nov 6, 2026
185 checks passed
@leshikus
leshikus deleted the cidb-read-url-from-ssm branch November 6, 2026 16:20
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Nov 6, 2026
@maxknv maxknv mentioned this pull request Nov 13, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clickhouse-gh clickhouse-gh[bot] clickhouse-gh[bot] left review comments
@maxknv maxknv maxknv approved these changes
@Felixoid Felixoid Awaiting requested review from Felixoid

Assignees

@maxknv maxknv

Labels

can be tested Allows running workflows for external contributors comp-ci-infrastructure CI/CD pipelines (GitHub Actions, CI scripts, runners). pr-ci pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@leshikus @maxknv @robot-ch-test-poll4