mgr/cephadm: updated nfs cluster create command and addressed review comments

Fixes: https://tracker.ceph.com/issues/71551
Signed-off-by: Shweta Bhosale <[email protected]>
(cherry picked from commit 0ae93ec)

Conflicts:
	src/cephadm/cephadm.py
	src/pybind/mgr/cephadm/module.py
	src/pybind/mgr/cephadm/serve.py
	src/pybind/mgr/nfs/module.py
	src/pybind/mgr/nfs/tests/test_nfs.py

Resolves: rhbz#2373703

Author: ShwetaBhosale1

src/cephadm/cephadm.py

@@ -768,15 +768,15 @@ def lookup_container_id_by_daemon_name(ctx: CephadmContext, fsid: str, name: str
     )
     daemons = [updater.expand(ctx, entry) for entry in daemon_entries]
     if not daemons:
-        raise Error('Failed to find daemon {}'.format(name))
+        raise Error(f'Failed to find daemon {name}')
     if len(daemons) > 1:
-        raise Error('Found multiple daemons matching name {}: {}'.format(name, daemons))
+        raise Error(f'Found multiple daemons matching name {name}: {daemons}')
 
     daemon = daemons[0]
     try:
         return daemon['container_id']
     except KeyError:
-        raise Error('Failed to get container id for {}'.format(daemon))
+        raise Error(f'Failed to get container id for {daemon}')
 
 
 def create_daemon_dirs(
@@ -1166,14 +1166,15 @@ 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():
-        if not ctx.skip_restart:
+        if not ctx.skip_restart_for_reconfig:
             # 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()
+        elif ctx.send_signal_to_daemon:
+            ctx.signal_name = ctx.send_signal_to_daemon
+            ctx.signal_number = None
+            command_signal(ctx)
 
 
 def clean_cgroup(ctx: CephadmContext, fsid: str, unit_name: str) -> None:
@@ -3586,8 +3587,7 @@ def command_unit(ctx):
 
 
 @infer_fsid
-def command_signal(ctx):
-    # type: (CephadmContext) -> int
+def command_signal(ctx: CephadmContext) -> int:
     if not ctx.fsid:
         raise Error('must pass --fsid to specify cluster')
 
@@ -5300,11 +5300,15 @@ def _add_deploy_parser_args(
         help='Set LimitCORE=infinity in ceph unit files'
     )
     parser_deploy.add_argument(
-        '--skip-restart',
+        '--skip-restart-for-reconfig',
         action='store_true',
         default=False,
         help='skip restart for non ceph daemons and perform default action'
     )
+    parser_deploy.add_argument(
+        '--send-signal-to-daemon',
+        help='Send signal to daemon'
+    )
 
 
 def _get_parser():

src/cephadm/cephadmlib/daemon_form.py

@@ -44,9 +44,6 @@ 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,8 +1,6 @@
 import logging
 import os
 import re
-import signal
-import subprocess
 from typing import Dict, List, Optional, Tuple, Union
 
 from ..call_wrappers import call, CallVerbosity
@@ -237,25 +235,3 @@ 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

@@ -2686,7 +2686,8 @@ def _daemon_action(self,
                        action: str,
                        image: Optional[str] = None,
                        spec: Optional[ServiceSpec] = None,
-                       skip_restart: bool = False) -> str:
+                       skip_restart_for_reconfig: bool = False,
+                       send_signal_to_daemon: Optional[str] = None) -> str:
         self._daemon_action_set_image(action, image, daemon_spec.daemon_type,
                                       daemon_spec.daemon_id)
 
@@ -2712,7 +2713,13 @@ 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'), skip_restart=skip_restart))
+                    CephadmServe(self)._create_daemon(
+                        [daemon_spec],
+                        reconfig=(action == 'reconfig'),
+                        skip_restart_for_reconfig=skip_restart_for_reconfig,
+                        send_signal_to_daemon=send_signal_to_daemon
+                    )
+                )
                 # 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,7 +1342,8 @@ 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
+            skip_restart_for_reconfig = False
+            send_signal_to_daemon = None
             if not last_config:
                 self.log.info('Reconfiguring %s (unknown last config time)...' % (
                     dd.name()))
@@ -1364,8 +1365,8 @@ def _check_daemons(self) -> None:
                     if not only_kmip_updated:
                         action = 'redeploy'
                     else:
-                        skip_restart = True
-
+                        skip_restart_for_reconfig = True
+                        send_signal_to_daemon = 'SIGHUP'
             elif spec is not None and hasattr(spec, 'extra_container_args') and dd.extra_container_args != spec.extra_container_args:
                 self.log.debug(
                     f'{dd.name()} container cli args {dd.extra_container_args} -> {spec.extra_container_args}')
@@ -1393,7 +1394,13 @@ 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, skip_restart=skip_restart)
+                    self.mgr._daemon_action(
+                        daemon_spec,
+                        action=action,
+                        spec=spec,
+                        skip_restart_for_reconfig=skip_restart_for_reconfig,
+                        send_signal_to_daemon=send_signal_to_daemon
+                    )
                     if self.mgr.cache.rm_scheduled_daemon_action(dd.hostname, dd.name()):
                         self.mgr.cache.save_host(dd.hostname)
                 except OrchestratorError as e:
@@ -1573,7 +1580,8 @@ async def _create_daemon(self,
                              daemon_specs: List[CephadmDaemonDeploySpec],
                              reconfig: bool = False,
                              osd_uuid_map: Optional[Dict[str, Any]] = None,
-                             skip_restart: bool = False
+                             skip_restart_for_reconfig: bool = False,
+                             send_signal_to_daemon: Optional[str] = None
                              ) -> Tuple[Dict[str, str], Dict[str, str]]:
 
         exchanges: List[exchange.Deploy] = []
@@ -1619,8 +1627,10 @@ 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
+            if skip_restart_for_reconfig:
+                daemon_params['skip_restart_for_reconfig'] = True
+            if send_signal_to_daemon:
+                daemon_params['send_signal_to_daemon'] = send_signal_to_daemon
 
             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,7 +265,6 @@ 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

@@ -16,6 +16,7 @@
 
 from orchestrator import DaemonDescription, OrchestratorError
 
+from cephadm import utils
 from cephadm.services.cephadmservice import AuthEntity, CephadmDaemonDeploySpec, CephService
 if TYPE_CHECKING:
     from ..module import CephadmOrchestrator
@@ -87,11 +88,11 @@ def get_dependencies(
         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_cert: {str(utils.md5_hash(nfs_spec.kmip_cert))}')
+            deps.append(f'kmip_key: {str(utils.md5_hash(nfs_spec.kmip_key))}')
+            deps.append(f'kmip_ca_cert: {str(utils.md5_hash(nfs_spec.kmip_ca_cert))}')
             deps.append(f'kmip_host_list: {nfs_spec.kmip_host_list}')
-        return deps
+        return sorted(deps)
 
     def generate_config(self, daemon_spec: CephadmDaemonDeploySpec) -> Tuple[Dict[str, Any], List[str]]:
         assert self.TYPE == daemon_spec.daemon_type
@@ -218,7 +219,7 @@ def get_cephadm_config() -> Dict[str, Any]:
             logger.debug('Generated cephadm config-json: %s' % config)
             return config
 
-        return get_cephadm_config(), sorted(self.get_dependencies(self.mgr, spec))
+        return get_cephadm_config(), self.get_dependencies(self.mgr, spec)
 
     def create_rados_config_obj(self,
                                 spec: NFSServiceSpec,

src/pybind/mgr/nfs/module.py

@@ -1,6 +1,7 @@
 import logging
 import threading
 from typing import Tuple, Optional, List, Dict, Any
+import yaml
 
 from mgr_module import MgrModule, CLICommand, Option, CLICheckNonemptyFileInput
 import object_format
@@ -153,12 +154,17 @@ def _cmd_nfs_cluster_create(self,
                                 ingress_mode: Optional[IngressType] = None,
                                 port: Optional[int] = None,
                                 enable_virtual_server: bool = False,
-                                kmip_cert: Optional[str] = None,
-                                kmip_key: Optional[str] = None,
-                                kmip_ca_cert: Optional[str] = None,
-                                kmip_host_list: Optional[List[str]] = None,
+                                inbuf: Optional[str] = None
                                 ) -> None:
         """Create an NFS Cluster"""
+        kmip_cert = kmip_key = kmip_ca_cert = kmip_host_list = None
+        if inbuf:
+            kmip_config = yaml.safe_load(inbuf)
+            kmip_cert = kmip_config.get('kmip_cert')
+            kmip_key = kmip_config.get('kmip_key')
+            kmip_ca_cert = kmip_config.get('kmip_ca_cert')
+            kmip_host_list = kmip_config.get('kmip_host_list')
+
         return self.nfs.create_nfs_cluster(cluster_id=cluster_id, placement=placement,
                                            virtual_ip=virtual_ip, ingress=ingress,
                                            ingress_mode=ingress_mode, port=port,

src/pybind/mgr/nfs/tests/test_nfs.py

@@ -14,7 +14,7 @@
 from ceph.utils import with_units_to_int, bytes_to_human
 from nfs import Module
 from nfs.export import ExportMgr, normalize_path
-from nfs.ganesha_conf import GaneshaConfParser, Export
+from nfs.ganesha_conf import GaneshaConfParser, Export, format_block
 from nfs.qos_conf import (
     RawBlock,
     QOS,
@@ -1665,6 +1665,27 @@ def _do_test_export_qos_bw_ops(self, qos_type, clust_bw_params, clust_ops_params
     def test_export_qos_bw_ops(self, qos_type, clust_bw_params, clust_ops_params, export_bw_params, export_ops_params):
         self._do_mock_test(self._do_test_export_qos_bw_ops, qos_type, clust_bw_params, clust_ops_params, export_bw_params, export_ops_params)
 
+    def _do_test_nfs_byok_export(self):
+        nfs_mod = Module('nfs', '', '')
+        conf = ExportMgr(nfs_mod)
+        
+        conf.create_export(
+            fsal_type='cephfs',
+            cluster_id=self.cluster_id,
+            fs_name='myfs',
+            path='/',
+            pseudo_path='/cephfs4',
+            read_only=False,
+            squash='root',
+            kmip_key_id='12345'
+        )
+        export = conf._fetch_export(self.cluster_id, '/cephfs4')
+        block = format_block(export.to_export_block())
+        assert block.startswith('EXPORT {\n    kmip_key_id = "12345";')
+
+    def test_nfs_byok_export(self):
+        self._do_mock_test(self._do_test_nfs_byok_export)
+
 
 @pytest.mark.parametrize(
     "path,expected",