mgr/cephadm: while updating nfs kmip certs do not restart service instead send SIGHUP to ganesha service

Signed-off-by: Shweta Bhosale <[email protected]>
(cherry picked from commit c99d6c6)

Conflicts:
	src/cephadm/cephadm.py
	src/pybind/mgr/cephadm/module.py
	src/pybind/mgr/cephadm/serve.py
	src/pybind/mgr/cephadm/services/nfs.py
	src/pybind/mgr/cephadm/templates/services/nfs/ganesha.conf.j2

Resolves: rhbz#2373703

Author: ShwetaBhosale1

src/cephadm/cephadm.py

@@ -1147,10 +1147,14 @@ def deploy_daemon(
     # If this was a reconfig and the daemon is not a Ceph daemon, restart it
     # so it can pick up potential changes to its configuration files
     if deployment_type == DeploymentType.RECONFIG and daemon_type not in ceph_daemons():
-        # ceph daemons do not need a restart; others (presumably) do to pick
-        # up the new config
-        call_throws(ctx, ['systemctl', 'reset-failed', ident.unit_name])
-        call_throws(ctx, ['systemctl', 'restart', ident.unit_name])
+        if not ctx.skip_restart:
+            # ceph daemons do not need a restart; others (presumably) do to pick
+            # up the new config
+            call_throws(ctx, ['systemctl', 'reset-failed', ident.unit_name])
+            call_throws(ctx, ['systemctl', 'restart', ident.unit_name])
+        else:
+            # perform default action
+            daemon_form_create(ctx, ident).perform_default_restart()
 
 
 def clean_cgroup(ctx: CephadmContext, fsid: str, unit_name: str) -> None:
@@ -5264,6 +5268,12 @@ def _add_deploy_parser_args(
         default=False,
         help='Set LimitCORE=infinity in ceph unit files'
     )
+    parser_deploy.add_argument(
+        '--skip-restart',
+        action='store_true',
+        default=False,
+        help='skip restart for non ceph daemons and perform default action'
+    )
 
 
 def _get_parser():

src/cephadm/cephadmlib/daemon_form.py

@@ -44,6 +44,9 @@ def identity(self) -> DaemonIdentity:
         """
         raise NotImplementedError()  # pragma: no cover
 
+    def perform_default_restart(self) -> int:
+        return 0
+
 
 DF = TypeVar('DF', bound=DaemonForm)
 

src/cephadm/cephadmlib/daemons/nfs.py

@@ -1,7 +1,8 @@
 import logging
 import os
 import re
-
+import signal
+import subprocess
 from typing import Dict, List, Optional, Tuple, Union
 
 from ..call_wrappers import call, CallVerbosity
@@ -236,3 +237,25 @@ def customize_container_args(
 
     def default_entrypoint(self) -> str:
         return self.entrypoint
+
+    def perform_default_restart(self) -> int:
+        pid = None
+        try:
+            output = subprocess.check_output(['pidof', 'ganesha.nfsd'])
+            pid = int(output.strip())
+        except subprocess.CalledProcessError:
+            return 1
+        if pid:
+            logger.debug(
+                f'Sending SIGHUP signal to ganesha.nfsd process, pid: {pid}'
+            )
+            try:
+                os.kill(pid, signal.SIGHUP)
+            except Exception:
+                logger.error(
+                    f'Not able to send SIGHUP signal to ganesha.nfsd process, pid: {pid}'
+                )
+                return 1
+            return 0
+        logger.error('Process ganesha.nfsd not found.')
+        return 1

src/pybind/mgr/cephadm/module.py

@@ -2685,7 +2685,8 @@ def _daemon_action(self,
                        daemon_spec: CephadmDaemonDeploySpec,
                        action: str,
                        image: Optional[str] = None,
-                       spec: Optional[ServiceSpec] = None) -> str:
+                       spec: Optional[ServiceSpec] = None,
+                       skip_restart: bool = False) -> str:
         self._daemon_action_set_image(action, image, daemon_spec.daemon_type,
                                       daemon_spec.daemon_id)
 
@@ -2711,7 +2712,7 @@ def _daemon_action(self,
                     daemon_spec)
             with self.async_timeout_handler(daemon_spec.host, f'cephadm deploy ({daemon_spec.daemon_type} daemon)'):
                 successes, failures = self.wait_async(
-                    CephadmServe(self)._create_daemon([daemon_spec], reconfig=(action == 'reconfig')))
+                    CephadmServe(self)._create_daemon([daemon_spec], reconfig=(action == 'reconfig'), skip_restart=skip_restart))
                 # we're only deploying one daemon here, so we expect successes or failures to container one entry
                 for res in [successes, failures]:
                     if daemon_spec.name() in res:

src/pybind/mgr/cephadm/serve.py

@@ -1342,6 +1342,7 @@ def _check_daemons(self) -> None:
             if last_deps is None:
                 last_deps = []
             action = self.mgr.cache.get_scheduled_daemon_action(dd.hostname, dd.name())
+            skip_restart = False
             if not last_config:
                 self.log.info('Reconfiguring %s (unknown last config time)...' % (
                     dd.name()))
@@ -1357,6 +1358,13 @@ def _check_daemons(self) -> None:
                     REDEPLOY_TRIGGERS = ['secure_monitoring_stack', 'mgmt-gateway']
                     if any(svc in e for e in diff for svc in REDEPLOY_TRIGGERS):
                         action = 'redeploy'
+                elif dd.daemon_type == 'nfs':
+                    # check what has changed, based on that decide action
+                    only_kmip_updated = all(s.startswith('kmip') for s in list(sym_diff))
+                    if not only_kmip_updated:
+                        action = 'redeploy'
+                    else:
+                        skip_restart = True
 
             elif spec is not None and hasattr(spec, 'extra_container_args') and dd.extra_container_args != spec.extra_container_args:
                 self.log.debug(
@@ -1385,7 +1393,7 @@ def _check_daemons(self) -> None:
                     action = 'redeploy'
                 try:
                     daemon_spec = CephadmDaemonDeploySpec.from_daemon_description(dd)
-                    self.mgr._daemon_action(daemon_spec, action=action, spec=spec)
+                    self.mgr._daemon_action(daemon_spec, action=action, spec=spec, skip_restart=skip_restart)
                     if self.mgr.cache.rm_scheduled_daemon_action(dd.hostname, dd.name()):
                         self.mgr.cache.save_host(dd.hostname)
                 except OrchestratorError as e:
@@ -1565,6 +1573,7 @@ async def _create_daemon(self,
                              daemon_specs: List[CephadmDaemonDeploySpec],
                              reconfig: bool = False,
                              osd_uuid_map: Optional[Dict[str, Any]] = None,
+                             skip_restart: bool = False
                              ) -> Tuple[Dict[str, str], Dict[str, str]]:
 
         exchanges: List[exchange.Deploy] = []
@@ -1610,6 +1619,8 @@ async def _create_daemon(self,
                 daemon_params['allow_ptrace'] = True
             if self.mgr.set_coredump_overrides:
                 daemon_params['limit_core_infinity'] = True
+            if skip_restart:
+                daemon_params['skip_restart'] = True
 
             daemon_spec, extra_container_args, extra_entrypoint_args = self._setup_extra_deployment_args(daemon_spec, daemon_params)
             init_containers = self._setup_init_containers(daemon_spec, daemon_params)

src/pybind/mgr/cephadm/service_discovery.py

@@ -12,7 +12,7 @@ class Server:  # type: ignore
 from mgr_module import ServiceInfoT
 from mgr_util import build_url
 from typing import Dict, List, TYPE_CHECKING, cast, Collection, Callable, NamedTuple, Optional, IO
-from cephadm.services.nfs import NFSService
+
 from cephadm.services.smb import SMBService
 from cephadm.services.monitoring import AlertmanagerService, NodeExporterService, PrometheusService
 import secrets
@@ -265,6 +265,7 @@ def nvmeof_sd_config(self) -> List[Dict[str, Collection[str]]]:
     def nfs_sd_config(self) -> List[Dict[str, Collection[str]]]:
         """Return <http_sd_config> compatible prometheus config for nfs service."""
         srv_entries = []
+        from cephadm.services.nfs import NFSService
         for dd in self.mgr.cache.get_daemons_by_type('nfs'):
             assert dd.hostname is not None
             nfs = cast(NFSService, service_registry.get_service('nfs'))

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

@@ -4,7 +4,7 @@
 import os
 import subprocess
 import tempfile
-from typing import Dict, Tuple, Any, List, cast, Optional
+from typing import Dict, Tuple, Any, List, cast, Optional, TYPE_CHECKING
 from configparser import ConfigParser
 from io import StringIO
 
@@ -17,6 +17,8 @@
 from orchestrator import DaemonDescription, OrchestratorError
 
 from cephadm.services.cephadmservice import AuthEntity, CephadmDaemonDeploySpec, CephService
+if TYPE_CHECKING:
+    from ..module import CephadmOrchestrator
 
 logger = logging.getLogger(__name__)
 
@@ -72,6 +74,25 @@ def get_daemon_nodeid(self, service_name: str, rank: Optional[int]) -> str:
                 return f'{service_name}.{rank}'
         return str(rank)
 
+    @classmethod
+    def get_dependencies(
+        cls,
+        mgr: "CephadmOrchestrator",
+        spec: Optional[ServiceSpec] = None,
+        daemon_type: Optional[str] = None
+    ) -> List[str]:
+        deps: List[str] = []
+        if not spec:
+            return deps
+        nfs_spec = cast(NFSServiceSpec, spec)
+        if (nfs_spec.kmip_cert and nfs_spec.kmip_key and nfs_spec.kmip_ca_cert and nfs_spec.kmip_host_list):
+            # add dependency of kmip fields
+            deps.append(f'kmip_cert: {nfs_spec.kmip_cert}')
+            deps.append(f'kmip_key: {nfs_spec.kmip_key}')
+            deps.append(f'kmip_ca_cert: {nfs_spec.kmip_ca_cert}')
+            deps.append(f'kmip_host_list: {nfs_spec.kmip_host_list}')
+        return deps
+
     def generate_config(self, daemon_spec: CephadmDaemonDeploySpec) -> Tuple[Dict[str, Any], List[str]]:
         assert self.TYPE == daemon_spec.daemon_type
 
@@ -80,8 +101,6 @@ def generate_config(self, daemon_spec: CephadmDaemonDeploySpec) -> Tuple[Dict[st
         host = daemon_spec.host
         spec = cast(NFSServiceSpec, self.mgr.spec_store[daemon_spec.service_name].spec)
 
-        deps: List[str] = []
-
         nodeid = self.get_daemon_nodeid(spec.service_name(), daemon_spec.rank)
 
         nfs_idmap_conf = '/etc/ganesha/idmap.conf'
@@ -199,7 +218,7 @@ def get_cephadm_config() -> Dict[str, Any]:
             logger.debug('Generated cephadm config-json: %s' % config)
             return config
 
-        return get_cephadm_config(), deps
+        return get_cephadm_config(), sorted(self.get_dependencies(self.mgr, spec))
 
     def create_rados_config_obj(self,
                                 spec: NFSServiceSpec,

src/pybind/mgr/cephadm/templates/services/nfs/ganesha.conf.j2

@@ -54,6 +54,7 @@ Ceph {
 {% endif %}
 
 {% if kmip_addrs %}
+
 KMIP {
             HOST {
 {% for kmip_addr in kmip_addrs %}

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

@@ -3748,6 +3748,45 @@ def test_nfs_config_bind_addr(self, _run_cephadm, cephadm_module: CephadmOrchest
                 assert "Bind_addr = 1.2.3.7" in ganesha_conf
 
 
+class TestNFS:
+    @patch("cephadm.serve.CephadmServe._run_cephadm")
+    @patch("cephadm.services.nfs.NFSService.fence_old_ranks", MagicMock())
+    @patch("cephadm.services.nfs.NFSService.run_grace_tool", MagicMock())
+    @patch("cephadm.services.nfs.NFSService.purge", MagicMock())
+    @patch("cephadm.services.nfs.NFSService.create_rados_config_obj", MagicMock())
+    def test_nfs_byok_config(self, _run_cephadm, cephadm_module: CephadmOrchestrator):
+        _run_cephadm.side_effect = async_side_effect(('{}', '', 0))
+
+        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.1']
+                }
+            })
+
+            nfs_spec = NFSServiceSpec(service_id="foo", placement=PlacementSpec(hosts=['test']),
+                                      kmip_cert='kmip_cert', kmip_key='kmip_key',
+                                      kmip_ca_cert='kmip_ca_cert', kmip_host_list=['test'])
+            with with_service(cephadm_module, nfs_spec) as _:
+                nfs_generated_conf, _ = service_registry.get_service('nfs').generate_config(
+                    CephadmDaemonDeploySpec(host='test', daemon_id='foo.test.0.0', service_name=nfs_spec.service_name()))
+                ganesha_conf = nfs_generated_conf['files']['ganesha.conf']
+                expected_kmip_block = (
+                    'KMIP {\n'
+                    '            HOST {\n'
+                    '                     addr = test;\n'
+                    '            }\n'
+                    '            cert = /etc/ganesha/kmip/kmip_cert.pem;\n'
+                    '            key = /etc/ganesha/kmip/kmip_key.pem;\n'
+                    '            ca = /etc/ganesha/kmip/kmip_ca_cert.pem;\n'
+                    '}\n'
+                )
+                assert expected_kmip_block in ganesha_conf
+                assert nfs_generated_conf['files']['kmip_cert.pem'] == 'kmip_cert'
+                assert nfs_generated_conf['files']['kmip_key.pem'] == 'kmip_key'
+                assert nfs_generated_conf['files']['kmip_ca_cert.pem'] == 'kmip_ca_cert'
+
+
 class TestCephFsMirror:
     @patch("cephadm.serve.CephadmServe._run_cephadm")
     def test_config(self, _run_cephadm, cephadm_module: CephadmOrchestrator):

src/python-common/ceph/deployment/service_spec.py

@@ -1,5 +1,3 @@
-
-
 import fnmatch
 import os
 import re