Add ring_ip option to object services

This will be used when finding their own devices in rings, defaulting to
the bind_ip.

Notably, this allows services to be containerized while servers_per_port
is enabled:

* For the object-server, the ring_ip should be set to the host ip and
  will be used to discover which ports need binding. Sockets will still
  be bound to the bind_ip (likely 0.0.0.0), with the assumption that the
  host will publish ports 1:1.

* For the replicator and reconstructor, the ring_ip will be used to
  discover which devices should be replicated. While bind_ip could
  previously be used for this, it would have required a separate config
  from the object-server.

Also rename object deamon's bind_ip attribute to ring_ip so that it's
more obvious wherever we're using the IP for ring lookups instead of
socket binding.

Co-Authored-By: Tim Burke <tim.burke@gmail.com>
Change-Id: I1c9bb8086994f7930acd8cda8f56e766938c2218

10 files changed, 123 insertions, 25 deletions

diff --git a/etc/object-server.conf-sample b/etc/object-server.conf-sample
index e7466bba8..2c93e23fd 100644
--- a/etc/object-server.conf-sample
+++ b/etc/object-server.conf-sample
@@ -22,6 +22,12 @@ bind_port = 6200
 # feature.
 # servers_per_port = 0
 #
+# If running in a container, servers_per_port may not be able to use the
+# bind_ip to lookup the ports in the ring.  You may instead override the port
+# lookup in the ring using the ring_ip.  Any devices/ports associted with the
+# ring_ip will be used when listening on the configured bind_ip address.
+# ring_ip = <bind_ip>
+#
 # Maximum concurrent requests per worker
 # max_clients = 1024
 #
diff --git a/swift/common/storage_policy.py b/swift/common/storage_policy.py
index 3045cc98d..88c892aa6 100644
--- a/swift/common/storage_policy.py
+++ b/swift/common/storage_policy.py
@@ -37,11 +37,11 @@ DEFAULT_EC_OBJECT_SEGMENT_SIZE = 1048576
 
 
 class BindPortsCache(object):
-    def __init__(self, swift_dir, bind_ip):
+    def __init__(self, swift_dir, ring_ip):
         self.swift_dir = swift_dir
         self.mtimes_by_ring_path = {}
         self.portsets_by_ring_path = {}
-        self.my_ips = set(whataremyips(bind_ip))
+        self.my_ips = set(whataremyips(ring_ip))
 
     def all_bind_ports_for_node(self):
         """
diff --git a/swift/common/utils.py b/swift/common/utils.py
index 06c9ded30..c38bf709a 100644
--- a/swift/common/utils.py
+++ b/swift/common/utils.py
@@ -2713,25 +2713,25 @@ def expand_ipv6(address):
     return socket.inet_ntop(socket.AF_INET6, packed_ip)
 
 
-def whataremyips(bind_ip=None):
+def whataremyips(ring_ip=None):
     """
     Get "our" IP addresses ("us" being the set of services configured by
     one `*.conf` file). If our REST listens on a specific address, return it.
     Otherwise, if listen on '0.0.0.0' or '::' return all addresses, including
     the loopback.
 
-    :param str bind_ip: Optional bind_ip from a config file; may be IP address
-                        or hostname.
+    :param str ring_ip: Optional ring_ip/bind_ip from a config file; may be
+                        IP address or hostname.
     :returns: list of Strings of ip addresses
     """
-    if bind_ip:
+    if ring_ip:
         # See if bind_ip is '0.0.0.0'/'::'
         try:
             _, _, _, _, sockaddr = socket.getaddrinfo(
-                bind_ip, None, 0, socket.SOCK_STREAM, 0,
+                ring_ip, None, 0, socket.SOCK_STREAM, 0,
                 socket.AI_NUMERICHOST)[0]
             if sockaddr[0] not in ('0.0.0.0', '::'):
-                return [bind_ip]
+                return [ring_ip]
         except socket.gaierror:
             pass
 
diff --git a/swift/common/wsgi.py b/swift/common/wsgi.py
index eb5c0ffe2..925d90359 100644
--- a/swift/common/wsgi.py
+++ b/swift/common/wsgi.py
@@ -866,8 +866,11 @@ class ServersPerPortStrategy(StrategyBase):
         self.swift_dir = conf.get('swift_dir', '/etc/swift')
         self.ring_check_interval = float(conf.get('ring_check_interval', 15))
 
-        bind_ip = conf.get('bind_ip', '0.0.0.0')
-        self.cache = BindPortsCache(self.swift_dir, bind_ip)
+        # typically ring_ip will be the same as bind_ip, but in a container the
+        # bind_ip might be differnt than the host ip address used to lookup
+        # devices/ports in the ring
+        ring_ip = conf.get('ring_ip', conf.get('bind_ip', '0.0.0.0'))
+        self.cache = BindPortsCache(self.swift_dir, ring_ip)
 
     def _reload_bind_ports(self):
         self.bind_ports = self.cache.all_bind_ports_for_node()
diff --git a/swift/container/sharder.py b/swift/container/sharder.py
index e933b7cb9..5ddd20645 100644
--- a/swift/container/sharder.py
+++ b/swift/container/sharder.py
@@ -2085,7 +2085,7 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
         self._zero_stats()
         self._local_device_ids = set()
         dirs = []
-        self.ips = whataremyips(bind_ip=self.bind_ip)
+        self.ips = whataremyips(self.bind_ip)
         for node in self.ring.devs:
             device_path = self._check_node(node)
             if not device_path:
diff --git a/swift/obj/reconstructor.py b/swift/obj/reconstructor.py
index 5294231d3..c3f35615a 100644
--- a/swift/obj/reconstructor.py
+++ b/swift/obj/reconstructor.py
@@ -169,7 +169,7 @@ class ObjectReconstructor(Daemon):
         self.devices_dir = conf.get('devices', '/srv/node')
         self.mount_check = config_true_value(conf.get('mount_check', 'true'))
         self.swift_dir = conf.get('swift_dir', '/etc/swift')
-        self.bind_ip = conf.get('bind_ip', '0.0.0.0')
+        self.ring_ip = conf.get('ring_ip', conf.get('bind_ip', '0.0.0.0'))
         self.servers_per_port = int(conf.get('servers_per_port', '0') or 0)
         self.port = None if self.servers_per_port else \
             int(conf.get('bind_port', 6200))
@@ -1275,7 +1275,7 @@ class ObjectReconstructor(Daemon):
         return jobs
 
     def get_policy2devices(self):
-        ips = whataremyips(self.bind_ip)
+        ips = whataremyips(self.ring_ip)
         policy2devices = {}
         for policy in self.policies:
             self.load_object_ring(policy)
diff --git a/swift/obj/replicator.py b/swift/obj/replicator.py
index 6e84c05dc..e6fed47b0 100644
--- a/swift/obj/replicator.py
+++ b/swift/obj/replicator.py
@@ -134,7 +134,7 @@ class ObjectReplicator(Daemon):
         self.devices_dir = conf.get('devices', '/srv/node')
         self.mount_check = config_true_value(conf.get('mount_check', 'true'))
         self.swift_dir = conf.get('swift_dir', '/etc/swift')
-        self.bind_ip = conf.get('bind_ip', '0.0.0.0')
+        self.ring_ip = conf.get('ring_ip', conf.get('bind_ip', '0.0.0.0'))
         self.servers_per_port = int(conf.get('servers_per_port', '0') or 0)
         self.port = None if self.servers_per_port else \
             int(conf.get('bind_port', 6200))
@@ -316,7 +316,7 @@ class ObjectReplicator(Daemon):
         This is the device names, e.g. "sdq" or "d1234" or something, not
         the full ring entries.
         """
-        ips = whataremyips(self.bind_ip)
+        ips = whataremyips(self.ring_ip)
         local_devices = set()
         for policy in self.policies:
             self.load_object_ring(policy)
@@ -907,7 +907,7 @@ class ObjectReplicator(Daemon):
             policies will be returned
         """
         jobs = []
-        ips = whataremyips(self.bind_ip)
+        ips = whataremyips(self.ring_ip)
         for policy in self.policies:
             # Skip replication if next_part_power is set. In this case
             # every object is hard-linked twice, but the replicator can't
diff --git a/test/unit/common/test_wsgi.py b/test/unit/common/test_wsgi.py
index 2471fd2de..731e70388 100644
--- a/test/unit/common/test_wsgi.py
+++ b/test/unit/common/test_wsgi.py
@@ -1604,6 +1604,35 @@ class TestServersPerPortStrategy(unittest.TestCase, CommonTestMixin):
         # This is one of the workers for port 6006 that already got reaped.
         self.assertIsNone(self.strategy.register_worker_exit(89))
 
+    def test_servers_per_port_in_container(self):
+        # normally there's no configured ring_ip
+        conf = {
+            'bind_ip': '1.2.3.4',
+        }
+        self.strategy = wsgi.ServersPerPortStrategy(conf, self.logger, 1)
+        self.assertEqual(self.mock_cache_class.call_args,
+                         mock.call('/etc/swift', '1.2.3.4'))
+        self.assertEqual({6006, 6007},
+                         self.strategy.cache.all_bind_ports_for_node())
+        ports = {item[1][0] for item in self.strategy.new_worker_socks()}
+        self.assertEqual({6006, 6007}, ports)
+
+        # but in a container we can override it
+        conf = {
+            'bind_ip': '1.2.3.4',
+            'ring_ip': '2.3.4.5'
+        }
+        self.strategy = wsgi.ServersPerPortStrategy(conf, self.logger, 1)
+        # N.B. our fake BindPortsCache always returns {6006, 6007}, but a real
+        # BindPortsCache would only return ports for devices that match the ip
+        # address in the ring
+        self.assertEqual(self.mock_cache_class.call_args,
+                         mock.call('/etc/swift', '2.3.4.5'))
+        self.assertEqual({6006, 6007},
+                         self.strategy.cache.all_bind_ports_for_node())
+        ports = {item[1][0] for item in self.strategy.new_worker_socks()}
+        self.assertEqual({6006, 6007}, ports)
+
     def test_shutdown_sockets(self):
         pid = 88
         for s, i in self.strategy.new_worker_socks():
diff --git a/test/unit/obj/test_reconstructor.py b/test/unit/obj/test_reconstructor.py
index c1876b93b..1fe25c2cc 100644
--- a/test/unit/obj/test_reconstructor.py
+++ b/test/unit/obj/test_reconstructor.py
@@ -271,7 +271,7 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
                         policy=policy, frag_index=scenarios[part_num](obj_set),
                         timestamp=utils.Timestamp(t))
 
-        ips = utils.whataremyips(self.reconstructor.bind_ip)
+        ips = utils.whataremyips(self.reconstructor.ring_ip)
         for policy in [p for p in POLICIES if p.policy_type == EC_POLICY]:
             self.ec_policy = policy
             self.ec_obj_ring = self.reconstructor.load_object_ring(
@@ -1286,7 +1286,7 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
         # verify reconstructor only deletes reverted nondurable fragments older
         # commit_window
         shutil.rmtree(self.ec_obj_path)
-        ips = utils.whataremyips(self.reconstructor.bind_ip)
+        ips = utils.whataremyips(self.reconstructor.ring_ip)
         local_devs = [dev for dev in self.ec_obj_ring.devs
                       if dev and dev['replication_ip'] in ips and
                       dev['replication_port'] ==
@@ -1348,7 +1348,7 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
         # visited by the reconstructor, despite having timestamp older than
         # reclaim_age
         shutil.rmtree(self.ec_obj_path)
-        ips = utils.whataremyips(self.reconstructor.bind_ip)
+        ips = utils.whataremyips(self.reconstructor.ring_ip)
         local_devs = [dev for dev in self.ec_obj_ring.devs
                       if dev and dev['replication_ip'] in ips and
                       dev['replication_port'] ==
@@ -1396,7 +1396,7 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
         # reclaim_age and commit_window is zero; this test illustrates the
         # potential data loss bug that commit_window addresses
         shutil.rmtree(self.ec_obj_path)
-        ips = utils.whataremyips(self.reconstructor.bind_ip)
+        ips = utils.whataremyips(self.reconstructor.ring_ip)
         local_devs = [dev for dev in self.ec_obj_ring.devs
                       if dev and dev['replication_ip'] in ips and
                       dev['replication_port'] ==
@@ -1446,7 +1446,7 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
         # survive being visited by the reconstructor if its timestamp is older
         # than reclaim_age
         shutil.rmtree(self.ec_obj_path)
-        ips = utils.whataremyips(self.reconstructor.bind_ip)
+        ips = utils.whataremyips(self.reconstructor.ring_ip)
         local_devs = [dev for dev in self.ec_obj_ring.devs
                       if dev and dev['replication_ip'] in ips and
                       dev['replication_port'] ==
@@ -1494,7 +1494,7 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
         # verify reconstructor only deletes objects that were actually reverted
         # when ssync is limited by max_objects_per_revert
         shutil.rmtree(self.ec_obj_path)
-        ips = utils.whataremyips(self.reconstructor.bind_ip)
+        ips = utils.whataremyips(self.reconstructor.ring_ip)
         local_devs = [dev for dev in self.ec_obj_ring.devs
                       if dev and dev['replication_ip'] in ips and
                       dev['replication_port'] ==
@@ -3084,6 +3084,36 @@ class BaseTestObjectReconstructor(unittest.TestCase):
 
 
 class TestObjectReconstructor(BaseTestObjectReconstructor):
+    def test_ring_ip_and_bind_ip(self):
+        # make clean base_conf
+        base_conf = dict(self.conf)
+        for key in ('bind_ip', 'ring_ip'):
+            base_conf.pop(key, None)
+
+        # default ring_ip is always 0.0.0.0
+        self.conf = base_conf
+        self._configure_reconstructor()
+        self.assertEqual('0.0.0.0', self.reconstructor.ring_ip)
+
+        # bind_ip works fine for legacy configs
+        self.conf = dict(base_conf)
+        self.conf['bind_ip'] = '192.168.1.42'
+        self._configure_reconstructor()
+        self.assertEqual('192.168.1.42', self.reconstructor.ring_ip)
+
+        # ring_ip works fine by-itself
+        self.conf = dict(base_conf)
+        self.conf['ring_ip'] = '192.168.1.43'
+        self._configure_reconstructor()
+        self.assertEqual('192.168.1.43', self.reconstructor.ring_ip)
+
+        # if you have both ring_ip wins
+        self.conf = dict(base_conf)
+        self.conf['bind_ip'] = '192.168.1.44'
+        self.conf['ring_ip'] = '192.168.1.45'
+        self._configure_reconstructor()
+        self.assertEqual('192.168.1.45', self.reconstructor.ring_ip)
+
     def test_handoffs_only_default(self):
         # sanity neither option added to default conf
         self.conf.pop('handoffs_first', None)
@@ -3276,7 +3306,7 @@ class TestObjectReconstructor(BaseTestObjectReconstructor):
             'replication_ip': '127.0.0.88',  # not local via IP
             'replication_port': self.port,
         })
-        self.reconstructor.bind_ip = '0.0.0.0'  # use whataremyips
+        self.reconstructor.ring_ip = '0.0.0.0'  # use whataremyips
         with mock.patch('swift.obj.reconstructor.whataremyips',
                         return_value=[self.ip]), \
                 mock.patch.object(self.policy.object_ring, '_devs',
@@ -3331,7 +3361,7 @@ class TestObjectReconstructor(BaseTestObjectReconstructor):
             'replication_ip': '127.0.0.88',  # not local via IP
             'replication_port': self.port,
         })
-        self.reconstructor.bind_ip = '0.0.0.0'  # use whataremyips
+        self.reconstructor.ring_ip = '0.0.0.0'  # use whataremyips
         with mock.patch('swift.obj.reconstructor.whataremyips',
                         return_value=[self.ip]), \
                 mock.patch.object(self.policy.object_ring, '_devs',
@@ -3372,7 +3402,7 @@ class TestObjectReconstructor(BaseTestObjectReconstructor):
             'replication_ip': self.ip,
             'replication_port': self.port,
         } for i, dev in enumerate(local_devs)]
-        self.reconstructor.bind_ip = '0.0.0.0'  # use whataremyips
+        self.reconstructor.ring_ip = '0.0.0.0'  # use whataremyips
         with mock.patch('swift.obj.reconstructor.whataremyips',
                         return_value=[self.ip]), \
                 mock.patch.object(self.policy.object_ring, '_devs',
diff --git a/test/unit/obj/test_replicator.py b/test/unit/obj/test_replicator.py
index d25c25490..feeb9303d 100644
--- a/test/unit/obj/test_replicator.py
+++ b/test/unit/obj/test_replicator.py
@@ -260,6 +260,36 @@ class TestObjectReplicator(unittest.TestCase):
         rmtree(self.testdir, ignore_errors=1)
         rmtree(self.recon_cache, ignore_errors=1)
 
+    def test_ring_ip_and_bind_ip(self):
+        # make clean base_conf
+        base_conf = dict(self.conf)
+        for key in ('bind_ip', 'ring_ip'):
+            base_conf.pop(key, None)
+
+        # default ring_ip is always 0.0.0.0
+        self.conf = base_conf
+        self._create_replicator()
+        self.assertEqual('0.0.0.0', self.replicator.ring_ip)
+
+        # bind_ip works fine for legacy configs
+        self.conf = dict(base_conf)
+        self.conf['bind_ip'] = '192.168.1.42'
+        self._create_replicator()
+        self.assertEqual('192.168.1.42', self.replicator.ring_ip)
+
+        # ring_ip works fine by-itself
+        self.conf = dict(base_conf)
+        self.conf['ring_ip'] = '192.168.1.43'
+        self._create_replicator()
+        self.assertEqual('192.168.1.43', self.replicator.ring_ip)
+
+        # if you have both ring_ip wins
+        self.conf = dict(base_conf)
+        self.conf['bind_ip'] = '192.168.1.44'
+        self.conf['ring_ip'] = '192.168.1.45'
+        self._create_replicator()
+        self.assertEqual('192.168.1.45', self.replicator.ring_ip)
+
     def test_handoff_replication_setting_warnings(self):
         conf_tests = [
             # (config, expected_warning)