Refactoring

ch_tools/chadmin/cli/object_storage_group.py

@@ -177,28 +177,15 @@ def _store_state_zk_save(
         create_zk_nodes(ctx, [path], make_parents=True)
 
     state_data = json.dumps(
-        {
-            ORPHANED_OBJECTS_SIZE_FIELD: total_size,
-            ORPHANED_OBJECTS_ERROR_MSG_FIELD: error_msg,
-        },
-        indent=4,
+        create_orphaned_objects_state(total_size, error_msg), indent=4
     )
 
     update_zk_nodes(ctx, [path], state_data.encode("utf-8"))
 
 
-def _store_state_local_save(
-    _: Context, total_size: int, error_msg: Optional[str]
-) -> None:
+def _store_state_local_save(_: Context, total_size: int, error_msg: str) -> None:
     with open(STATE_LOCAL_PATH, "w", encoding="utf-8") as file:
-        json.dump(
-            {
-                ORPHANED_OBJECTS_SIZE_FIELD: total_size,
-                ORPHANED_OBJECTS_ERROR_MSG_FIELD: error_msg,
-            },
-            file,
-            indent=4,
-        )
+        json.dump(create_orphaned_objects_state(total_size, error_msg), file, indent=4)
 
 
 def _print_response(ctx: Context, dry_run: bool, deleted: int, total_size: int) -> None:
@@ -224,3 +211,10 @@ def _table_formatter(stats):
     print_response(
         ctx, clean_stats, default_format="table", table_formatter=_table_formatter
     )
+
+
+def create_orphaned_objects_state(total_size: int, error_msg: str) -> dict:
+    return {
+        ORPHANED_OBJECTS_SIZE_FIELD: total_size,
+        ORPHANED_OBJECTS_ERROR_MSG_FIELD: error_msg,
+    }

ch_tools/monrun_checks/ch_orphaned_objects.py

@@ -1,11 +1,13 @@
 import json
+from typing import Tuple
 
 import click
 
 from ch_tools.chadmin.cli.object_storage_group import (
     ORPHANED_OBJECTS_ERROR_MSG_FIELD,
     ORPHANED_OBJECTS_SIZE_FIELD,
     STATE_LOCAL_PATH,
+    create_orphaned_objects_state,
 )
 from ch_tools.chadmin.internal.zookeeper import get_zk_node
 from ch_tools.common.result import CRIT, OK, WARNING, Result
@@ -49,34 +51,25 @@ def orphaned_objects_command(
 ) -> Result:
     _check_mutually_exclusive(state_local, state_zk_path)
 
-    state = dict()
-    if state_local:
-        state = _local_get_orphaned_objects_state()
-
-    if state_zk_path:
-        state = _zk_get_orphaned_objects_state(ctx, state_zk_path)
-
-    total_size = state.get(ORPHANED_OBJECTS_SIZE_FIELD)
-    error_msg = state.get(ORPHANED_OBJECTS_ERROR_MSG_FIELD)
-
-    msg = ""
-    if total_size is None:
-        msg += f'Orphaned objects state not have field "{ORPHANED_OBJECTS_SIZE_FIELD}".'
-    if error_msg is None:
-        msg += f'Orphaned objects state not have field "{ORPHANED_OBJECTS_ERROR_MSG_FIELD}".'
+    state = _get_orphaned_objects_state(ctx, state_local, state_zk_path)
 
-    if msg != "":
+    valid, msg = _orphaned_objects_state_validate(state)
+    if not valid:
         return Result(CRIT, msg)
 
+    total_size = state[ORPHANED_OBJECTS_SIZE_FIELD]
+    error_msg = state[ORPHANED_OBJECTS_ERROR_MSG_FIELD]
+
     if error_msg != "":
         return Result(CRIT, error_msg)
 
-    msg = f"Total size: {total_size}"
-    if isinstance(total_size, int) and total_size >= crit:
-        return Result(CRIT, msg)
-    if isinstance(total_size, int) and total_size >= warn:
-        return Result(WARNING, msg)
-    return Result(OK, msg)
+    result_msg = f"Total size: {total_size}"
+    if total_size >= crit:
+        return Result(CRIT, result_msg)
+    if total_size >= warn:
+        return Result(WARNING, result_msg)
+
+    return Result(OK, result_msg)
 
 
 def _check_mutually_exclusive(state_local, state_zk_path):
@@ -91,22 +84,43 @@ def _check_mutually_exclusive(state_local, state_zk_path):
         )
 
 
+def _orphaned_objects_state_validate(state: dict) -> Tuple[bool, str]:
+    total_size = state.get(ORPHANED_OBJECTS_SIZE_FIELD)
+    error_msg = state.get(ORPHANED_OBJECTS_ERROR_MSG_FIELD)
+
+    msg = ""
+    if total_size is None:
+        msg += f'Orphaned objects state not have field "{ORPHANED_OBJECTS_SIZE_FIELD}".'
+    if error_msg is None:
+        msg += f'Orphaned objects state not have field "{ORPHANED_OBJECTS_ERROR_MSG_FIELD}".'
+
+    return msg == "", msg
+
+
+def _get_orphaned_objects_state(
+    ctx: click.Context, state_local: bool, state_zk_path: str
+) -> dict:
+    state = dict()
+
+    if state_local:
+        state = _local_get_orphaned_objects_state()
+
+    if state_zk_path:
+        state = _zk_get_orphaned_objects_state(ctx, state_zk_path)
+
+    return state
+
+
 def _local_get_orphaned_objects_state() -> dict:
     try:
         with open(STATE_LOCAL_PATH, mode="r", encoding="utf-8") as file:
             return json.load(file)
     except Exception as e:
-        return {
-            ORPHANED_OBJECTS_SIZE_FIELD: 0,
-            ORPHANED_OBJECTS_ERROR_MSG_FIELD: str(e),
-        }
+        return create_orphaned_objects_state(0, str(e))
 
 
 def _zk_get_orphaned_objects_state(ctx: click.Context, state_zk_path: str) -> dict:
     try:
         return json.loads(get_zk_node(ctx, state_zk_path))
     except Exception as e:
-        return {
-            ORPHANED_OBJECTS_SIZE_FIELD: 0,
-            ORPHANED_OBJECTS_ERROR_MSG_FIELD: str(e),
-        }
+        return create_orphaned_objects_state(0, str(e))

tests/features/monrun.feature

@@ -502,3 +502,14 @@ Feature: ch-monitoring tool
     """
     1;Unknown error: One of these options must be provided: --state-local, --state-zk-path
     """
+
+  Scenario: Check clickhouse orphaned objects with not empty error_msg
+    When we create file /tmp/object_storage_cleanup_state.json with data "{ \"orphaned_objects_size\": 0,  \"error_msg\": \"ERROR\" }"
+    And we execute command on clickhouse01
+    """
+    ch-monitoring orphaned-objects --state-local
+    """
+    Then we get response
+    """
+    2;ERROR
+    """

tests/steps/common.py

@@ -88,15 +88,21 @@ def step_get_response_not_contains(context, entry):
     assert_that(context.response, is_not(contains_string(entry)))  # type: ignore
 
 
+@when("we create file {file_path} with data '{data}'")
+def step_create_file(context, file_path, data):
+    container = docker.get_container(context, "clickhouse01")
+    result = container.exec_run(
+        ["bash", "-c", f'echo "{data}" > {file_path}'], user="root"
+    )
+    assert result.exit_code == 0
+
+
 @then("we get file {file_path}")
 def step_get_file(context, file_path):
     container = docker.get_container(context, "clickhouse01")
-    context.command = context.text.strip()
     result = container.exec_run(["bash", "-c", f"cat {file_path}"], user="root")
     context.response = result.output.decode().strip()
-    context.exit_code = result.exit_code
-    if context.exit_code != 0:
-        assert_that("", equal_to(context.text))
+    assert result.exit_code == 0
     assert_that(context.response, equal_to(context.text))