Skip to content

ROX-11909: save kuttl JUnit XML to artifacts #2562

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.

Sign up for GitHub

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

Merged
ivan-degtiarenko merged 9 commits into from
Oct 10, 2022
Merged

ROX-11909: save kuttl JUnit XML to artifacts #2562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
384e710
Store kuttl XML as prow artifact
ivan-degtiarenko Aug 1, 2022
03fd7fe
Move storing test results to post_tests.py
ivan-degtiarenko Sep 7, 2022
52540a6
Add test_results field to BaseTest and use it in post_tests.py
ivan-degtiarenko Sep 20, 2022
8fe5662
Set add_test_result to True by default
ivan-degtiarenko Sep 20, 2022
f9436ad
Add test_results parameter to all post test
ivan-degtiarenko Sep 22, 2022
05dcb5b
Address Marcin's feedback
ivan-degtiarenko Oct 5, 2022
ad4e310
Fix FinalPost
ivan-degtiarenko Oct 5, 2022
014ba59
Fix for-loop
ivan-degtiarenko Oct 6, 2022
324572c
Address Marcin's final comments.
ivan-degtiarenko Oct 7, 2022
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
7 changes: 6 additions & 1 deletion .openshift-ci/ci_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
class BaseTest:
def __init__(self):
self.test_outputs = []
self.test_results = {}

def run_with_graceful_kill(self, args, timeout, post_start_hook=None):
with subprocess.Popen(args) as cmd:
Expand Down Expand Up @@ -61,6 +62,10 @@ def __init__(self):
"operator/build/kuttl-test-artifacts",
"operator/build/kuttl-test-artifacts-upgrade",
]
self.test_results = {
"kuttl-test-artifacts": "operator/build/kuttl-test-artifacts",
"kuttl-test-artifacts-upgrade": "operator/build/kuttl-test-artifacts-upgrade",
}

def run(self):
print("Deploying operator")
Expand All @@ -72,7 +77,7 @@ def run(self):
print("Executing operator upgrade test")
self.run_with_graceful_kill(
Copy link
Contributor

@gavin-stackrox gavin-stackrox Aug 4, 2022

Choose a reason for hiding this comment

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

If a test fails I would expect this to raise an error and the junit logs would not be gathered. What about using the post_start_hook=set_dirs_after_start convention used elsewhere in this file? Or just set test_output_dirs in a OperatorE2eTest __init__()?

Copy link
Contributor

@porridge porridge Aug 4, 2022

Choose a reason for hiding this comment

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

Hm, good point, this will only gather logs when we don't need them, :-)
Another way would be to change this artifact gathering into a context manager, or something?

Copy link
Contributor Author

@ivan-degtiarenko ivan-degtiarenko Aug 4, 2022

Choose a reason for hiding this comment

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

@gavin-stackrox do you suggest creating new post_test like StoreTestResults? Because StoreArtifacts doesn't add them to JUnit panel

Copy link
Contributor

@gavin-stackrox gavin-stackrox Aug 4, 2022

Choose a reason for hiding this comment

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

@ivan-degtiarenko actually, now that I think about it a bit more, maybe a better approach for the operator e2e tests is something similar to what is done with UIE2eTest. i.e. it is just a wrapper around tests/e2e/run-ui-e2e.sh which can then do all the error handling and JUnit artifact saving via store_test_results().

["make", "-C", "operator", "test-upgrade"],
OperatorE2eTest.UPGRADE_TEST_TIMEOUT_SEC,
OperatorE2eTest.UPGRADE_TEST_TIMEOUT_SEC
)

print("Executing operator e2e tests")
Expand Down
59 changes: 37 additions & 22 deletions .openshift-ci/post_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@


class PostTestsConstants:

API_TIMEOUT = 5 * 60
COLLECT_TIMEOUT = 10 * 60
CHECK_TIMEOUT = 5 * 60
STORE_TIMEOUT = 5 * 60
FIXUP_TIMEOUT = 5 * 60
ARTIFACTS_TIMEOUT = 3 * 60
# Where the QA tests store failure logs:
# qa-tests-backend/src/main/groovy/common/Constants.groovy
QA_TEST_DEBUG_LOGS = "/tmp/qa-tests-backend-logs"
Expand All @@ -30,13 +30,13 @@ class PostTestsConstants:


class NullPostTest:
def run(self, test_outputs=None):
def run(self, test_outputs=None, test_results=None):
pass


class RunWithBestEffortMixin:
def __init__(
self,
self,
):
self.exitstatus = 0
self.failed_commands: List[List[str]] = []
Expand Down Expand Up @@ -68,17 +68,29 @@ class StoreArtifacts(RunWithBestEffortMixin):
"""For tests that only need to store artifacts"""

def __init__(
self,
artifact_destination_prefix=None,
self,
artifact_destination_prefix=None,
):
super().__init__()
self.artifact_destination_prefix = artifact_destination_prefix
self.data_to_store = []

def run(self, test_outputs=None):
def run(self, test_outputs=None, test_results=None):
Copy link
Contributor

@porridge porridge Sep 27, 2022

Choose a reason for hiding this comment

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

This may be a bit confusing, since it does process test_outputs but not test_results. I can think of two ways:

  • barf if test_results is not None (better fail loud than silent), or
  • move the functionality to this class

self.store_artifacts(test_outputs)
self.add_test_results(test_results)
self.handle_run_failure()

def add_test_results(self, test_results):
if not test_results:
return
print("Storing test results in JUnit format")
for to_dir, from_dir in test_results.items():
self.run_with_best_effort(
["scripts/ci/store-artifacts.sh", "store_test_results",
from_dir, to_dir],
timeout=PostTestsConstants.ARTIFACTS_TIMEOUT,
)

def store_artifacts(self, test_outputs=None):
if test_outputs is not None:
self.data_to_store = test_outputs + self.data_to_store
Expand All @@ -101,10 +113,10 @@ class PostClusterTest(StoreArtifacts):
"""The standard cluster test suite of debug gathering and analysis"""

def __init__(
self,
collect_central_artifacts=True,
check_stackrox_logs=False,
artifact_destination_prefix=None,
self,
collect_central_artifacts=True,
check_stackrox_logs=False,
artifact_destination_prefix=None,
):
super().__init__(artifact_destination_prefix=artifact_destination_prefix)
self._check_stackrox_logs = check_stackrox_logs
Expand All @@ -118,7 +130,7 @@ def __init__(
]
self.collect_central_artifacts = collect_central_artifacts

def run(self, test_outputs=None):
def run(self, test_outputs=None, test_results=None):
self.collect_collector_metrics()
if self.collect_central_artifacts and self.wait_for_central_api():
self.get_central_debug_dump()
Expand All @@ -128,6 +140,7 @@ def run(self, test_outputs=None):
if self._check_stackrox_logs:
self.check_stackrox_logs()
self.store_artifacts(test_outputs)
self.add_test_results(test_results)
self.handle_run_failure()

def wait_for_central_api(self):
Expand Down Expand Up @@ -209,17 +222,17 @@ class CheckStackroxLogs(StoreArtifacts):
"""When only stackrox logs and checks are required"""

def __init__(
self,
check_for_stackrox_restarts=False,
check_for_errors_in_stackrox_logs=False,
artifact_destination_prefix=None,
self,
check_for_stackrox_restarts=False,
check_for_errors_in_stackrox_logs=False,
artifact_destination_prefix=None,
):
super().__init__(artifact_destination_prefix=artifact_destination_prefix)
self._check_for_stackrox_restarts = check_for_stackrox_restarts
self._check_for_errors_in_stackrox_logs = check_for_errors_in_stackrox_logs
self.central_is_responsive = False

def run(self, test_outputs=None):
def run(self, test_outputs=None, test_results=None):
Copy link
Contributor

@porridge porridge Sep 27, 2022

Choose a reason for hiding this comment

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

Same comment as with the StoreArtifacts implementation.

self.central_is_responsive = self.wait_for_central_api()
if self.central_is_responsive:
self.collect_stackrox_logs()
Expand All @@ -228,6 +241,7 @@ def run(self, test_outputs=None):
if self._check_for_errors_in_stackrox_logs:
self.check_for_errors_in_stackrox_logs()
self.store_artifacts(test_outputs)
self.add_test_results(test_results)
self.handle_run_failure()

def wait_for_central_api(self):
Expand Down Expand Up @@ -272,11 +286,11 @@ class FinalPost(StoreArtifacts):
"""Collect logs that accumulate over multiple tests and other final steps"""

def __init__(
self,
store_qa_test_debug_logs=False,
store_qa_spock_results=False,
artifact_destination_prefix="final",
handle_e2e_progress_failures=True,
self,
store_qa_test_debug_logs=False,
store_qa_spock_results=False,
artifact_destination_prefix="final",
handle_e2e_progress_failures=True,
):
super().__init__(artifact_destination_prefix=artifact_destination_prefix)
self._store_qa_test_debug_logs = store_qa_test_debug_logs
Expand All @@ -287,8 +301,9 @@ def __init__(
self.data_to_store.append(PostTestsConstants.QA_SPOCK_RESULTS)
self._handle_e2e_progress_failures = handle_e2e_progress_failures

def run(self, test_outputs=None):
def run(self, test_outputs=None, test_results=None):
Copy link
Contributor

@porridge porridge Sep 27, 2022

Choose a reason for hiding this comment

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

Ditto

self.store_artifacts()
self.add_test_results(test_results)
self.fixup_artifacts_content_type()
self.make_artifacts_help()
self.handle_run_failure()
Expand Down
3 changes: 2 additions & 1 deletion .openshift-ci/runners.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ def run_test_set(self, test_set):
hold = err
try:
self.log_event("About to run post test", test_set)
test_set["post_test"].run(test_outputs=test_set["test"].test_outputs)
test_set["post_test"].run(test_outputs=test_set["test"].test_outputs,
test_results=test_set["test"].test_results)
self.log_event("post test completed", test_set)
except Exception as err:
self.log_event("ERROR: post test failed", test_set)
Expand Down