[trac945] fix some config_update() problems

- fixes the uninitialized problem
- does not set values to None if they are not updated while others are
- restores previous config if there is an error in the new one
(+tests)

Also added a few TODO's, which are needed, but not as urgently as these (eg take defaults from spec, which would require more changes than i want to do right now, as does making a separate class for storing these values so they don't all have to be backed up when applying a new config)

src/bin/zonemgr/tests/zonemgr_test.py

@@ -434,6 +434,14 @@ class TestZonemgrRefresh(unittest.TestCase):
         self.assertTrue(zone_state == ZONE_REFRESHING)
 
     def test_update_config_data(self):
+        # make sure it doesn't fail if we only provide secondary zones
+        config_data = {
+                    "secondary_zones": [ { "name": "example.net.",
+                                           "class": "IN" } ]
+                }
+        self.zone_refresh.update_config_data(config_data)
+
+        # update all values
         config_data = {
                     "lowerbound_refresh" : 60,
                     "lowerbound_retry" : 30,
@@ -449,6 +457,37 @@ class TestZonemgrRefresh(unittest.TestCase):
         self.assertEqual(0.25, self.zone_refresh._refresh_jitter)
         self.assertEqual(0.75, self.zone_refresh._reload_jitter)
 
+        # make sure they are not reset when we only update one
+        config_data = {
+                    "reload_jitter" : 0.35,
+                }
+        self.zone_refresh.update_config_data(config_data)
+        self.assertEqual(60, self.zone_refresh._lowerbound_refresh)
+        self.assertEqual(30, self.zone_refresh._lowerbound_retry)
+        self.assertEqual(19800, self.zone_refresh._max_transfer_timeout)
+        self.assertEqual(0.25, self.zone_refresh._refresh_jitter)
+        self.assertEqual(0.35, self.zone_refresh._reload_jitter)
+
+        # and make sure we restore the previous config if something
+        # goes wrong
+        config_data = {
+                    "lowerbound_refresh" : 61,
+                    "lowerbound_retry" : 31,
+                    "max_transfer_timeout" : 19801,
+                    "refresh_jitter" : 0.21,
+                    "reload_jitter" : 0.71,
+                    "secondary_zones": [ { "name": "doesnotexist",
+                                           "class": "IN" } ]
+                }
+        self.assertRaises(ZonemgrException,
+                          self.zone_refresh.update_config_data,
+                          config_data)
+        self.assertEqual(60, self.zone_refresh._lowerbound_refresh)
+        self.assertEqual(30, self.zone_refresh._lowerbound_retry)
+        self.assertEqual(19800, self.zone_refresh._max_transfer_timeout)
+        self.assertEqual(0.25, self.zone_refresh._refresh_jitter)
+        self.assertEqual(0.35, self.zone_refresh._reload_jitter)
+
     def test_shutdown(self):
         self.zone_refresh._check_sock = self.zone_refresh._master_socket
         listener = self.zone_refresh.run_timer()

src/bin/zonemgr/zonemgr.py.in

@@ -101,6 +101,11 @@ class ZonemgrRefresh:
         self._check_sock = slave_socket
         self._db_file = db_file
         self._zonemgr_refresh_info = {}
+        self._lowerbound_refresh = None
+        self._lowerbound_retry = None
+        self._max_transfer_timeout = None
+        self._refresh_jitter = None
+        self._reload_jitter = None
         self.update_config_data(config_data)
         self._running = False
 
@@ -404,37 +409,59 @@ class ZonemgrRefresh:
 
     def update_config_data(self, new_config):
         """ update ZonemgrRefresh config """
+        # TODO: we probably want to store all this info in a nice
+        # class, so that we don't have to backup and restore every
+        # single value.
+        # TODO2: We also don't use get_default_value yet
         backup = self._zonemgr_refresh_info.copy()
+
+        # store the values so we can restore them if there is a problem
+        lowerbound_refresh_backup = self._lowerbound_refresh
+        self._lowerbound_refresh = new_config.get('lowerbound_refresh') or self._lowerbound_refresh
+
+        lowerbound_retry_backup = self._lowerbound_retry
+        self._lowerbound_retry = new_config.get('lowerbound_retry') or self._lowerbound_retry
+
+        max_transfer_timeout_backup = self._max_transfer_timeout
+        self._max_transfer_timeout = new_config.get('max_transfer_timeout') or self._max_transfer_timeout
+
+        refresh_jitter_backup = self._refresh_jitter
+        self._refresh_jitter = new_config.get('refresh_jitter') or self._refresh_jitter
+
+        reload_jitter_backup = self._reload_jitter
+        self._reload_jitter = new_config.get('reload_jitter') or self._reload_jitter
         try:
             required = {}
-            # Add new zones
-            for secondary_zone in new_config.get('secondary_zones'):
-                name = secondary_zone['name']
-                # Be tolerant to sclerotic users who forget the final dot
-                if name[-1] != '.':
-                    name = name + '.'
-                name_class = (name, secondary_zone['class'])
-                required[name_class] = True
-                # Add it only if it isn't there already
-                if not name_class in self._zonemgr_refresh_info:
-                    self.zonemgr_add_zone(name_class)
-            # Drop the zones that are no longer there
-            # Do it in two phases, python doesn't like deleting while iterating
-            to_drop = []
-            for old_zone in self._zonemgr_refresh_info:
-                if not old_zone in required:
-                    to_drop.append(old_zone)
-            for drop in to_drop:
-                del self._zonemgr_refresh_info[drop]
+            secondary_zones = new_config.get('secondary_zones')
+            if secondary_zones is not None:
+                # Add new zones
+                for secondary_zone in new_config.get('secondary_zones'):
+                    name = secondary_zone['name']
+                    # Be tolerant to sclerotic users who forget the final dot
+                    if name[-1] != '.':
+                        name = name + '.'
+                    name_class = (name, secondary_zone['class'])
+                    required[name_class] = True
+                    # Add it only if it isn't there already
+                    if not name_class in self._zonemgr_refresh_info:
+                        self.zonemgr_add_zone(name_class)
+                # Drop the zones that are no longer there
+                # Do it in two phases, python doesn't like deleting while iterating
+                to_drop = []
+                for old_zone in self._zonemgr_refresh_info:
+                    if not old_zone in required:
+                        to_drop.append(old_zone)
+                for drop in to_drop:
+                    del self._zonemgr_refresh_info[drop]
         # If we are not able to find it in database, restore the original
         except:
             self._zonemgr_refresh_info = backup
+            self._lowerbound_refresh = lowerbound_refresh_backup
+            self._lowerbound_retry = lowerbound_retry_backup
+            self._max_transfer_timeout = max_transfer_timeout_backup
+            self._refresh_jitter = refresh_jitter_backup
+            self._reload_jitter = reload_jitter_backup
             raise
-        self._lowerbound_refresh = new_config.get('lowerbound_refresh')
-        self._lowerbound_retry = new_config.get('lowerbound_retry')
-        self._max_transfer_timeout = new_config.get('max_transfer_timeout')
-        self._refresh_jitter = new_config.get('refresh_jitter')
-        self._reload_jitter = new_config.get('reload_jitter')
 
 class Zonemgr:
     """Zone manager class."""