From: Francis Dupont Date: Wed, 23 Jan 2019 15:07:45 +0000 (+0100) Subject: [313-return-a-list-of-all-reservations-by-subnet-id] Addressed more comments X-Git-Tag: 429-Updated-StampedValue-to-support-reals_base~77 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=12a3d0b2af12a03d48e6fdec3276ca5b02c12e60;p=thirdparty%2Fkea.git [313-return-a-list-of-all-reservations-by-subnet-id] Addressed more comments --- diff --git a/doc/guide/hooks.xml b/doc/guide/hooks.xml index c3e340cfe1..8aa341a7d0 100644 --- a/doc/guide/hooks.xml +++ b/doc/guide/hooks.xml @@ -1663,7 +1663,48 @@ An example result returned when the query was malformed: retrieved by using subnet-id value of zero (0). - + For instance for retrieving host reservations for the + subnet 1: + +{ + "command": "reservation-get-all", + "arguments": { + "subnet-id": 1 + } +} + returns found some IPv4 hosts: + +{ + "arguments": { + "hosts": [ + { + "boot-file-name": "bootfile.efi", + "client-classes": [ ], + "hostname": "somehost.example.org", + "hw-address": "01:02:03:04:05:06", + "ip-address": "192.0.2.100", + "next-server": "192.0.0.2", + "option-data": [ ], + "server-hostname": "server-hostname.example.org" + }, + ... + { + "boot-file-name": "bootfile.efi", + "client-classes": [ ], + "hostname": "otherhost.example.org", + "hw-address": "01:02:03:04:05:ff", + "ip-address": "192.0.2.200", + "next-server": "192.0.0.2", + "option-data": [ ], + "server-hostname": "server-hostname.example.org" + } + ] + }, + "result": 0, + "text": "72 IPv4 host(s) found." +} + + The response returned by reservation-get-all can be very long. The DHCP server does not handle DHCP @@ -1690,14 +1731,83 @@ An example result returned when the query was malformed: uses to chain page queries. - - The first page is queried without source-index and from, for next pages source-index and from should be set using the preceeding result source-index and next. After the last page the returned list is empty, count is 0 and result 3 is returned. + For instance for retrieving host reservations for the + subnet 1 requesting the first page can be done by: + +{ + "command": "reservation-get-page", + "arguments": { + "subnet-id": 1, + "limit": 10 + } +} + leaving source-index and from to their zero default values. + + + Some hosts are returned with informations to get the next page: + +{ + "arguments": { + "count": 72, + "hosts": [ + { + "boot-file-name": "bootfile.efi", + "client-classes": [ ], + "hostname": "somehost.example.org", + "hw-address": "01:02:03:04:05:06", + "ip-address": "192.0.2.100", + "next-server": "192.0.0.2", + "option-data": [ ], + "server-hostname": "server-hostname.example.org" + }, + ... + { + "boot-file-name": "bootfile.efi", + "client-classes": [ ], + "hostname": "otherhost.example.org", + "hw-address": "01:02:03:04:05:ff", + "ip-address": "192.0.2.200", + "next-server": "192.0.0.2", + "option-data": [ ], + "server-hostname": "server-hostname.example.org" + } + ], + "next": 1234567, + "source-index": 0 + }, + "result": 0, + "text": "72 IPv4 host(s) found." +} + To get the next page this can be sent: + +{ + "command": "reservation-get-page", + "arguments": { + "subnet-id": 1, + "source-index": 1, + "from": 1234567, + "limit": 10 + } +} + responses after the last page look like: + +{ + "arguments": { + "count": 0, + "hosts": [ ], + "source-index": 2, + }, + "result": 3, + "0 IPv4 host(s) found." +} + + This command is more complex than reservation-get-all, but lets users retrieve larger host reservations lists by smaller diff --git a/src/lib/dhcpsrv/cql_host_data_source.cc b/src/lib/dhcpsrv/cql_host_data_source.cc index 308fd1cc85..a94121e437 100644 --- a/src/lib/dhcpsrv/cql_host_data_source.cc +++ b/src/lib/dhcpsrv/cql_host_data_source.cc @@ -321,26 +321,6 @@ public: // associated with a host using subnet identifier. static constexpr StatementTag GET_HOST_BY_IPV6_SUBNET_ID = "GET_HOST_BY_IPV6_SUBNET_ID"; - - // Retrieves host information along with the IPv4 options associated - // with it using a subnet identifier. First page. - static constexpr StatementTag GET_HOST_BY_IPV4_SUBNET_ID_LIMIT = - "GET_HOST_BY_IPV4_SUBNET_ID_LIMIT"; - - // Retrieves host information along with the IPv4 options associated - // with it using a subnet identifier. Next page. - static constexpr StatementTag GET_HOST_BY_IPV4_SUBNET_ID_PAGE = - "GET_HOST_BY_IPV4_SUBNET_ID_PAGE"; - - // Retrieves host information along with the IPv6 options associated - // with it using a subnet identifier. First page. - static constexpr StatementTag GET_HOST_BY_IPV6_SUBNET_ID_LIMIT = - "GET_HOST_BY_IPV6_SUBNET_ID_LIMIT"; - - // Retrieves host information along with the IPv6 options associated - // with it using a subnet identifier. Next page. - static constexpr StatementTag GET_HOST_BY_IPV6_SUBNET_ID_PAGE = - "GET_HOST_BY_IPV6_SUBNET_ID_PAGE"; /// @} /// @brief Cassandra statements @@ -452,10 +432,6 @@ constexpr StatementTag CqlHostExchange::GET_HOST_BY_IPV6_SUBNET_ID_AND_ADDRESS; constexpr StatementTag CqlHostExchange::DELETE_HOST; constexpr StatementTag CqlHostExchange::GET_HOST_BY_IPV4_SUBNET_ID; constexpr StatementTag CqlHostExchange::GET_HOST_BY_IPV6_SUBNET_ID; -constexpr StatementTag CqlHostExchange::GET_HOST_BY_IPV4_SUBNET_ID_LIMIT; -constexpr StatementTag CqlHostExchange::GET_HOST_BY_IPV4_SUBNET_ID_PAGE; -constexpr StatementTag CqlHostExchange::GET_HOST_BY_IPV6_SUBNET_ID_LIMIT; -constexpr StatementTag CqlHostExchange::GET_HOST_BY_IPV6_SUBNET_ID_PAGE; StatementMap CqlHostExchange::tagged_statements_ = { {INSERT_HOST, @@ -870,154 +846,6 @@ StatementMap CqlHostExchange::tagged_statements_ = { "FROM host_reservations " "WHERE host_ipv6_subnet_id = ? " "ALLOW FILTERING " - }}, - - {GET_HOST_BY_IPV4_SUBNET_ID_LIMIT, - {GET_HOST_BY_IPV4_SUBNET_ID_LIMIT, - "SELECT " - "id, " - "host_identifier, " - "host_identifier_type, " - "host_ipv4_subnet_id, " - "host_ipv6_subnet_id, " - "host_ipv4_address, " - "host_ipv4_next_server, " - "host_ipv4_server_hostname, " - "host_ipv4_boot_file_name, " - "auth_key, " - "hostname, " - "user_context, " - "host_ipv4_client_classes, " - "host_ipv6_client_classes, " - "reserved_ipv6_prefix_address, " - "reserved_ipv6_prefix_length, " - "reserved_ipv6_prefix_address_type, " - "iaid, " - "option_universe, " - "option_code, " - "option_value, " - "option_formatted_value, " - "option_space, " - "option_is_persistent, " - "option_client_class, " - "option_subnet_id, " - "option_user_context, " - "option_scope_id " - "FROM host_reservations " - "WHERE host_ipv4_subnet_id = ? " - "LIMIT ? " - "ALLOW FILTERING " - }}, - - {GET_HOST_BY_IPV4_SUBNET_ID_PAGE, - {GET_HOST_BY_IPV4_SUBNET_ID_PAGE, - "SELECT " - "id, " - "host_identifier, " - "host_identifier_type, " - "host_ipv4_subnet_id, " - "host_ipv6_subnet_id, " - "host_ipv4_address, " - "host_ipv4_next_server, " - "host_ipv4_server_hostname, " - "host_ipv4_boot_file_name, " - "auth_key, " - "hostname, " - "user_context, " - "host_ipv4_client_classes, " - "host_ipv6_client_classes, " - "reserved_ipv6_prefix_address, " - "reserved_ipv6_prefix_length, " - "reserved_ipv6_prefix_address_type, " - "iaid, " - "option_universe, " - "option_code, " - "option_value, " - "option_formatted_value, " - "option_space, " - "option_is_persistent, " - "option_client_class, " - "option_subnet_id, " - "option_user_context, " - "option_scope_id " - "FROM host_reservations " - "WHERE host_ipv4_subnet_id = ? AND TOKEN(id) > TOKEN(?) " - "LIMIT ? " - "ALLOW FILTERING " - }}, - - {GET_HOST_BY_IPV6_SUBNET_ID_LIMIT, - {GET_HOST_BY_IPV6_SUBNET_ID_LIMIT, - "SELECT " - "id, " - "host_identifier, " - "host_identifier_type, " - "host_ipv4_subnet_id, " - "host_ipv6_subnet_id, " - "host_ipv4_address, " - "host_ipv4_next_server, " - "host_ipv4_server_hostname, " - "host_ipv4_boot_file_name, " - "auth_key, " - "hostname, " - "user_context, " - "host_ipv4_client_classes, " - "host_ipv6_client_classes, " - "reserved_ipv6_prefix_address, " - "reserved_ipv6_prefix_length, " - "reserved_ipv6_prefix_address_type, " - "iaid, " - "option_universe, " - "option_code, " - "option_value, " - "option_formatted_value, " - "option_space, " - "option_is_persistent, " - "option_client_class, " - "option_subnet_id, " - "option_user_context, " - "option_scope_id " - "FROM host_reservations " - "WHERE host_ipv6_subnet_id = ? " - "LIMIT ? " - "ALLOW FILTERING " - }}, - - {GET_HOST_BY_IPV6_SUBNET_ID_PAGE, - {GET_HOST_BY_IPV6_SUBNET_ID_PAGE, - "SELECT " - "id, " - "host_identifier, " - "host_identifier_type, " - "host_ipv4_subnet_id, " - "host_ipv6_subnet_id, " - "host_ipv4_address, " - "host_ipv4_next_server, " - "host_ipv4_server_hostname, " - "host_ipv4_boot_file_name, " - "auth_key, " - "hostname, " - "user_context, " - "host_ipv4_client_classes, " - "host_ipv6_client_classes, " - "reserved_ipv6_prefix_address, " - "reserved_ipv6_prefix_length, " - "reserved_ipv6_prefix_address_type, " - "iaid, " - "option_universe, " - "option_code, " - "option_value, " - "option_formatted_value, " - "option_space, " - "option_is_persistent, " - "option_client_class, " - "option_subnet_id, " - "option_user_context, " - "option_scope_id " - "FROM host_reservations " - "WHERE host_ipv6_subnet_id = ? AND TOKEN(id) > TOKEN(?) " - "LIMIT ? " - "ALLOW FILTERING " }} }; @@ -1700,6 +1528,9 @@ public: /// @brief Implementation of @ref CqlHostDataSource::getPage4() /// + /// Not implemented. + /// @todo: implement it. + /// /// See @ref CqlHostDataSource::getPage4() for parameter details. /// /// @param subnet_id identifier of the subnet to which hosts belong @@ -1713,6 +1544,9 @@ public: /// @brief Implementation of @ref CqlHostDataSource::getPage6() /// + /// Not implemented. + /// @todo: implement it. + /// /// See @ref CqlHostDataSource::getPage6() for parameter details. /// /// @param subnet_id identifier of the subnet to which hosts belong @@ -2143,71 +1977,29 @@ CqlHostDataSourceImpl::getAll6(const SubnetID& subnet_id) const { return (result); } -ConstHostCollection -CqlHostDataSourceImpl::getPage4(const SubnetID& subnet_id, - uint64_t lower_host_id, - const HostPageSize& page_size) const { - // Convert to CQL data types. - cass_int32_t host_ipv4_subnet_id = static_cast(subnet_id); - - // Bind to array. - AnyArray where_values; - where_values.add(&host_ipv4_subnet_id); +// There are some problems implementing this for Cassandra. +// Attempts show the per page ordering does not work and +// it is not possible to order by TOKEN(host_id). +// If the issue solved by paging is the Kea API overhead then +// a solution is to get and cache all reservations and to handle +// paging at the API level. - cass_int64_t id = static_cast(lower_host_id); - if (id) { - where_values.add(&id); - } - - cass_int32_t page_size_data = - static_cast(page_size.page_size_); - where_values.add(&page_size_data); - - // Run statement. - ConstHostCollection result = - getHostCollection(id == 0 ? - CqlHostExchange::GET_HOST_BY_IPV4_SUBNET_ID_LIMIT : - CqlHostExchange::GET_HOST_BY_IPV4_SUBNET_ID_PAGE, - where_values); - - // Note the result is not ordered (or ordered following TOKEN). - - return (result); +ConstHostCollection +CqlHostDataSourceImpl::getPage4(const SubnetID& /*subnet_id*/, + uint64_t /*lower_host_id*/, + const HostPageSize& /*page_size*/) const { + isc_throw(NotImplemented, + "reservation-get-page is not supported by Cassandra"); } ConstHostCollection -CqlHostDataSourceImpl::getPage6(const SubnetID& subnet_id, - uint64_t lower_host_id, - const HostPageSize& page_size) const { - // Convert to CQL data types. - cass_int32_t host_ipv6_subnet_id = static_cast(subnet_id); - - // Bind to array. - AnyArray where_values; - where_values.add(&host_ipv6_subnet_id); - - cass_int64_t id = static_cast(lower_host_id); - if (id) { - where_values.add(&id); - } - - cass_int32_t page_size_data = - static_cast(page_size.page_size_); - where_values.add(&page_size_data); - - // Run statement. - ConstHostCollection result = - getHostCollection(id == 0 ? - CqlHostExchange::GET_HOST_BY_IPV6_SUBNET_ID_LIMIT : - CqlHostExchange::GET_HOST_BY_IPV6_SUBNET_ID_PAGE, - where_values); - - // Note the result is not ordered (or ordered following TOKEN). - - return (result); +CqlHostDataSourceImpl::getPage6(const SubnetID& /*subnet_id*/, + uint64_t /*lower_host_id*/, + const HostPageSize& /*page_size*/) const { + isc_throw(NotImplemented, + "reservation-get-page is not supported by Cassandra"); } - ConstHostCollection CqlHostDataSourceImpl::getAll4(const asiolink::IOAddress& address) const { // Convert to CQL data types. diff --git a/src/lib/dhcpsrv/cql_host_data_source.h b/src/lib/dhcpsrv/cql_host_data_source.h index d5dc713736..c9f5a18512 100644 --- a/src/lib/dhcpsrv/cql_host_data_source.h +++ b/src/lib/dhcpsrv/cql_host_data_source.h @@ -192,8 +192,7 @@ public: /// @brief Returns range of hosts in a DHCPv4 subnet. /// - /// This method returns a page of @c Host objects which represent - /// reservations in a specified subnet. + /// Not implemented. /// /// @param subnet_id Subnet identifier. /// @param source_index Index of the source (unused). @@ -210,8 +209,7 @@ public: /// @brief Returns range of hosts in a DHCPv6 subnet. /// - /// This method returns a page of @c Host objects which represent - /// reservations in a specified subnet. + /// Not implemented. /// /// @param subnet_id Subnet identifier. /// @param source_index Index of the source (unused). diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 3a57cbb3a4..4e6c25bb28 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -2325,17 +2325,23 @@ TaggedStatementArray tagged_statements = { { "persistent, user_context, dhcp_client_class, dhcp6_subnet_id, host_id, scope_id) " " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}, + // Delete a single IPv4 reservation by subnet id and reserved address. {MySqlHostDataSourceImpl::DEL_HOST_ADDR4, "DELETE FROM hosts WHERE dhcp4_subnet_id = ? AND ipv4_address = ?"}, + // Delete a single IPv4 reservation by subnet id and identifier. {MySqlHostDataSourceImpl::DEL_HOST_SUBID4_ID, "DELETE FROM hosts WHERE dhcp4_subnet_id = ? AND dhcp_identifier_type=? " "AND dhcp_identifier = ?"}, + // Delete a single IPv6 reservation by subnet id and identifier. {MySqlHostDataSourceImpl::DEL_HOST_SUBID6_ID, "DELETE FROM hosts WHERE dhcp6_subnet_id = ? AND dhcp_identifier_type=? " "AND dhcp_identifier = ?"}, + // Retrieves host information along with the DHCPv4 options associated with + // it. Left joining the dhcp4_options table results in multiple rows being + // returned for the same host. Hosts are retrieved by IPv4 subnet id. {MySqlHostDataSourceImpl::GET_HOST_SUBID4, "SELECT h.host_id, h.dhcp_identifier, h.dhcp_identifier_type, " "h.dhcp4_subnet_id, h.dhcp6_subnet_id, h.ipv4_address, h.hostname, " @@ -2350,6 +2356,10 @@ TaggedStatementArray tagged_statements = { { "WHERE h.dhcp4_subnet_id = ? " "ORDER BY h.host_id, o.option_id"}, + // Retrieves host information, IPv6 reservations and DHCPv6 options + // associated with a host. The number of rows returned is a multiplication + // of number of IPv6 reservations and DHCPv6 options. Hosts are retrieved + // by IPv6 subnet id. {MySqlHostDataSourceImpl::GET_HOST_SUBID6, "SELECT h.host_id, h.dhcp_identifier, " "h.dhcp_identifier_type, h.dhcp4_subnet_id, " @@ -2370,6 +2380,11 @@ TaggedStatementArray tagged_statements = { { "WHERE h.dhcp6_subnet_id = ? " "ORDER BY h.host_id, o.option_id, r.reservation_id"}, + // Retrieves host information along with the DHCPv4 options associated with + // it. Left joining the dhcp4_options table results in multiple rows being + // returned for the same host. Hosts are retrieved by IPv4 subnet id + // and with a host id greater than the start one. + // The number of hosts returned is lower or equal to the limit. {MySqlHostDataSourceImpl::GET_HOST_SUBID4_PAGE, "SELECT h.host_id, h.dhcp_identifier, h.dhcp_identifier_type, " "h.dhcp4_subnet_id, h.dhcp6_subnet_id, h.ipv4_address, h.hostname, " @@ -2378,13 +2393,19 @@ TaggedStatementArray tagged_statements = { { "h.dhcp4_boot_file_name, h.auth_key, " "o.option_id, o.code, o.value, o.formatted_value, o.space, " "o.persistent, o.user_context " - "FROM hosts AS h " + "FROM ( SELECT * FROM hosts AS h " + "WHERE h.dhcp4_subnet_id = ? AND h.host_id > ? " + "ORDER BY h.host_id " + "LIMIT ? ) AS h " "LEFT JOIN dhcp4_options AS o " "ON h.host_id = o.host_id " - "WHERE h.dhcp4_subnet_id = ? AND h.host_id > ? " - "ORDER BY h.host_id, o.option_id " - "LIMIT ?"}, + "ORDER BY h.host_id, o.option_id"}, + // Retrieves host information, IPv6 reservations and DHCPv6 options + // associated with a host. The number of rows returned is a multiplication + // of number of IPv6 reservations and DHCPv6 options. Hosts are retrieved + // by IPv6 subnet id and with a host id greater than the start one. + // The number of hosts returned is lower or equal to the limit. {MySqlHostDataSourceImpl::GET_HOST_SUBID6_PAGE, "SELECT h.host_id, h.dhcp_identifier, " "h.dhcp_identifier_type, h.dhcp4_subnet_id, " @@ -2397,14 +2418,15 @@ TaggedStatementArray tagged_statements = { { "o.persistent, o.user_context, " "r.reservation_id, r.address, r.prefix_len, r.type, " "r.dhcp6_iaid " - "FROM hosts AS h " + "FROM ( SELECT * FROM hosts AS h " + "WHERE h.dhcp6_subnet_id = ? AND h.host_id > ? " + "ORDER BY h.host_id " + "LIMIT ? ) AS h " "LEFT JOIN dhcp6_options AS o " "ON h.host_id = o.host_id " "LEFT JOIN ipv6_reservations AS r " "ON h.host_id = r.host_id " - "WHERE h.dhcp6_subnet_id = ? AND h.host_id > ? " - "ORDER BY h.host_id, o.option_id, r.reservation_id " - "LIMIT ?"} + "ORDER BY h.host_id, o.option_id, r.reservation_id"} } }; diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index f92142398e..d6a9d22924 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -1784,11 +1784,12 @@ TaggedStatementArray tagged_statements = { { " h.dhcp4_boot_file_name, h.auth_key, " " o.option_id, o.code, o.value, o.formatted_value, o.space, " " o.persistent, o.user_context " - "FROM hosts AS h " + "FROM ( SELECT * FROM hosts AS h " + " WHERE h.dhcp4_subnet_id = $1 AND h.host_id > $2 " + " ORDER BY h.host_id " + " LIMIT $3 ) AS h " "LEFT JOIN dhcp4_options AS o ON h.host_id = o.host_id " - "WHERE h.dhcp4_subnet_id = $1 AND h.host_id > $2 " - "ORDER BY h.host_id, o.option_id " - "LIMIT $3" + "ORDER BY h.host_id, o.option_id" }, // PgSqlHostDataSourceImpl::GET_HOST_SUBID6_PAGE @@ -1810,12 +1811,13 @@ TaggedStatementArray tagged_statements = { { " o.option_id, o.code, o.value, o.formatted_value, o.space, " " o.persistent, o.user_context, " " r.reservation_id, r.address, r.prefix_len, r.type, r.dhcp6_iaid " - "FROM hosts AS h " + "FROM ( SELECT * FROM hosts AS h " + " WHERE h.dhcp6_subnet_id = $1 AND h.host_id > $2 " + " ORDER BY h.host_id " + " LIMIT $3 ) AS h " "LEFT JOIN dhcp6_options AS o ON h.host_id = o.host_id " "LEFT JOIN ipv6_reservations AS r ON h.host_id = r.host_id " - "WHERE h.dhcp6_subnet_id = $1 AND h.host_id > $2 " - "ORDER BY h.host_id, o.option_id, r.reservation_id " - "LIMIT $3" + "ORDER BY h.host_id, o.option_id, r.reservation_id" } } }; diff --git a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h index 8661dfae38..4954876e05 100644 --- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h +++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h @@ -206,6 +206,22 @@ public: /// @param id Identifier type (hwaddr or duid). void testGetPage6(const Host::IdentifierType& id); + /// @brief Test that Verifies that pages of complex host reservations + /// are not truncated, i.e. the limit applies on the number of hosts + /// and not on the number of rows. + /// + /// Uses gtest macros to report failures. + /// @param id Identifier type (hwaddr or duid). + // void testGetPageLimit4(const Host::IdentifierType& id); + + /// @brief Test that Verifies that pages of complex host reservations + /// are not truncated, i.e. the limit applies on the number of hosts + /// and not on the number of rows. + /// + /// Uses gtest macros to report failures. + /// @param id Identifier type (hwaddr or duid). + // void testGetPageLimit6(const Host::IdentifierType& id); + /// @brief Test inserts several hosts with unique IPv4 address and /// checks that they can be retrieved properly. ///