github/krkn
1
0
Fork 0
mirror of https://github.com/krkn-chaos/krkn.git synced 2026-08-25 09:27:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: validate container_name in container scenario plugin (#1409) (#1410)

Browse Source 
* fix: validate container_name in container scenario plugin

The container scenario plugin removed pods and incremented killed_count
even when the requested container_name was never found, causing scenarios
to silently report success with no disruption (issue #1409).

Track whether a container was actually found and killed; only increment
killed_count on a real kill, skip pods that lack the target container,
and raise a clear RuntimeError once all pods are exhausted without a kill.

Adds unit tests covering invalid, valid, empty, heterogeneous, and
count-exceeds-target scenarios.

Closes #1409

* fix: report actual kill count in container-not-found error

When the candidate pod list is exhausted without finding the target
container, the RuntimeError now reports how many containers were
actually killed ("N of M requested container(s) were killed") instead
of always claiming "No containers were killed", which was inaccurate in
partial-success cases.

* fix: only raise container-not-found error when nothing was killed

Address review feedback: the "not found in any matching pod" error was
raised even after one or more containers had already been killed (when
count exceeds the number of pods containing the target), making the
message contradictory.

Now that error only fires when killed_count == 0. When some kills
already happened but the candidate list is exhausted, the loop falls
through to the existing "Trying to kill more containers than were found"
error, which accurately describes that case.

---------

Co-authored-by: augmentcode[bot] <185243770+augmentcode[bot]@users.noreply.github.com>
Co-authored-by: Darshan Jain <darjain@redhat.com>
This commit is contained in:
augmentcode[bot]
2026-07-01 00:52:01 +05:30
committed by GitHub
co-authored by lnx01 Darshan Jain
parent 55d8bf7062
commit a81d084c93
2 changed files with 138 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
krkn/scenario_plugins/container/container_scenario_plugin.py
+11 -1
View File
@@ -158,6 +158,7 @@ class ContainerScenarioPlugin(AbstractScenarioPlugin):
selected_container_pod = container_pod_list[
random.randint(0, len(container_pod_list) - 1)
]
container_found = False
for c_name in selected_container_pod[2]:
if container_name != "":
if c_name == container_name:
@@ -175,6 +176,7 @@ class ContainerScenarioPlugin(AbstractScenarioPlugin):
c_name,
kubecli,
)
container_found = True
break
else:
killed_container_list.append(
@@ -187,9 +189,17 @@ class ContainerScenarioPlugin(AbstractScenarioPlugin):
c_name,
kubecli,
)
container_found = True
break
container_pod_list.remove(selected_container_pod)
killed_count += 1
if container_found:
killed_count += 1
elif killed_count == 0 and len(container_pod_list) == 0:
logging.error("Scenario " + scenario_name + " failed")
raise RuntimeError(
f"Container '{container_name}' not found in any matching pod. "
f"No containers were killed."
)
logging.info("Scenario " + scenario_name + " successfully injected")
return killed_container_list
tests/test_container_scenario_plugin.py
+127 -1
View File
@@ -10,7 +10,7 @@ Assisted By: Claude Code
"""
import unittest
from unittest.mock import MagicMock
from unittest.mock import MagicMock, patch
from krkn_lib.k8s import KrknKubernetes
from krkn_lib.telemetry.ocp import KrknTelemetryOpenshift
@@ -30,6 +30,43 @@ class TestContainerScenarioPlugin(unittest.TestCase):
"""Clean up after each test to prevent state leakage"""
self.plugin = None
@staticmethod
def _container_obj(name):
"""Build a mock container object exposing a ``name`` attribute."""
container = MagicMock()
container.name = name
return container
def _make_kubecli(self, pod_containers, namespace="test-ns"):
"""
Build a mocked KrknKubernetes whose ``list_pods``/``get_pod_info``
reflect the given ``{pod_name: [container_names]}`` mapping.
"""
kubecli = MagicMock(spec=KrknKubernetes)
kubecli.list_pods.return_value = list(pod_containers.keys())
def _get_pod_info(pod, ns):
info = MagicMock()
info.containers = [
self._container_obj(name) for name in pod_containers[pod]
]
return info
kubecli.get_pod_info.side_effect = _get_pod_info
kubecli.exec_cmd_in_pod.return_value = ""
return kubecli
@staticmethod
def _scenario(container_name="", count=1, namespace="test-ns"):
return {
"name": "test-container-scenario",
"namespace": namespace,
"label_selector": "app=test",
"container_name": container_name,
"count": count,
"action": 1,
}
def test_get_scenario_types(self):
"""
Test get_scenario_types returns correct scenario type
@@ -39,6 +76,95 @@ class TestContainerScenarioPlugin(unittest.TestCase):
self.assertEqual(result, ["container_scenarios"])
self.assertEqual(len(result), 1)
def test_invalid_container_name_raises(self):
"""Invalid container name (absent in all pods) must raise RuntimeError."""
kubecli = self._make_kubecli(
{"pod1": ["c1", "c2"], "pod2": ["c1", "c2"]}
)
scenario = self._scenario(container_name="nonexistent", count=1)
with patch.object(self.plugin, "retry_container_killing") as mock_kill:
with self.assertRaises(RuntimeError) as ctx:
self.plugin.container_killing_in_pod(scenario, kubecli)
self.assertIn("nonexistent", str(ctx.exception))
self.assertIn("not found in any matching pod", str(ctx.exception))
mock_kill.assert_not_called()
def test_valid_container_name_kills(self):
"""Valid container name present in all pods kills the right containers."""
kubecli = self._make_kubecli(
{
"pod1": ["target", "sidecar"],
"pod2": ["target", "sidecar"],
"pod3": ["target", "sidecar"],
}
)
scenario = self._scenario(container_name="target", count=2)
with patch.object(self.plugin, "retry_container_killing") as mock_kill:
killed = self.plugin.container_killing_in_pod(scenario, kubecli)
self.assertEqual(len(killed), 2)
self.assertEqual(mock_kill.call_count, 2)
for entry in killed:
self.assertEqual(entry[2], "target")
def test_empty_container_name_kills_first(self):
"""Empty container name kills the first container of each selected pod."""
kubecli = self._make_kubecli(
{"pod1": ["c1", "c2"], "pod2": ["c1", "c2"]}
)
scenario = self._scenario(container_name="", count=2)
with patch.object(self.plugin, "retry_container_killing") as mock_kill:
killed = self.plugin.container_killing_in_pod(scenario, kubecli)
self.assertEqual(len(killed), 2)
self.assertEqual(mock_kill.call_count, 2)
for entry in killed:
self.assertEqual(entry[2], "c1")
@patch("krkn.scenario_plugins.container.container_scenario_plugin.random.randint")
def test_heterogeneous_pods_skips_non_matching(self, mock_randint):
"""Pods without the target container are skipped; kill still succeeds."""
mock_randint.return_value = 0
kubecli = self._make_kubecli(
{
"other1": ["c1"],
"other2": ["c1"],
"target-pod": ["target"],
}
)
scenario = self._scenario(container_name="target", count=1)
with patch.object(self.plugin, "retry_container_killing") as mock_kill:
killed = self.plugin.container_killing_in_pod(scenario, kubecli)
self.assertEqual(len(killed), 1)
self.assertEqual(killed[0][0], "target-pod")
self.assertEqual(killed[0][2], "target")
self.assertEqual(mock_kill.call_count, 1)
@patch("krkn.scenario_plugins.container.container_scenario_plugin.random.randint")
def test_count_exceeds_pods_with_target_raises(self, mock_randint):
"""count higher than pods containing the target exhausts list and raises."""
mock_randint.return_value = 0
kubecli = self._make_kubecli(
{
"target-pod": ["target"],
"other1": ["c1"],
"other2": ["c1"],
}
)
scenario = self._scenario(container_name="target", count=2)
with patch.object(self.plugin, "retry_container_killing") as mock_kill:
with self.assertRaises(RuntimeError):
self.plugin.container_killing_in_pod(scenario, kubecli)
self.assertEqual(mock_kill.call_count, 1)
if __name__ == "__main__":
unittest.main()