kola: support denylist Warn feature #3539

nikita-dubrovskii · 2023-07-18T14:02:05Z

We bubble denylisted tests with 'Warn: true' option as warnings rather than hard failures:
```
kola -p qemu run --parallel 8 ext.config.ntp.* --output-dir tmp/kola

⏭️  Skipping kola test pattern "ext.config.ntp.chrony.dhcp-propagation":
  👉 https://github.com/coreos/fedora-coreos-tracker/issues/1508
⚠️  Warning kola test pattern "ext.config.ntp.timesyncd.dhcp-propagation", snoozing expired on Jul 20 2023:
  👉 https://github.com/coreos/fedora-coreos-tracker/issues/1508
=== RUN   ext.config.ntp.chrony.coreos-platform-chrony-config
=== RUN   ext.config.ntp.timesyncd.dhcp-propagation
--- PASS: ext.config.ntp.chrony.coreos-platform-chrony-config (27.56s)
--- WARN: ext.config.ntp.timesyncd.dhcp-propagation (90.72s)
        cluster.go:162: Error: Unit kola-runext.service exited with code 1
        cluster.go:162: 2023-07-27T06:24:18Z cli: Unit kola-runext.service exited with code 1
        harness.go:1236: kolet failed: : kolet run-test-unit failed: Process exited with status 1
FAIL, output in tmp/kola
+ rc=0
+ set +x
```

dustymabe · 2023-07-20T17:59:19Z

mantle/kola/harness.go

+	// Ignore the error when only denied tests with Warn:true feaute failed
+	if runErr != nil && allTestsWarnOnError(testResults.getResults()) {
+		return nil
+	}


I think the goal here is that in our pipeline we are able to know when a test that fails is warn: true and "warn" in the pipline (warn in jenkins is different; the build continues but does get flagged with yellow warn status). This is crucial because otherwise we don't get notified when a warning happens.

I'm thinking we should surface this up and return a non-zero return code, but defined return code so that callers (like jenkins) can adjust their behavior. For example if we returned 77 here jenkins could check the return code if it's non-zero and if it is 77 then warn instead of fail. Here is an example where we added a new option to change the return code in #1152

I played around with this idea and posted some code over in https://github.com/dustymabe/coreos-assembler/commits/PR3539 - let me know what you think?

I'd add 77 with a separate PR, but not here (to keep warn:true feature support simple).

I'd add 77 with a separate PR, but not here (to keep warn:true feature support simple).

Yes, if you don't mind we can open a followup PR for something like changing the exit code and then update coreos-ci-lib to change behavior of kola() based on that.

mantle/kola/harness.go

dustymabe · 2023-07-21T03:37:42Z

mantle/kola/harness.go

+		if IsWarningOnFailure(test) {
+			continue
+		}


do we want this?

I think so. For example denylisted tagged warn: true tests shouldn't affect allow-rerun-success tag=foo failed tests.

ahh I see now. if a warn:true test didn't have tag=foo then the loop would exit early because it found a test that didn't allow rerun success, but we don't want this to count against that.

Maybe the better thing to do here would be to not rerun warn:true tests. i.e. we'd filter it out in getRerunnable( somehow. I'm not sure if that's a great idea or not, but spitballing.

i've removed this check and now kola simply reruns everything. Anyhow we sort denylist + warn out later

mantle/kola/harness.go

dustymabe

mostly LGTM - some comments

mantle/kola/harness.go

dustymabe · 2023-07-28T01:21:09Z

mantle/kola/harness.go

+		if IsWarningOnFailure(test) {
+			continue
+		}


ahh I see now. if a warn:true test didn't have tag=foo then the loop would exit early because it found a test that didn't allow rerun success, but we don't want this to count against that.

Maybe the better thing to do here would be to not rerun warn:true tests. i.e. we'd filter it out in getRerunnable( somehow. I'm not sure if that's a great idea or not, but spitballing.

mantle/kola/harness.go

dustymabe · 2023-07-28T01:37:20Z

mantle/harness/testresult/status.go

 	Fail TestResult = "FAIL"
+	Warn TestResult = "WARN"
 	Skip TestResult = "SKIP"
 	Pass TestResult = "PASS"


random comment: In the future I think it would be cool if we could use colors for these outputs. I'm thinking red, yellow, white, green.

what's about s/white/blue/ ?

Did this work for you? I can't get colors to output locally.

For me that works. tmux and gnome-terminal have no problems

ok I got it working :)

what was wrong?

ok now the CI unittests are failing because they don't expect the color in the output. We could add the expected color to the CI tests.. Alternatively I wonder if we should try to check to see if the output is to a terminal (tty) or not and add the color based on that. Something similar to https://github.com/coreos/rpm-ostree/blob/dd370d13d3d41da0e3d61fc9ace558d6bc349f7f/src/libpriv/rpmostree-editor.cxx#L37-L55

Another example is NetworkManager. nmcli c show will show colors, but nmcli c show | cat won't because the output is to a pipe.

what was wrong?

I didn't have the code checked out right locally :(

dustymabe

LGTM

coreos#3344 We bubble denylisted tests with 'Warn: true' option as warnings rather than hard failures: ``` kola -p qemu run --parallel 8 ext.config.ntp.* --output-dir tmp/kola ⏭️ Skipping kola test pattern "ext.config.ntp.chrony.dhcp-propagation": 👉 coreos/fedora-coreos-tracker#1508 ⚠️ Warning kola test pattern "ext.config.ntp.timesyncd.dhcp-propagation", snoozing expired on Jul 20 2023: 👉 coreos/fedora-coreos-tracker#1508 === RUN ext.config.ntp.chrony.coreos-platform-chrony-config === RUN ext.config.ntp.timesyncd.dhcp-propagation --- PASS: ext.config.ntp.chrony.coreos-platform-chrony-config (27.56s) --- WARN: ext.config.ntp.timesyncd.dhcp-propagation (90.72s) cluster.go:162: Error: Unit kola-runext.service exited with code 1 cluster.go:162: 2023-07-27T06:24:18Z cli: Unit kola-runext.service exited with code 1 harness.go:1236: kolet failed: : kolet run-test-unit failed: Process exited with status 1 FAIL, output in tmp/kola + rc=0 + set +x ``` Co-authored-by: Dusty Mabe <[email protected]>

With the introduction of warn: true the logic gets complicated. This is an attempt to simplify the logic and handle all corner cases.

dustymabe · 2023-07-28T17:40:54Z

/retest

openshift-ci · 2023-07-28T20:09:30Z

@nikita-dubrovskii: 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/rhcos	`5c2f264`	link	true	`/test rhcos`

Full PR test history. Your PR dashboard.

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.

dustymabe · 2023-07-29T01:37:50Z

the prow/rhcos failure seems unrelated to this. Merging.

warn: true was added in coreos/coreos-assembler#3539 and can be used to make test failures non-fatal in the pipeline. Let's strip those from the denylist too on promotion.

This is a new feature that was added recently in coreos/coreos-assembler#3539. Now when a snooze expires we can configure it to warn us and continue to build rather than marking the entire run as a failure.

warn: true was added in coreos/coreos-assembler#3539 and can be used to make test failures non-fatal in the pipeline. Let's strip those from the denylist too on promotion.

This is a new feature that was added recently in coreos/coreos-assembler#3539. Now when a snooze expires we can configure it to warn us and continue to build rather than marking the entire run as a failure.

nikita-dubrovskii mentioned this pull request Jul 18, 2023

kola: mark denylisted tests with 'warn: true' coreos/fedora-coreos-config#2512

Closed

nikita-dubrovskii force-pushed the issue-3344 branch 2 times, most recently from 1071f29 to 0f06f66 Compare July 20, 2023 13:31

nikita-dubrovskii mentioned this pull request Jul 20, 2023

kola: mark denylisted tests with 'warn: true' coreos/fedora-coreos-config#2517

Closed

dustymabe reviewed Jul 21, 2023

View reviewed changes

nikita-dubrovskii force-pushed the issue-3344 branch 2 times, most recently from 5ccf356 to fdedefa Compare July 27, 2023 06:43

dustymabe previously approved these changes Jul 28, 2023

View reviewed changes

nikita-dubrovskii dismissed dustymabe’s stale review via 38b353d July 28, 2023 06:23

nikita-dubrovskii force-pushed the issue-3344 branch from 38b353d to 94beb35 Compare July 28, 2023 06:59

dustymabe previously approved these changes Jul 28, 2023

View reviewed changes

nikita-dubrovskii and others added 2 commits July 28, 2023 16:33

mantle/kola: simplify logic in ParseDenyListYaml

5c2f264

With the introduction of warn: true the logic gets complicated. This is an attempt to simplify the logic and handle all corner cases.

nikita-dubrovskii dismissed dustymabe’s stale review via 5c2f264 July 28, 2023 14:35

nikita-dubrovskii force-pushed the issue-3344 branch from 94beb35 to e35315b Compare July 28, 2023 14:35

dustymabe approved these changes Jul 28, 2023

View reviewed changes

dustymabe enabled auto-merge (rebase) July 28, 2023 14:35

nikita-dubrovskii force-pushed the issue-3344 branch from e35315b to 5c2f264 Compare July 28, 2023 14:37

dustymabe merged commit f38e1df into coreos:main Jul 29, 2023

dustymabe mentioned this pull request Jul 29, 2023

kola/harness.go: add info to recently expired snooze #3515

Closed

nikita-dubrovskii deleted the issue-3344 branch July 29, 2023 09:46

dustymabe mentioned this pull request Aug 22, 2023

scripts/promote-config: strip warn: out of denylist on promotion coreos/fedora-coreos-releng-automation#178

Merged

dustymabe mentioned this pull request Aug 23, 2023

denylist: mark snoozed tests as warn: true coreos/fedora-coreos-config#2561

Merged

nikita-dubrovskii mentioned this pull request Oct 12, 2023

Add support for "known fails" for kola run #1165

Closed

jlebon mentioned this pull request Oct 12, 2023

kola: support denylist warn or warning feature #3344

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kola: support denylist Warn feature #3539

kola: support denylist Warn feature #3539

nikita-dubrovskii commented Jul 18, 2023 •

edited

Loading

dustymabe Jul 20, 2023

dustymabe Jul 21, 2023

nikita-dubrovskii Jul 24, 2023

dustymabe Jul 28, 2023

nikita-dubrovskii Jul 28, 2023

dustymabe Jul 21, 2023

nikita-dubrovskii Jul 24, 2023

dustymabe Jul 28, 2023

nikita-dubrovskii Jul 28, 2023

dustymabe left a comment

dustymabe Jul 28, 2023

dustymabe Jul 28, 2023

nikita-dubrovskii Jul 28, 2023

dustymabe Jul 28, 2023

dustymabe Jul 28, 2023

nikita-dubrovskii Jul 28, 2023

dustymabe Jul 28, 2023

nikita-dubrovskii Jul 28, 2023

dustymabe Jul 28, 2023 •

edited

Loading

dustymabe Jul 28, 2023

dustymabe Jul 28, 2023

dustymabe left a comment

dustymabe commented Jul 28, 2023

openshift-ci bot commented Jul 28, 2023

dustymabe commented Jul 29, 2023

kola: support denylist Warn feature #3539

kola: support denylist Warn feature #3539

Conversation

nikita-dubrovskii commented Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustymabe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustymabe Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustymabe left a comment

Choose a reason for hiding this comment

dustymabe commented Jul 28, 2023

openshift-ci bot commented Jul 28, 2023

dustymabe commented Jul 29, 2023

nikita-dubrovskii commented Jul 18, 2023 •

edited

Loading

dustymabe Jul 28, 2023 •

edited

Loading