From 47397a45c585f1f0b835d5ea5afca592e7f29d9d Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sun, 1 Dec 2024 17:06:43 +0100 Subject: [PATCH] tests: make test_config_substitutions idempotent Now the following passed: $ bin=$(which clickhouse); g=$PWD; docker run --privileged -it --rm --volume=$bin:/clickhouse --volume=$bin:/usr/share/clickhouse_fresh --volume=/dev/null:/usr/bin/clickhouse-odbc-bridge --volume=/dev/null:/usr/share/clickhouse-odbc-bridge_fresh --volume=$g/programs/server:/clickhouse-config --volume=$g:/ClickHouse --volume=$g/docker/test/integration/runner/compose:/compose:ro --volume=clickhouse-integration-tests:/var/lib/docker -e PYTEST_ADDOPTS='-vvv -s --pdb --count 2 test_config_substitutions/test.py' --name ch clickhouse/integration-tests-runner Signed-off-by: Azat Khuzhin --- tests/integration/helpers/cluster.py | 15 +++ .../test_config_substitutions/test.py | 93 ++++++++++--------- 2 files changed, 66 insertions(+), 42 deletions(-) diff --git a/tests/integration/helpers/cluster.py b/tests/integration/helpers/cluster.py index 504a70d3865..46d2ea3f902 100644 --- a/tests/integration/helpers/cluster.py +++ b/tests/integration/helpers/cluster.py @@ -17,6 +17,7 @@ import subprocess import time import traceback import urllib.parse +from contextlib import contextmanager from functools import cache from pathlib import Path from typing import Any, List, Sequence, Tuple, Union @@ -4542,6 +4543,20 @@ class ClickHouseInstance: if key != "DSN": f.write(key + "=" + value + "\n") + @contextmanager + def with_replace_config(self, path, replacement): + """Create a copy of existing config (if exists) and revert on leaving the context""" + self.exec_in_container( + ["bash", "-c", f"test ! -f {path} || mv --no-clobber {path} {path}.bak"] + ) + self.exec_in_container( + ["bash", "-c", "echo '{}' > {}".format(replacement, path)] + ) + yield + self.exec_in_container( + ["bash", "-c", f"test ! -f {path}.bak || mv {path}.bak {path}"] + ) + def replace_config(self, path_to_config, replacement): self.exec_in_container( ["bash", "-c", "echo '{}' > {}".format(replacement, path_to_config)] diff --git a/tests/integration/test_config_substitutions/test.py b/tests/integration/test_config_substitutions/test.py index 80a1fecb4ca..659b6ac8e61 100644 --- a/tests/integration/test_config_substitutions/test.py +++ b/tests/integration/test_config_substitutions/test.py @@ -21,6 +21,7 @@ node3 = cluster.add_instance( "configs/config_zk_include_test.xml", ], with_zookeeper=True, + stay_alive=True, ) node4 = cluster.add_instance( "node4", @@ -133,7 +134,7 @@ def test_config(start_cluster): def test_config_from_env_overrides(start_cluster): - node7.replace_config( + with node7.with_replace_config( "/etc/clickhouse-server/users.d/000-users_with_env_subst.xml", """ @@ -155,13 +156,14 @@ def test_config_from_env_overrides(start_cluster): """, - ) - with pytest.raises( - QueryRuntimeException, - match="Failed to preprocess config '/etc/clickhouse-server/users.xml': Exception: Element has value and does not have 'replace' attribute, can't process from_env substitution", ): - node7.query("SYSTEM RELOAD CONFIG") - node7.replace_config( + with pytest.raises( + QueryRuntimeException, + match="Failed to preprocess config '/etc/clickhouse-server/users.xml': Exception: Element has value and does not have 'replace' attribute, can't process from_env substitution", + ): + node7.query("SYSTEM RELOAD CONFIG") + + with node7.with_replace_config( "/etc/clickhouse-server/users.d/000-users_with_env_subst.xml", """ @@ -183,26 +185,27 @@ def test_config_from_env_overrides(start_cluster): """, - ) - node7.query("SYSTEM RELOAD CONFIG") + ): + node7.query("SYSTEM RELOAD CONFIG") def test_config_merge_from_env_overrides(start_cluster): + node7.query("SYSTEM RELOAD CONFIG") assert ( node7.query( "SELECT value FROM system.server_settings WHERE name='max_thread_pool_size'" ) == "10000\n" ) - node7.replace_config( + with node7.with_replace_config( "/etc/clickhouse-server/config.d/010-server_with_env_subst.xml", """ 9000 """, - ) - node7.query("SYSTEM RELOAD CONFIG") + ): + node7.query("SYSTEM RELOAD CONFIG") def test_include_config(start_cluster): @@ -223,6 +226,7 @@ def test_include_config(start_cluster): def test_allow_databases(start_cluster): + node5.query("DROP DATABASE IF EXISTS db1 SYNC") node5.query("CREATE DATABASE db1") node5.query( "CREATE TABLE db1.test_table(date Date, k1 String, v1 Int32) ENGINE = MergeTree(date, (k1, date), 8192)" @@ -293,6 +297,9 @@ def test_allow_databases(start_cluster): def test_config_multiple_zk_substitutions(start_cluster): + # NOTE: background_pool_size cannot be decreased, so let's restart ClickHouse to make the test idempotent (i.e. runned multiple times) + node3.restart_clickhouse() + node3.query("SYSTEM RELOAD CONFIG") assert ( node3.query( "SELECT value FROM system.merge_tree_settings WHERE name='min_bytes_for_wide_part'" @@ -319,34 +326,36 @@ def test_config_multiple_zk_substitutions(start_cluster): ) zk = cluster.get_kazoo_client("zoo1") - zk.create( - path="/background_pool_size", - value=b"72", - makepath=True, - ) - - node3.replace_config( - "/etc/clickhouse-server/config.d/config_zk_include_test.xml", - """ - - - 44 - - - 1 - 1111 - - - - -""", - ) - - node3.query("SYSTEM RELOAD CONFIG") - - assert ( - node3.query( - "SELECT value FROM system.server_settings WHERE name='background_pool_size'" + try: + zk.create( + path="/background_pool_size", + value=b"72", + makepath=True, ) - == "72\n" - ) + + with node3.with_replace_config( + "/etc/clickhouse-server/config.d/config_zk_include_test.xml", + """ + + + 44 + + + 1 + 1111 + + + + + """, + ): + node3.query("SYSTEM RELOAD CONFIG") + + assert ( + node3.query( + "SELECT value FROM system.server_settings WHERE name='background_pool_size'" + ) + == "72\n" + ) + finally: + zk.delete(path="/background_pool_size")