mgr/nfs: Add Spec parameter for ganesha.conf Bind_addr

Fixes: https://tracker.ceph.com/issues/71031

Signed-off-by: Shweta Bhosale <[email protected]>

Author: ShwetaBhosale1

doc/cephadm/services/nfs.rst

@@ -47,6 +47,11 @@ Alternatively, an NFS service can be applied using a YAML specification.
       hosts:
         - host1
         - host2
+    networks:
+    - 1.2.3.4/24
+    ip_addrs:
+      host1: 10.0.0.100
+      host2: 10.0.0.101
     spec:
       port: 12345
       monitoring_port: 567
@@ -59,7 +64,13 @@ Alternatively, an NFS service can be applied using a YAML specification.
 
 In this example, we run the server on the non-default ``port`` of
 12345 (instead of the default 2049) on ``host1`` and ``host2``.
-The default monitoring port can be customized using the ``monitoring_port``
+You can bind the NFS data port to a specific IP address using either the
+``ip_addrs`` or ``networks`` section. If ``ip_addrs`` is provided and
+the specified IP is assigned to the host, that IP will be used. If the
+IP is not present but ``networks`` is specified, an IP matching one of
+the given networks will be selected. If neither condition is met, the
+daemon will not start on that node.
+The default NFS monitoring port can be customized using the ``monitoring_port``
 parameter. Additionally, you can specify the ``monitoring_ip_addrs`` or
 ``monitoring_networks`` parameters to bind the monitoring port to a specific
 IP address or network. If ``monitoring_ip_addrs`` is provided and the specified

src/pybind/mgr/cephadm/schedule.py

@@ -460,18 +460,34 @@ def get_candidates(self) -> List[DaemonPlacement]:
                 "placement spec is empty: no hosts, no label, no pattern, no count")
 
         # allocate an IP?
-        if self.spec.networks:
+        if self.spec.networks or self.spec.ip_addrs:
             orig = ls.copy()
             ls = []
             for p in orig:
-                ip = self.find_ip_on_host(p.hostname, self.spec.networks)
+                ip = None
+                # daemon can have specific ip if 'ip_addrs' is spcified in spec, we can use this
+                # parameter for all services, if they need to bind to specific ip
+                # If ip not present and networks is passed, ip of that network will be used
+                if self.spec.ip_addrs:
+                    ip = self.spec.ip_addrs.get(p.hostname)
+                    host_ips: List[str] = []
+                    for net_details in self.networks.get(p.hostname, {}).values():
+                        for ips in net_details.values():
+                            host_ips.extend(ips)
+                    if ip and ip not in host_ips:
+                        logger.debug(f"IP {ip} is not configured on host {p.hostname}.")
+                        ip = None
+                if not ip and self.spec.networks:
+                    ip = self.find_ip_on_host(p.hostname, self.spec.networks)
                 if ip:
                     ls.append(DaemonPlacement(daemon_type=self.primary_daemon_type,
                                               hostname=p.hostname, network=p.network,
                                               name=p.name, ports=p.ports, ip=ip))
                 else:
                     logger.debug(
-                        f'Skipping {p.hostname} with no IP in network(s) {self.spec.networks}'
+                        f"Skipping {p.hostname} with no IP in provided networks or ip_addrs "
+                        f"{f'networks: {self.spec.networks}' if self.spec.networks else ''}"
+                        f"{f'ip_addrs: {self.spec.ip_addrs}' if self.spec.ip_addrs else ''}"
                     )
 
         if self.filter_new_host:

src/pybind/mgr/cephadm/service_discovery.py

@@ -22,6 +22,7 @@ class Server:  # type: ignore
 from cephadm.services.ingress import IngressSpec
 from cephadm.services.cephadmservice import CephExporterService
 from cephadm.services.nvmeof import NvmeofService
+from cephadm.services.service_registry import service_registry
 
 if TYPE_CHECKING:
     from cephadm.module import CephadmOrchestrator
@@ -263,8 +264,10 @@ def nfs_sd_config(self) -> List[Dict[str, Collection[str]]]:
         srv_entries = []
         for dd in self.mgr.cache.get_daemons_by_type('nfs'):
             assert dd.hostname is not None
-            addr = dd.ip if dd.ip else self.mgr.inventory.get_addr(dd.hostname)
-            port = NFSService.DEFAULT_EXPORTER_PORT
+            nfs = cast(NFSService, service_registry.get_service('nfs'))
+            monitoring_ip, monitoring_port = nfs.get_monitoring_details(dd.service_name(), dd.hostname)
+            addr = monitoring_ip or dd.ip or self.mgr.inventory.get_addr(dd.hostname)
+            port = monitoring_port or NFSService.DEFAULT_EXPORTER_PORT
             srv_entries.append({
                 'targets': [build_url(host=addr, port=port).lstrip('/')],
                 'labels': {'instance': dd.hostname}

src/pybind/mgr/cephadm/services/nfs.py

@@ -98,32 +98,28 @@ def generate_config(self, daemon_spec: CephadmDaemonDeploySpec) -> Tuple[Dict[st
         self.create_rados_config_obj(spec)
 
         port = daemon_spec.ports[0] if daemon_spec.ports else 2049
-        monitoring_port = spec.monitoring_port if spec.monitoring_port else 9587
+        monitoring_ip, monitoring_port = self.get_monitoring_details(daemon_spec.service_name, host)
 
         # create the RGW keyring
         rgw_user = f'{rados_user}-rgw'
         rgw_keyring = self.create_rgw_keyring(daemon_spec)
+        bind_addr = ''
         if spec.virtual_ip and not spec.enable_haproxy_protocol:
             bind_addr = spec.virtual_ip
             daemon_spec.port_ips = {str(port): spec.virtual_ip}
-        else:
-            bind_addr = daemon_spec.ip if daemon_spec.ip else ''
+            # update daemon spec ip for prometheus, as monitoring will happen on this
+            # ip, if no monitor ip specified
+            daemon_spec.ip = bind_addr
+        elif daemon_spec.ip:
+            bind_addr = daemon_spec.ip
+            daemon_spec.port_ips = {str(port): daemon_spec.ip}
         if not bind_addr:
             logger.warning(f'Bind address in {daemon_type}.{daemon_id}\'s ganesha conf is defaulting to empty')
         else:
             logger.debug("using haproxy bind address: %r", bind_addr)
 
-        # check if monitor needs to be bind on specific ip
-        monitoring_addr = spec.monitoring_ip_addrs.get(host) if spec.monitoring_ip_addrs else None
-        if monitoring_addr and monitoring_addr not in self.mgr.cache.get_host_network_ips(host):
-            logger.debug(f"Monitoring IP {monitoring_addr} is not configured on host {daemon_spec.host}.")
-            monitoring_addr = None
-        if not monitoring_addr and spec.monitoring_networks:
-            monitoring_addr = self.mgr.get_first_matching_network_ip(daemon_spec.host, spec, spec.monitoring_networks)
-            if not monitoring_addr:
-                logger.debug(f"No IP address found in the network {spec.monitoring_networks} on host {daemon_spec.host}.")
-        if monitoring_addr:
-            daemon_spec.port_ips.update({str(monitoring_port): monitoring_addr})
+        if monitoring_ip:
+            daemon_spec.port_ips.update({str(monitoring_port): monitoring_ip})
 
         # generate the ganesha config
         def get_ganesha_conf() -> str:
@@ -136,7 +132,7 @@ def get_ganesha_conf() -> str:
                 "url": f'rados://{POOL_NAME}/{spec.service_id}/{spec.rados_config_name()}',
                 # fall back to default NFS port if not present in daemon_spec
                 "port": port,
-                "monitoring_addr": monitoring_addr,
+                "monitoring_addr": monitoring_ip,
                 "monitoring_port": monitoring_port,
                 "bind_addr": bind_addr,
                 "haproxy_hosts": [],
@@ -386,3 +382,18 @@ def _haproxy_hosts(self) -> List[str]:
                     # one address per interface/subnet is enough
                     cluster_ips.append(addrs[0])
         return cluster_ips
+
+    def get_monitoring_details(self, service_name: str, host: str) -> Tuple[Optional[str], Optional[int]]:
+        spec = cast(NFSServiceSpec, self.mgr.spec_store[service_name].spec)
+        monitoring_port = spec.monitoring_port if spec.monitoring_port else 9587
+
+        # check if monitor needs to be bind on specific ip
+        monitoring_addr = spec.monitoring_ip_addrs.get(host) if spec.monitoring_ip_addrs else None
+        if monitoring_addr and monitoring_addr not in self.mgr.cache.get_host_network_ips(host):
+            logger.debug(f"Monitoring IP {monitoring_addr} is not configured on host {host}.")
+            monitoring_addr = None
+        if not monitoring_addr and spec.monitoring_networks:
+            monitoring_addr = self.mgr.get_first_matching_network_ip(host, spec, spec.monitoring_networks)
+            if not monitoring_addr:
+                logger.debug(f"No IP address found in the network {spec.monitoring_networks} on host {host}.")
+        return monitoring_addr, monitoring_port

src/pybind/mgr/cephadm/tests/test_service_discovery.py

@@ -1,5 +1,6 @@
 from unittest.mock import MagicMock
 from cephadm.service_discovery import Root
+from cephadm.services.service_registry import service_registry
 
 
 class FakeDaemonDescription:
@@ -39,35 +40,56 @@ def get_daemons_by_service(self, service_type):
                 FakeDaemonDescription('1.2.3.5', [9200], 'node1')]
 
     def get_daemons_by_type(self, daemon_type):
-        return [FakeDaemonDescription('1.2.3.4', [9100], 'node0', 'ingress', 'haproxy'),
-                FakeDaemonDescription('1.2.3.5', [9200], 'node1', 'ingress', 'haproxy')]
+        if daemon_type == 'ingress':
+            return [FakeDaemonDescription('1.2.3.4', [9100], 'node0', 'ingress', 'haproxy'),
+                    FakeDaemonDescription('1.2.3.5', [9200], 'node1', 'ingress', 'haproxy')]
+        else:
+            return [FakeDaemonDescription('1.2.3.4', [1234], 'node0', daemon_type, daemon_type),
+                    FakeDaemonDescription('1.2.3.5', [1234], 'node1', daemon_type, daemon_type)]
 
 
 class FakeInventory:
     def get_addr(self, name: str):
         return '1.2.3.4'
 
 
+class FakeNFSServiceSpec:
+    def __init__(self, port):
+        self.monitoring_port = None
+        self.monitoring_ip_addrs = None
+        self.monitoring_networks = None
+
+
+class FakeIngressServiceSpec:
+    def __init__(self, port):
+        self.monitor_port = port
+
+
 class FakeServiceSpec:
     def __init__(self, port):
         self.monitor_port = port
 
 
 class FakeSpecDescription:
-    def __init__(self, port):
-        self.spec = FakeServiceSpec(port)
+    def __init__(self, service, port):
+        if service == 'ingress':
+            self.spec = FakeIngressServiceSpec(port)
+        elif service == 'nfs':
+            self.spec = FakeNFSServiceSpec(port)
+        else:
+            self.spec = FakeServiceSpec(port)
 
 
 class FakeSpecStore():
     def __init__(self, mgr):
         self.mgr = mgr
-        self._specs = {'ingress': FakeSpecDescription(9049)}
+        self._specs = {'ingress': FakeSpecDescription('ingress', 9049), 'nfs': FakeSpecDescription('nfs', 9587)}
 
     def __contains__(self, name):
         return name in self._specs
 
     def __getitem__(self, name):
-        return self._specs['ingress']
+        return self._specs[name]
 
 
 class FakeMgr:
@@ -80,6 +102,7 @@ def __init__(self):
         self.inventory = FakeInventory()
         self.cache = FakeCache()
         self.spec_store = FakeSpecStore(self)
+        service_registry.init_services(self)
 
     def get_mgr_id(self):
         return 'mgr-1'