Merge pull request ceph#62927 from ShwetaBhosale1/fix_issue_71031_add_Monitoring_Addr_param_to_ganesha.conf

mgr/nfs: Add Monitoring_Addr parameter to ganesha.conf and provide option for user to provide ips for bind_addr

Reviewed-by: Adam King <[email protected]>
Reviewed-by: Anthony D'Atri <[email protected]>

Author: adk3798

doc/cephadm/services/nfs.rst

@@ -47,11 +47,37 @@ 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
+      monitoring_ip_addrs:
+        host1: 10.0.0.123
+        host2: 10.0.0.124
+      monitoring_networks:
+      - 192.168.124.0/24
+
 
 In this example, we run the server on the non-default ``port`` of
 12345 (instead of the default 2049) on ``host1`` and ``host2``.
+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
+IP address is assigned to the host, that IP address will be used. If the IP
+address is not present and ``monitoring_networks`` is specified, an IP address
+that matches one of the specified networks will be used. If neither condition
+is met, the default binding will happen on all available network interfaces.
 
 The specification can then be applied by running the following command:
 

src/pybind/mgr/cephadm/inventory.py

@@ -1597,6 +1597,14 @@ def get_scheduled_daemon_action(self, host: str, daemon: str) -> Optional[str]:
 
         return self.scheduled_daemon_actions.get(host, {}).get(daemon)
 
+    def get_host_network_ips(self, host: str) -> List[str]:
+        return [
+            ip
+            for net_details in self.networks.get(host, {}).values()
+            for ips in net_details.values()
+            for ip in ips
+        ]
+
 
 class NodeProxyCache:
     def __init__(self, mgr: 'CephadmOrchestrator') -> None:

src/pybind/mgr/cephadm/module.py

@@ -1068,8 +1068,14 @@ def update_failed_daemon_health_check(self) -> None:
             self.set_health_warning('CEPHADM_FAILED_DAEMON', f'{len(failed_daemons)} failed cephadm daemon(s)', len(
                 failed_daemons), failed_daemons)
 
-    def get_first_matching_network_ip(self, host: str, sspec: ServiceSpec) -> Optional[str]:
-        sspec_networks = sspec.networks
+    def get_first_matching_network_ip(
+        self,
+        host: str,
+        sspec: ServiceSpec,
+        sspec_networks: Optional[List[str]] = None
+    ) -> Optional[str]:
+        if not sspec_networks:
+            sspec_networks = sspec.networks
         for subnet, ifaces in self.cache.networks.get(host, {}).items():
             host_network = ipaddress.ip_network(subnet)
             for spec_network_str in sspec_networks:

src/pybind/mgr/cephadm/schedule.py

@@ -467,18 +467,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

@@ -21,6 +21,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
 
 from ceph.deployment.service_spec import SMBSpec
 
@@ -265,8 +266,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,20 +98,29 @@ 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_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)
 
+        if monitoring_ip:
+            daemon_spec.port_ips.update({str(monitoring_port): monitoring_ip})
+
         # generate the ganesha config
         def get_ganesha_conf() -> str:
             context: Dict[str, Any] = {
@@ -123,7 +132,8 @@ 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_port": spec.monitoring_port if spec.monitoring_port else 9587,
+                "monitoring_addr": monitoring_ip,
+                "monitoring_port": monitoring_port,
                 "bind_addr": bind_addr,
                 "haproxy_hosts": [],
                 "nfs_idmap_conf": nfs_idmap_conf,
@@ -372,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/templates/services/nfs/ganesha.conf.j2

@@ -12,6 +12,9 @@ NFS_CORE_PARAM {
 {% endif %}
 {% if haproxy_hosts %}
         HAProxy_Hosts = {{ haproxy_hosts|join(", ") }};
+{% endif %}
+{% if monitoring_addr %}
+	Monitoring_Addr = {{ monitoring_addr }};
 {% endif %}
         Monitoring_Port = {{ 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,15 +40,31 @@ 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
@@ -58,20 +75,25 @@ def metrics_exporter_port(self):
 
 
 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:
@@ -84,6 +106,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'