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

kola: support tags for --allow-rerun-success #3430

Merged
merged 1 commit into from
May 3, 2023

Conversation

nikita-dubrovskii
Copy link
Contributor

New format is:

--allow-rerun-success tags=tag1[,tag2]

To allow all tests simply:

--allow-rerun-success tags=all

Issue: coreos/fedora-coreos-pipeline#842

@nikita-dubrovskii nikita-dubrovskii force-pushed the rerun_tagged branch 2 times, most recently from 20d391e to c1c8b79 Compare April 17, 2023 07:31
mantle/cmd/kola/kola.go Outdated Show resolved Hide resolved
mantle/cmd/kola/kola.go Outdated Show resolved Hide resolved
mantle/cmd/kola/kola.go Outdated Show resolved Hide resolved
mantle/cmd/kola/kola.go Show resolved Hide resolved
mantle/kola/harness.go Outdated Show resolved Hide resolved
@nikita-dubrovskii nikita-dubrovskii force-pushed the rerun_tagged branch 2 times, most recently from 0f492ec to be90192 Compare April 21, 2023 06:42
@cgwalters cgwalters requested a review from dustymabe May 1, 2023 13:16
@nikita-dubrovskii nikita-dubrovskii force-pushed the rerun_tagged branch 2 times, most recently from 3219559 to 3bc0f4f Compare May 2, 2023 12:05
mantle/kola/harness.go Outdated Show resolved Hide resolved
dustymabe
dustymabe previously approved these changes May 2, 2023
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM.

Did this work well in local testing? I ran some brief tests but I didn't spend a lot of time on it.

@dustymabe
Copy link
Member

I think we need to pair this with a PR to the pipeline to fixup the calls there and I think we'll also need to backport this change to older COSA branches.

One other thing maybe we should do here is have --allow-rerun-success tags=xyz imply --rerun so the user doesn't have to specify both.

@jlebon - WDYT?

New format is:
```
--allow-rerun-success tags=tag1[,tag2]
```

To allow all tests simply:
```
--allow-rerun-success tags=all
```

Co-authored-by: Dusty Mabe <dusty@dustymabe.com>

Issue: coreos/fedora-coreos-pipeline#842
@nikita-dubrovskii
Copy link
Contributor Author

nikita-dubrovskii commented May 2, 2023

LGTM.

Did this work well in local testing? I ran some brief tests but I didn't spend a lot of time on it.

yep, tested locally with enabling/disabling host's network during tests. Here is an example:

$cosa kola run --rerun --allow-rerun-success tags=foo,needs-internet  ext.config.rpm-ostree-countme
$cosa kola run --rerun --allow-rerun-success tags=all  ext.config.rpm-ostree-countme

Output looks good:

=== RUN   ext.config.rpm-ostree-countme
....
--- FAIL: ext.config.rpm-ostree-countme (25.78s)
        cluster.go:162: Error: Unit kola-runext.service exited with code 1
        cluster.go:162: 2023-05-02T14:14:00Z cli: Unit kola-runext.service exited with code 1
        harness.go:1147: kolet failed: : kolet run-test-unit failed: Process exited with status 1
FAIL, output in tmp/kola
...
======== Re-running failed tests (flake detection) ========
--- PASS: ext.config.rpm-ostree-countme (28.55s)
PASS, output in tmp/kola/rerun
+ rc=0

dustymabe added a commit to dustymabe/fedora-coreos-pipeline that referenced this pull request May 2, 2023
@dustymabe
Copy link
Member

SGTM.. ok I don't think we need to backport this before merging because the only tests in the pipeline using --allow-rerun-success aren't run downstream right now. We just need to get the existing uses in the pipeline updated. I opened coreos/fedora-coreos-pipeline#868 for that.

@jlebon
Copy link
Member

jlebon commented May 2, 2023

One other thing maybe we should do here is have --allow-rerun-success tags=xyz imply --rerun so the user doesn't have to specify both.

Yeah, that seems reasonable to me. It's not strictly new to this PR so I wouldn't block on it.

@dustymabe dustymabe merged commit 864cece into coreos:main May 3, 2023
2 checks passed
dustymabe added a commit to coreos/fedora-coreos-pipeline that referenced this pull request May 3, 2023
@nikita-dubrovskii nikita-dubrovskii deleted the rerun_tagged branch May 3, 2023 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants