Skip to content

Fix port overlapping in ytsaurus tests#88165

Merged
azat merged 2 commits intoClickHouse:masterfrom
MikhailBurdukov:fix_ytsaurus_port
Oct 8, 2025
Merged

Fix port overlapping in ytsaurus tests#88165
azat merged 2 commits intoClickHouse:masterfrom
MikhailBurdukov:fix_ytsaurus_port

Conversation

@MikhailBurdukov
Copy link
Contributor

@MikhailBurdukov MikhailBurdukov commented Oct 7, 2025

Continuation of: #88134

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

...

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@MikhailBurdukov MikhailBurdukov marked this pull request as ready for review October 7, 2025 08:32
@MikhailBurdukov
Copy link
Contributor Author

MikhailBurdukov commented Oct 7, 2025

@azat FYI

Also I haven't managed this by now: #88134 (comment)

Just to avoid blocking anyone, let's leave it as it is, and I'll figure out if it's possible or not in the background.

@azat azat self-assigned this Oct 7, 2025
@azat azat added can be tested Allows running workflows for external contributors 🍃 green ci 🌿 Fixing flaky tests in CI labels Oct 7, 2025
@clickhouse-gh
Copy link
Contributor

clickhouse-gh bot commented Oct 7, 2025 •
edited
Loading

Workflow [PR], commit [000edf8]

Summary: ✅

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Oct 7, 2025
@azat
Copy link
Member

azat commented Oct 7, 2025

Thanks!

Also I haven't managed this by now: #88134 (comment)

It should be as simple as the patch below, but, it turns out not that simple due to wait_ytsaurus_to_start() that goes via host network. So ok for now.

patch 
diff --git a/tests/integration/compose/docker_compose_ytsaurus.yml b/tests/integration/compose/docker_compose_ytsaurus.yml
index bdc5c116aff..4aaaba843b1 100644
--- a/tests/integration/compose/docker_compose_ytsaurus.yml
+++ b/tests/integration/compose/docker_compose_ytsaurus.yml
@@ -8,6 +8,4 @@ services:
             YT_USE_HOSTS: 0
             YT_TOKEN: password
             YT_PROXY: ytsaurus_backend1:${YTSAURUS_PROXY_PORT}
-        ports:
-            - ${YTSAURUS_PROXY_PORT}:${YTSAURUS_PROXY_PORT}
         entrypoint: yt_local start --proxy-port ${YTSAURUS_PROXY_PORT} --local-cypress-dir /var/lib/yt/local-cypress --ytserver-all-path /usr/bin/ytserver-all --sync --fqdn ytsaurus_backend1 --proxy-config '{coordinator={public_fqdn="ytsaurus_backend1:80"}}' --rpc-proxy-count 0 --rpc-proxy-port 8002 --node-count 1 --queue-agent-count 1 --address-resolver-confi>

P.S. do you have any ideas on how to make those tests faster? They looks very slow at first glance

@azat azat enabled auto-merge October 7, 2025 11:41
auto-merge was automatically disabled October 7, 2025 12:37

Head branch was pushed to by a user without write access

MikhailBurdukov
MikhailBurdukov commented Oct 7, 2025
View reviewed changes
tests/integration/test_ytsaurus/yt_helpers.py
Comment on lines +41 to 43
retry_count=100,
time_to_sleep=0.5,
):
Copy link
Contributor Author

@MikhailBurdukov MikhailBurdukov Oct 7, 2025

Choose a reason for hiding this comment

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

That should make tests faster, but I don't have real numbers. If it's still slow, ping me and we will look into it more systematically. @azat

@azat azat enabled auto-merge October 7, 2025 12:51
@azat azat added this pull request to the merge queue Oct 8, 2025
Merged via the queue into ClickHouse:master with commit 07c5999 Oct 8, 2025
123 checks passed
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Oct 8, 2025
@Algunenano Algunenano mentioned this pull request Oct 8, 2025
Closed
@MikhailBurdukov MikhailBurdukov mentioned this pull request Oct 8, 2025
Merged
1 task
@CurtizJ CurtizJ mentioned this pull request Oct 9, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azat azat azat approved these changes

Assignees

@azat azat

Labels

can be tested Allows running workflows for external contributors 🍃 green ci 🌿 Fixing flaky tests in CI pr-not-for-changelog This PR should not be mentioned in the changelog 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

@MikhailBurdukov @azat @robot-ch-test-poll