common/pick_address: fail if cannot bind with specified network family

this change partially reverts 9f75dfbf364f5140b3f291e0a2c6769bc3d8cbac

we should not proceed against user's will if dual stack is specified but
only one network for a network family can be found. the right fix is
have better error message and documentation, not to tolerate the
failure.

Fixes: https://tracker.ceph.com/issues/46845
Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit d752acafa0d99c3d7cacfaaaf3ae51770e251aff)

diff --git a/src/common/pick_address.cc b/src/common/pick_address.cc
index 79513bb652d5b5c5e70958c7095c738b0948869a..e6da4248b5017c2c629b9eb14184026097ada5f2 100644 (file)
--- a/src/common/pick_address.cc
+++ b/src/common/pick_address.cc
@@ -279,32 +279,6 @@ static int fill_in_one_address(
   return 0;
 }
 
-unsigned networks_address_family_coverage(CephContext *cct, const std::string &networks) {
-  std::list<string> nets;
-  get_str_list(networks, nets);
-  unsigned found_ipv = 0;
-
-  for (auto& s : nets) {
-    struct sockaddr_storage net;
-    unsigned prefix_len;
-    if (!parse_network(s.c_str(), &net, &prefix_len)) {
-      lderr(cct) << "unable to parse network: " << s << dendl;
-      exit(1);
-    }
-
-    switch (net.ss_family) {
-    case AF_INET:
-      found_ipv |= CEPH_PICK_ADDRESS_IPV4;
-      break;
-    case AF_INET6:
-      found_ipv |= CEPH_PICK_ADDRESS_IPV6;
-      break;
-    }
-  }
-
-  return found_ipv;
-}
-
 int pick_addresses(
   CephContext *cct,
   unsigned flags,
@@ -378,7 +352,6 @@ int pick_addresses(
       !networks.empty()) {
     int ipv4_r = !(ipv & CEPH_PICK_ADDRESS_IPV4) ? 0 : -1;
     int ipv6_r = !(ipv & CEPH_PICK_ADDRESS_IPV6) ? 0 : -1;
-    unsigned found_ipv = networks_address_family_coverage(cct, networks);
     // first try on preferred numa node (if >= 0), then anywhere.
     while (true) {
       // note: pass in ipv to filter the matching addresses
@@ -399,11 +372,6 @@ int pick_addresses(
                                      networks, interfaces, addrs,
                                      preferred_numa_node);
       }
-      if (found_ipv != 0 && (found_ipv & ipv != ipv)) {
-        lderr(cct) << "An IP address was found, but not enough networks to cover both address families. "
-                   << "An IPv4 and IPv6 network is required for dual stack. Continuing with one stack" << dendl;
-        break;
-      }
       if (ipv4_r >= 0 && ipv6_r >= 0) {
        break;
       }

diff --git a/src/common/pick_address.h b/src/common/pick_address.h
index 80e9d102c028a8bc2615a5c6ad8258c7ae540ea3..ba9473a9649a3751d86459f3d9d07c5ec69a5870 100644 (file)
--- a/src/common/pick_address.h
+++ b/src/common/pick_address.h
@@ -85,11 +85,4 @@ int get_iface_numa_node(
   const std::string& iface,
   int *node);
 
-/**
- * Return a bitmap of address families that are covered by networks
- *
- * @param cct context (used for logging)
- * @param string of networks
- */
-unsigned networks_address_family_coverage(CephContext *cct, const std::string &networks);
 #endif

diff --git a/src/test/test_ipaddr.cc b/src/test/test_ipaddr.cc
index 6f6c39fcb2b2589a914b0c9929de2361dbf70465..d904a6819fffde92ada756d041959c2d376f2284 100644 (file)
--- a/src/test/test_ipaddr.cc
+++ b/src/test/test_ipaddr.cc
@@ -931,26 +931,20 @@ TEST(pick_address, filtering)
 
 TEST(pick_address, ipv4_ipv6_enabled)
 {
-  struct ifaddrs one, two;
+  struct ifaddrs one;
   struct sockaddr_in a_one;
-  struct sockaddr_in6 a_two;
 
-  one.ifa_next = &two;
+  one.ifa_next = NULL;
   one.ifa_addr = (struct sockaddr*)&a_one;
   one.ifa_name = eth0;
 
-  two.ifa_next = NULL;
-  two.ifa_addr = (struct sockaddr*)&a_two;
-  two.ifa_name = eth0;
-
   ipv4(&a_one, "10.1.1.2");
-  ipv6(&a_two, "2001:1234:5678:90ab::cdef");
 
   CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
   cct->_conf._clear_safe_to_start_threads();  // so we can set configs
 
   cct->_conf.set_val("public_addr", "");
-  cct->_conf.set_val("public_network", "10.1.1.0/24, 2001::/16");
+  cct->_conf.set_val("public_network", "10.1.1.0/24");
   cct->_conf.set_val("public_network_interface", "");
   cct->_conf.set_val("cluster_addr", "");
   cct->_conf.set_val("cluster_network", "");
@@ -964,15 +958,11 @@ TEST(pick_address, ipv4_ipv6_enabled)
                           CEPH_PICK_ADDRESS_MSGR1,
                           &one, &av);
     cout << av << std::endl;
-    ASSERT_EQ(0, r);
-    // Got 2 address
-    ASSERT_EQ(2u, av.v.size());
-    ASSERT_EQ(string("v1:[2001:1234:5678:90ab::cdef]:0/0"), stringify(av.v[0]));
-    ASSERT_EQ(string("v1:10.1.1.2:0/0"), stringify(av.v[1]));
+    ASSERT_EQ(-1, r);
   }
 }
 
-TEST(pick_address, only_ipv6_enabled)
+TEST(pick_address, ipv4_ipv6_enabled2)
 {
   struct ifaddrs one;
   struct sockaddr_in6 a_one;
@@ -993,76 +983,6 @@ TEST(pick_address, only_ipv6_enabled)
   cct->_conf.set_val("cluster_network", "");
   cct->_conf.set_val("cluster_network_interface", "");
   cct->_conf.set_val("ms_bind_ipv6", "true");
-  cct->_conf.set_val("ms_bind_ipv4", "false");
-
-  entity_addrvec_t av;
-  {
-    int r = pick_addresses(cct,
-                          CEPH_PICK_ADDRESS_PUBLIC |
-                          CEPH_PICK_ADDRESS_MSGR1,
-                          &one, &av);
-    cout << av << std::endl;
-    ASSERT_EQ(0, r);
-    ASSERT_EQ(1u, av.v.size());
-    ASSERT_EQ(string("v1:[2001:1234:5678:90ab::cdef]:0/0"), stringify(av.v[0]));
-  }
-}
-
-TEST(pick_address, only_ipv4_enabled)
-{
-  struct ifaddrs one;
-  struct sockaddr_in a_one;
-
-  one.ifa_next = NULL;
-  one.ifa_addr = (struct sockaddr*)&a_one;
-  one.ifa_name = eth0;
-
-  ipv4(&a_one, "10.1.1.2");
-
-  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
-  cct->_conf._clear_safe_to_start_threads();  // so we can set configs
-
-  cct->_conf.set_val("public_addr", "");
-  cct->_conf.set_val("public_network", "10.1.1.0/24");
-  cct->_conf.set_val("public_network_interface", "");
-  cct->_conf.set_val("cluster_addr", "");
-  cct->_conf.set_val("cluster_network", "");
-  cct->_conf.set_val("cluster_network_interface", "");
-  
-  entity_addrvec_t av;
-  {
-    int r = pick_addresses(cct,
-                          CEPH_PICK_ADDRESS_PUBLIC |
-                          CEPH_PICK_ADDRESS_MSGR1,
-                          &one, &av);
-    cout << av << std::endl;
-    ASSERT_EQ(0, r);
-    ASSERT_EQ(1u, av.v.size());
-    ASSERT_EQ(string("v1:10.1.1.2:0/0"), stringify(av.v[0]));
-  }
-}
-
-TEST(pick_address, ipv4_ipv6_enabled_not_enough_networks)
-{
-  struct ifaddrs one;
-  struct sockaddr_in a_one;
-
-  one.ifa_next = NULL;
-  one.ifa_addr = (struct sockaddr*)&a_one;
-  one.ifa_name = eth0;
-
-  ipv4(&a_one, "10.1.1.2");
-
-  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
-  cct->_conf._clear_safe_to_start_threads();  // so we can set configs
-
-  cct->_conf.set_val("public_addr", "");
-  cct->_conf.set_val("public_network", "10.1.1.0/24");
-  cct->_conf.set_val("public_network_interface", "");
-  cct->_conf.set_val("cluster_addr", "");
-  cct->_conf.set_val("cluster_network", "");
-  cct->_conf.set_val("cluster_network_interface", "");
-  cct->_conf.set_val("ms_bind_ipv6", "true");
 
   entity_addrvec_t av;
   {
@@ -1072,31 +992,6 @@ TEST(pick_address, ipv4_ipv6_enabled_not_enough_networks)
                           &one, &av);
     cout << av << std::endl;
     ASSERT_EQ(-1, r);
-    ASSERT_EQ(1u, av.v.size());
-    ASSERT_EQ(string("v2:10.1.1.2:0/0"), stringify(av.v[0]));
   }
 }
 
-TEST(networks_address_family_coverage, just_ipv4)
-{
-  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
-  std::string networks = "10.0.0.0/24";
-  unsigned r = networks_address_family_coverage(cct, networks);
-  ASSERT_EQ(CEPH_PICK_ADDRESS_IPV4, r);
-}
-
-TEST(networks_address_family_coverage, just_ipv6)
-{
-  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
-  std::string networks = "2001::/16";
-  unsigned r = networks_address_family_coverage(cct, networks);
-  ASSERT_EQ(CEPH_PICK_ADDRESS_IPV6, r);
-}
-
-TEST(networks_address_family_coverage, ipv6_and_ipv4)
-{
-  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
-  std::string networks = "2001::/16, 10.0.0.0/16";
-  unsigned r = networks_address_family_coverage(cct, networks);
-  ASSERT_EQ(CEPH_PICK_ADDRESS_IPV4 | CEPH_PICK_ADDRESS_IPV6, r);
-}