diff --git a/src/v/cloud_storage/tests/topic_mount_handler_test.cc b/src/v/cloud_storage/tests/topic_mount_handler_test.cc index 449dcc000895b..bb921fb7c150c 100644 --- a/src/v/cloud_storage/tests/topic_mount_handler_test.cc +++ b/src/v/cloud_storage/tests/topic_mount_handler_test.cc @@ -43,6 +43,15 @@ static const model::topic_namespace test_tp_ns_override{ static ss::abort_source never_abort; static constexpr model::cloud_credentials_source config_file{ model::cloud_credentials_source::config_file}; + +static cluster::topic_configuration +get_topic_configuration(cluster::topic_properties topic_props) { + auto topic_cfg = cluster::topic_configuration( + test_tp_ns.ns, test_tp_ns.tp, 1, 1); + topic_cfg.properties = std::move(topic_props); + return topic_cfg; +} + } // namespace struct TopicMountHandlerFixture @@ -64,14 +73,6 @@ struct TopicMountHandlerFixture pool.stop().get(); } - cluster::topic_configuration - get_topic_configuration(cluster::topic_properties topic_props) const { - auto topic_cfg = cluster::topic_configuration( - test_tp_ns.ns, test_tp_ns.tp, 1, 1); - topic_cfg.properties = std::move(topic_props); - return topic_cfg; - } - ss::sharded pool; ss::sharded remote; }; diff --git a/src/v/cloud_storage/topic_mount_handler.cc b/src/v/cloud_storage/topic_mount_handler.cc index 9ae6247cb7ac7..d432a73076371 100644 --- a/src/v/cloud_storage/topic_mount_handler.cc +++ b/src/v/cloud_storage/topic_mount_handler.cc @@ -57,6 +57,10 @@ ss::future topic_mount_handler::check_mount( _bucket, manifest_path, parent, existence_check_type::manifest); // If the manifest exists, it is possible to mount the topic. + // If the result is anything but download_result::success, it is + // assumed that the mount manifest does not exist, or is unreachable for + // whatever reason. In that case, it is simple enough for the user to + // reissue a request to `mount_topic()`. co_return (exists_result == download_result::success) ? topic_mount_result::success : topic_mount_result::mount_manifest_does_not_exist; @@ -72,6 +76,8 @@ ss::future topic_mount_handler::commit_mount( const auto delete_result = co_await _remote.delete_object( _bucket, manifest_path, parent); + // If the upload_result is anything but upload_result::success, + // the mount manifest was not deleted, and we cannot mount the topic. co_return (delete_result == upload_result::success) ? topic_mount_result::success : topic_mount_result::mount_manifest_not_deleted; diff --git a/src/v/cloud_storage/topic_mount_handler.h b/src/v/cloud_storage/topic_mount_handler.h index db7cd93138b98..e5d640fd161eb 100644 --- a/src/v/cloud_storage/topic_mount_handler.h +++ b/src/v/cloud_storage/topic_mount_handler.h @@ -44,7 +44,7 @@ class topic_mount_handler { ss::future mount_topic( const cluster::topic_configuration& topic_cfg, retry_chain_node& parent); - // Perform the mounting process by deleting the topic mount manifest. + // Perform the unmounting process by uploading the topic mount manifest. // topic_cfg should be the recovered topic configuration from a topic // manifest in cloud storage. If it has a value, the remote_label stored in // the topic properties is used as the "source" label. Otherwise, the