fix(nemesis.py): abort nemesis on stress command failure

[dimakr]

Abort the nemesis flow early when a stress command fails, as continuing it is often invalid due to the cluster being in an unexpected state.
For example, if the _prepare_test_table routine fails, subsequent steps that depend on the test table (or attempt disruptions on top of it) will also fail.

This change adds a check for results of stress command triggered within the nemesis code, ensuring the nemesis halts early if the stress command is unsuccessful.

Fixes: #8722

Testing

• GrowShrinkClusterNemesis, both c-s and s-b loads are executed during the nemesis
• TruncateMonkey, both c-s and s-b loads are executed during the nemesis
• Tested locally to ensure/simulate a failed stress command - added an invalid parameter to cassandra-stress command in Nemesis._prepare_test_table (here

  scylla-cluster-tests/sdcm/nemesis.py

  Line 2072 in 532ed3f

  stress_cmd = "cassandra-stress write n=400000 cl=QUORUM -mode native cql3 " \

  ), which makes the stress command fail.
  Executed PR-provision-test configuration for TruncateMonkey nemesis class. As a result the disrupt_truncate disruption was started few times, but interrupted early as the stress command in _prepare_test_table
  was failing:

< t:2024-12-20 22:04:24,417 f:file_logger.py  l:101  c:sdcm.sct_events.file_logger p:INFO  > sdcm.exceptions.NemesisStressFailure: Aborting 'TruncateMonkey' nemesis as 'cassandra-stress write whoa n=400000 cl=QUORUM -mode native cql3 -schema 'replication(strategy=NetworkTopologyStrategy,replication_factor=3)' -log interval=5' stress command failed with the following errors:
< t:2024-12-20 22:04:24,417 f:file_logger.py  l:101  c:sdcm.sct_events.file_logger p:INFO  >   on node 'PR-provision-test-dmitriy-loader-node-a2eaf144-2': ['Stress command completed with bad status 1: ']
< t:2024-12-20 22:04:24,417 f:file_logger.py  l:101  c:sdcm.sct_events.file_logger p:INFO  >   on node 'PR-provision-test-dmitriy-loader-node-a2eaf144-1': ['Stress command completed with bad status 1: ']

PR pre-checks (self review)

• I added the relevant backport labels
• I didn't leave commented-out/debugging code

Reminders

• Add New configuration option and document them (in sdcm/sct_config.py)
• Add unit tests to cover my changes (under unit-test/ folder)
• Update the Readme/doc folder relevant to this change (if needed)

[fruch]

@dimakr

can you run individual nemesis on at least two of the changed nemesis, to show case it's working (it can be AWS or docker backend if they are relevant)

[vponomaryov]

Here we run the same command on each of the existing loaders.
In this case we are totally ok to get n_loaders - 1 failures.
We need result at least from some of them.

Probably it is the case for some other nemesis.

It runs a single command on multiple loaders just because of the default parameter round_robin=False defined in the self.tester.run_stress_thread.

So, probably we should have some passes and some fails checks.

[dimakr]

Changed the check in the new method so that a nemesis would be aborted if all 'n_loadersstress commands failed. Checked this for both cases -round_robin` enabled and disabled.

[dimakr]

can you run individual nemesis on at least two of the changed nemesis, to show case it's working (it can be AWS or docker backend if they are relevant)

Executed longevity-5gb-1h-nemesis config for GrowShrinkClusterNemesis and TruncateMonkey nemeses. Linked the jobs in the PR descriptions. In both cases the c-s and s-b commands are executed (and stop_nemesis_on_error check is performed) during the nemesis.

[fruch]

Maybe we need to align this on all stress commands ?

[dimakr]

This new stop_nemesis_on_stress_errors method is now applied to results of stress commands that are triggered from nemesis.py. There are 3 commands that are executed this way in nemesis code - c-s, s-b, and cql-stress-cassandra-stress.
All of them (with this^ change included) are returning tuple of (loader, result, event).

I'm not sure, if/which stress commands are also triggered from nemesis.py.

[fruch]

I think regardless of what is being used right now, it would be good to align them to return identically from this function.

[vponomaryov]

Agree, would be less confusion if we had single return type for all cases.

[fruch]

I've just set down with @dimakr, and we agreed that there would be a follow up for this change

that would incorporate stop_nemesis_on_stress_errors() into verify verify_results() and align the signatures of get_results() and verify_results()

[fruch]

Why is the super call needed here ? What are we trying to skip ? And why ?

[dimakr]

This is to directly call the get_results method to get all results from threads. Some command specific get_results implementation would filter out failed commands. E.g. in c-s the get_results performs:

    def get_results(self) -> list[dict | None]:
        ret = []
        results = super().get_results()

        for _, result, event in results:
            if not result:
                # Silently skip if stress command threw error, since it was already reported in _run_stress
                continue
...
        return ret

[fruch]

It's a bit specific c-s, we should at least put a comment explaining it, it's quite unusual to skip up the MRO like that, I barely understand the reason it's c-s is doing that, let's at least point to the reason we jump it.