Merge pull request #46458 from ClickHouse/fix-install-check

Improve install_check.py

tests/ci/compress_files.py

@@ -6,11 +6,11 @@ import os
 
 def compress_file_fast(path, archive_path):
     if archive_path.endswith(".zst"):
-        subprocess.check_call("zstd < {} > {}".format(path, archive_path), shell=True)
+        subprocess.check_call(f"zstd < {path} > {archive_path}", shell=True)
     elif os.path.exists("/usr/bin/pigz"):
-        subprocess.check_call("pigz < {} > {}".format(path, archive_path), shell=True)
+        subprocess.check_call(f"pigz < {path} > {archive_path}", shell=True)
     else:
-        subprocess.check_call("gzip < {} > {}".format(path, archive_path), shell=True)
+        subprocess.check_call(f"gzip < {path} > {archive_path}", shell=True)
 
 
 def compress_fast(path, archive_path, exclude=None):
@@ -28,9 +28,9 @@ def compress_fast(path, archive_path, exclude=None):
     if exclude is None:
         exclude_part = ""
     elif isinstance(exclude, list):
-        exclude_part = " ".join(["--exclude {}".format(x) for x in exclude])
+        exclude_part = " ".join([f"--exclude {x}" for x in exclude])
     else:
-        exclude_part = "--exclude {}".format(str(exclude))
+        exclude_part = f"--exclude {exclude}"
 
     fname = os.path.basename(path)
     if os.path.isfile(path):
@@ -38,9 +38,7 @@ def compress_fast(path, archive_path, exclude=None):
     else:
         path += "/.."
 
-    cmd = "tar {} {} -cf {} -C {} {}".format(
-        program_part, exclude_part, archive_path, path, fname
-    )
+    cmd = f"tar {program_part} {exclude_part} -cf {archive_path} -C {path} {fname}"
     logging.debug("compress_fast cmd: %s", cmd)
     subprocess.check_call(cmd, shell=True)
 
@@ -70,11 +68,9 @@ def decompress_fast(archive_path, result_path=None):
         )
 
     if result_path is None:
-        subprocess.check_call(
-            "tar {} -xf {}".format(program_part, archive_path), shell=True
-        )
+        subprocess.check_call(f"tar {program_part} -xf {archive_path}", shell=True)
     else:
         subprocess.check_call(
-            "tar {} -xf {} -C {}".format(program_part, archive_path, result_path),
+            f"tar {program_part} -xf {archive_path} -C {result_path}",
             shell=True,
         )

tests/ci/install_check.py

@@ -7,7 +7,7 @@ import logging
 import sys
 import subprocess
 from pathlib import Path
-
+from shutil import copy2
 from typing import Dict
 
 from github import Github
@@ -19,6 +19,7 @@ from clickhouse_helper import (
     prepare_tests_results_for_clickhouse,
 )
 from commit_status_helper import post_commit_status, update_mergeable_check
+from compress_files import compress_fast
 from docker_pull_helper import get_image_with_version, DockerImage
 from env_helper import CI, TEMP_PATH as TEMP, REPORTS_PATH
 from get_robot_token import get_best_robot_token
@@ -34,15 +35,22 @@ from upload_result_helper import upload_results
 RPM_IMAGE = "clickhouse/install-rpm-test"
 DEB_IMAGE = "clickhouse/install-deb-test"
 TEMP_PATH = Path(TEMP)
+LOGS_PATH = TEMP_PATH / "tests_logs"
 SUCCESS = "success"
 FAILURE = "failure"
+OK = "OK"
+FAIL = "FAIL"
 
 
 def prepare_test_scripts():
     server_test = r"""#!/bin/bash
+set -e
+trap "bash -ex /packages/preserve_logs.sh" ERR
 systemctl start clickhouse-server
 clickhouse-client -q 'SELECT version()'"""
     keeper_test = r"""#!/bin/bash
+set -e
+trap "bash -ex /packages/preserve_logs.sh" ERR
 systemctl start clickhouse-keeper
 for i in {1..20}; do
     echo wait for clickhouse-keeper to being up
@@ -50,15 +58,18 @@ for i in {1..20}; do
 done
 for i in {1..5}; do
     echo wait for clickhouse-keeper to answer on mntr request
-    exec 13<>/dev/tcp/127.0.0.1/9181
-    echo mntr >&13
-    cat <&13 | grep zk_version && break || sleep 1
+    {
+        exec 13<>/dev/tcp/127.0.0.1/9181
+        echo mntr >&13
+        cat <&13 | grep zk_version
+    } && break || sleep 1
     exec 13>&-
 done
 exec 13>&-"""
     binary_test = r"""#!/bin/bash
-chmod +x /packages/clickhouse
-/packages/clickhouse install
+set -e
+trap "bash -ex /packages/preserve_logs.sh" ERR
+/packages/clickhouse.copy install
 clickhouse-server start --daemon
 for i in {1..5}; do
     clickhouse-client -q 'SELECT version()' && break || sleep 1
@@ -70,15 +81,26 @@ for i in {1..20}; do
 done
 for i in {1..5}; do
     echo wait for clickhouse-keeper to answer on mntr request
-    exec 13<>/dev/tcp/127.0.0.1/9181
-    echo mntr >&13
-    cat <&13 | grep zk_version && break || sleep 1
+    {
+        exec 13<>/dev/tcp/127.0.0.1/9181
+        echo mntr >&13
+        cat <&13 | grep zk_version
+    } && break || sleep 1
     exec 13>&-
 done
 exec 13>&-"""
+    preserve_logs = r"""#!/bin/bash
+journalctl -u clickhouse-server > /tests_logs/clickhouse-server.service || :
+journalctl -u clickhouse-keeper > /tests_logs/clickhouse-keeper.service || :
+cp /var/log/clickhouse-server/clickhouse-server.* /tests_logs/ || :
+cp /var/log/clickhouse-keeper/clickhouse-keeper.* /tests_logs/ || :
+chmod a+rw -R /tests_logs
+exit 1
+"""
     (TEMP_PATH / "server_test.sh").write_text(server_test, encoding="utf-8")
     (TEMP_PATH / "keeper_test.sh").write_text(keeper_test, encoding="utf-8")
     (TEMP_PATH / "binary_test.sh").write_text(binary_test, encoding="utf-8")
+    (TEMP_PATH / "preserve_logs.sh").write_text(preserve_logs, encoding="utf-8")
 
 
 def test_install_deb(image: DockerImage) -> TestResults:
@@ -143,27 +165,41 @@ def test_install(image: DockerImage, tests: Dict[str, str]) -> TestResults:
         stopwatch = Stopwatch()
         container_name = name.lower().replace(" ", "_").replace("/", "_")
         log_file = TEMP_PATH / f"{container_name}.log"
+        logs = [log_file]
         run_command = (
             f"docker run --rm --privileged --detach --cap-add=SYS_PTRACE "
-            f"--volume={TEMP_PATH}:/packages {image}"
+            f"--volume={LOGS_PATH}:/tests_logs --volume={TEMP_PATH}:/packages {image}"
         )
-        logging.info("Running docker container: `%s`", run_command)
-        container_id = subprocess.check_output(
-            run_command, shell=True, encoding="utf-8"
-        ).strip()
-        (TEMP_PATH / "install.sh").write_text(command)
-        install_command = f"docker exec {container_id} bash -ex /packages/install.sh"
-        with TeePopen(install_command, log_file) as process:
-            retcode = process.wait()
-            if retcode == 0:
-                status = SUCCESS
-            else:
-                status = FAILURE
+
+        for retry in range(1, 4):
+            for file in LOGS_PATH.glob("*"):
+                file.unlink()
+
+            logging.info("Running docker container: `%s`", run_command)
+            container_id = subprocess.check_output(
+                run_command, shell=True, encoding="utf-8"
+            ).strip()
+            (TEMP_PATH / "install.sh").write_text(command)
+            install_command = (
+                f"docker exec {container_id} bash -ex /packages/install.sh"
+            )
+            with TeePopen(install_command, log_file) as process:
+                retcode = process.wait()
+                if retcode == 0:
+                    status = OK
+                    break
+
+                status = FAIL
+            copy2(log_file, LOGS_PATH)
+            archive_path = TEMP_PATH / f"{container_name}-{retry}.tar.gz"
+            compress_fast(
+                LOGS_PATH.as_posix(),
+                archive_path.as_posix(),
+            )
+            logs.append(archive_path)
 
         subprocess.check_call(f"docker kill -s 9 {container_id}", shell=True)
-        test_results.append(
-            TestResult(name, status, stopwatch.duration_seconds, [log_file])
-        )
+        test_results.append(TestResult(name, status, stopwatch.duration_seconds, logs))
 
     return test_results
 
@@ -222,6 +258,7 @@ def main():
     args = parse_args()
 
     TEMP_PATH.mkdir(parents=True, exist_ok=True)
+    LOGS_PATH.mkdir(parents=True, exist_ok=True)
 
     pr_info = PRInfo()
 
@@ -245,18 +282,30 @@ def main():
     if args.download:
 
         def filter_artifacts(path: str) -> bool:
-            return (
-                path.endswith(".deb")
-                or path.endswith(".rpm")
-                or path.endswith(".tgz")
-                or path.endswith("/clickhouse")
-            )
+            is_match = False
+            if args.deb or args.rpm:
+                is_match = is_match or path.endswith("/clickhouse")
+            if args.deb:
+                is_match = is_match or path.endswith(".deb")
+            if args.rpm:
+                is_match = is_match or path.endswith(".rpm")
+            if args.tgz:
+                is_match = is_match or path.endswith(".tgz")
+            return is_match
 
         download_builds_filter(
             args.check_name, REPORTS_PATH, TEMP_PATH, filter_artifacts
         )
 
     test_results = []  # type: TestResults
+    ch_binary = Path(TEMP_PATH) / "clickhouse"
+    if ch_binary.exists():
+        # make a copy of clickhouse to avoid redownload of exctracted binary
+        ch_binary.chmod(0o755)
+        ch_copy = ch_binary.parent / "clickhouse.copy"
+        copy2(ch_binary, ch_binary.parent / "clickhouse.copy")
+        subprocess.check_output(f"{ch_copy.absolute()} local -q 'SELECT 1'", shell=True)
+
     if args.deb:
         test_results.extend(test_install_deb(docker_images[DEB_IMAGE]))
     if args.rpm:
@@ -266,9 +315,11 @@ def main():
         test_results.extend(test_install_tgz(docker_images[RPM_IMAGE]))
 
     state = SUCCESS
+    test_status = OK
     description = "Packages installed successfully"
-    if FAILURE in (result.status for result in test_results):
+    if FAIL in (result.status for result in test_results):
         state = FAILURE
+        test_status = FAIL
         description = "Failed to install packages: " + ", ".join(
             result.name for result in test_results
         )
@@ -298,7 +349,7 @@ def main():
     prepared_events = prepare_tests_results_for_clickhouse(
         pr_info,
         test_results,
-        state,
+        test_status,
         stopwatch.duration_seconds,
         stopwatch.start_time_str,
         report_url,