Merge branch 'MDB-25320-move_clickhouse_cleanup' into MDB-25318-clean…

…up-after-replicated-databases

ch_tools/chadmin/cli/zookeeper_group.py

@@ -7,7 +7,7 @@
 from ch_tools.chadmin.internal.table_replica import get_table_replica
 from ch_tools.chadmin.internal.zookeeper import (
     check_zk_node,
-    clean_zk_nodes,
+    clean_zk_metadata_for_hosts,
     create_zk_nodes,
     delete_zk_nodes,
     get_zk_node,
@@ -42,8 +42,19 @@
     help="Do not try to get parameters from clickhouse config.xml.",
     default=False,
 )
+@option(
+    "-c",
+    "--chroot",
+    "zk_root_path",
+    type=str,
+    help="Cluster ZooKeeper root path. If not specified,the root path will be used.",
+    required=False,
+    default="/",
+)
 @pass_context
-def zookeeper_group(ctx, host, port, timeout, zkcli_identity, no_chroot, no_ch_config):
+def zookeeper_group(
+    ctx, host, port, timeout, zkcli_identity, no_chroot, no_ch_config, zk_root_path
+):
     """ZooKeeper management commands.
 
     ZooKeeper command runs client which connects to Zookeeper node.
@@ -58,6 +69,7 @@ def zookeeper_group(ctx, host, port, timeout, zkcli_identity, no_chroot, no_ch_c
         "zkcli_identity": zkcli_identity,
         "no_chroot": no_chroot,
         "no_ch_config": no_ch_config,
+        "zk_root_path": zk_root_path,
     }
 
 
@@ -269,18 +281,11 @@ def delete_ddl_task_command(ctx, tasks):
     delete_zk_nodes(ctx, paths)
 
 
-@zookeeper_group.command(name="clickhouse-hosts-cleanup")
-@option("-f", "--fqdn", type=str, help="Removed FQDNs, comma separated", required=True)
-@option(
-    "-c",
-    "--root",
-    "zk_root_path",
-    type=str,
-    help="Cluster ZooKeeper root path. If not specified,the root path will be used.",
-    required=False,
-    default="/",
+@zookeeper_group.command(
+    name="cleanup-removed-hosts-metadata",
+    help="Remove metadata from Zookeeper for specified hosts.",
 )
+@argument("fqdn", type=ListParamType())
 @pass_context
-def clickhouse_hosts_command(ctx, fqdn, zk_root_path):
-    hosts = fqdn.split(",")
-    clean_zk_nodes(ctx, zk_root_path, hosts)
+def clickhouse_hosts_command(ctx, fqdn):
+    clean_zk_metadata_for_hosts(ctx, fqdn)

ch_tools/chadmin/internal/zookeeper.py

@@ -148,7 +148,7 @@ def _find_paths(zk, root_path, included_paths_regexp, excluded_paths_regexp=None
     return list(paths)
 
 
-def clean_zk_nodes(ctx, zk_root_path, nodes):
+def clean_zk_metadata_for_hosts(ctx, nodes):
     """
     Perform cleanup in zookeeper after deleting hosts in the cluster or whole cluster deleting.
     """
@@ -172,7 +172,11 @@ def _find_parents(zk, root_path, nodes, parent_name):
         ]
         included_paths = [".*/" + parent_name + "/" + node for node in nodes]
         paths = _find_paths(zk, root_path, included_paths, excluded_paths)
-        return [os.sep.join(path.split(os.sep)[:-2]) for path in paths]
+        # Paths will be like */shard1/replicas/hostname. But we need */shard1.
+        # Go up for 2 directories.
+        paths = [os.sep.join(path.split(os.sep)[:-2]) for path in paths]
+        # One path might be in list several times. Make list unique.
+        return list(set(paths))
 
     def _set_replicas_is_lost(zk, table_paths, nodes):
         """
@@ -223,7 +227,7 @@ def _absent_if_empty_child(zk, path, child):
         ):
             _try_delete_zk_node(zk, path)
 
-    zk_root_path = _format_path(ctx, zk_root_path)
+    zk_root_path = _format_path(ctx, "/")
     with zk_client(ctx) as zk:
         table_paths = _find_parents(zk, zk_root_path, nodes, "replicas")
         _set_replicas_is_lost(zk, table_paths, nodes)
@@ -254,6 +258,7 @@ def _get_zk_client(ctx):
     zkcli_identity = args.get("zkcli_identity")
     no_chroot = args.get("no_chroot", False)
     no_ch_config = args.get("no_ch_config", False)
+    zk_root_path = args.get("zk_root_path", None)
 
     if no_ch_config:
         if not host:
@@ -267,7 +272,9 @@ def _get_zk_client(ctx):
             f'{host if host else node["host"]}:{port if port else node["port"]}'
             for node in zk_config.nodes
         )
-        if not no_chroot and zk_config.root is not None:
+        if not zk_root_path:
+            connect_str += zk_root_path
+        elif not no_chroot and zk_config.root is not None:
             connect_str += zk_config.root
 
         if zkcli_identity is None:

tests/modules/chadmin.py

@@ -1,10 +1,13 @@
+import logging
+
+
 class Chadmin:
     def __init__(self, container):
         self._container = container
 
     def exec_cmd(self, cmd):
         ch_admin_cmd = f"chadmin {cmd}"
-        print("CMD: " + ch_admin_cmd)
+        logging.debug("chadmin command:", ch_admin_cmd)
         result = self._container.exec_run(["bash", "-c", ch_admin_cmd], user="root")
         return result
 
@@ -24,7 +27,7 @@ def zk_list(self, zk_node, no_ch_config=True):
         return self.exec_cmd(cmd)
 
     def zk_cleanup(self, fqdn, zk_root, no_ch_config=True):
-        cmd = "zookeeper {use_config} clickhouse-hosts-cleanup --root {root} --fqdn {hosts}".format(
+        cmd = "zookeeper {use_config} --chroot {root} cleanup-removed-hosts-metadata {hosts}".format(
             use_config="--no-ch-config" if no_ch_config else "",
             root=zk_root,
             hosts=fqdn,