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.

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

Jump to bottom

release-22.2: roachtest: set default cluster settings, report timeout failures correctly, roachprod: capture ssh logs #96903

Merged
merged 5 commits into from
Mar 8, 2023
Merged

release-22.2: roachtest: set default cluster settings, report timeout failures correctly, roachprod: capture ssh logs #96903

merged 5 commits into from
Mar 8, 2023

Conversation

smg260
Copy link
Contributor

@smg260 smg260 commented Feb 9, 2023 •
edited
Loading

Backport:

Please see individual PRs for details.

/cc @cockroachdb/release

Epic: None
Release note: None
Release justification: test only change

@blathers-crl
Copy link

blathers-crl bot commented Feb 9, 2023

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@smg260 smg260 changed the title release-22.2: roachtest: set default cluster settings, capture ssh logs, report timeout failures correctly, release-22.2: roachtest: set default cluster settings, report timeout failures correctly, roachprod: capture ssh logs Feb 10, 2023
renatolabs and others added 5 commits March 7, 2023 18:13
@renatolabs
roachprod: fix roachprod init.
82250a4 
When a cluster is started with the `--skip-init` option, the caller
can run `roachprod init` at any time to initialize the
cluster. Unfortunately, the code used to initialize the cluster was
duplicated: one copy existed in the `start` path, and another in the
`init` path. Since the latter is used far less frequently, it had a
bug that went unnoticed: it hardcoded the first node index as `0`,
when node indices start at 1.

This commit fixes the issue by updating the constant and sharing code
between `init `and `start`.

Fixes cockroachdb#88226.

Release note: None
@renatolabs
roachprod: set default cluster settings when starting
3765da1 
In cockroachdb#88514, the cluster start logic was refactored to reuse the same
code across `init` and `start`, fixing a bug in the former. However,
the refactoring overlooked the fact that we previously always set the
default cluster settings when there's more than one node in the
cluster.

This fixes that by setting the default cluster settings in that case;
one particularly important cluster setting is the license key,
necessary for some roachtests.

Fixes cockroachdb#88660.
Fixes cockroachdb#88665.
Fixes cockroachdb#88666.
Fixes cockroachdb#88710.

Release note: None
roachtest: capture verbose SSH logging on failures
44b6979 
Currently we do not capture SSH logs in the event of a
command failure, which can be useful in debugging issues,
transient or otherwise.

This commit enables logging via the ssh switch -vvv and
specifying a log filename, to be stored under an ssh/
directory in the test log root. The debug file is deleted
upon successful (zero) exit of the command, but preserved
for non-zero exits for further inspection.

Additionally,
- The name of the log is consistent with the
corresponding run log and encodes a node number and
timestamp.
- SSH sessions must now be initialised with the
command itself to re-inforce its single use nature.
- Debug friendly command names can optionally be
specified to influence the name of the run/ssh logs.
- Retry options can optionally be omitted from any call
to ParallelE to disable retries

Release note: None
Epic: CRDB-21386
roachtest: Make ssh debug logging optional for node-wait/monitor.
48c49cd 
Wait commands are issued every 500ms returning a non zero exit
code until nodes have started. This results in a large number
of ssh debug logs during cluster creation.

Also adopts functional options.

Release note: None
Epic: none
roachtest: report timeout failures accordingly
54871f4 
Timeout failures are recorded at actual timeout, with
subsequent failures secondary.

`addFailure` accepts a depth parameter and no longer
includes context cancellation, which is done separately.

Epic: none
Fixes: cockroachdb#91237
Release note: None
@smg260 smg260 force-pushed the backport22.2-88514-88716-92667-95762-96743 branch from e621749 to 54871f4 Compare March 7, 2023 18:13
@smg260 smg260 marked this pull request as ready for review March 7, 2023 23:45
@smg260 smg260 requested a review from a team as a code owner March 7, 2023 23:45
@smg260 smg260 requested review from herkolategan and renatolabs and removed request for a team March 7, 2023 23:45
renatolabs
renatolabs approved these changes Mar 8, 2023
View reviewed changes
Copy link
Contributor

@renatolabs renatolabs left a comment

Choose a reason for hiding this comment

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

Thanks for backporting!

herkolategan
herkolategan approved these changes Mar 8, 2023
View reviewed changes
Copy link
Collaborator

@herkolategan herkolategan left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@smg260 smg260 merged commit c71d4c2 into cockroachdb:release-22.2 Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@renatolabs renatolabs renatolabs approved these changes

@herkolategan herkolategan herkolategan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@smg260 @cockroach-teamcity @herkolategan @renatolabs