mirror of
https://github.com/krkn-chaos/krkn.git
synced 2026-08-25 09:27:36 +00:00
fix: improve logging clarity , reduce noise, fix log levels, and reword misleading messages (#1562)
* fix: improve logging clarity — reduce noise, fix log levels, and reword misleading messages - Move per-directory rollback WARNING to DEBUG; add single INFO summary - Change "No rollback context directories found" from WARNING to DEBUG - Change "Skip cleanup/execution" from WARNING to DEBUG - Fix misleading Cerberus "still failing" message: early-return when disabled, use correct log levels, remove inaccurate wording - Change "No error logs collected" from INFO to DEBUG - Reword "Detected distribution" to "Detected cluster platform" - Reword OCP metadata collection log for clarity Closes #1559 Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address review comments — fix tests, reword log messages - Add cerberus_enabled = True to 4 publish_kraken_status tests broken by early-return guard - Add new test for cerberus_disabled early-return path - Reword rollback skip summary to neutral wording - Remove capitalize() from platform detection log Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
+13
-13
@@ -91,34 +91,34 @@ def get_status(start_time, end_time):
|
||||
return cerberus_status
|
||||
|
||||
|
||||
def publish_kraken_status( start_time, end_time):
|
||||
def publish_kraken_status(start_time, end_time):
|
||||
"""
|
||||
Publish kraken status to cerberus
|
||||
Check cerberus health status after a scenario run and act on exit_on_failure.
|
||||
"""
|
||||
if not cerberus_enabled:
|
||||
return
|
||||
|
||||
cerberus_status = get_status(start_time, end_time)
|
||||
if not cerberus_status:
|
||||
if exit_on_failure:
|
||||
logging.info(
|
||||
"Cerberus status is not healthy and post action scenarios "
|
||||
"are still failing, exiting kraken run"
|
||||
logging.error(
|
||||
"Cerberus reports cluster is NOT healthy, exiting kraken run"
|
||||
)
|
||||
sys.exit(1)
|
||||
else:
|
||||
logging.info(
|
||||
"Cerberus status is not healthy and post action scenarios "
|
||||
"are still failing"
|
||||
logging.warning(
|
||||
"Cerberus reports cluster is NOT healthy"
|
||||
)
|
||||
else:
|
||||
if exit_on_failure:
|
||||
logging.info(
|
||||
"Cerberus status is healthy but post action scenarios "
|
||||
"are still failing, exiting kraken run"
|
||||
logging.warning(
|
||||
"Cerberus reports cluster is healthy, "
|
||||
"but exit_on_failure is set — exiting kraken run"
|
||||
)
|
||||
sys.exit(1)
|
||||
else:
|
||||
logging.info(
|
||||
"Cerberus status is healthy but post action scenarios "
|
||||
"are still failing"
|
||||
"Cerberus reports cluster is healthy"
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -221,14 +221,19 @@ class RollbackConfig(metaclass=SingletonMeta):
|
||||
return []
|
||||
|
||||
rollback_context_directories = []
|
||||
skipped_count = 0
|
||||
for dir in os.listdir(cls().versions_directory):
|
||||
if cls.is_rollback_context_directory_format(dir, run_uuid):
|
||||
rollback_context_directories.append(dir)
|
||||
else:
|
||||
logger.warning(f"Directory {dir} does not match expected pattern of <timestamp>-<run_uuid>")
|
||||
skipped_count += 1
|
||||
logger.debug(f"Directory {dir} does not match expected pattern of <timestamp>-<run_uuid>")
|
||||
|
||||
if skipped_count > 0:
|
||||
logger.info(f"Skipped {skipped_count} non-matching entries in rollback versions directory")
|
||||
|
||||
if not rollback_context_directories:
|
||||
logger.warning(f"No rollback context directories found for run UUID {run_uuid}")
|
||||
logger.debug(f"No rollback context directories found for run UUID {run_uuid}")
|
||||
return []
|
||||
|
||||
|
||||
|
||||
@@ -155,7 +155,7 @@ def execute_rollback_version_files(
|
||||
# Get the rollback versions directory
|
||||
version_files = RollbackConfig.search_rollback_version_files(run_uuid, scenario_type)
|
||||
if not version_files:
|
||||
logger.warning(f"Skip execution for run_uuid={run_uuid or '*'}, scenario_type={scenario_type or '*'}")
|
||||
logger.debug(f"Skip execution for run_uuid={run_uuid or '*'}, scenario_type={scenario_type or '*'}")
|
||||
return
|
||||
|
||||
# Execute all version files in the directory
|
||||
@@ -206,7 +206,7 @@ def cleanup_rollback_version_files(run_uuid: str, scenario_type: str):
|
||||
# Get the rollback versions directory
|
||||
version_files = RollbackConfig.search_rollback_version_files(run_uuid, scenario_type)
|
||||
if not version_files:
|
||||
logger.warning(f"Skip cleanup for run_uuid={run_uuid}, scenario_type={scenario_type or '*'}")
|
||||
logger.debug(f"Skip cleanup for run_uuid={run_uuid}, scenario_type={scenario_type or '*'}")
|
||||
return
|
||||
|
||||
# Remove all version files in the directory
|
||||
|
||||
+3
-3
@@ -268,7 +268,7 @@ def main(options, command: Optional[str], out: Optional[dict] = None) -> int:
|
||||
distribution = "kubernetes"
|
||||
if ocpcli.is_openshift():
|
||||
distribution = "openshift"
|
||||
logging.info("Detected distribution %s" % (distribution))
|
||||
logging.info("Detected cluster platform: %s" % (distribution))
|
||||
|
||||
# find node kraken might be running on
|
||||
kubecli.find_kraken_node()
|
||||
@@ -599,7 +599,7 @@ def main(options, command: Optional[str], out: Optional[dict] = None) -> int:
|
||||
# through OCP specific APIs
|
||||
if distribution == "openshift":
|
||||
logging.info(
|
||||
"collecting OCP cluster metadata, this may take few minutes...."
|
||||
"Collecting OCP cluster metadata (nodes, resources, network plugins)..."
|
||||
)
|
||||
telemetry_ocp.collect_cluster_metadata(chaos_telemetry)
|
||||
else:
|
||||
@@ -611,7 +611,7 @@ def main(options, command: Optional[str], out: Optional[dict] = None) -> int:
|
||||
logging.info(f"Collected {len(error_logs)} error logs for telemetry")
|
||||
chaos_telemetry.error_logs = error_logs
|
||||
else:
|
||||
logging.info("No error logs collected during chaos run")
|
||||
logging.debug("No error logs collected during chaos run")
|
||||
chaos_telemetry.error_logs = []
|
||||
if resiliency_obj and hist_window is None:
|
||||
try:
|
||||
|
||||
@@ -160,9 +160,19 @@ class TestCerberusSetup(unittest.TestCase):
|
||||
|
||||
self.assertEqual(cm.exception.code, 1)
|
||||
|
||||
@patch('krkn.cerberus.setup.get_status')
|
||||
def test_publish_kraken_status_cerberus_disabled_skips(self, mock_get_status):
|
||||
"""Test publish_kraken_status returns immediately when cerberus is disabled"""
|
||||
cerberus_setup.cerberus_enabled = False
|
||||
|
||||
cerberus_setup.publish_kraken_status(0, 100)
|
||||
|
||||
mock_get_status.assert_not_called()
|
||||
|
||||
@patch('krkn.cerberus.setup.get_status')
|
||||
def test_publish_kraken_status_healthy_exit_on_failure_false(self, mock_get_status):
|
||||
"""Test publish_kraken_status when cluster is healthy and exit_on_failure is False"""
|
||||
cerberus_setup.cerberus_enabled = True
|
||||
cerberus_setup.exit_on_failure = False
|
||||
mock_get_status.return_value = True
|
||||
|
||||
@@ -173,6 +183,7 @@ class TestCerberusSetup(unittest.TestCase):
|
||||
@patch('krkn.cerberus.setup.get_status')
|
||||
def test_publish_kraken_status_healthy_exit_on_failure_true(self, mock_get_status):
|
||||
"""Test publish_kraken_status when cluster is healthy and exit_on_failure is True"""
|
||||
cerberus_setup.cerberus_enabled = True
|
||||
cerberus_setup.exit_on_failure = True
|
||||
mock_get_status.return_value = True
|
||||
|
||||
@@ -185,6 +196,7 @@ class TestCerberusSetup(unittest.TestCase):
|
||||
@patch('krkn.cerberus.setup.get_status')
|
||||
def test_publish_kraken_status_unhealthy_exit_on_failure_false(self, mock_get_status):
|
||||
"""Test publish_kraken_status when cluster is unhealthy and exit_on_failure is False"""
|
||||
cerberus_setup.cerberus_enabled = True
|
||||
cerberus_setup.exit_on_failure = False
|
||||
mock_get_status.return_value = False
|
||||
|
||||
@@ -195,6 +207,7 @@ class TestCerberusSetup(unittest.TestCase):
|
||||
@patch('krkn.cerberus.setup.get_status')
|
||||
def test_publish_kraken_status_unhealthy_exit_on_failure_true(self, mock_get_status):
|
||||
"""Test publish_kraken_status when cluster is unhealthy and exit_on_failure is True"""
|
||||
cerberus_setup.cerberus_enabled = True
|
||||
cerberus_setup.exit_on_failure = True
|
||||
mock_get_status.return_value = False
|
||||
|
||||
|
||||
Reference in New Issue
Block a user