]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/cephadm: select IPs/interface based on VIP for keepalive conf
authorAdam King <adking@redhat.com>
Tue, 1 Aug 2023 20:32:06 +0000 (16:32 -0400)
committerAdam King <adking@redhat.com>
Fri, 4 Aug 2023 19:20:48 +0000 (15:20 -0400)
We need to make sure the keepalive conf sets
the unicast src and peer IPs to be the ones
in the same subnet as the VIP we're setting up,
as well as specify the correct interface. Otherwise,
the keepalive daemons don't speak to each other
properly and all end up going into MASTER state.

Fixes: https://tracker.ceph.com/issues/62276
Signed-off-by: Adam King <adking@redhat.com>
src/pybind/mgr/cephadm/services/ingress.py
src/pybind/mgr/cephadm/templates/services/ingress/keepalived.conf.j2
src/pybind/mgr/cephadm/tests/test_services.py

index 134c9ce9a39d9364a46118ed271acc6bfa84ca9b..39fb5c27fcab3a3a86ebd20b08f284100184d993 100644 (file)
@@ -247,56 +247,35 @@ class IngressService(CephService):
         host = daemon_spec.host
         hosts = sorted(list(set([host] + [str(d.hostname) for d in daemons])))
 
-        # interface
-        bare_ips = []
-        if spec.virtual_ip:
-            bare_ips.append(str(spec.virtual_ip).split('/')[0])
-        elif spec.virtual_ips_list:
-            bare_ips = [str(vip).split('/')[0] for vip in spec.virtual_ips_list]
-        interface = None
-        for bare_ip in bare_ips:
+        def _get_valid_interface_and_ip(vip: str, host: str) -> Tuple[str, str]:
+            # interface
+            bare_ip = ipaddress.ip_interface(vip).ip
+            host_ip = ''
+            interface = None
             for subnet, ifaces in self.mgr.cache.networks.get(host, {}).items():
                 if ifaces and ipaddress.ip_address(bare_ip) in ipaddress.ip_network(subnet):
                     interface = list(ifaces.keys())[0]
+                    host_ip = ifaces[interface][0]
                     logger.info(
                         f'{bare_ip} is in {subnet} on {host} interface {interface}'
                     )
                     break
-            else:  # nobreak
-                continue
-            break
-        # try to find interface by matching spec.virtual_interface_networks
-        if not interface and spec.virtual_interface_networks:
-            for subnet, ifaces in self.mgr.cache.networks.get(host, {}).items():
-                if subnet in spec.virtual_interface_networks:
-                    interface = list(ifaces.keys())[0]
-                    logger.info(
-                        f'{spec.virtual_ip} will be configured on {host} interface '
-                        f'{interface} (which has guiding subnet {subnet})'
-                    )
-                    break
-        if not interface:
-            raise OrchestratorError(
-                f"Unable to identify interface for {spec.virtual_ip} on {host}"
-            )
-
-        # Use interface as vrrp_interface for vrrp traffic if vrrp_interface_network not set on the spec
-        vrrp_interface = None
-        if not spec.vrrp_interface_network:
-            vrrp_interface = interface
-        else:
-            for subnet, ifaces in self.mgr.cache.networks.get(host, {}).items():
-                if subnet == spec.vrrp_interface_network:
-                    vrrp_interface = list(ifaces.keys())[0]
-                    logger.info(
-                        f'vrrp will be configured on {host} interface '
-                        f'{vrrp_interface} (which has guiding subnet {subnet})'
-                    )
-                    break
-            else:
+            # try to find interface by matching spec.virtual_interface_networks
+            if not interface and spec.virtual_interface_networks:
+                for subnet, ifaces in self.mgr.cache.networks.get(host, {}).items():
+                    if subnet in spec.virtual_interface_networks:
+                        interface = list(ifaces.keys())[0]
+                        host_ip = ifaces[interface][0]
+                        logger.info(
+                            f'{spec.virtual_ip} will be configured on {host} interface '
+                            f'{interface} (which is in subnet {subnet})'
+                        )
+                        break
+            if not interface:
                 raise OrchestratorError(
-                    f"Unable to identify vrrp interface for {spec.vrrp_interface_network} on {host}"
+                    f"Unable to identify interface for {spec.virtual_ip} on {host}"
                 )
+            return interface, host_ip
 
         # script to monitor health
         script = '/usr/bin/false'
@@ -341,7 +320,36 @@ class IngressService(CephService):
         # other_ips in conf file and converter to ips
         if host in hosts:
             hosts.remove(host)
-        other_ips = [utils.resolve_ip(self.mgr.inventory.get_addr(h)) for h in hosts]
+        host_ips: List[str] = []
+        other_ips: List[List[str]] = []
+        interfaces: List[str] = []
+        for vip in virtual_ips:
+            interface, ip = _get_valid_interface_and_ip(vip, host)
+            host_ips.append(ip)
+            interfaces.append(interface)
+            ips: List[str] = []
+            for h in hosts:
+                _, ip = _get_valid_interface_and_ip(vip, h)
+                ips.append(ip)
+            other_ips.append(ips)
+
+        # Use interface as vrrp_interface for vrrp traffic if vrrp_interface_network not set on the spec
+        vrrp_interfaces: List[str] = []
+        if not spec.vrrp_interface_network:
+            vrrp_interfaces = interfaces
+        else:
+            for subnet, ifaces in self.mgr.cache.networks.get(host, {}).items():
+                if subnet == spec.vrrp_interface_network:
+                    vrrp_interface = [list(ifaces.keys())[0]] * len(interfaces)
+                    logger.info(
+                        f'vrrp will be configured on {host} interface '
+                        f'{vrrp_interface} (which is in subnet {subnet})'
+                    )
+                    break
+            else:
+                raise OrchestratorError(
+                    f"Unable to identify vrrp interface for {spec.vrrp_interface_network} on {host}"
+                )
 
         keepalived_conf = self.mgr.template.render(
             'services/ingress/keepalived.conf.j2',
@@ -349,14 +357,14 @@ class IngressService(CephService):
                 'spec': spec,
                 'script': script,
                 'password': password,
-                'interface': interface,
-                'vrrp_interface': vrrp_interface,
+                'interfaces': interfaces,
+                'vrrp_interfaces': vrrp_interfaces,
                 'virtual_ips': virtual_ips,
                 'first_virtual_router_id': spec.first_virtual_router_id,
                 'states': states,
                 'priorities': priorities,
                 'other_ips': other_ips,
-                'host_ip': utils.resolve_ip(self.mgr.inventory.get_addr(host)),
+                'host_ips': host_ips,
             }
         )
 
index dfab7e342451d0a802b91c42322ba4d8e6bcf967..e19f556c6f42727247600466ffdd7097df9b80a5 100644 (file)
@@ -11,7 +11,7 @@ vrrp_script check_backend {
 vrrp_instance VI_{{ x }} {
   state {{ states[x] }}
   priority {{ priorities[x] }}
-  interface {{ vrrp_interface }}
+  interface {{ vrrp_interfaces[x] }}
   virtual_router_id {{ first_virtual_router_id + x }}
   advert_int 1
   authentication {
@@ -19,15 +19,15 @@ vrrp_instance VI_{{ x }} {
       auth_pass {{ password }}
   }
 {% if not spec.use_keepalived_multicast %}
-  unicast_src_ip {{ host_ip }}
+  unicast_src_ip {{ host_ips[x] }}
   unicast_peer {
-    {% for ip in other_ips %}
+    {% for ip in other_ips[x] %}
     {{ ip }}
     {% endfor %}
   }
 {% endif %}
   virtual_ipaddress {
-    {{ virtual_ips[x] }} dev {{ interface }}
+    {{ virtual_ips[x] }} dev {{ interfaces[x] }}
   }
   track_script {
       check_backend
index 25e557631e118314313e2ac2babe0013aa727a1d..8ab71c2bbdaaed465f441b4413fa1f4a27240b57 100644 (file)
@@ -1657,7 +1657,7 @@ class TestIngressService:
         with with_host(cephadm_module, 'test', addr='1.2.3.7'):
             cephadm_module.cache.update_host_networks('test', {
                 '1.2.3.0/24': {
-                    'if0': ['1.2.3.4/32']
+                    'if0': ['1.2.3.4']
                 }
             })
 
@@ -1701,7 +1701,7 @@ class TestIngressService:
                                 'auth_type PASS\n      '
                                 'auth_pass 12345\n  '
                                 '}\n  '
-                                'unicast_src_ip 1.2.3.7\n  '
+                                'unicast_src_ip 1.2.3.4\n  '
                                 'unicast_peer {\n  '
                                 '}\n  '
                                 'virtual_ipaddress {\n    '
@@ -1780,7 +1780,7 @@ class TestIngressService:
         with with_host(cephadm_module, 'test'):
             cephadm_module.cache.update_host_networks('test', {
                 '1.2.3.0/24': {
-                    'if0': ['1.2.3.4/32']
+                    'if0': ['1.2.3.1']
                 }
             })
 
@@ -1824,7 +1824,7 @@ class TestIngressService:
                                 'auth_type PASS\n      '
                                 'auth_pass 12345\n  '
                                 '}\n  '
-                                'unicast_src_ip 1::4\n  '
+                                'unicast_src_ip 1.2.3.1\n  '
                                 'unicast_peer {\n  '
                                 '}\n  '
                                 'virtual_ipaddress {\n    '
@@ -1905,7 +1905,7 @@ class TestIngressService:
         with with_host(cephadm_module, 'test', addr='1.2.3.7'):
             cephadm_module.cache.update_host_networks('test', {
                 '1.2.3.0/24': {
-                    'if0': ['1.2.3.4/32']
+                    'if0': ['1.2.3.1']
                 }
             })
 
@@ -1950,7 +1950,7 @@ class TestIngressService:
                                 'auth_type PASS\n      '
                                 'auth_pass 12345\n  '
                                 '}\n  '
-                                'unicast_src_ip 1.2.3.7\n  '
+                                'unicast_src_ip 1.2.3.1\n  '
                                 'unicast_peer {\n  '
                                 '}\n  '
                                 'virtual_ipaddress {\n    '
@@ -2022,6 +2022,104 @@ class TestIngressService:
 
                 assert haproxy_generated_conf[0] == haproxy_expected_conf
 
+    @patch("cephadm.serve.CephadmServe._run_cephadm")
+    def test_keepalive_config_multi_interface_vips(self, _run_cephadm, cephadm_module: CephadmOrchestrator):
+        _run_cephadm.side_effect = async_side_effect(('{}', '', 0))
+
+        with with_host(cephadm_module, 'test', addr='1.2.3.1'):
+            with with_host(cephadm_module, 'test2', addr='1.2.3.2'):
+                cephadm_module.cache.update_host_networks('test', {
+                    '1.2.3.0/24': {
+                        'if0': ['1.2.3.1']
+                    },
+                    '100.100.100.0/24': {
+                        'if1': ['100.100.100.1']
+                    }
+                })
+                cephadm_module.cache.update_host_networks('test2', {
+                    '1.2.3.0/24': {
+                        'if0': ['1.2.3.2']
+                    },
+                    '100.100.100.0/24': {
+                        'if1': ['100.100.100.2']
+                    }
+                })
+
+                # Check the ingress with multiple VIPs
+                s = RGWSpec(service_id="foo", placement=PlacementSpec(count=1),
+                            rgw_frontend_type='beast')
+
+                ispec = IngressSpec(service_type='ingress',
+                                    service_id='test',
+                                    placement=PlacementSpec(hosts=['test', 'test2']),
+                                    backend_service='rgw.foo',
+                                    frontend_port=8089,
+                                    monitor_port=8999,
+                                    monitor_user='admin',
+                                    monitor_password='12345',
+                                    keepalived_password='12345',
+                                    virtual_ips_list=["1.2.3.100/24", "100.100.100.100/24"])
+                with with_service(cephadm_module, s) as _, with_service(cephadm_module, ispec) as _:
+                    keepalived_generated_conf = cephadm_module.cephadm_services['ingress'].keepalived_generate_config(
+                        CephadmDaemonDeploySpec(host='test', daemon_id='ingress', service_name=ispec.service_name()))
+
+                    keepalived_expected_conf = {
+                        'files':
+                            {
+                                'keepalived.conf':
+                                    '# This file is generated by cephadm.\n'
+                                    'vrrp_script check_backend {\n    '
+                                    'script "/usr/bin/curl http://1.2.3.1:8999/health"\n    '
+                                    'weight -20\n    '
+                                    'interval 2\n    '
+                                    'rise 2\n    '
+                                    'fall 2\n}\n\n'
+                                    'vrrp_instance VI_0 {\n  '
+                                    'state MASTER\n  '
+                                    'priority 100\n  '
+                                    'interface if0\n  '
+                                    'virtual_router_id 50\n  '
+                                    'advert_int 1\n  '
+                                    'authentication {\n      '
+                                    'auth_type PASS\n      '
+                                    'auth_pass 12345\n  '
+                                    '}\n  '
+                                    'unicast_src_ip 1.2.3.1\n  '
+                                    'unicast_peer {\n    '
+                                    '1.2.3.2\n  '
+                                    '}\n  '
+                                    'virtual_ipaddress {\n    '
+                                    '1.2.3.100/24 dev if0\n  '
+                                    '}\n  '
+                                    'track_script {\n      '
+                                    'check_backend\n  }\n'
+                                    '}\n'
+                                    'vrrp_instance VI_1 {\n  '
+                                    'state BACKUP\n  '
+                                    'priority 90\n  '
+                                    'interface if1\n  '
+                                    'virtual_router_id 51\n  '
+                                    'advert_int 1\n  '
+                                    'authentication {\n      '
+                                    'auth_type PASS\n      '
+                                    'auth_pass 12345\n  '
+                                    '}\n  '
+                                    'unicast_src_ip 100.100.100.1\n  '
+                                    'unicast_peer {\n    '
+                                    '100.100.100.2\n  '
+                                    '}\n  '
+                                    'virtual_ipaddress {\n    '
+                                    '100.100.100.100/24 dev if1\n  '
+                                    '}\n  '
+                                    'track_script {\n      '
+                                    'check_backend\n  }\n'
+                                    '}\n'
+                            }
+                    }
+
+                    # check keepalived config
+                    assert keepalived_generated_conf[0] == keepalived_expected_conf
+
     @patch("cephadm.serve.CephadmServe._run_cephadm")
     @patch("cephadm.services.nfs.NFSService.fence_old_ranks", MagicMock())
     @patch("cephadm.services.nfs.NFSService.run_grace_tool", MagicMock())
@@ -2033,7 +2131,7 @@ class TestIngressService:
         with with_host(cephadm_module, 'test', addr='1.2.3.7'):
             cephadm_module.cache.update_host_networks('test', {
                 '1.2.3.0/24': {
-                    'if0': ['1.2.3.4/32']
+                    'if0': ['1.2.3.1']
                 }
             })
 
@@ -2080,7 +2178,7 @@ class TestIngressService:
                                 'auth_type PASS\n      '
                                 'auth_pass 12345\n  '
                                 '}\n  '
-                                'unicast_src_ip 1.2.3.7\n  '
+                                'unicast_src_ip 1.2.3.1\n  '
                                 'unicast_peer {\n  '
                                 '}\n  '
                                 'virtual_ipaddress {\n    '