ROX-11909: save kuttl JUnit XML to artifacts by ivan-degtiarenko · Pull Request #2562 · stackrox/stackrox

ivan-degtiarenko · 2022-07-31T14:13:33Z

Description

Prow can display tests that fail in its JUnit panel. e.g. https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/branch-ci-stackrox-stackrox-master-merge-go-unit-tests/1550162970292523008

For Prow to display tests that fail in its JUnit panel it needs files in JUnit XML format copied to $ARTIFACT_DIR. This PR does that for openshift-4-operator-e2e-tests, which are run via kuttl.

Checklist

Investigated and inspected CI test results
~~- [ ] Unit test and regression tests added~~
Evaluated and added CHANGELOG entry if required
~~- [ ] Determined and documented upgrade steps~~
~~- [ ] Documented user facing changes (create PR based on openshift/openshift-docs and merge into rhacs-docs)~~

If any of these don't apply, please comment below.

Testing Performed

Verified in CI that artifacts are in place, see ci/prow/openshift-4-operator-e2e-tests

ghost · 2022-07-31T15:00:53Z

Images are ready for the commit at 324572c.

To use with deploy scripts, first export MAIN_IMAGE_TAG=3.72.x-227-g324572c9d4.

ivan-degtiarenko · 2022-07-31T15:58:22Z

/retest

ivan-degtiarenko · 2022-08-03T23:29:30Z

/retest

porridge

Awesome!

porridge · 2022-08-04T11:02:14Z

.openshift-ci/ci_tests.py

        )

+        print("Storing test-e2e-deployed results in JUnit format")
+        self.run_with_graceful_kill(


Would be nice to refactor this print+run into a couple of methods:

one in OperatorE2eTest which contains pieces specific to kuttl artifacts, or perhaps specific to operator tests; it would delegate to:

another generic method in BaseTest that calls store-artifacts.sh and would potentially be useful to other subclasses.

gavin-stackrox · 2022-08-04T14:48:48Z

.openshift-ci/ci_tests.py

@@ -66,7 +67,15 @@ def run(self):
        print("Executing operator upgrade test")
        self.run_with_graceful_kill(


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__()?

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?

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

@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().

gavin-stackrox · 2022-09-07T01:08:53Z

.openshift-ci/post_tests.py

+        print("Storing operator test results in JUnit format")
+        self.run_with_best_effort(
+            ["scripts/ci/store-artifacts.sh", "store_test_results",
+             "operator/build/kuttl-test-artifacts"],


I think based on @porridge changes there will be different dirs now for different tests. i.e. not all are "operator/build/kuttl-test-artifacts"

what about adding an array of junit test output passed to PostClusterTest() then this change is not so operator specific.

ivan-degtiarenko · 2022-09-16T09:52:51Z

/retest

ivan-degtiarenko · 2022-09-23T11:25:06Z

/retest

porridge · 2022-09-27T08:15:06Z

/test openshift-newest-operator-e2e-tests
/test openshift-oldest-operator-e2e-tests

porridge

LGTM overall, except it seems like this might better fit in the parent class...

porridge · 2022-09-27T08:17:00Z

.openshift-ci/ci_tests.py

-            "operator/build/kuttl-test-artifacts-upgrade",
-        ]
+        self.test_results = {
+            "operator/build/kuttl-test-artifacts": "kuttl-test-artifacts",


I think it might be better to have the dict the other way, since we primarily want to make sure the destination name is unique..

porridge · 2022-09-27T08:18:43Z

.openshift-ci/post_tests.py

        collect_central_artifacts=True,
        check_stackrox_logs=False,
        artifact_destination_prefix=None,
+        add_test_result=True,


I think keeping this in plural to match the test_results argument name would be better for consistency.

porridge · 2022-09-27T08:22:24Z

.openshift-ci/post_tests.py

        self.data_to_store = []

-    def run(self, test_outputs=None):
+    def run(self, test_outputs=None, test_results=None):


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

porridge · 2022-09-27T08:22:55Z

.openshift-ci/post_tests.py

        )

+    def add_test_result(self, test_results):
+        print("Storing operator test results in JUnit format")


This is in general not operator specific.

Suggested change

print("Storing operator test results in JUnit format")

print("Storing test results in JUnit format")

porridge · 2022-09-27T08:23:52Z

.openshift-ci/post_tests.py

+        for fromDir, toDir in test_results:
+            self.run_with_best_effort(
+                ["scripts/ci/store-artifacts.sh", "store_test_results",
+                 fromDir, toDir],


The rest of this file seems to avoid camelCase.

Suggested change

for fromDir, toDir in test_results:

self.run_with_best_effort(

["scripts/ci/store-artifacts.sh", "store_test_results",

fromDir, toDir],

for from_dir, to_dir in test_results:

self.run_with_best_effort(

["scripts/ci/store-artifacts.sh", "store_test_results",

from_dir, to_dir],

porridge · 2022-09-27T08:25:00Z

.openshift-ci/post_tests.py

        self.central_is_responsive = False

-    def run(self, test_outputs=None):
+    def run(self, test_outputs=None, test_results=None):


Same comment as with the StoreArtifacts implementation.

porridge · 2022-09-27T08:25:13Z

.openshift-ci/post_tests.py

        self._handle_e2e_progress_failures = handle_e2e_progress_failures

-    def run(self, test_outputs=None):
+    def run(self, test_outputs=None, test_results=None):


porridge · 2022-09-27T08:28:18Z

And let's see whether this actually works in the operator jobs :-)

porridge · 2022-09-28T05:14:45Z

Unfortunately:

****
**** 09:54:36: final post completed
****
Traceback (most recent call last):
  File "/go/src/github.com/stackrox/stackrox/scripts/ci/jobs/openshift_newest_operator_e2e_tests.py", line 16, in <module>
    final_post=FinalPost(handle_e2e_progress_failures=False),
  File "/go/src/github.com/stackrox/stackrox/.openshift-ci/runners.py", line 83, in run
    raise hold
  File "/go/src/github.com/stackrox/stackrox/.openshift-ci/runners.py", line 57, in run
    self.run_test_set(test_set)
  File "/go/src/github.com/stackrox/stackrox/.openshift-ci/runners.py", line 113, in run_test_set
    raise hold
  File "/go/src/github.com/stackrox/stackrox/.openshift-ci/runners.py", line 104, in run_test_set
    test_set["post_test"].run(test_outputs=test_set["test"].test_outputs,
AttributeError: 'OperatorE2eTest' object has no attribute 'test_outputs'

openshift-ci · 2022-10-06T12:15:59Z

@ivan-degtiarenko: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/gke-ui-e2e-tests	`0d153c6`	link	false	`/test gke-ui-e2e-tests`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

ivan-degtiarenko · 2022-10-06T21:25:30Z

/test openshift-newest-operator-e2e-tests
/test openshift-oldest-operator-e2e-tests

porridge

NICE!
Just a couple of nitpicks.

porridge · 2022-10-07T07:27:30Z

.openshift-ci/post_tests.py

        self.handle_run_failure()

+    def add_test_results(self, test_results):
+        if test_results is None:


It's generally considered more pythonic to say:

Suggested change

if test_results is None:

if not test_results:

This catches the empty list case too (and I guess it makes sense because we don't want to log the following line if we actually have nothing to store).

porridge · 2022-10-07T07:29:13Z

.openshift-ci/post_tests.py

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


To keep the order consistent with StoreArtifacts.run:

Suggested change

self.add_test_results(test_results)

self.store_artifacts(test_outputs)

self.store_artifacts(test_outputs)

self.add_test_results(test_results)

gavin-stackrox

LGTM!

I'd prefer to avoid the reformatting but I left the door open for that by not adding the formatter to CI: https://github.com/stackrox/stackrox/blob/master/.openshift-ci/README.md#workflow-aliases-for-testformatlint

It would be nice to have an example of a failed kuttl test result in the PR description as proof of work.

porridge · 2022-10-10T05:05:19Z

/test openshift-newest-operator-e2e-tests
/test openshift-oldest-operator-e2e-tests

porridge · 2022-10-10T05:08:26Z

It would be nice to have an example of a failed kuttl test result in the PR description as proof of work.

Let's see how #3362 goes

porridge · 2022-10-10T08:02:39Z

Failure cases.

Hm, these actually seem to have failed for some unrelated reason 🤔

time="2022-10-10T05:30:41Z" level=warning msg="missing \"ROX_IMAGE_FLAVOR\" build argument. Try adding \"--build-arg ROX_IMAGE_FLAVOR=<VALUE>\" to the command line"

Could this be a symptom of a necessary rebase @ivan-degtiarenko ?

Or perhaps this message is a red herring.. let me try rerunning...

porridge · 2022-10-10T09:18:16Z

Now that's better!

ivan-degtiarenko · 2022-10-10T10:18:36Z

@porridge thanks for testing 👍 Merging the PR

Update SCANNER_VERSION and COLLECTOR_VERSION to reference images from their respective UBI9 migration PRs: - Scanner: 2.38.x-166-g16d772deeb (PR #2562) - includes arm64 build fix - Collector: 3.23.x-132-g98b7877e58 (PR #2815) This enables e2e tests to verify all three components work correctly together with UBI9. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

In UBI9, update-ca-trust requires the -o flag to specify the output directory when running as a non-root user. This matches the workaround applied in the scanner repo (PR #2562). See: https://bugzilla.redhat.com/show_bug.cgi?id=2241240 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

ivan-degtiarenko added the do-not-merge/work-in-progress label Jul 31, 2022

github-actions bot added the area/operator label Jul 31, 2022

ivan-degtiarenko force-pushed the ivan/add-kuttl-junit-to-prow branch 4 times, most recently from 9a5f410 to e732b8f Compare August 3, 2022 18:50

ivan-degtiarenko changed the title ~~WIP: Try to save kuttl XML to artifacts~~ ROX-11909: save kuttl JUnit XML to artifacts Aug 4, 2022

openshift-ci bot removed the do-not-merge/work-in-progress label Aug 4, 2022

ivan-degtiarenko requested review from gavin-stackrox and porridge August 4, 2022 10:53

porridge approved these changes Aug 4, 2022

View reviewed changes

gavin-stackrox suggested changes Aug 4, 2022

View reviewed changes

porridge mentioned this pull request Aug 19, 2022

ROX-12057: collect operator e2e test artifacts #2732

Merged

5 tasks

ivan-degtiarenko force-pushed the ivan/add-kuttl-junit-to-prow branch from e732b8f to 0d153c6 Compare September 7, 2022 00:50

gavin-stackrox suggested changes Sep 7, 2022

View reviewed changes

ivan-degtiarenko requested a review from gavin-stackrox September 7, 2022 12:05

porridge reviewed Sep 27, 2022

View reviewed changes

ivan-degtiarenko added ci-eks-tests labels Oct 5, 2022

Fix for-loop

014ba59

ivan-degtiarenko removed ci-eks-tests area/central area/ui labels Oct 6, 2022

porridge approved these changes Oct 7, 2022

View reviewed changes

ivan-degtiarenko requested review from gavin-stackrox and removed request for gavin-stackrox October 7, 2022 08:50

Address Marcin's final comments.

324572c

gavin-stackrox approved these changes Oct 7, 2022

View reviewed changes

ivan-degtiarenko merged commit 2117bc0 into master Oct 10, 2022

ivan-degtiarenko deleted the ivan/add-kuttl-junit-to-prow branch October 10, 2022 10:19

		@@ -66,7 +67,15 @@ def run(self):
		print("Executing operator upgrade test")
		self.run_with_graceful_kill(

	print("Storing operator test results in JUnit format")
	print("Storing test results in JUnit format")

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

Conversation

ivan-degtiarenko commented Jul 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Testing Performed

Uh oh!

ghost commented Jul 31, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivan-degtiarenko commented Jul 31, 2022

Uh oh!

ivan-degtiarenko commented Aug 3, 2022

Uh oh!

porridge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivan-degtiarenko commented Sep 16, 2022

Uh oh!

ivan-degtiarenko commented Sep 23, 2022

Uh oh!

porridge commented Sep 27, 2022

Uh oh!

porridge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

porridge commented Sep 27, 2022

Uh oh!

porridge commented Sep 28, 2022

Uh oh!

openshift-ci bot commented Oct 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivan-degtiarenko commented Oct 6, 2022

Uh oh!

porridge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gavin-stackrox left a comment

Choose a reason for hiding this comment

Uh oh!

porridge commented Oct 10, 2022

Uh oh!

porridge commented Oct 10, 2022

Uh oh!

porridge commented Oct 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

porridge commented Oct 10, 2022

Uh oh!

ivan-degtiarenko commented Jul 31, 2022 •

edited

Loading

ghost commented Jul 31, 2022 •

edited by ghost

Loading

openshift-ci bot commented Oct 6, 2022 •

edited

Loading

porridge commented Oct 10, 2022 •

edited

Loading