mirror of
https://github.com/krkn-chaos/krkn.git
synced 2026-08-25 09:27:36 +00:00
fix: skip unnecessary wait_duration sleep after the last scenario (#1558)
* fix: skip unnecessary wait_duration sleep after the last scenario The wait_duration sleep was running unconditionally after every scenario, including the final one. This wasted up to 60s (default) on single-scenario runs where there is no next scenario to wait for. Closes #1556 Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix: use wait_duration as soak window for SLO evaluation Move end_timestamp capture to after the wait_duration sleep so the resiliency score / SLO evaluation window includes the settling period. This ensures delayed impacts during cooldown are captured in metrics. The sleep now runs for every scenario (including the last) since it serves as a measurement soak window, not just an inter-scenario delay. Closes krkn-chaos#1556 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:
@@ -123,7 +123,6 @@ class AbstractScenarioPlugin(ABC):
|
||||
logging.info(
|
||||
f"Running {self.__class__.__name__}: {self.get_scenario_types()} -> {scenario_config}"
|
||||
)
|
||||
# pass all the parameters by kwargs to make `set_rollback_context_decorator` get the `run_uuid` and `scenario_type`
|
||||
return_value = self.run(
|
||||
run_uuid=run_uuid,
|
||||
scenario=scenario_config,
|
||||
@@ -142,11 +141,16 @@ class AbstractScenarioPlugin(ABC):
|
||||
run_uuid, scenario_telemetry.scenario_type
|
||||
)
|
||||
else:
|
||||
# execute rollback files based on the return value
|
||||
execute_rollback_version_files(
|
||||
telemetry, run_uuid, scenario_telemetry.scenario_type
|
||||
)
|
||||
scenario_telemetry.exit_status = return_value
|
||||
|
||||
logging.info(
|
||||
f"waiting {wait_duration}s for cluster to stabilize "
|
||||
f"before collecting metrics"
|
||||
)
|
||||
time.sleep(wait_duration)
|
||||
scenario_telemetry.end_timestamp = time.time()
|
||||
start_time = int(scenario_telemetry.start_timestamp)
|
||||
end_time = int(scenario_telemetry.end_timestamp)
|
||||
@@ -182,10 +186,8 @@ class AbstractScenarioPlugin(ABC):
|
||||
if scenario_telemetry.exit_status != 0:
|
||||
failed_scenarios.append(scenario_config)
|
||||
scenario_telemetries.append(scenario_telemetry)
|
||||
cerberus.publish_kraken_status(start_time,end_time)
|
||||
logging.info(f"waiting {wait_duration} before running the next scenario")
|
||||
time.sleep(wait_duration)
|
||||
|
||||
cerberus.publish_kraken_status(start_time, end_time)
|
||||
|
||||
return failed_scenarios, scenario_telemetries
|
||||
|
||||
|
||||
|
||||
@@ -353,6 +353,82 @@ class TestAbstractScenarioPluginCerberusIntegration(unittest.TestCase):
|
||||
self.assertEqual(telemetries[0].exit_status, 1)
|
||||
|
||||
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.cerberus.publish_kraken_status')
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.cleanup_rollback_version_files')
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.utils.collect_and_put_ocp_logs')
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.signal_handler.signal_context')
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.os.path.exists', return_value=True)
|
||||
@patch('time.sleep')
|
||||
@patch('time.time')
|
||||
def test_end_timestamp_includes_wait_duration_soak(
|
||||
self, mock_time, mock_sleep, mock_exists, mock_signal_ctx, mock_collect_logs,
|
||||
mock_cleanup, mock_cerberus_publish
|
||||
):
|
||||
"""Test that end_timestamp is captured AFTER wait_duration sleep (soak window)"""
|
||||
mock_signal_ctx.return_value.__enter__ = Mock()
|
||||
mock_signal_ctx.return_value.__exit__ = Mock(return_value=False)
|
||||
|
||||
# Simulate: start=1000, scenario ends, sleep 60s, end_timestamp=1060
|
||||
time_values = iter([1000.0, 1060.0])
|
||||
mock_time.side_effect = lambda: next(time_values)
|
||||
|
||||
krkn_config = {
|
||||
"tunings": {"wait_duration": 60},
|
||||
"telemetry": {"events_backup": False}
|
||||
}
|
||||
|
||||
scenarios_list = ["scenario1.yaml"]
|
||||
|
||||
failed_scenarios, telemetries = self.plugin.run_scenarios(
|
||||
"test-uuid",
|
||||
scenarios_list,
|
||||
krkn_config,
|
||||
self.mock_telemetry
|
||||
)
|
||||
|
||||
# sleep should be called with the configured wait_duration
|
||||
mock_sleep.assert_called_once_with(60)
|
||||
# end_timestamp should reflect post-soak time (1060), not pre-soak
|
||||
self.assertEqual(telemetries[0].start_timestamp, 1000.0)
|
||||
self.assertEqual(telemetries[0].end_timestamp, 1060.0)
|
||||
# SLO window should span the full 60s soak period
|
||||
call_args = mock_cerberus_publish.call_args[0]
|
||||
self.assertEqual(call_args[0], 1000)
|
||||
self.assertEqual(call_args[1], 1060)
|
||||
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.cerberus.publish_kraken_status')
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.cleanup_rollback_version_files')
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.utils.collect_and_put_ocp_logs')
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.signal_handler.signal_context')
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.os.path.exists', return_value=True)
|
||||
@patch('time.sleep')
|
||||
def test_wait_duration_runs_for_every_scenario_including_last(
|
||||
self, mock_sleep, mock_exists, mock_signal_ctx, mock_collect_logs,
|
||||
mock_cleanup, mock_cerberus_publish
|
||||
):
|
||||
"""Test that wait_duration sleep runs for ALL scenarios (soak window, not just inter-scenario delay)"""
|
||||
mock_signal_ctx.return_value.__enter__ = Mock()
|
||||
mock_signal_ctx.return_value.__exit__ = Mock(return_value=False)
|
||||
|
||||
krkn_config = {
|
||||
"tunings": {"wait_duration": 30},
|
||||
"telemetry": {"events_backup": False}
|
||||
}
|
||||
|
||||
scenarios_list = ["scenario1.yaml", "scenario2.yaml", "scenario3.yaml"]
|
||||
|
||||
self.plugin.run_scenarios(
|
||||
"test-uuid",
|
||||
scenarios_list,
|
||||
krkn_config,
|
||||
self.mock_telemetry
|
||||
)
|
||||
|
||||
# sleep should be called once per scenario (including the last)
|
||||
self.assertEqual(mock_sleep.call_count, 3)
|
||||
for call in mock_sleep.call_args_list:
|
||||
self.assertEqual(call[0][0], 30)
|
||||
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.cerberus.publish_kraken_status')
|
||||
@patch('krkn.scenario_plugins.abstract_scenario_plugin.os.path.exists', return_value=False)
|
||||
@patch('time.sleep')
|
||||
|
||||
Reference in New Issue
Block a user