Skip to content

Conversation

travisdowns
Copy link
Member

@travisdowns travisdowns commented Sep 24, 2025

This should be done only once, don't do it on service stop, but in the @cluster cleanup code, before we stop redpanda.

This saves six seconds on every test, since the second export (due to the second stop) took six seconds. Hat tip to Stephan for noticing this.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v25.2.x
  • v25.1.x
  • v24.3.x

Release Notes

  • none

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes test execution time by moving cluster configuration export to the @cluster cleanup phase instead of during service stop. This eliminates duplicate configuration exports when services are stopped multiple times, saving approximately 6 seconds per test.

Key changes:

  • Moves export_cluster_config() call from stop() method to @cluster cleanup
  • Adds abstract method definition for export_cluster_config() in base class
  • Extracts configuration export logic into separate method

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/rptest/services/redpanda.py Extracts cluster config export from stop() method and adds abstract method definition
tests/rptest/services/cluster.py Calls export_cluster_config() during @cluster cleanup phase

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Sep 24, 2025

Retry command for Build#72845

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/tests/audit_log_test.py::AuditLogTestInvalidConfig.test_invalid_config@{"audit_transport_mode":"kclient"}
tests/rptest/tests/audit_log_test.py::AuditLogTestInvalidConfigMTLS.test_invalid_config_mtls@{"audit_transport_mode":"kclient"}

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Sep 24, 2025

CI test results

test results on build#72845
test_class test_method test_arguments test_kind job_url test_status passed reason test_history
AuditLogTestInvalidConfig test_invalid_config {"audit_transport_mode": "kclient"} integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c49-3b84-426f-9e67-abaa137d548a FAIL 0/21 The test has failed across all retries https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=AuditLogTestInvalidConfig&test_method=test_invalid_config
AuditLogTestInvalidConfig test_invalid_config {"audit_transport_mode": "kclient"} integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c4d-5cb9-461e-9234-28766361c3be FAIL 0/21 The test has failed across all retries https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=AuditLogTestInvalidConfig&test_method=test_invalid_config
AuditLogTestInvalidConfigMTLS test_invalid_config_mtls {"audit_transport_mode": "kclient"} integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c49-3b85-412f-b4ca-ad248932eb9e FAIL 0/21 The test has failed across all retries https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=AuditLogTestInvalidConfigMTLS&test_method=test_invalid_config_mtls
AuditLogTestInvalidConfigMTLS test_invalid_config_mtls {"audit_transport_mode": "kclient"} integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c4d-5cbb-4aea-ad11-cd0d5ab992e7 FAIL 0/21 The test has failed across all retries https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=AuditLogTestInvalidConfigMTLS&test_method=test_invalid_config_mtls
EndToEndCloudTopicsTest test_write null integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c4d-5cbd-4976-a6b6-4b4acbf607a9 FLAKY 14/21 upstream reliability is '92.96875'. current run reliability is '66.66666666666666'. drift is 26.30208 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=EndToEndCloudTopicsTest&test_method=test_write
ClusterRateQuotaTest test_client_response_and_produce_throttle_mechanism null integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c4d-5cb7-4e71-9365-2aaa4ce651ab FLAKY 13/21 upstream reliability is '81.25613346418056'. current run reliability is '61.904761904761905'. drift is 19.35137 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=ClusterRateQuotaTest&test_method=test_client_response_and_produce_throttle_mechanism
ClusterRateQuotaTest test_client_response_throttle_mechanism_applies_to_next_request null integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c49-3b85-4635-a3af-6ef2a2cafbf9 FLAKY 12/21 upstream reliability is '84.8'. current run reliability is '57.14285714285714'. drift is 27.65714 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=ClusterRateQuotaTest&test_method=test_client_response_throttle_mechanism_applies_to_next_request
ControllerLogLimitMirrorMakerTests test_mirror_maker_with_limits null integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c4d-5cba-4596-9286-6863e93df805 FLAKY 19/21 upstream reliability is '99.57264957264957'. current run reliability is '90.47619047619048'. drift is 9.09646 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=ControllerLogLimitMirrorMakerTests&test_method=test_mirror_maker_with_limits
DisablingPartitionsTest test_disable null integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c49-3b87-4452-9699-3cd70040c6e3 FLAKY 17/21 upstream reliability is '84.25925925925925'. current run reliability is '80.95238095238095'. drift is 3.30688 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=DisablingPartitionsTest&test_method=test_disable
TopicRecoveryTest test_prevent_recovery {"cloud_storage_type": 1} integration https://buildkite.com/redpanda/redpanda/builds/72845#01997c49-3b86-4c6e-a1e1-60c2fd8d6ae2 FLAKY 20/21 upstream reliability is '96.6542750929368'. current run reliability is '95.23809523809523'. drift is 1.41618 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=TopicRecoveryTest&test_method=test_prevent_recovery
test results on build#72911
test_class test_method test_arguments test_kind job_url test_status passed reason test_history
AuditLogTestKafkaApi test_no_auth_enabled {"audit_transport_mode": "rpc"} integration https://buildkite.com/redpanda/redpanda/builds/72911#01997edb-f69b-4df3-a226-c3fd1e706dda FLAKY 20/21 upstream reliability is '100.0'. current run reliability is '95.23809523809523'. drift is 4.7619 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=AuditLogTestKafkaApi&test_method=test_no_auth_enabled
ClusterRateQuotaTest test_client_response_and_produce_throttle_mechanism null integration https://buildkite.com/redpanda/redpanda/builds/72911#01997ee2-916b-4720-8cbb-ed8ac27057b5 FAIL 0/1 https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=ClusterRateQuotaTest&test_method=test_client_response_and_produce_throttle_mechanism
ClusterRateQuotaTest test_client_response_throttle_mechanism null integration https://buildkite.com/redpanda/redpanda/builds/72911#01997ee2-916c-4cad-a232-8416b00eeb51 FLAKY 16/21 upstream reliability is '81.93493150684932'. current run reliability is '76.19047619047619'. drift is 5.74446 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=ClusterRateQuotaTest&test_method=test_client_response_throttle_mechanism
RandomNodeOperationsTest test_node_operations {"cloud_storage_type": 2, "compaction_mode": "adjacent_merge", "enable_failures": true, "mixed_versions": false, "with_iceberg": false} integration https://buildkite.com/redpanda/redpanda/builds/72911#01997edb-f693-4cca-b714-ad6d5de0c52d FLAKY 20/21 upstream reliability is '100.0'. current run reliability is '95.23809523809523'. drift is 4.7619 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=RandomNodeOperationsTest&test_method=test_node_operations
WriteCachingFailureInjectionE2ETest test_crash_all {"use_transactions": false} integration https://buildkite.com/redpanda/redpanda/builds/72911#01997edb-f699-47bf-8be1-a07ce85edacb FLAKY 17/21 upstream reliability is '94.83101391650099'. current run reliability is '80.95238095238095'. drift is 13.87863 and the allowed drift is set to 50. The test should PASS https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=WriteCachingFailureInjectionE2ETest&test_method=test_crash_all

@travisdowns travisdowns force-pushed the td-export-cluster-config-once branch from f10ba82 to dccc4ab Compare September 24, 2025 18:10
@travisdowns
Copy link
Member Author

dccc4ab moves slightly the config check since in this audit test is produces an exact bad log line (because auditing is misconfigurated, this request fails), so just move it below the BLL check.

@travisdowns travisdowns marked this pull request as draft September 24, 2025 19:03
@travisdowns travisdowns force-pushed the td-export-cluster-config-once branch from dccc4ab to 532e7a3 Compare September 24, 2025 19:19
@travisdowns travisdowns marked this pull request as ready for review September 24, 2025 19:20
This should be done only once, don't do it on service stop, but in the
@cluster cleanup code, before we stop redpanda.

This saves six seconds on every test, since the second export (due to
the second stop) took six seconds. Hat tip to Stephan for noticing
this.
@travisdowns
Copy link
Member Author

travisdowns commented Sep 25, 2025

this push of c6d0059 removes some checks I added a while ago because we needed to swap the Service and RedpandaABC order, as ABC must occur after Service in the MRO for @AbstractMethod in ABC to be implemented in Service. We can be quite sure by now that the two base classes concerned do have those attributes by now!

@StephanDollberg StephanDollberg merged commit a758e48 into redpanda-data:dev Sep 25, 2025
17 checks passed
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.

3 participants