[v23.3.x] "enable by default spillover manifest" testing followups

tests/rptest/infinite_retention/infinite_retention_test.py

@@ -197,8 +197,7 @@ def __init__(self, test_context, *args, **kwargs):
             'cloud_storage_manifest_max_upload_interval_sec':
             self.params.manifest_upload_interval,
             # Default retention is infinite
-            'delete_retention_ms':
-            self.params.retention_ms,
+            'delete_retention_ms': self.params.retention_ms,
             # enable merging to generate more re-uploads
             'cloud_storage_enable_segment_merging':
             self.params.cloud_storage_enable_segment_merging,
@@ -224,6 +223,9 @@ def __init__(self, test_context, *args, **kwargs):
             # evict it frequently
             'cloud_storage_materialized_manifest_ttl_ms':
             self.params.cloud_storage_materialized_manifest_ttl_ms,
+
+            # disable spillover: it will be explicitly set in the tests when needed
+            'cloud_storage_spillover_manifest_size': None,
         }
         super().__init__(test_context, node_prealloc_count=1, *args, **kwargs)
 

tests/rptest/scale_tests/tiered_storage_single_partition_test.py

@@ -92,6 +92,8 @@ def throughput_test(self, restart_stress):
             f"Configuring to spill metadata after {spillover_segments} segments"
         )
         self.redpanda.set_cluster_config({
+            'cloud_storage_spillover_manifest_size':
+            None,
             'cloud_storage_spillover_manifest_max_segments':
             spillover_segments
         })

tests/rptest/services/admin.py

@@ -379,7 +379,7 @@ def get_cluster_config_schema(self, node=None):
         return self._request("GET", "cluster_config/schema", node=node).json()
 
     def patch_cluster_config(self,
-                             upsert: dict[str, str] = {},
+                             upsert: dict[str, str | int | None] = {},
                              remove: list[str] = [],
                              force=False,
                              dry_run=False,

tests/rptest/services/redpanda.py

@@ -2532,19 +2532,15 @@ def set_cluster_config(self,
         Update cluster configuration and wait for all nodes to report that they
         have seen the new config.
 
-        :param values: dict of property name to value. if value is None, key will be removed from cluster config
+        :param values: dict of property name to value.
         :param expect_restart: set to true if you wish to permit a node restart for needs_restart=yes properties.
                                If you set such a property without this flag, an assertion error will be raised.
         """
         if admin_client is None:
             admin_client = self._admin
 
-        patch_result = admin_client.patch_cluster_config(
-            upsert={
-                k: v
-                for k, v in values.items() if v is not None
-            },
-            remove=[k for k, v in values.items() if v is None])
+        patch_result = admin_client.patch_cluster_config(upsert=values,
+                                                         remove=[])
         new_version = patch_result['config_version']
 
         self._wait_for_config_version(new_version,
@@ -3264,14 +3260,6 @@ def write_bootstrap_cluster_config(self):
             conf.update(
                 dict(http_authentication=self._security.http_authentication))
 
-        # in redpanda, cloud_storage_spillover_manifest_size takes preference over cloud_storage_spillover_manifest_max_segments.
-        # disable the former to use the latter
-        assert conf.get('cloud_storage_spillover_manifest_size', None) is None or \
-            conf.get('cloud_storage_spillover_manifest_max_segments', None) is None, \
-                  "cannot set cloud_storage_spillover_manifest_max_segments if cloud_storage_spillover_manifest_size is already set, it will not be used by redpanda"
-        if 'cloud_storage_spillover_manifest_max_segments' in conf:
-            conf['cloud_storage_spillover_manifest_size'] = None
-
         conf_yaml = yaml.dump(conf)
         for node in self.nodes:
             self.logger.info(

tests/rptest/tests/tiered_storage_model_test.py

@@ -15,7 +15,7 @@
 from threading import Condition
 from collections import defaultdict
 
-from ducktape.mark import matrix, ignore
+from ducktape.mark import matrix
 from ducktape.tests.test import TestContext
 from ducktape.utils.util import wait_until
 
@@ -547,7 +547,6 @@ def shutdown_bg_tasks(self):
         self.stop_flag = True
         self.thread_pool.shutdown()
 
-    @ignore  #see https://github.com/redpanda-data/redpanda/issues/16208
     @cluster(num_nodes=4)
     @matrix(cloud_storage_type=get_cloud_storage_type(),
             test_case=get_tiered_storage_test_cases(fast_run=True))

tests/rptest/tests/timequery_test.py

@@ -312,7 +312,7 @@ def set_up_cluster(self, cloud_storage: bool, batch_cache: bool,
             'cloud_storage_cache_chunk_size':
             self.chunk_size,
 
-            # Avoid confiugring spillover so tests can do it themselves.
+            # Avoid configuring spillover so tests can do it themselves.
             'cloud_storage_spillover_manifest_size':
             None,
         })

tests/rptest/tests/upgrade_test.py

@@ -506,7 +506,9 @@ def verify():
                 'cloud_storage_manifest_max_upload_interval_sec':
                 1,
                 'cloud_storage_spillover_manifest_max_segments':
-                2
+                2,
+                'cloud_storage_spillover_manifest_size':
+                None,
             },
                                        node=new_version_node)