From 54d76920019991854ccf86c213b70298bc02dd8e Mon Sep 17 00:00:00 2001
From: Erik Olof Gunnar Andersson <eandersson@blizzard.com>
Date: Thu, 24 Aug 2023 11:37:17 +0200
Subject: [PATCH] Raise error if producer configured with no valid tasks

There is no legit reason to have the producer running
if there are no tasks running.

Also improved the test coverage.

Change-Id: I05a69cf80785c912aaef2e29bfc0a1e5d57ceb6c
---
 designate/producer/service.py                 | 16 ++++--
 designate/tests/unit/producer/test_service.py | 50 +++++++++++++------
 2 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/designate/producer/service.py b/designate/producer/service.py
index 3fcec6a0b..c5720724b 100644
--- a/designate/producer/service.py
+++ b/designate/producer/service.py
@@ -19,6 +19,7 @@ import oslo_messaging as messaging
 
 from designate.central import rpcapi
 from designate import coordination
+from designate import exceptions
 from designate.producer import tasks
 from designate import quota
 from designate import service
@@ -84,9 +85,16 @@ class Service(service.RPCService):
         self._partitioner.start()
         self._partitioner.watch_partition_change(self._rebalance)
 
-        enabled_tasks = CONF['service:producer'].enabled_tasks
-        for task in tasks.PeriodicTask.get_extensions(enabled_tasks):
-            LOG.debug("Registering task %s", task)
+        enabled_tasks = tasks.PeriodicTask.get_extensions(
+            CONF['service:producer'].enabled_tasks
+        )
+        if not enabled_tasks:
+            raise exceptions.ConfigurationError(
+                'No periodic tasks found matching: %s' %
+                CONF['service:producer'].enabled_tasks
+            )
+        for task in enabled_tasks:
+            LOG.debug('Registering task %s', task)
 
             # Instantiate the task
             task = task()
@@ -102,5 +110,5 @@ class Service(service.RPCService):
         self.coordination.stop()
 
     def _rebalance(self, my_partitions, members, event):
-        LOG.info("Received rebalance event %s", event)
+        LOG.info('Received rebalance event %s', event)
         self.partition_range = my_partitions
diff --git a/designate/tests/unit/producer/test_service.py b/designate/tests/unit/producer/test_service.py
index a4e4d8af7..80c93d49c 100644
--- a/designate/tests/unit/producer/test_service.py
+++ b/designate/tests/unit/producer/test_service.py
@@ -24,10 +24,10 @@ from oslo_config import cfg
 from oslo_config import fixture as cfg_fixture
 import oslotest.base
 
+from designate import exceptions
 from designate.producer import service
 import designate.service
 from designate.tests import fixtures
-from designate.tests.unit import RoObject
 
 CONF = cfg.CONF
 
@@ -38,39 +38,59 @@ class ProducerTest(oslotest.base.BaseTestCase):
         conf = self.useFixture(cfg_fixture.Config(CONF))
         conf.conf([], project='designate')
 
-        service.CONF = RoObject({
-            'service:producer': RoObject({
-                'enabled_tasks': None,  # enable all tasks
-            }),
-            'producer_task:zone_purge': '',
-        })
         super(ProducerTest, self).setUp()
         self.stdlog = fixtures.StandardLogging()
         self.useFixture(self.stdlog)
 
+        self.tg = mock.Mock()
         self.service = service.Service()
+        self.service.coordination = mock.Mock()
         self.service.rpc_server = mock.Mock()
+        self.service.tg = self.tg
         self.service._storage = mock.Mock()
         self.service._quota = mock.Mock()
         self.service._quota.limit_check = mock.Mock()
 
-    @mock.patch.object(service.tasks, 'PeriodicTask')
     @mock.patch.object(service.coordination, 'Partitioner')
     @mock.patch.object(designate.service.RPCService, 'start')
-    def test_service_start(self, mock_rpc_start, mock_partitioner,
-                           mock_periodic_task):
-        self.service.coordination = mock.Mock()
+    def test_service_start(self, mock_rpc_start, mock_partitioner):
+        CONF.set_override('enabled_tasks', None, 'service:producer')
+
+        mock_partition = mock.Mock()
+        mock_partitioner.return_value = mock_partition
 
         self.service.start()
 
-        self.assertTrue(mock_rpc_start.called)
+        mock_rpc_start.assert_called_with()
+        mock_partition.watch_partition_change.assert_called()
+        mock_partition.start.assert_called()
+
+        # Make sure that tasks were added to the tg timer.
+        self.tg.add_timer.assert_called()
+        self.assertEqual(6, self.tg.add_timer.call_count)
+
+    @mock.patch.object(service.coordination, 'Partitioner')
+    @mock.patch.object(designate.service.RPCService, 'start')
+    def test_service_start_all_extension_disabled(self, mock_rpc_start,
+                                                  mock_partitioner):
+        CONF.set_override('enabled_tasks', [], 'service:producer')
+        self.assertRaisesRegex(
+            exceptions.ConfigurationError,
+            r'No periodic tasks found matching: \[\]',
+            self.service.start,
+        )
+
+        CONF.set_override('enabled_tasks', ['None'], 'service:producer')
+        self.assertRaisesRegex(
+            exceptions.ConfigurationError,
+            r'No periodic tasks found matching: \[\'None\'\]',
+            self.service.start,
+        )
 
     def test_service_stop(self):
-        self.service.coordination.stop = mock.Mock()
-
         self.service.stop()
 
-        self.assertTrue(self.service.coordination.stop.called)
+        self.service.coordination.stop.assert_called()
 
         self.assertIn('Stopping producer service', self.stdlog.logger.output)