cloud_storage: Move s3_imposter to cloud_io

[Lazin]

The s3_imposter is used in several subsystems already (iceberg/datalake/cloud_storage/cloud_topics). It makes sense to move it into test_utils to make dependencies less confusing.

The PR just moves s3_imposter.h(cc) to test_utils and updates build files and include paths.

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
• v24.3.x
• v24.2.x
• v24.1.x

Release Notes

• none

[dotnwat]

in v::random, for example, we only keep generic random utilities. it's v::subsystem that will have subsystem specific random generators.

do we want to put all sub-system specific test utilities into a single place? i think the answer to that question determines if this change makes sense. the current trend seems to be have A depends_on B implies many times that A::test depends_on B::test_utils.

[andrwng]

Just curious what the motivation for this change is. Also, did you consider putting this in, for example, cloud_io/tests? test_utils seems to generally contain things that might be useful for all tests with the exception of archival.h which seems like an outlier

[andrwng]

Curious in general why this couldn't have just moved the existing library definition from cloud_storage/BUILD. Like why does this depend on cloud_storage?

It's surprising this would depend on these too:

• cluster
• disk_log_builder

I think a lot of the boost deps would be better as implementation_deps

[Lazin]

I extracted this PR from the cloud topics branch, and there I think I just copied this stuff from one place to another. Will clean this up.

[Lazin]

I think moving to v::cloud_io makes sense. I'm OK with this as long as it's not part of the cloud_storage. When it's in cloud_storage it's weird because tests for other subsystems depend on cloud_storage even though the actual subsystem doesn't.

[vbotbuildovich]

CI test results

test results on build#61228

test_id	test_kind	job_url	test_status	passed
rptest.tests.compaction_recovery_test.CompactionRecoveryUpgradeTest.test_index_recovery_after_upgrade	ducktape	https://buildkite.com/redpanda/redpanda/builds/61228#0194a90f-7d68-43cf-b18e-4e324d979213	FLAKY	1/2

test results on build#61249

test_id	test_kind	job_url	test_status	passed
gtest_raft_rpunit.gtest_raft_rpunit	unit	https://buildkite.com/redpanda/redpanda/builds/61249#0194a9ca-3fd7-42ca-8392-2cb6fbfcc8cc	FLAKY	1/2
rptest.tests.delete_records_test.DeleteRecordsTest.test_delete_records_with_transactions.cloud_storage_enabled=True	ducktape	https://buildkite.com/redpanda/redpanda/builds/61249#0194aa25-d142-4430-8dbc-88c21aed2102	FLAKY	1/2

test results on build#61287

test_id	test_kind	job_url	test_status	passed
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery	ducktape	https://buildkite.com/redpanda/redpanda/builds/61287#0194aca7-2b75-4806-919b-f7d553388f4e	FLAKY	1/2
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery	ducktape	https://buildkite.com/redpanda/redpanda/builds/61287#0194acc3-1f58-485a-a356-087206449889	FLAKY	1/2
rptest.tests.datalake.compaction_test.CompactionGapsTest.test_translation_no_gaps.cloud_storage_type=CloudStorageType.S3	ducktape	https://buildkite.com/redpanda/redpanda/builds/61287#0194acc3-1f59-4955-9d85-6b15220b807d	FLAKY	1/3
rptest.tests.raft_availability_test.RaftAvailabilityTest.test_leader_transfers_recovery.acks=-1	ducktape	https://buildkite.com/redpanda/redpanda/builds/61287#0194aca7-2b74-4a4c-8b22-e4bd3a1a99a5	FLAKY	1/2
test_kafka_protocol_unit_rpunit.test_kafka_protocol_unit_rpunit	unit	https://buildkite.com/redpanda/redpanda/builds/61287#0194ac63-9f29-4fb7-93bb-64f206bc0c02	FLAKY	1/2

test results on build#61315

test_id	test_kind	job_url	test_status	passed
rptest.tests.scaling_up_test.ScalingUpTest.test_scaling_up_with_recovered_topic	ducktape	https://buildkite.com/redpanda/redpanda/builds/61315#0194ae9e-8fbb-46fc-b710-9c367976383f	FLAKY	1/3

[Lazin]

Moved to cloud_io/tests

[dotnwat]

I think moving to v::cloud_io makes sense. I'm OK with this as long as it's not part of the cloud_storage. When it's in cloud_storage it's weird because tests for other subsystems depend on cloud_storage even though the actual subsystem doesn't.

+1 to rehoming into cloud_io

[dotnwat]

bazel build needs a few tweeks

ERROR: /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-04bdc3fc7bbed132d-1/redpanda/redpanda/src/v/datalake/coordinator/tests/BUILD:83:18: no such target '//src/v/cloud_storage/tests:s3_imposter': target 's3_imposter' not declared in package 'src/v/cloud_storage/tests' defined by /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-04bdc3fc7bbed132d-1/redpanda/redpanda/src/v/cloud_storage/tests/BUILD and referenced by '//src/v/datalake/coordinator/tests:iceberg_snapshot_remover_test'
--
  | Analyzing: 1724 targets (1415 packages loaded, 84344 targets configured)

[dotnwat]

s3_imposter.{cc,h} didn't change, so why did implementation_deps change?

[Lazin]

@andrwng pointed out that some of the deps are not used

[andrwng]

Mm I don't think that's quite related though. I'd pointed out there are some unnecessary things in deps, but Noah's pointing out that the implementation_deps list was removed in this move.

IIUC the general guidance is that implementation_deps should include things that are in the cc file but not the header, and deps should include things required by the header.

[dotnwat]

andrew is right. iobuf, for example, is included in .cc but not in .h so it at least looks to be a candidate for implementation_deps. similarily for other things in the deps = [] list, but i think at least not regressing on the list makes sense. i don't know which ones are unused, but that is separate.