-
Notifications
You must be signed in to change notification settings - Fork 36
Prevent indefinite hang during chaos execution when Kubernetes API is unreachable #124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
WHOIM1205
wants to merge
1
commit into
krkn-chaos:main
Choose a base branch
from
WHOIM1205:fix/run-shell-timeout
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| """ | ||
| Tests for the run_shell function in krkn_ai.utils | ||
| """ | ||
|
|
||
| import pytest | ||
| import time | ||
| from krkn_ai.utils import run_shell | ||
|
|
||
|
|
||
| class TestRunShellBasic: | ||
| """Test basic run_shell functionality""" | ||
|
|
||
| def test_successful_command(self): | ||
| """Test that a successful command returns output and exit code 0""" | ||
| logs, returncode = run_shell("echo 'hello world'") | ||
| assert returncode == 0 | ||
| assert "hello world" in logs | ||
|
|
||
| def test_failed_command(self): | ||
| """Test that a failed command returns non-zero exit code""" | ||
| logs, returncode = run_shell("ls /nonexistent_directory_12345") | ||
| assert returncode != 0 | ||
|
|
||
| def test_multiline_output(self): | ||
| """Test that multiline output is captured correctly""" | ||
| logs, returncode = run_shell("echo -e 'line1\\nline2\\nline3'") | ||
| assert returncode == 0 | ||
| assert "line1" in logs | ||
| assert "line2" in logs | ||
| assert "line3" in logs | ||
|
|
||
|
|
||
| class TestRunShellTimeout: | ||
| """Test run_shell timeout functionality""" | ||
|
|
||
| def test_command_completes_before_timeout(self): | ||
| """Test that a fast command completes normally with timeout set""" | ||
| logs, returncode = run_shell("echo 'fast'", timeout=10) | ||
| assert returncode == 0 | ||
| assert "fast" in logs | ||
|
|
||
| def test_command_times_out(self): | ||
| """Test that a slow command is terminated when timeout expires""" | ||
| start_time = time.time() | ||
| logs, returncode = run_shell("sleep 30", timeout=2) | ||
| elapsed = time.time() - start_time | ||
|
|
||
| # Should return -1 on timeout | ||
| assert returncode == -1 | ||
| # Should complete in roughly 2 seconds (with some buffer for termination) | ||
| assert elapsed < 10, f"Command took {elapsed} seconds, expected ~2 seconds" | ||
|
|
||
| def test_timeout_none_allows_completion(self): | ||
| """Test that timeout=None allows command to complete normally""" | ||
| logs, returncode = run_shell("sleep 1", timeout=None) | ||
| assert returncode == 0 | ||
|
|
||
| def test_partial_output_on_timeout(self): | ||
| """Test that partial output is captured even on timeout""" | ||
| # Command that continuously outputs then gets killed | ||
| # Using a bash script that outputs before sleeping | ||
| logs, returncode = run_shell( | ||
| "bash -c 'for i in 1 2 3; do echo line$i; done; sleep 30'", timeout=2 | ||
| ) | ||
| assert returncode == -1 | ||
| assert "line1" in logs | ||
|
|
||
| def test_zero_timeout_immediately_kills(self): | ||
| """Test that timeout=0 immediately kills the process""" | ||
| # Note: timeout=0 means no timeout in the current implementation | ||
| # This test documents the behavior | ||
| start_time = time.time() | ||
| logs, returncode = run_shell("sleep 5", timeout=1) | ||
| elapsed = time.time() - start_time | ||
|
|
||
| assert returncode == -1 | ||
| assert elapsed < 5 | ||
|
|
||
|
|
||
| class TestRunShellLogging: | ||
| """Test run_shell logging behavior""" | ||
|
|
||
| def test_do_not_log_suppresses_output(self): | ||
| """Test that do_not_log=True suppresses debug logging""" | ||
| # This test mainly verifies the parameter is accepted | ||
| logs, returncode = run_shell("echo 'test'", do_not_log=True) | ||
| assert returncode == 0 | ||
| assert "test" in logs | ||
|
|
||
| def test_do_not_log_false_allows_output(self): | ||
| """Test that do_not_log=False allows debug logging""" | ||
| logs, returncode = run_shell("echo 'test'", do_not_log=False) | ||
| assert returncode == 0 | ||
| assert "test" in logs | ||
|
|
||
|
|
||
| class TestRunShellEdgeCases: | ||
| """Test edge cases for run_shell""" | ||
|
|
||
| def test_empty_output_command(self): | ||
| """Test command that produces no output""" | ||
| logs, returncode = run_shell("true") | ||
| assert returncode == 0 | ||
| assert logs == "" | ||
|
|
||
| def test_special_characters_in_output(self): | ||
| """Test that special characters are handled correctly""" | ||
| logs, returncode = run_shell("echo 'special: $HOME \"quotes\" `backticks`'") | ||
| assert returncode == 0 | ||
| assert "special:" in logs | ||
|
|
||
| def test_large_output(self): | ||
| """Test that large output is captured correctly""" | ||
| # Generate 1000 lines of output | ||
| logs, returncode = run_shell("seq 1 1000") | ||
| assert returncode == 0 | ||
| assert "1" in logs | ||
| assert "1000" in logs |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel we should disable it by default and let end-user configure it per their use-case. Krkn Scenarios can take over 240 seconds duration depending on the scenario itself. If we start restricting this here without knowing the user application setup or the cluster details and type of scenarios they plan to run, then we might end up killing Krkn-AI tests half way through the scenario execution itself.