From 3c222a8705bd67e4bff4f03df822770cc2e4893f Mon Sep 17 00:00:00 2001
From: Patrick Seidensal <pseidensal@suse.com>
Date: Fri, 26 Feb 2021 17:27:17 +0100
Subject: [PATCH] cephadm: `cephadm ls` broken for SUSE downstream alertmanager
 container

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

Signed-off-by: Patrick Seidensal <pseidensal@suse.com>
---
 src/cephadm/cephadm               | 49 +++++++++++++++++++------------
 src/cephadm/tests/test_cephadm.py | 36 +++++++++++++++++++++++
 2 files changed, 67 insertions(+), 18 deletions(-)

diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm
index 219908a3291..23ea2c7a618 100755
--- a/src/cephadm/cephadm
+++ b/src/cephadm/cephadm
@@ -281,6 +281,35 @@ class Monitoring(object):
         },
     }  # type: ignore
 
+    @staticmethod
+    def get_version(ctx, container_id, daemon_type):
+        # type: (CephadmContext, str, str) -> str
+        """
+        :param: daemon_type Either "prometheus", "alertmanager" or "node-exporter"
+        """
+        assert daemon_type in ('prometheus', 'alertmanager', 'node-exporter')
+        cmd = daemon_type.replace('-', '_')
+        code = -1
+        err = ''
+        version = ''
+        if daemon_type == 'alertmanager':
+            for cmd in ['alertmanager', 'prometheus-alertmanager']:
+                _, err, code = call(ctx, [
+                        ctx.container_path, 'exec', container_id, cmd,
+                        '--version'
+                    ], verbosity=CallVerbosity.SILENT)
+                if code == 0:
+                    break
+            cmd = 'alertmanager'  # reset cmd for version extraction
+        else:
+            _, err, code = call(ctx, [
+                ctx.container_path, 'exec', container_id, cmd, '--version'
+            ])
+        if code == 0 and \
+            err.startswith('%s, version ' % cmd):
+            version = err.split(' ')[2]
+        return version
+
 ##################################
 def populate_files(config_dir, config_files, uid, gid):
     # type: (str, Dict, int, int) -> None
@@ -4562,24 +4591,8 @@ def list_daemons(ctx, detail=True, legacy_dir=None):
                                 elif daemon_type in ['prometheus',
                                                      'alertmanager',
                                                      'node-exporter']:
-                                    cmd = daemon_type.replace('-', '_')
-                                    if daemon_type == 'alertmanager':
-                                        for cmd in ['alertmanager', 'prometheus-alertmanager']:
-                                            out, err, code = call(ctx, [
-                                                    container_path, 'exec', container_id, cmd,
-                                                    '--version'
-                                                ], verbosity=CallVerbosity.SILENT)
-                                            if code == 0:
-                                                break
-                                        cmd = 'alertmanager'  # reset cmd for version extraction
-                                    else:
-                                        out, err, code = call(ctx, [
-                                            container_path, 'exec', container_id, cmd, '--version'
-                                        ])
-                                    if not code and \
-                                       err.startswith('%s, version ' % cmd):
-                                        version = err.split(' ')[2]
-                                        seen_versions[image_id] = version
+                                    version = Monitoring.get_version(ctx, container_id, daemon_type)
+                                    seen_versions[image_id] = version
                                 elif daemon_type == 'haproxy':
                                     out, err, code = call(ctx,
                                         [container_path, 'exec', container_id,
diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py
index 349ead59f89..fe2be2c5981 100644
--- a/src/cephadm/tests/test_cephadm.py
+++ b/src/cephadm/tests/test_cephadm.py
@@ -807,3 +807,39 @@ class TestMaintenance:
     def test_parser_BAD(self):
         with pytest.raises(SystemExit):
             cd._parse_args(['host-maintenance', 'wah'])
+
+
+class TestMonitoring(object):
+    @mock.patch('cephadm.call')
+    def test_get_version_alertmanager(self, _call):
+        ctx = mock.Mock()
+        daemon_type = 'alertmanager'
+
+        # binary `prometheus`
+        _call.return_value = '', '{}, version 0.16.1'.format(daemon_type), 0
+        version = cd.Monitoring.get_version(ctx, 'container_id', daemon_type)
+        assert version == '0.16.1'
+
+        # binary `prometheus-alertmanager`
+        _call.side_effect = (
+            ('', '', 1),
+            ('', '{}, version 0.16.1'.format(daemon_type), 0),
+        )
+        version = cd.Monitoring.get_version(ctx, 'container_id', daemon_type)
+        assert version == '0.16.1'
+
+    @mock.patch('cephadm.call')
+    def test_get_version_prometheus(self, _call):
+        ctx = mock.Mock()
+        daemon_type = 'prometheus'
+        _call.return_value = '', '{}, version 0.16.1'.format(daemon_type), 0
+        version = cd.Monitoring.get_version(ctx, 'container_id', daemon_type)
+        assert version == '0.16.1'
+
+    @mock.patch('cephadm.call')
+    def test_get_version_node_exporter(self, _call):
+        ctx = mock.Mock()
+        daemon_type = 'node-exporter'
+        _call.return_value = '', '{}, version 0.16.1'.format(daemon_type.replace('-', '_')), 0
+        version = cd.Monitoring.get_version(ctx, 'container_id', daemon_type)
+        assert version == '0.16.1'
-- 
2.39.5