fix(scenarios): fix network_chaos_ng variable shadowing and instance_count condition

[NETIZEN-11]

Description

This PR fixes two bugs in krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py:

• Fixed incorrect condition len(config) > 1, which was checking the length of a dictionary instead of the scenario list. Updated to len(scenario_config) > 1 so inter-scenario wait is applied only when multiple scenarios are configured
• Corrected inverted instance_count guard from instance_count > len(targets) to len(targets) > instance_count, ensuring sampling only occurs when there are more targets than requested

These fixes ensure correct scenario execution flow and prevent invalid sampling behavior.

---

Type of change

• Refactor
• New feature
• Bug fix
• Optimization

---

Related Tickets & Documents

• Related Issue #: fix(scenarios): fix cerberus call signature, missing imports, prometh… #1208 (fix/scenario-plugins-and-utils)
• Closes #: fix(scenarios): fix cerberus call signature, missing imports, prometh… #1208

---

Checklist before requesting a review

• Ensure the changes and proposed solution have been discussed in the relevant issue
• I have performed a self-review of my code by running krkn and specific scenario
• Added unit tests with above 80% coverage

---

Tests Performed

python run_kraken.py

# Output:
No runtime errors observed.
Scenario execution behaves correctly with proper delay handling.
Instance selection logic works as expected.

[qodo-code-review]

Review Summary by Qodo

Fix network_chaos_ng variable shadowing and instance_count condition

🐞 Bug fix

Walkthroughs

Description

• Fixed variable shadowing: changed len(config) to len(scenario_config) for correct
  inter-scenario wait logic
• Corrected inverted instance_count condition from instance_count > len(targets) to
  len(targets) > instance_count
• Added Apache 2.0 license header to the file

Diagram

flowchart LR
  A["Bug: len(config) checks dict length"] -->|Fix| B["Use len(scenario_config) for scenario list"]
  C["Bug: Inverted instance_count condition"] -->|Fix| D["Correct to len(targets) > instance_count"]
  E["Missing license header"] -->|Add| F["Apache 2.0 license header"]
  B --> G["Correct scenario execution flow"]
  D --> G
  F --> G

Loading

File Changes

1. krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py 🐞 Bug fix +18/-2

Fix variable shadowing and instance_count logic bugs

• Added Apache 2.0 license header at the beginning of the file
• Fixed variable shadowing bug: changed len(config) > 1 to len(scenario_config) > 1 to correctly
 check scenario list length instead of dictionary length
• Corrected inverted instance_count guard condition from instance_count > len(targets) to
 len(targets) > instance_count to ensure sampling only occurs when there are more targets than
 requested

krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Negative instance_count sampling ☑ 🐞 Bug ≡ Correctness

Description

NetworkChaosNgScenarioPlugin.run() now samples targets when len(targets) > instance_count, which
becomes true for any negative instance_count and will crash at runtime when calling
random.sample() with a negative sample size. BaseNetworkChaosConfig.validate() does not validate
instance_count type/range, so negative values from YAML can reach this code path.

Code

krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py[R70-75]

                    if (
                        network_chaos_config.instance_count != 0
-                        and network_chaos_config.instance_count > len(targets)
+                        and len(targets) > network_chaos_config.instance_count
                    ):
                        targets = random.sample(
                            targets, network_chaos_config.instance_count

Evidence

The plugin’s new condition will enter the sampling block for negative instance_count values
(because any non-negative len(targets) is greater than a negative integer), and there is no config
validation preventing negative instance_count from being constructed from YAML-loaded config.

krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py[70-76]
krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py[45-52]
krkn/scenario_plugins/network_chaos_ng/models.py[12-46]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`instance_count` is not validated for type/range. After the PR change, a negative `instance_count` will satisfy the sampling guard and lead to a runtime exception when sampling targets.

## Issue Context
`scenario_config` is loaded directly from YAML and expanded into dataclasses without runtime type enforcement, and `BaseNetworkChaosConfig.validate()` currently does not check `instance_count`.

## Fix Focus Areas
- krkn/scenario_plugins/network_chaos_ng/models.py[12-46]
- krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py[70-76]

## Suggested change
1) In `BaseNetworkChaosConfig.validate()`, add checks:
- `instance_count` must be an `int`
- `instance_count` must be `>= 0` (or `>= 1` if you want to disallow 0)
2) In the sampling guard, require a strictly positive count before sampling, e.g. `if isinstance(instance_count, int) and instance_count > 0 and len(targets) > instance_count:`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Sleeps after last module 🐞 Bug ➹ Performance

Description

When multiple modules exist, the plugin sleeps after every module (including the last one) because
it only checks len(scenario_config) > 1. This adds an unnecessary delay and logs that it is
waiting “before running the next” module even when there is no next module.

Code

krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py[R82-85]

+                    if len(scenario_config) > 1:
                        logging.info(
                            f"waiting {network_chaos_config.wait_duration} seconds before running the next "
                            f"Network Chaos NG Module"

Evidence

The sleep condition depends only on total module count, not whether the current iteration is the
final module in scenario_config, so the sleep runs unconditionally after the last module whenever
len(scenario_config) > 1.

krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py[52-88]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The plugin sleeps even after the last module in a multi-module scenario file, adding an extra `wait_duration` at the end.

## Issue Context
The condition checks only `len(scenario_config) > 1` and does not check the current module index.

## Fix Focus Areas
- krkn/scenario_plugins/network_chaos_ng/network_chaos_ng_scenario_plugin.py[52-88]

## Suggested change
Iterate with an index (`for i, config in enumerate(scenario_config):`) and sleep only when `i < len(scenario_config) - 1`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[paigerube14]

are you able to squash your commits into 1 and we can get this merged! 👍

[NETIZEN-11]

@paigerube14 I've squashed the commits into a single commit and force-pushed the changes. Please review.