[master] Merge branch 'trac2576'

src/bin/loadzone/loadzone.py.in

@@ -200,29 +200,6 @@ class LoadZoneRunner:
         logger.info(LOADZONE_SQLITE3_USING_DEFAULT_CONFIG, default_db_file)
         return '{"database_file": "' + default_db_file + '"}'
 
-    def __cancel_create(self):
-        '''sqlite3-only hack: delete the zone just created on load failure.
-
-        This should eventually be done via generic datasrc API, but right now
-        we don't have that interface.  Leaving the zone in this situation
-        is too bad, so we handle it with a workaround.
-
-        '''
-        if self._datasrc_type is not 'sqlite3':
-            return
-
-        import sqlite3          # we need the module only here
-        import json
-
-        # If we are here, the following should basically succeed; since
-        # this is considered a temporary workaround we don't bother to catch
-        # and recover rare failure cases.
-        dbfile = json.loads(self._datasrc_config)['database_file']
-        with sqlite3.connect(dbfile) as conn:
-            cur = conn.cursor()
-            cur.execute("DELETE FROM zones WHERE name = ?",
-                        [self._zone_name.to_text()])
-
     def _report_progress(self, loaded_rrs):
         '''Dump the current progress report to stdout.
 
@@ -274,10 +251,10 @@ class LoadZoneRunner:
                 self.__loaded_rrs >= self._report_interval):
                 sys.stdout.write('\n')
         except Exception as ex:
-            # release any remaining lock held in the client/loader
-            loader, datasrc_client = None, None
+            # release any remaining lock held in the loader
+            loader = None
             if created:
-                self.__cancel_create()
+                datasrc_client.delete_zone(self._zone_name)
                 logger.error(LOADZONE_CANCEL_CREATE_ZONE, self._zone_name,
                              self._zone_class)
             raise LoadFailure(str(ex))

src/lib/python/isc/datasrc/client_inc.cc

@@ -100,15 +100,12 @@ Note: This is a tentative API, and this method is likely to change or\n\
 be removed in the near future. For that reason, it currently provides\n\
 a default implementation that throws NotImplemented.\n\
 \n\
-Apart from the two exceptions mentioned below, in theory this call can\n\
-throw anything, depending on the implementation of the datasource\n\
-backend.\n\
-\n\
 Exceptions:\n\
   NotImplemented If the datasource backend does not support direct\n\
              zone creation.\n\
   DataSourceError If something goes wrong in the data source while\n\
-             creating the zone.\n\
+             creating the zone, or there are some really unexpected\n\
+             errors.\n\
 \n\
 Parameters:\n\
   name       The (fully qualified) name of the zone to create\n\
@@ -117,6 +114,42 @@ Return Value(s): True if the zone was added, False if it already\n\
 existed\n\
 ";
 
+const char* const DataSourceClient_deleteZone_doc = "\
+delete_zone(zone_name) -> bool\n\
+\n\
+Delete a zone from the data source.\n\
+\n\
+This method also checks if the specified zone exists in the data\n\
+source, and returns true/false depending on whether the zone\n\
+existed/not existed, respectively. In either case, on successful\n\
+return it ensures the data source does not contain the specified name\n\
+of the zone.\n\
+\n\
+Note: This is a tentative API, and this method is likely to change or\n\
+be removed in the near future. For that reason, it currently provides\n\
+a default implementation that throws NotImplemented. Note also that\n\
+this method does not delete other database records related to the\n\
+zone, such as zone's resource records or differences corresponding to\n\
+updates made in the zone. This is primarily for implementation\n\
+simplicity (in the currently intended usage there wouldn't be such\n\
+other data at the time of this call anyway) and due to the fact that\n\
+details of managing zones is still in flux. Once the design in this\n\
+area is fixed we may revisit the behavior.\n\
+\n\
+Exceptions:\n\
+  NotImplemented If the datasource backend does not support direct\n\
+             zone deletion.\n\
+  DataSourceError If something goes wrong in the data source while\n\
+             deleting the zone, or there are some really unexpected\n\
+             errors.\n\
+\n\
+Parameters:\n\
+  zone_name  The (fully qualified) name of the zone to be deleted\n\
+\n\
+Return Value(s): true if the zone previously existed and has been\n\
+deleted by this method; false if the zone didn't exist.\n\
+";
+
 const char* const DataSourceClient_getIterator_doc = "\
 get_iterator(name, separate_rrs=False) -> ZoneIterator\n\
 \n\

src/lib/python/isc/datasrc/client_python.cc

@@ -123,6 +123,36 @@ DataSourceClient_createZone(PyObject* po_self, PyObject* args) {
 }
 
 PyObject*
+DataSourceClient_deleteZone(PyObject* po_self, PyObject* args) {
+    s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
+    PyObject* po_name;
+    if (PyArg_ParseTuple(args, "O!", &name_type, &po_name)) {
+        try {
+            if (self->client->deleteZone(PyName_ToName(po_name))) {
+                Py_RETURN_TRUE;
+            } else {
+                Py_RETURN_FALSE;
+            }
+        } catch (const DataSourceError& dse) {
+            PyErr_SetString(getDataSourceException("Error"), dse.what());
+            return (NULL);
+        } catch (const isc::NotImplemented& ni) {
+            PyErr_SetString(getDataSourceException("NotImplemented"),
+                            ni.what());
+            return (NULL);
+        } catch (const std::exception& exc) {
+            PyErr_SetString(getDataSourceException("Error"), exc.what());
+            return (NULL);
+        } catch (...) {
+            PyErr_SetString(getDataSourceException("Error"),
+                            "Unexpected exception");
+            return (NULL);
+        }
+    }
+    return (NULL);
+}
+
+PyObject*
 DataSourceClient_getIterator(PyObject* po_self, PyObject* args) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
     PyObject* name_obj;
@@ -263,6 +293,8 @@ PyMethodDef DataSourceClient_methods[] = {
       DataSourceClient_findZone_doc },
     { "create_zone", DataSourceClient_createZone, METH_VARARGS,
       DataSourceClient_createZone_doc },
+    { "delete_zone", DataSourceClient_deleteZone, METH_VARARGS,
+      DataSourceClient_deleteZone_doc },
     { "get_iterator",
       DataSourceClient_getIterator, METH_VARARGS,
       DataSourceClient_getIterator_doc },

src/lib/python/isc/datasrc/tests/datasrc_test.py

@@ -634,23 +634,27 @@ class DataSrcUpdater(unittest.TestCase):
         self.assertEqual(None, iterator.get_soa())
         self.assertEqual(None, iterator.get_next_rrset())
 
-    def test_create_zone_args(self):
+    def test_create_or_delete_zone_args(self):
         dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
 
-        self.assertRaises(TypeError, dsc.create_zone)
-        self.assertRaises(TypeError, dsc.create_zone, 1)
-        self.assertRaises(TypeError, dsc.create_zone, None)
-        self.assertRaises(TypeError, dsc.create_zone, "foo.")
-        self.assertRaises(TypeError, dsc.create_zone,
-                          isc.dns.Name("example.org"), 1)
+        for method in [dsc.create_zone, dsc.delete_zone]:
+            self.assertRaises(TypeError, method)
+            self.assertRaises(TypeError, method, 1)
+            self.assertRaises(TypeError, method, None)
+            self.assertRaises(TypeError, method, "foo.")
+            self.assertRaises(TypeError, method, isc.dns.Name("example.org"),
+                              1)
 
-    def test_create_zone(self):
+    def test_create_delete_zone(self):
         dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
         # Note, using example.org here, which should not exist
         zone_name = isc.dns.Name("example.org")
         self.assertIsNone(dsc.get_updater(zone_name, True))
         self.assertRaises(isc.datasrc.Error, dsc.get_iterator, zone_name)
 
+        # delete_zone should return False, meaning it didn't exist.
+        self.assertFalse(dsc.delete_zone(zone_name))
+
         self.assertTrue(dsc.create_zone(zone_name))
 
         # should exist now, we should be able to get an updater
@@ -663,6 +667,12 @@ class DataSrcUpdater(unittest.TestCase):
         # Trying to create it again should return False
         self.assertFalse(dsc.create_zone(zone_name))
 
+        # Now that it exists, delete_zone should return True, and it cannot
+        # be found any more.
+        self.assertTrue(dsc.delete_zone(zone_name))
+        self.assertEqual(isc.datasrc.DataSourceClient.NOTFOUND,
+                         dsc.find_zone(zone_name)[0])
+
     def test_create_zone_locked(self):
         zone_name = isc.dns.Name("example.org")
         dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
@@ -670,10 +680,13 @@ class DataSrcUpdater(unittest.TestCase):
 
         # Should fail since db is locked
         self.assertRaises(isc.datasrc.Error, dsc.create_zone, zone_name)
+        self.assertRaises(isc.datasrc.Error, dsc.delete_zone,
+                          isc.dns.Name('example.com'))
 
-        # Cancel updater, then create should succeed
+        # Cancel updater, then create/delete should succeed
         updater = None
         self.assertTrue(dsc.create_zone(zone_name))
+        self.assertTrue(dsc.delete_zone(zone_name))
 
     def test_create_zone_not_implemented(self):
         mem_cfg = '{ "type": "memory", "class": "IN", "zones": [] }';