[#680,!426] Delete embedded options when subnet gets deleted in mysql_cb.

configure.ac

@@ -1722,6 +1722,7 @@ AC_CONFIG_FILES([Makefile
                  src/share/database/scripts/mysql/upgrade_6.0_to_7.0.sh
                  src/share/database/scripts/mysql/upgrade_7.0_to_8.0.sh
                  src/share/database/scripts/mysql/upgrade_8.0_to_8.1.sh
+                 src/share/database/scripts/mysql/upgrade_8.1_to_8.2.sh
                  src/share/database/scripts/mysql/wipe_data.sh
                  src/share/database/scripts/pgsql/Makefile
                  src/share/database/scripts/pgsql/upgrade_1.0_to_2.0.sh

src/bin/admin/tests/mysql_tests.sh.in

@@ -258,7 +258,7 @@ mysql_upgrade_test() {
 
     assert_str_eq "1.0" ${version} "Expected kea-admin to return %s, returned value was %s"
 
-    # Ok, we have a 1.0 database. Let's upgrade it to 8.1
+    # Ok, we have a 1.0 database. Let's upgrade it to 8.2
     ${keaadmin} db-upgrade mysql -u $db_user -p $db_password -n $db_name -d $db_scripts_dir
     ERRCODE=$?
 
@@ -689,7 +689,7 @@ EOF
     qry="SELECT COUNT(*) FROM parameter_data_type";
     run_statement "parameter_data_type count" "$qry" 4;
 
-    # Schema upgrade from 8.0 to 8.1
+    # Schema upgrade from 8.0 to 8.2
 
     # New lifetime bounds.
 
@@ -709,9 +709,9 @@ EOF
     qry="select subnet_prefix, client_class, interface, modification_ts, preferred_lifetime, min_preferred_lifetime, max_preferred_lifetime, rapid_commit, rebind_timer, relay, renew_timer, require_client_classes, reservation_mode, shared_network_name, subnet_id, user_context, valid_lifetime, min_valid_lifetime, max_valid_lifetime, calculate_tee_times, t1_percent, t2_percent from dhcp6_subnet"
     run_statement "dhcp6_subnet" "$qry"
 
-    # Verify upgraded schema reports version 8.1
+    # Verify upgraded schema reports version 8.2
     version=$(${keaadmin} db-version mysql -u $db_user -p $db_password -n $db_name -d $db_scripts_dir)
-    assert_str_eq "8.1" ${version} "Expected kea-admin to return %s, returned value was %s"
+    assert_str_eq "8.2" ${version} "Expected kea-admin to return %s, returned value was %s"
 
     # Let's wipe the whole database
     mysql_wipe

src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc

@@ -2597,7 +2597,8 @@ MySqlConfigBackendDHCPv4Impl::MySqlConfigBackendDHCPv4Impl(const DatabaseConnect
 }
 
 MySqlConfigBackendDHCPv4::MySqlConfigBackendDHCPv4(const DatabaseConnection::ParameterMap& parameters)
-    : impl_(new MySqlConfigBackendDHCPv4Impl(parameters)) {
+    : base_impl_(new MySqlConfigBackendDHCPv4Impl(parameters)), impl_() {
+    impl_ = boost::dynamic_pointer_cast<MySqlConfigBackendDHCPv4Impl>(base_impl_);
 }
 
 Subnet4Ptr

src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h

@@ -7,6 +7,7 @@
 #ifndef MYSQL_CONFIG_BACKEND_DHCP4_H
 #define MYSQL_CONFIG_BACKEND_DHCP4_H
 
+#include <mysql_cb_impl.h>
 #include <database/database_connection.h>
 #include <dhcpsrv/config_backend_dhcp4.h>
 #include <mysql_cb_log.h>
@@ -538,12 +539,15 @@ public:
     /// This should be called by the hook lib unload() function.
     static void unregisterBackendType();
 
-private:
+protected:
+
+    /// @brief Pointer to the base implementation of the backend shared by
+    /// DHCPv4 and DHCPv6 servers.
+    boost::shared_ptr<MySqlConfigBackendImpl> base_impl_;
 
     /// @brief Pointer to the implementation of the @c MySqlConfigBackendDHCPv4
     /// class.
     boost::shared_ptr<MySqlConfigBackendDHCPv4Impl> impl_;
-
 };
 
 /// @brief Pointer to the @c MySqlConfigBackendDHCPv4 class.

src/hooks/dhcp/mysql_cb/mysql_cb_impl.h

@@ -110,7 +110,7 @@ public:
     explicit MySqlConfigBackendImpl(const db::DatabaseConnection::ParameterMap& parameters);
 
     /// @brief Destructor.
-    ~MySqlConfigBackendImpl();
+    virtual ~MySqlConfigBackendImpl();
 
     /// @brief Creates MySQL binding from a @c Triplet.
     ///

src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <config.h>
 #include <mysql_cb_dhcp4.h>
+#include <mysql_cb_impl.h>
 #include <database/db_exceptions.h>
 #include <database/server.h>
 #include <database/testutils/schema.h>
@@ -20,6 +21,7 @@
 #include <dhcpsrv/testutils/generic_backend_unittest.h>
 #include <mysql/testutils/mysql_schema.h>
 #include <boost/shared_ptr.hpp>
+#include <mysql.h>
 #include <gtest/gtest.h>
 #include <map>
 #include <sstream>
@@ -33,6 +35,24 @@ using namespace isc::dhcp::test;
 
 namespace {
 
+/// @brief Test implementation of the MySQL configuration backend.
+///
+/// It exposes protected members of the @c MySqlConfigBackendDHCPv4.
+class TestMySqlConfigBackendDHCPv4 : public MySqlConfigBackendDHCPv4 {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// @param parameters A data structure relating keywords and values
+    /// concerned with the database.
+    explicit TestMySqlConfigBackendDHCPv4(const DatabaseConnection::ParameterMap& parameters)
+        : MySqlConfigBackendDHCPv4(parameters) {
+    }
+
+    using MySqlConfigBackendDHCPv4::base_impl_;
+
+};
+
 /// @brief Test fixture class for @c MySqlConfigBackendDHCPv4.
 ///
 /// @todo The tests we're providing here only test cases when the
@@ -58,7 +78,7 @@ public:
             // Create MySQL connection and use it to start the backend.
             DatabaseConnection::ParameterMap params =
                 DatabaseConnection::parse(validMySQLConnectionString());
-            cbptr_.reset(new MySqlConfigBackendDHCPv4(params));
+            cbptr_.reset(new TestMySqlConfigBackendDHCPv4(params));
 
         } catch (...) {
             std::cerr << "*** ERROR: unable to open database. The test\n"
@@ -85,6 +105,43 @@ public:
         destroyMySQLSchema();
     }
 
+    /// @brief Counts rows in a selected table in MySQL database.
+    ///
+    /// This method can be used to verify that some configuration elements were
+    /// deleted from a selected table as a result of cascade delete or a trigger.
+    /// For example, deleting a subnet should trigger deletion of its address
+    /// pools and options. By counting the rows on each table we can determine
+    /// whether the deletion took place on all tables for which it was expected.
+    ///
+    /// @param table Table name.
+    /// @return Number of rows in the specified table.
+    size_t countRows(const std::string& table) const {
+        auto p = boost::dynamic_pointer_cast<TestMySqlConfigBackendDHCPv4>(cbptr_);
+        if (!p) {
+            ADD_FAILURE() << "cbptr_ does not cast to TestMySqlConfigBackendDHCPv4";
+            return (0);
+        }
+
+        // Reuse the existing connection of the backend.
+        auto impl = boost::dynamic_pointer_cast<MySqlConfigBackendImpl>(p->base_impl_);
+        auto& conn = impl->conn_;
+
+        // Execute a simple select query on all rows.
+        std::string query = "SELECT * FROM " + table;
+        auto status = mysql_query(conn.mysql_, query.c_str());
+        if (status != 0) {
+            ADD_FAILURE() << "Query failed: " << mysql_error(conn.mysql_);
+            return (0);
+        }
+
+        // Get the number of rows returned and free the result.
+        MYSQL_RES * res = mysql_store_result(conn.mysql_);
+        unsigned numrows = static_cast<unsigned>(mysql_num_rows(res));
+        mysql_free_result(res);
+
+        return (numrows);
+    }
+
     /// @brief Creates several servers used in tests.
     void initTestServers() {
         test_servers_.push_back(Server::create(ServerTag("server1"), "this is server 1"));
@@ -614,6 +671,8 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAndDeleteAllServers) {
     // The number of deleted server should be equal to the number of
     // inserted servers. The logical 'all' server should be excluded.
     EXPECT_EQ(test_servers_.size() - 1, deleted_servers);
+
+    EXPECT_EQ(1, countRows("dhcp4_server"));
 }
 
 // This test verifies that the global parameter can be added, updated and
@@ -1915,6 +1974,27 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getSharedNetworkSubnets4) {
     EXPECT_TRUE(isEquivalent(returned_list, test_list));
 }
 
+TEST_F(MySqlConfigBackendDHCPv4Test, subnetOptions) {
+    EXPECT_NO_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ALL(), test_subnets_[0]));
+    EXPECT_EQ(3, countRows("dhcp4_options"));
+
+    EXPECT_NO_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ALL(), test_subnets_[1]));
+    EXPECT_EQ(2, countRows("dhcp4_options"));
+
+    EXPECT_NO_THROW(cbptr_->deleteSubnet4(ServerSelector::ALL(), test_subnets_[1]->getID()));
+    EXPECT_EQ(0, countRows("dhcp4_subnet"));
+    EXPECT_EQ(0, countRows("dhcp4_pool"));
+    EXPECT_EQ(0, countRows("dhcp4_options"));
+
+    EXPECT_NO_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ALL(), test_subnets_[0]));
+    EXPECT_EQ(3, countRows("dhcp4_options"));
+
+    EXPECT_NO_THROW(cbptr_->deleteSubnet4(ServerSelector::ALL(), test_subnets_[0]->getID()));
+    EXPECT_EQ(0, countRows("dhcp4_subnet"));
+    EXPECT_EQ(0, countRows("dhcp4_pool"));
+    EXPECT_EQ(0, countRows("dhcp4_options"));
+}
+
 // Test that shared network can be inserted, fetched, updated and then
 // fetched again.
 TEST_F(MySqlConfigBackendDHCPv4Test, getSharedNetwork4) {

src/lib/mysql/mysql_constants.h

@@ -52,7 +52,7 @@ const int MLM_MYSQL_FETCH_FAILURE = 0;
 /// @name Current database schema version values.
 //@{
 const uint32_t MYSQL_SCHEMA_VERSION_MAJOR = 8;
-const uint32_t MYSQL_SCHEMA_VERSION_MINOR = 1;
+const uint32_t MYSQL_SCHEMA_VERSION_MINOR = 2;
 
 //@}
 

src/share/database/scripts/mysql/.gitignore

@@ -9,4 +9,5 @@
 /upgrade_6.0_to_7.0.sh
 /upgrade_7.0_to_8.0.sh
 /upgrade_8.0_to_8.1.sh
+/upgrade_8.1_to_8.2.sh
 /wipe_data.sh

src/share/database/scripts/mysql/Makefile.am

@@ -14,6 +14,7 @@ sqlscripts_DATA += upgrade_5.2_to_6.0.sh
 sqlscripts_DATA += upgrade_6.0_to_7.0.sh
 sqlscripts_DATA += upgrade_7.0_to_8.0.sh
 sqlscripts_DATA += upgrade_8.0_to_8.1.sh
+sqlscripts_DATA += upgrade_8.1_to_8.2.sh
 sqlscripts_DATA += wipe_data.sh
 
 DISTCLEANFILES = upgrade_1.0_to_2.0.sh
@@ -26,7 +27,7 @@ DISTCLEANFILES += upgrade_5.1_to_5.2.sh
 DISTCLEANFILES += upgrade_5.2_to_6.0.sh
 DISTCLEANFILES += upgrade_6.0_to_7.0.sh
 DISTCLEANFILES += upgrade_7.0_to_8.0.sh
-DISTCLEANFILES += upgrade_8.0_to_8.1.sh
+DISTCLEANFILES += upgrade_8.1_to_8.2.sh
 DISTCLEANFILES += wipe_data.sh
 
 EXTRA_DIST = ${sqlscripts_DATA}

src/share/database/scripts/mysql/dhcpdb_create.mysql

@@ -2377,6 +2377,40 @@ SET version = '8', minor = '1';
 
 # This line concludes database upgrade to version 8.1.
 
+# Do not perform cascade deletion of the data in the dhcp4_pool because
+# the cascade deletion does not execute triggers associated with the table.
+# Instead we are going to use triggers on the dhcp4_subnet table.
+ALTER TABLE dhcp4_pool
+    DROP FOREIGN KEY fk_dhcp4_pool_subnet_id;
+
+ALTER TABLE dhcp4_pool
+    ADD CONSTRAINT fk_dhcp4_pool_subnet_id FOREIGN KEY (subnet_id)
+    REFERENCES dhcp4_subnet (subnet_id)
+    ON DELETE NO ACTION ON UPDATE CASCADE;
+
+
+# Drop existing trigger on the dhcp4_subnet table.
+DROP TRIGGER dhcp4_subnet_ADEL;
+
+# Create new trigger which will delete pools associated with the subnet and
+# the options associated with the subnet.
+DELIMITER $$
+CREATE TRIGGER dhcp4_subnet_BDEL BEFORE DELETE ON dhcp4_subnet
+    FOR EACH ROW
+    BEGIN
+        CALL createAuditEntryDHCP4('dhcp4_subnet', OLD.subnet_id, "delete");
+        DELETE FROM dhcp4_pool WHERE subnet_id = OLD.subnet_id;
+        DELETE FROM dhcp4_options WHERE dhcp4_subnet_id = OLD.subnet_id;
+    END $$
+DELIMITER ;
+
+
+# Update the schema version number
+UPDATE schema_version
+SET version = '8', minor = '2';
+
+# This line concludes database upgrade to version 8.2.
+
 
 # Notes:
 #

src/share/database/scripts/mysql/upgrade_8.1_to_8.2.sh.in

@@ -0,0 +1,59 @@
+#!/bin/sh
+
+prefix=@prefix@
+# Include utilities. Use installed version if available and
+# use build version if it isn't.
+if [ -e @datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh ]; then
+    . @datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh
+else
+    . @abs_top_builddir@/src/bin/admin/admin-utils.sh
+fi
+
+VERSION=`mysql_version "$@"`
+
+if [ "$VERSION" != "8.1" ]; then
+    printf "This script upgrades 8.1 to 8.2. Reported version is $VERSION. Skipping upgrade.\n"
+    exit 0
+fi
+
+mysql "$@" <<EOF
+
+# Do not perform cascade deletion of the data in the dhcp4_pool because
+# the cascade deletion does not execute triggers associated with the table.
+# Instead we are going to use triggers on the dhcp4_subnet table.
+ALTER TABLE dhcp4_pool
+    DROP FOREIGN KEY fk_dhcp4_pool_subnet_id;
+
+ALTER TABLE dhcp4_pool
+    ADD CONSTRAINT fk_dhcp4_pool_subnet_id FOREIGN KEY (subnet_id)
+    REFERENCES dhcp4_subnet (subnet_id)
+    ON DELETE NO ACTION ON UPDATE CASCADE;
+
+
+# Drop existing trigger on the dhcp4_subnet table.
+DROP TRIGGER dhcp4_subnet_ADEL;
+
+# Create new trigger which will delete pools associated with the subnet and
+# the options associated with the subnet.
+DELIMITER $$
+CREATE TRIGGER dhcp4_subnet_BDEL BEFORE DELETE ON dhcp4_subnet
+    FOR EACH ROW
+    BEGIN
+        CALL createAuditEntryDHCP4('dhcp4_subnet', OLD.subnet_id, "delete");
+        DELETE FROM dhcp4_pool WHERE subnet_id = OLD.subnet_id;
+        DELETE FROM dhcp4_options WHERE dhcp4_subnet_id = OLD.subnet_id;
+    END $$
+DELIMITER ;
+
+
+# Update the schema version number
+UPDATE schema_version
+SET version = '8', minor = '2';
+
+# This line concludes database upgrade to version 8.2.
+
+EOF
+
+RESULT=$?
+
+exit $?