Try not to return loopback addresses with kolla_address in ipv4

kolla_address does not check if the returned ip address is a loopback
for ipv4 addresses, this change checks, and always try to return the
first non loopback address, defaulting to the primary in case only
loopbacks are available.

Closes-Bug: #2085699
Change-Id: Ie8a8b6156e6db025aba94e80a4b3aa2bf72d73ef
Signed-off-by: Bertrand Lanson <bertrand.lanson@protonmail.com>

2 files changed, 105 insertions, 0 deletions

diff --git a/kolla_ansible/kolla_address.py b/kolla_ansible/kolla_address.py
index e83037d57..52d46e166 100644
--- a/kolla_ansible/kolla_address.py
+++ b/kolla_ansible/kolla_address.py
@@ -23,6 +23,17 @@ from kolla_ansible.exception import FilterError
 from kolla_ansible.helpers import _call_bool_filter
 
 
+def is_loopback(address):
+    """Determines if the provided IP address is a loopback address.
+
+    :param address: string representing an IP address (IPv4 or IPv6)
+    :returns: bool indicating whether the address is a loopback address
+    """
+
+    ip = ipaddress.ip_address(address)
+    return ip.is_loopback
+
+
 @pass_context
 def kolla_address(context, network_name, hostname=None, override_var=None):
     """returns IP address on the requested network
@@ -124,6 +135,25 @@ def kolla_address(context, network_name, hostname=None, override_var=None):
 
     if address_family == 'ipv4':
         address = af_interface.get('address')
+
+        # NOTE(ednxzu): we do the following code in case the primary
+        # address of the interface is a loopback address. This is
+        # because sometimes, for example when doing BGP to the host,
+        # the host's ip address will be mounted on the loopback interface,
+        # as a secondary ip. This ensures that if there is a secondary
+        # address on a loopback interface, this secondary address is used.
+
+        non_loopback_addresses = []
+        if address and is_loopback(address):
+            ipv4_secondaries = interface.get("ipv4_secondaries", [])
+            non_loopback_addresses = [
+                sec["address"] for sec in ipv4_secondaries
+                if not is_loopback(sec["address"])
+            ]
+
+        if non_loopback_addresses:
+            address = non_loopback_addresses[0]
+
     elif address_family == 'ipv6':
         # ipv6 has no concept of a secondary address
         # explicitly exclude the vip addresses
diff --git a/kolla_ansible/tests/unit/test_address_filters.py b/kolla_ansible/tests/unit/test_address_filters.py
index 4be75528f..63b2a315f 100644
--- a/kolla_ansible/tests/unit/test_address_filters.py
+++ b/kolla_ansible/tests/unit/test_address_filters.py
@@ -19,6 +19,7 @@ import unittest
 import jinja2
 
 from kolla_ansible.exception import FilterError
+from kolla_ansible.kolla_address import is_loopback
 from kolla_ansible.kolla_address import kolla_address
 from kolla_ansible.kolla_url import kolla_url
 from kolla_ansible.put_address_in_context import put_address_in_context
@@ -390,6 +391,80 @@ class TestKollaAddressFilter(unittest.TestCase):
         self.assertEqual(
             addr, kolla_address(context, 'api', None, override_var))
 
+    def test_is_loopback_ipv4(self):
+        self.assertTrue(is_loopback("127.0.0.1"))
+        self.assertFalse(is_loopback("192.0.2.1"))
+
+    def test_is_loopback_ipv6(self):
+        self.assertTrue(is_loopback("::1"))
+        self.assertFalse(is_loopback("2001:db8::1"))
+
+    def test_loopback_primary_with_nonloopback_secondary(self):
+        loopback_addr = "127.0.0.1"
+        nonloopback_addr = "192.0.2.1"
+        context = self._make_context({
+            "inventory_hostname": "primary",
+            "hostvars": {
+                "primary": {
+                    "api_address_family": "ipv4",
+                    "api_interface": "lo",
+                    "ansible_facts": {
+                        "lo": {
+                            "ipv4": {"address": loopback_addr},
+                            "ipv4_secondaries": [
+                                {"address": nonloopback_addr},
+                            ],
+                        },
+                    },
+                },
+            },
+        })
+        self.assertEqual(nonloopback_addr, kolla_address(context, "api"))
+
+    def test_loopback_primary_with_only_loopback_secondaries(self):
+        loopback_addr = "127.0.0.1"
+        loopback_secondary = "127.0.0.2"
+        context = self._make_context({
+            "inventory_hostname": "primary",
+            "hostvars": {
+                "primary": {
+                    "api_address_family": "ipv4",
+                    "api_interface": "lo",
+                    "ansible_facts": {
+                        "lo": {
+                            "ipv4": {"address": loopback_addr},
+                            "ipv4_secondaries": [
+                                {"address": loopback_secondary},
+                            ],
+                        },
+                    },
+                },
+            },
+        })
+        self.assertEqual(loopback_addr, kolla_address(context, "api"))
+
+    def test_nonloopback_primary_ignores_secondaries(self):
+        primary_addr = "192.0.2.10"
+        secondary_addr = "192.0.2.11"
+        context = self._make_context({
+            "inventory_hostname": "primary",
+            "hostvars": {
+                "primary": {
+                    "api_address_family": "ipv4",
+                    "api_interface": "eth0",
+                    "ansible_facts": {
+                        "eth0": {
+                            "ipv4": {"address": primary_addr},
+                            "ipv4_secondaries": [
+                                {"address": secondary_addr},
+                            ],
+                        },
+                    },
+                },
+            },
+        })
+        self.assertEqual(primary_addr, kolla_address(context, "api"))
+
 
 class TestKollaUrlFilter(unittest.TestCase):