From: Volker Theile <vtheile@suse.com>
Date: Fri, 30 Oct 2020 08:22:30 +0000 (+0100)
Subject: mgr/cephadm: Allow customizing mgr/cephadm/lsmcli_blink_lights_cmd per host
X-Git-Tag: v16.1.0~584^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cd79c9912ab35ee6296d613edc7830410a141e05;p=ceph.git

mgr/cephadm: Allow customizing mgr/cephadm/lsmcli_blink_lights_cmd per host

* Rename key name from 'lsmcli_blink_lights_cmd' to 'blink_device_light_cmd'
* Refactor TemplateMgr::render() method to use the Ceph common behavior how to name store/module option keys. The old implementation required a key like 'mgr/cephadm/services_nfs_ganesha.conf' instead of 'mgr/cephadm/services/nfs/ganesha.conf' or 'mgr/cephadm/mgr0_blink_device_light_cmd' instead of 'mgr/cephadm/mgr0/blink_device_light_cmd'.

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

Signed-off-by: Volker Theile <vtheile@suse.com>
---

diff --git a/doc/rados/operations/devices.rst b/doc/rados/operations/devices.rst
index 2dd0917222cf5..d404c3f10e776 100644
--- a/doc/rados/operations/devices.rst
+++ b/doc/rados/operations/devices.rst
@@ -47,6 +47,23 @@ By default, the `identification` light is used.
 
      ceph orch status
 
+The command behind the scene to blink the drive LEDs is `lsmcli`. If you need
+to customize this command you can configure this via a Jinja2 template::
+
+   ceph config-key set mgr/cephadm/blink_device_light_cmd "<template>"
+   ceph config-key set mgr/cephadm/<host>/blink_device_light_cmd "lsmcli local-disk-{{ ident_fault }}-led-{{'on' if on else 'off'}} --path '{{ path or dev }}'"
+
+The Jinja2 template is rendered using the following arguments:
+
+* ``on``
+    A boolean value.
+* ``ident_fault``
+    A string containing `ident` or `fault`.
+* ``dev``
+    A string containing the device ID, e.g. `SanDisk_X400_M.2_2280_512GB_162924424784`.
+* ``path``
+    A string containing the device path, e.g. `/dev/sda`.
+
 .. _enabling-monitoring:
 
 Enabling monitoring
diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index fe10fbb74cbb5..95648bd584148 100644
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -18,7 +18,6 @@ import os
 import random
 import tempfile
 import multiprocessing.pool
-import shutil
 import subprocess
 
 from ceph.deployment import inventory
@@ -1566,30 +1565,30 @@ To check that the host is reachable:
 
         If you must, you can customize this via::
 
-          ceph config-key set mgr/cephadm/lsmcli_blink_lights_cmd '<my jinja2 template>'
+          ceph config-key set mgr/cephadm/blink_device_light_cmd '<my jinja2 template>'
+          ceph config-key set mgr/cephadm/<host>/blink_device_light_cmd '<my jinja2 template>'
 
-        See templates/lsmcli_blink_lights_cmd.j2
+        See templates/blink_device_light_cmd.j2
         """
         @forall_hosts
         def blink(host, dev, path):
-            j2_ctx = {
-                'on': on,
-                'ident_fault': ident_fault,
-                'dev': dev,
-                'path': path
-            }
-
-            lsmcli_blink_lights_cmd = self.template.render('lsmcli_blink_lights_cmd.j2', j2_ctx)
-
-            cmd = shlex.split(lsmcli_blink_lights_cmd)
+            cmd_line = self.template.render('blink_device_light_cmd.j2',
+                                            {
+                                                'on': on,
+                                                'ident_fault': ident_fault,
+                                                'dev': dev,
+                                                'path': path
+                                            },
+                                            host=host)
+            cmd_args = shlex.split(cmd_line)
 
             out, err, code = self._run_cephadm(
-                host, 'osd', 'shell', ['--'] + cmd,
+                host, 'osd', 'shell', ['--'] + cmd_args,
                 error_ok=True)
             if code:
                 raise OrchestratorError(
                     'Unable to affect %s light for %s:%s. Command: %s' % (
-                        ident_fault, host, dev, ' '.join(cmd)))
+                        ident_fault, host, dev, ' '.join(cmd_args)))
             self.log.info('Set %s light for %s:%s %s' % (
                 ident_fault, host, dev, 'on' if on else 'off'))
             return "Set %s light for %s:%s %s" % (
diff --git a/src/pybind/mgr/cephadm/template.py b/src/pybind/mgr/cephadm/template.py
index 1374aba0d7c84..778bc26c0e771 100644
--- a/src/pybind/mgr/cephadm/template.py
+++ b/src/pybind/mgr/cephadm/template.py
@@ -66,7 +66,10 @@ class TemplateMgr:
         }
         self.mgr = mgr
 
-    def render(self, name: str, context: Optional[dict] = None, managed_context=True) -> str:
+    def render(self, name: str,
+               context: Optional[dict] = None,
+               managed_context=True,
+               host: Optional[str] = None) -> str:
         """Render a string from a template with context.
 
         :param name: template name. e.g. services/nfs/ganesha.conf.j2
@@ -77,6 +80,9 @@ class TemplateMgr:
         :param managed_context: to inject default context like managed header or not, defaults
             to True
         :type managed_context: bool, optional
+        :param host: The host name used to build the key to access
+            the module's persistent key-value store.
+        :type host: Optional[str], optional
         :return: the templated string
         :rtype: str
         """
@@ -86,8 +92,17 @@ class TemplateMgr:
         if context is not None:
             ctx = {**ctx, **context}
 
-        store_name = name.replace('/', '_').rstrip('.j2')
+        # Check if the given name exists in the module's persistent
+        # key-value store, e.g.
+        # - blink_device_light_cmd
+        # - <host>/blink_device_light_cmd
+        # - services/nfs/ganesha.conf
+        store_name = name.rstrip('.j2')
         custom_template = self.mgr.get_store(store_name, None)
+        if host and custom_template is None:
+            store_name = '{}/{}'.format(host, store_name)
+            custom_template = self.mgr.get_store(store_name, None)
+
         if custom_template:
             return self.engine.render_plain(custom_template, ctx)
         else:
diff --git a/src/pybind/mgr/cephadm/templates/blink_device_light_cmd.j2 b/src/pybind/mgr/cephadm/templates/blink_device_light_cmd.j2
new file mode 100644
index 0000000000000..dab115833dc32
--- /dev/null
+++ b/src/pybind/mgr/cephadm/templates/blink_device_light_cmd.j2
@@ -0,0 +1 @@
+lsmcli local-disk-{{ ident_fault }}-led-{{'on' if on else 'off'}} --path '{{ path or dev }}'
diff --git a/src/pybind/mgr/cephadm/templates/lsmcli_blink_lights_cmd.j2 b/src/pybind/mgr/cephadm/templates/lsmcli_blink_lights_cmd.j2
deleted file mode 100644
index dab115833dc32..0000000000000
--- a/src/pybind/mgr/cephadm/templates/lsmcli_blink_lights_cmd.j2
+++ /dev/null
@@ -1 +0,0 @@
-lsmcli local-disk-{{ ident_fault }}-led-{{'on' if on else 'off'}} --path '{{ path or dev }}'
diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py
index 52be4ea7d2762..349665ff08d0d 100644
--- a/src/pybind/mgr/cephadm/tests/test_cephadm.py
+++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py
@@ -645,12 +645,26 @@ class TestCephadm(object):
     def test_blink_device_light_custom(self, _run_cephadm, cephadm_module):
         _run_cephadm.return_value = '{}', '', 0
         with with_host(cephadm_module, 'test'):
-            cephadm_module.set_store('lsmcli_blink_lights_cmd', 'echo hello')
-            c = cephadm_module.blink_device_light('ident', True, [('test', '', 'dev')])
+            cephadm_module.set_store('blink_device_light_cmd', 'echo hello')
+            c = cephadm_module.blink_device_light('ident', True, [('test', '', '/dev/sda')])
             assert wait(cephadm_module, c) == ['Set ident light for test: on']
             _run_cephadm.assert_called_with('test', 'osd', 'shell', [
                                             '--', 'echo', 'hello'], error_ok=True)
 
+    @mock.patch("cephadm.module.CephadmOrchestrator._run_cephadm")
+    def test_blink_device_light_custom_per_host(self, _run_cephadm, cephadm_module):
+        _run_cephadm.return_value = '{}', '', 0
+        with with_host(cephadm_module, 'mgr0'):
+            cephadm_module.set_store('mgr0/blink_device_light_cmd',
+                                     'xyz --foo --{{ ident_fault }}={{\'on\' if on else \'off\'}} \'{{ path or dev }}\'')
+            c = cephadm_module.blink_device_light(
+                'fault', True, [('mgr0', 'SanDisk_X400_M.2_2280_512GB_162924424784', '')])
+            assert wait(cephadm_module, c) == [
+                'Set fault light for mgr0:SanDisk_X400_M.2_2280_512GB_162924424784 on']
+            _run_cephadm.assert_called_with('mgr0', 'osd', 'shell', [
+                '--', 'xyz', '--foo', '--fault=on', 'SanDisk_X400_M.2_2280_512GB_162924424784'
+            ], error_ok=True)
+
     @pytest.mark.parametrize(
         "spec, meth",
         [