Commit 2b845cb0 authored by Thomas Markwalder's avatar Thomas Markwalder

[#399,!215] Addressed review comments

parent 6dc4c06c
...@@ -441,8 +441,10 @@ TEST_F(Dhcp4CBTest, mergeSharedNetworks) { ...@@ -441,8 +441,10 @@ TEST_F(Dhcp4CBTest, mergeSharedNetworks) {
staged_network = networks->getByName("two"); staged_network = networks->getByName("two");
ASSERT_TRUE(staged_network); ASSERT_TRUE(staged_network);
// Subnet3, which is in db2 should not have been merged, since it is // Subnet3, which is in db2 should not have been merged.
// backend data is first found, first used. // We queried db1 first and the query returned data. In
// other words, we iterate over the backends, asking for
// data. We use the first data, we find.
staged_network = networks->getByName("three"); staged_network = networks->getByName("three");
ASSERT_FALSE(staged_network); ASSERT_FALSE(staged_network);
} }
......
...@@ -24,8 +24,8 @@ CfgSharedNetworks4::merge(const CfgSharedNetworks4& other) { ...@@ -24,8 +24,8 @@ CfgSharedNetworks4::merge(const CfgSharedNetworks4& other) {
auto& index = networks_.get<SharedNetworkNameIndexTag>(); auto& index = networks_.get<SharedNetworkNameIndexTag>();
// Iterate over the subnets to be merged. They will replace the existing // Iterate over the subnets to be merged. They will replace the existing
// subnets with the same id. All new subnets will be inserted into the // subnets with the same id. All new subnets will be inserted into this
// configuration into which we're merging. // configuration.
auto other_networks = other.getAll(); auto other_networks = other.getAll();
for (auto other_network = other_networks->begin(); for (auto other_network = other_networks->begin();
other_network != other_networks->end(); ++other_network) { other_network != other_networks->end(); ++other_network) {
......
...@@ -124,7 +124,8 @@ public: ...@@ -124,7 +124,8 @@ public:
/// @return true if there is a network with a specified server identifier. /// @return true if there is a network with a specified server identifier.
bool hasNetworkWithServerId(const asiolink::IOAddress& server_id) const; bool hasNetworkWithServerId(const asiolink::IOAddress& server_id) const;
/// @brief Merges specified shared network configuration into this configuration. /// @brief Merges specified shared network configuration into this
/// configuration.
/// ///
/// This method merges networks from the @c other configuration into this /// This method merges networks from the @c other configuration into this
/// configuration. The general rule is that existing networks are replaced /// configuration. The general rule is that existing networks are replaced
...@@ -132,14 +133,14 @@ public: ...@@ -132,14 +133,14 @@ public:
/// ///
/// For each network in @c other, do the following: /// For each network in @c other, do the following:
/// ///
/// - Any associated subnets are removed. Shared networks retreived from /// - Any associated subnets are removed. Shared networks retrieved from
/// config backends, do not carry their associated subnets (if any) with them. /// config backends, do not carry their associated subnets (if any) with
/// Subnet assignments are maintained by subnet merges. /// them. (Subnet assignments are maintained by subnet merges).
/// - If a shared network of the same name, already exists in this /// - If a shared network of the same name already exists in this
/// configuration: /// configuration:
/// - All of its associated subnets are movde to the "other" network /// - All of its associated subnets are moved to the "other" network.
/// - The existing network is removed from this configuration /// - The existing network is removed from this configuration.
/// - The "other" network is added /// - The "other" network is added to this configuration.
/// ///
/// @warning The merge operation may affect the @c other configuration. /// @warning The merge operation may affect the @c other configuration.
/// Therefore, the caller must not rely on the data held in the @c other /// Therefore, the caller must not rely on the data held in the @c other
......
...@@ -64,7 +64,8 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks, ...@@ -64,7 +64,8 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks,
// subnets with the same id. All new subnets will be inserted into the // subnets with the same id. All new subnets will be inserted into the
// configuration into which we're merging. // configuration into which we're merging.
auto other_subnets = other.getAll(); auto other_subnets = other.getAll();
for (auto other_subnet = other_subnets->begin(); other_subnet != other_subnets->end(); for (auto other_subnet = other_subnets->begin();
other_subnet != other_subnets->end();
++other_subnet) { ++other_subnet) {
// Check if there is a subnet with the same ID. // Check if there is a subnet with the same ID.
...@@ -74,7 +75,7 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks, ...@@ -74,7 +75,7 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks,
// Subnet found. // Subnet found.
auto existing_subnet = *subnet_it; auto existing_subnet = *subnet_it;
// Continue if the existing subnet and other subnet // If the existing subnet and other subnet
// are the same instance skip it. // are the same instance skip it.
if (existing_subnet == *other_subnet) { if (existing_subnet == *other_subnet) {
continue; continue;
...@@ -106,7 +107,7 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks, ...@@ -106,7 +107,7 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks,
} else { } else {
// This implies the shared-network collection we were given // This implies the shared-network collection we were given
// is out of sync with the subnets we were given. // is out of sync with the subnets we were given.
isc_throw(InvalidOperation, "Cannot assign subnet ID of '" isc_throw(InvalidOperation, "Cannot assign subnet ID of "
<< (*other_subnet)->getID() << (*other_subnet)->getID()
<< " to shared network: " << network_name << " to shared network: " << network_name
<< ", network does not exist"); << ", network does not exist");
......
...@@ -60,9 +60,9 @@ public: ...@@ -60,9 +60,9 @@ public:
/// are the authority on their shared network assignments. It is also /// are the authority on their shared network assignments. It is also
/// assumed that @ networks is the list of shared networks that should be /// assumed that @ networks is the list of shared networks that should be
/// used in making assignments. The general concept is that the overarching /// used in making assignments. The general concept is that the overarching
/// merge process will first merge shared networks and then pass that list of /// merge process will first merge shared networks and then pass that list
/// networks into this method. Subnets from @c other are then merged into this /// of networks into this method. Subnets from @c other are then merged
/// configuration as follows: /// into this configuration as follows:
/// ///
/// For each subnet in @c other: /// For each subnet in @c other:
/// ///
...@@ -70,7 +70,7 @@ public: ...@@ -70,7 +70,7 @@ public:
/// -# If it belongs to a shared network, remove it from that network /// -# If it belongs to a shared network, remove it from that network
/// -# Remove the subnet from this configuration and discard it /// -# Remove the subnet from this configuration and discard it
/// ///
/// - Add the subnet from other to this configuration. /// - Add the subnet from @c other to this configuration.
/// - If that subnet is associated to shared network, find that network /// - If that subnet is associated to shared network, find that network
/// in @ networks and add that subnet to it. /// in @ networks and add that subnet to it.
/// ///
...@@ -81,7 +81,7 @@ public: ...@@ -81,7 +81,7 @@ public:
/// merged configuration. /// merged configuration.
/// ///
/// @param networks collection of shared networks that to which assignments /// @param networks collection of shared networks that to which assignments
/// should be added. In other words, the list of shared networks that belong /// should be added. In other words, the list of shared networks that belong
/// to the same SrvConfig instance we are merging into. /// to the same SrvConfig instance we are merging into.
/// @param other the subnet configuration to be merged into this /// @param other the subnet configuration to be merged into this
/// configuration. /// configuration.
......
// Copyright (C) 2017-2018 Internet Systems Consortium, Inc. ("ISC") // Copyright (C) 2017-2019 Internet Systems Consortium, Inc. ("ISC")
// //
// This Source Code Form is subject to the terms of the Mozilla Public // This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this // License, v. 2.0. If a copy of the MPL was not distributed with this
...@@ -319,12 +319,14 @@ SharedNetwork6::add(const Subnet6Ptr& subnet) { ...@@ -319,12 +319,14 @@ SharedNetwork6::add(const Subnet6Ptr& subnet) {
Impl::add(subnets_, subnet); Impl::add(subnets_, subnet);
// Associate the subnet with this network. // Associate the subnet with this network.
setSharedNetwork(subnet); setSharedNetwork(subnet);
subnet->setSharedNetworkName(name_);
} }
void void
SharedNetwork6::del(const SubnetID& subnet_id) { SharedNetwork6::del(const SubnetID& subnet_id) {
Subnet6Ptr subnet = Impl::del<Subnet6Ptr>(subnets_, subnet_id); Subnet6Ptr subnet = Impl::del<Subnet6Ptr>(subnets_, subnet_id);
clearSharedNetwork(subnet); clearSharedNetwork(subnet);
subnet->setSharedNetworkName("");
} }
void void
......
...@@ -193,7 +193,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { ...@@ -193,7 +193,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) {
SharedNetwork4Ptr network2(new SharedNetwork4("network2")); SharedNetwork4Ptr network2(new SharedNetwork4("network2"));
network2->setValid(Triplet<uint32_t>(200)); network2->setValid(Triplet<uint32_t>(200));
// Create network3 with one subnets. // Create network3 with one subnet.
SharedNetwork4Ptr network3(new SharedNetwork4("network3")); SharedNetwork4Ptr network3(new SharedNetwork4("network3"));
network3->setValid(Triplet<uint32_t>(300)); network3->setValid(Triplet<uint32_t>(300));
ASSERT_NO_THROW(network3->add(subnet3)); ASSERT_NO_THROW(network3->add(subnet3));
...@@ -240,7 +240,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { ...@@ -240,7 +240,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) {
// Should still have 3 networks. // Should still have 3 networks.
// Network1 should have doubled its valid life time but still only have // Network1 should have doubled its valid lifetime but still only have
// the orignal two subnets. Merge should discard assocations on CB // the orignal two subnets. Merge should discard assocations on CB
// subnets and preserve the associations from existing config. // subnets and preserve the associations from existing config.
ASSERT_EQ(3, cfg_to.getAll()->size()); ASSERT_EQ(3, cfg_to.getAll()->size());
...@@ -251,7 +251,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { ...@@ -251,7 +251,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) {
ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet<uint32_t>(200), ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet<uint32_t>(200),
std::vector<SubnetID>())); std::vector<SubnetID>()));
// Network1 should have doubled its valid life time and still subnet3. // Network1 should have doubled its valid lifetime and still subnet3.
ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network3", Triplet<uint32_t>(600), ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network3", Triplet<uint32_t>(600),
std::vector<SubnetID>{SubnetID(3)})); std::vector<SubnetID>{SubnetID(3)}));
} }
......
...@@ -159,7 +159,7 @@ TEST(CfgSubnets4Test, mergeSubnets) { ...@@ -159,7 +159,7 @@ TEST(CfgSubnets4Test, mergeSubnets) {
// Empty network pointer. // Empty network pointer.
SharedNetwork4Ptr no_network; SharedNetwork4Ptr no_network;
// Add Subnets1,2, and 4 to shared networks. // Add Subnets 1, 2 and 4 to shared networks.
ASSERT_NO_THROW(shared_network1->add(subnet1)); ASSERT_NO_THROW(shared_network1->add(subnet1));
ASSERT_NO_THROW(shared_network2->add(subnet2)); ASSERT_NO_THROW(shared_network2->add(subnet2));
ASSERT_NO_THROW(shared_network2->add(subnet4)); ASSERT_NO_THROW(shared_network2->add(subnet4));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment