Skip to content
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

Lisa should not cleanup failed environment if keep_environment=failed #3006

Merged
merged 7 commits into from
Jan 2, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion tests_e2e/orchestrator/lib/agent_test_suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ class KeepEnvironment(object):
No = 'no' # Always delete resources created by the test suite


class TestFailedException(Exception):
def __init__(self, env_name: str, test_cases: List[str]):
msg = "Test suite {0} failed.".format(env_name)
if test_cases:
msg += " Failed tests: " + ','.join(test_cases)
super().__init__(msg)


class _TestNode(object):
"""
Name and IP address of a test VM
Expand Down Expand Up @@ -539,6 +547,7 @@ def _execute(self) -> None:
log_path: Path = self._log_path / f"env-{self._environment_name}.log"
with set_current_thread_log(log_path):
start_time: datetime.datetime = datetime.datetime.now()
failed_cases = []

try:
# Log the environment's name and the variables received from the runbook (note that we need to expand the names of the test suites)
Expand All @@ -563,7 +572,10 @@ def _execute(self) -> None:
for suite in self._test_suites:
log.info("Executing test suite %s", suite.name)
self._lisa_log.info("Executing Test Suite %s", suite.name)
test_suite_success = self._execute_test_suite(suite, test_context) and test_suite_success
case_success = self._execute_test_suite(suite, test_context)
test_suite_success = case_success and test_suite_success
if not case_success:
failed_cases.append(suite.name)

finally:
if self._collect_logs == CollectLogs.Always or self._collect_logs == CollectLogs.Failed and not test_suite_success:
Expand All @@ -588,6 +600,13 @@ def _execute(self) -> None:
if unexpected_error:
self._mark_log_as_failed()

# Check if any test failures or unexpected errors occurred. If so, raise an Exception here so that
# lisa marks the environment as failed. Otherwise, lisa would mark this environment as passed and
# clean up regardless of the value of 'keep_environment'. This should be the last thing that
# happens during suite execution.
if not test_suite_success or unexpected_error:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just add a comment explaining the reason of this exception

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

raise TestFailedException(self._environment_name, failed_cases)

def _execute_test_suite(self, suite: TestSuiteInfo, test_context: AgentTestContext) -> bool:
"""
Executes the given test suite and returns True if all the tests in the suite succeeded.
Expand Down
Loading