Skip to content

Commit c6d0059

Browse files
committed
rptest: move cluster config export to @cluster
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.
1 parent cc0f5a5 commit c6d0059

File tree

2 files changed

+24
-20
lines changed

2 files changed

+24
-20
lines changed

tests/rptest/services/cluster.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ def _do_post_test_checks(
154154
redpanda.cloud_storage_diagnostics()
155155
raise
156156

157+
redpanda.export_cluster_config()
158+
157159
if isinstance(redpanda, RedpandaService):
158160
if test_failed:
159161
redpanda.cloud_storage_diagnostics()

tests/rptest/services/redpanda.py

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,13 +1212,13 @@ class has both implementation and abstract methods, only for methods which
12121212

12131213
def __init__(self, *args, **kwargs) -> None:
12141214
super().__init__(*args, **kwargs)
1215-
# Now ensure that this base is part of the right type of object.
1216-
# For these checks to work, this __init__ method should appear in
1217-
# the MRO after all the __init__ methods that would set these properties
1218-
self._check_attr("context", TestContext)
1219-
self._check_attr("logger", Logger)
12201215
self._usage_stats = UsageStats()
12211216

1217+
@property
1218+
@abstractmethod
1219+
def logger(self) -> Logger:
1220+
pass
1221+
12221222
@property
12231223
def usage_stats(self) -> UsageStats:
12241224
"""
@@ -1248,6 +1248,9 @@ def kafka_client_security(self) -> KafkaClientSecurity:
12481248
on this broker."""
12491249
pass
12501250

1251+
def export_cluster_config(self) -> None:
1252+
self.logger.debug("export_cluster_config not implemented for this service")
1253+
12511254
def wait_until(
12521255
self,
12531256
fn: Callable[[], Any],
@@ -1331,16 +1334,6 @@ def wrapped():
13311334
logger=logger,
13321335
)
13331336

1334-
def _check_attr(self, name: str, t: Type) -> None:
1335-
v = getattr(self, name, None)
1336-
mro = self.__class__.__mro__
1337-
assert v is not None, (
1338-
f"RedpandaServiceABC was missing attribute {name} after __init__: {mro}\n"
1339-
)
1340-
assert isinstance(v, t), (
1341-
f"{name} had wrong type, expected {t} but was {type(v)}"
1342-
)
1343-
13441337
def _extract_samples(
13451338
self, metrics: Generator[Metric, Any, None], sample_pattern: str, node: Any
13461339
) -> list[MetricSample]:
@@ -1582,7 +1575,6 @@ def __init__(
15821575
# we save the test context under both names since RedpandaService and Service
15831576
# save them under these two names, respetively
15841577
self.context = self._context = context
1585-
self.logger = context.logger
15861578

15871579
super().__init__()
15881580

@@ -1667,6 +1659,10 @@ def __init__(
16671659
f"Not enough brokers: test needs {self._min_brokers} but cluster has {node_count}"
16681660
)
16691661

1662+
@property
1663+
def logger(self) -> Logger:
1664+
return self._context.logger
1665+
16701666
@property
16711667
def kubectl(self) -> KubectlTool:
16721668
assert self.__kubectl, "kubectl accessed before cluster was started?"
@@ -2379,7 +2375,7 @@ def copy_from_pod(params):
23792375
return {}
23802376

23812377

2382-
class RedpandaService(RedpandaServiceABC, Service):
2378+
class RedpandaService(Service, RedpandaServiceABC):
23832379
PERSISTENT_ROOT = "/var/lib/redpanda"
23842380
TRIM_LOGS_KEY = "trim_logs"
23852381
DATA_DIR = os.path.join(PERSISTENT_ROOT, "data")
@@ -4449,11 +4445,11 @@ def get_version_if_not_head(self, node):
44494445
return self.get_version(node)
44504446
return None
44514447

4452-
def stop(self, **kwargs: Any) -> None:
4448+
def export_cluster_config(self) -> None:
44534449
"""
4454-
Override default stop() to execude stop_node in parallel
4450+
Export the cluster configuration of all nodes to the ducktape log
4451+
directory for later inspection.
44554452
"""
4456-
self._stop_time = time.time() # The last time stop is invoked
44574453
self.logger.info("%s: exporting cluster config" % self.who_am_i())
44584454

44594455
service_dir = os.path.join(
@@ -4477,6 +4473,12 @@ def stop(self, **kwargs: Any) -> None:
44774473
# will not be able to get it from the admin API
44784474
self.logger.info(f"{self.who_am_i()}: error getting config: {e}")
44794475

4476+
def stop(self, **kwargs: Any) -> None:
4477+
"""
4478+
Override default stop() to execude stop_node in parallel
4479+
"""
4480+
self._stop_time = time.time() # The last time stop is invoked
4481+
44804482
self.logger.info("%s: stopping service" % self.who_am_i())
44814483

44824484
self.for_nodes(self.nodes, lambda n: self.stop_node(n, **kwargs))

0 commit comments

Comments
 (0)