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

roachtest: improve the cluster-init roachtest #69940

Merged
merged 4 commits into from
Sep 13, 2021

Conversation

knz
Copy link
Contributor

@knz knz commented Sep 8, 2021

Fixes #67105
Fixes #66516.
Fixes #69949.
Fixes #70003

Release justification: non-production code changes

Release note: None

knz added 2 commits September 8, 2021 23:01
Release justification: non-production code changes

Release note: None
See discussion here;
cockroachdb#67105 (comment)

Release justification: non-production code changes

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz requested review from a team and stevendanna and removed request for a team September 8, 2021 21:15
@knz knz requested review from a team and tbg September 10, 2021 09:45
tbg added 2 commits September 13, 2021 12:30
Release justification: test-only change
Release note: None
Give it a progress bar and a rudimentary context cancellation policy.

Release justification: testing-only changes
Release note: None
@tbg tbg self-requested a review September 13, 2021 10:34
Copy link
Member

@tbg tbg left a comment

Choose a reason for hiding this comment

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

I just pushed two commits, one of which makes this test use c.Start (the other is a minor cleanup). I verified that the test works using

TEST=acceptance/cluster-init LOCAL=y SHORT=y COUNT=1 ./pkg/cmd/roachtest/roachstress.sh

As far as I'm concerned the result is ready for the merge, but since I wrote it perhaps someone else on this review (server or test-eng) wants to sign off officially.

I would also suggest closing #67105 (tests should not manage their own CRDB processes, and when they do it's fair that they would wipe the cluster before termination) and also #66516 (since the test is now idiomatic).

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @stevendanna and @tbg)

@tbg tbg removed their request for review September 13, 2021 10:34
Copy link
Contributor Author

@knz knz left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @stevendanna)

@tbg
Copy link
Member

tbg commented Sep 13, 2021

bors r=knz
TFTR!

@craig
Copy link
Contributor

craig bot commented Sep 13, 2021

Build succeeded:

@craig craig bot merged commit d69481b into cockroachdb:master Sep 13, 2021
knz added a commit to knz/cockroach that referenced this pull request Sep 15, 2021
This is an edulcorated backport of cockroachdb#69940, which cannot be backported
as-is due to severe roachtest infra changes.

Release justification: test-only changes

Release note: None
@knz knz deleted the 20210908-cluster-init branch September 17, 2021 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants