Merge pull request #45598 from rkachach/fix_issue_55008

mgr/cephadm: Adding image tag and date to cephadm startup messages

Reviewed-by: Adam King <adking@redhat.com>
Reviewed-by: John Mulligan <jmulligan@redhat.com>

src/cephadm/cephadm

@@ -113,6 +113,7 @@ cached_stdin = None
 
 class EndPoint:
     """EndPoint representing an ip:port format"""
+
     def __init__(self, ip: str, port: int) -> None:
         self.ip = ip
         self.port = port
@@ -124,6 +125,28 @@ class EndPoint:
         return f'{self.ip}:{self.port}'
 
 
+class ContainerInfo:
+    def __init__(self, container_id: str,
+                 image_name: str,
+                 image_id: str,
+                 start: str,
+                 version: str) -> None:
+        self.container_id = container_id
+        self.image_name = image_name
+        self.image_id = image_id
+        self.start = start
+        self.version = version
+
+    def __eq__(self, other: Any) -> bool:
+        if not isinstance(other, ContainerInfo):
+            return NotImplemented
+        return (self.container_id == other.container_id
+                and self.image_name == other.image_name
+                and self.image_id == other.image_id
+                and self.start == other.start
+                and self.version == other.version)
+
+
 class BaseConfig:
 
     def __init__(self) -> None:
@@ -330,7 +353,7 @@ class UnauthorizedRegistryError(Error):
 
 
 class Ceph(object):
-    daemons = ('mon', 'mgr', 'mds', 'osd', 'rgw', 'rbd-mirror',
+    daemons = ('mon', 'mgr', 'osd', 'mds', 'rgw', 'rbd-mirror',
                'crash', 'cephfs-mirror')
 
 ##################################
@@ -2034,7 +2057,7 @@ def infer_image(func: FuncT) -> FuncT:
         if not ctx.image:
             ctx.image = os.environ.get('CEPHADM_IMAGE')
         if not ctx.image:
-            ctx.image = get_last_local_ceph_image(ctx, ctx.container_engine.path)
+            ctx.image = infer_local_ceph_image(ctx, ctx.container_engine.path)
         if not ctx.image:
             ctx.image = _get_default_image(ctx)
         return func(ctx)
@@ -2066,24 +2089,70 @@ def default_image(func: FuncT) -> FuncT:
     return cast(FuncT, _default_image)
 
 
-def get_last_local_ceph_image(ctx: CephadmContext, container_path: str) -> Optional[str]:
+def get_container_info(ctx: CephadmContext, daemon_filter: str, by_name: bool) -> Optional[ContainerInfo]:
     """
+    :param ctx: Cephadm context
+    :param daemon_filter: daemon name or type
+    :param by_name: must be set to True if daemon name is provided
+    :return: Container information or None
+    """
+    def daemon_name_or_type(daemon: Dict[str, str]) -> str:
+        return daemon['name'] if by_name else daemon['name'].split('.', 1)[0]
+
+    if by_name and '.' not in daemon_filter:
+        logger.warning(f'Trying to get container info using invalid daemon name {daemon_filter}')
+        return None
+    daemons = list_daemons(ctx, detail=False)
+    matching_daemons = [d for d in daemons if daemon_name_or_type(d) == daemon_filter and d['fsid'] == ctx.fsid]
+    if matching_daemons:
+        d_type, d_id = matching_daemons[0]['name'].split('.', 1)
+        out, _, code = get_container_stats(ctx, ctx.container_engine.path, ctx.fsid, d_type, d_id)
+        if not code:
+            (container_id, image_name, image_id, start, version) = out.strip().split(',')
+            return ContainerInfo(container_id, image_name, image_id, start, version)
+    return None
+
+
+def infer_local_ceph_image(ctx: CephadmContext, container_path: str) -> Optional[str]:
+    """
+     Infer the local ceph image based on the following priority criteria:
+       1- the image specified by --image arg (if provided).
+       2- the same image as the daemon container specified by --name arg (if provided).
+       3- image used by any ceph container running on the host. In this case we use daemon types.
+       4- if no container is found then we use the most ceph recent image on the host.
+
+     Note: any selected container must have the same fsid inferred previously.
+
     :return: The most recent local ceph image (already pulled)
     """
+    # '|' special character is used to separate the output fields into:
+    #  - Repository@digest
+    #  - Image Id
+    #  - Image Tag
+    #  - Image creation date
     out, _, _ = call_throws(ctx,
                             [container_path, 'images',
                              '--filter', 'label=ceph=True',
                              '--filter', 'dangling=false',
-                             '--format', '{{.Repository}}@{{.Digest}}'])
+                             '--format', '{{.Repository}}@{{.Digest}}|{{.ID}}|{{.Tag}}|{{.CreatedAt}}'])
-    return _filter_last_local_ceph_image(out)
 
+    container_info = None
+    daemon_name = ctx.name if ('name' in ctx and ctx.name and '.' in ctx.name) else None
+    daemons_ls = [daemon_name] if daemon_name is not None else Ceph.daemons  # daemon types: 'mon', 'mgr', etc
+    for daemon in daemons_ls:
+        container_info = get_container_info(ctx, daemon, daemon_name is not None)
+        if container_info is not None:
+            logger.debug(f"Using container info for daemon '{daemon}'")
+            break
 
-def _filter_last_local_ceph_image(out):
-    # type: (str) -> Optional[str]
     for image in out.splitlines():
-        if image and not image.endswith('@'):
+        if image and not image.isspace():
-            logger.info('Using recent ceph image %s' % image)
+            (digest, image_id, tag, created_date) = image.lstrip().split('|')
-            return image
+            if container_info is not None and image_id not in container_info.image_id:
+                continue
+            if digest and not digest.endswith('@'):
+                logger.info(f"Using ceph image with id '{image_id}' and tag '{tag}' created on {created_date}\n{digest}")
+                return digest
     return None
 
 

src/cephadm/tests/fixtures.py

@@ -98,6 +98,7 @@ def with_cephadm_ctx(
          mock.patch('cephadm.call_timeout', return_value=0), \
          mock.patch('cephadm.find_executable', return_value='foo'), \
          mock.patch('cephadm.is_available', return_value=True), \
+         mock.patch('cephadm.get_container_info', return_value=None), \
          mock.patch('cephadm.json_loads_retry', return_value={'epoch' : 1}), \
          mock.patch('socket.gethostname', return_value=hostname):
         ctx: cd.CephadmContext = cd.cephadm_init_ctx(cmd)

src/cephadm/tests/test_cephadm.py

@@ -348,14 +348,203 @@ class TestCephAdm(object):
         result = cd.dict_get_join({'a': 1}, 'a')
         assert result == 1
 
-    def test_last_local_images(self):
+    @mock.patch('os.listdir', return_value=[])
-        out = '''
+    @mock.patch('cephadm.logger')
-docker.io/ceph/daemon-base@
+    def test_infer_local_ceph_image(self, _logger, _listdir):
-docker.io/ceph/ceph:v15.2.5
+        ctx = cd.CephadmContext()
-docker.io/ceph/daemon-base:octopus
+        ctx.fsid = '00000000-0000-0000-0000-0000deadbeez'
-        '''
+        ctx.container_engine = mock_podman()
-        image = cd._filter_last_local_ceph_image(out)
+
-        assert image == 'docker.io/ceph/ceph:v15.2.5'
+        # make sure the right image is selected when container is found
+        cinfo = cd.ContainerInfo('935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972',
+                                 'registry.hub.docker.com/rkachach/ceph:custom-v0.5',
+                                 '514e6a882f6e74806a5856468489eeff8d7106095557578da96935e4d0ba4d9d',
+                                 '2022-04-19 13:45:20.97146228 +0000 UTC',
+                                 '')
+        out = '''quay.ceph.io/ceph-ci/ceph@sha256:87f200536bb887b36b959e887d5984dd7a3f008a23aa1f283ab55d48b22c6185|dad864ee21e9|master|2022-03-23 16:29:19 +0000 UTC
+        quay.ceph.io/ceph-ci/ceph@sha256:b50b130fcda2a19f8507ddde3435bb4722266956e1858ac395c838bc1dcf1c0e|514e6a882f6e|pacific|2022-03-23 15:58:34 +0000 UTC
+        docker.io/ceph/ceph@sha256:939a46c06b334e094901560c8346de33c00309e3e3968a2db240eb4897c6a508|666bbfa87e8d|v15.2.5|2020-09-16 14:15:15 +0000 UTC'''
+        with mock.patch('cephadm.call_throws', return_value=(out, '', '')):
+            with mock.patch('cephadm.get_container_info', return_value=cinfo):
+                image = cd.infer_local_ceph_image(ctx, ctx.container_engine)
+                assert image == 'quay.ceph.io/ceph-ci/ceph@sha256:b50b130fcda2a19f8507ddde3435bb4722266956e1858ac395c838bc1dcf1c0e'
+
+        # make sure first valid image is used when no container_info is found
+        out = '''quay.ceph.io/ceph-ci/ceph@sha256:87f200536bb887b36b959e887d5984dd7a3f008a23aa1f283ab55d48b22c6185|dad864ee21e9|master|2022-03-23 16:29:19 +0000 UTC
+        quay.ceph.io/ceph-ci/ceph@sha256:b50b130fcda2a19f8507ddde3435bb4722266956e1858ac395c838bc1dcf1c0e|514e6a882f6e|pacific|2022-03-23 15:58:34 +0000 UTC
+        docker.io/ceph/ceph@sha256:939a46c06b334e094901560c8346de33c00309e3e3968a2db240eb4897c6a508|666bbfa87e8d|v15.2.5|2020-09-16 14:15:15 +0000 UTC'''
+        with mock.patch('cephadm.call_throws', return_value=(out, '', '')):
+            with mock.patch('cephadm.get_container_info', return_value=None):
+                image = cd.infer_local_ceph_image(ctx, ctx.container_engine)
+                assert image == 'quay.ceph.io/ceph-ci/ceph@sha256:87f200536bb887b36b959e887d5984dd7a3f008a23aa1f283ab55d48b22c6185'
+
+        # make sure images without digest are discarded (no container_info is found)
+        out = '''quay.ceph.io/ceph-ci/ceph@|||
+        docker.io/ceph/ceph@|||
+        docker.io/ceph/ceph@sha256:939a46c06b334e094901560c8346de33c00309e3e3968a2db240eb4897c6a508|666bbfa87e8d|v15.2.5|2020-09-16 14:15:15 +0000 UTC'''
+        with mock.patch('cephadm.call_throws', return_value=(out, '', '')):
+            with mock.patch('cephadm.get_container_info', return_value=None):
+                image = cd.infer_local_ceph_image(ctx, ctx.container_engine)
+                assert image == 'docker.io/ceph/ceph@sha256:939a46c06b334e094901560c8346de33c00309e3e3968a2db240eb4897c6a508'
+
+
+
+    @pytest.mark.parametrize('daemon_filter, by_name, daemon_list, container_stats, output',
+        [
+            # get container info by type ('mon')
+            (
+                'mon',
+                False,
+                [
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                    {'name': 'mgr.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                ],
+                ("935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972,registry.hub.docker.com/rkachach/ceph:custom-v0.5,666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4,2022-04-19 13:45:20.97146228 +0000 UTC,",
+                 "",
+                 0),
+                cd.ContainerInfo('935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972',
+                                 'registry.hub.docker.com/rkachach/ceph:custom-v0.5',
+                                 '666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4',
+                                 '2022-04-19 13:45:20.97146228 +0000 UTC',
+                                 '')
+            ),
+            # get container info by name ('mon.ceph-node-0')
+            (
+                'mon.ceph-node-0',
+                True,
+                [
+                    {'name': 'mgr.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                ],
+                ("935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972,registry.hub.docker.com/rkachach/ceph:custom-v0.5,666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4,2022-04-19 13:45:20.97146228 +0000 UTC,",
+                 "",
+                 0),
+                cd.ContainerInfo('935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972',
+                                 'registry.hub.docker.com/rkachach/ceph:custom-v0.5',
+                                 '666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4',
+                                 '2022-04-19 13:45:20.97146228 +0000 UTC',
+                                 '')
+            ),
+            # get container info by name (same daemon but two different fsids)
+            (
+                'mon.ceph-node-0',
+                True,
+                [
+                    {'name': 'mon.ceph-node-0', 'fsid': '10000000-0000-0000-0000-0000deadbeef'},
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                ],
+                ("935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972,registry.hub.docker.com/rkachach/ceph:custom-v0.5,666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4,2022-04-19 13:45:20.97146228 +0000 UTC,",
+                 "",
+                 0),
+                cd.ContainerInfo('935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972',
+                                 'registry.hub.docker.com/rkachach/ceph:custom-v0.5',
+                                 '666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4',
+                                 '2022-04-19 13:45:20.97146228 +0000 UTC',
+                                 '')
+            ),
+            # get container info by type (bad container stats: 127 code)
+            (
+                'mon',
+                False,
+                [
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-FFFF-0000-0000-0000deadbeef'},
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                ],
+                ("",
+                 "",
+                 127),
+                None
+            ),
+            # get container info by name (bad container stats: 127 code)
+            (
+                'mon.ceph-node-0',
+                True,
+                [
+                    {'name': 'mgr.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                ],
+                ("",
+                 "",
+                 127),
+                None
+            ),
+            # get container info by invalid name (doens't contain '.')
+            (
+                'mon-ceph-node-0',
+                True,
+                [
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                ],
+                ("935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972,registry.hub.docker.com/rkachach/ceph:custom-v0.5,666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4,2022-04-19 13:45:20.97146228 +0000 UTC,",
+                 "",
+                 0),
+                None
+            ),
+            # get container info by invalid name (empty)
+            (
+                '',
+                True,
+                [
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                ],
+                ("935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972,registry.hub.docker.com/rkachach/ceph:custom-v0.5,666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4,2022-04-19 13:45:20.97146228 +0000 UTC,",
+                 "",
+                 0),
+                None
+            ),
+            # get container info by invalid type (empty)
+            (
+                '',
+                False,
+                [
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-0000-0000-0000-0000deadbeef'},
+                ],
+                ("935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972,registry.hub.docker.com/rkachach/ceph:custom-v0.5,666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4,2022-04-19 13:45:20.97146228 +0000 UTC,",
+                 "",
+                 0),
+                None
+            ),
+            # get container info by name: no match (invalid fsid)
+            (
+                'mon',
+                False,
+                [
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-1111-0000-0000-0000deadbeef'},
+                    {'name': 'mon.ceph-node-0', 'fsid': '00000000-2222-0000-0000-0000deadbeef'},
+                ],
+                ("935b549714b8f007c6a4e29c758689cf9e8e69f2e0f51180506492974b90a972,registry.hub.docker.com/rkachach/ceph:custom-v0.5,666bbfa87e8df05702d6172cae11dd7bc48efb1d94f1b9e492952f19647199a4,2022-04-19 13:45:20.97146228 +0000 UTC,",
+                 "",
+                 0),
+                None
+            ),
+            # get container info by name: no match
+            (
+                'mon.ceph-node-0',
+                True,
+                [],
+                None,
+                None
+            ),
+            # get container info by type: no match
+            (
+                'mgr',
+                False,
+                [],
+                None,
+                None
+            ),
+        ])
+    def test_get_container_info(self, daemon_filter, by_name, daemon_list, container_stats, output):
+        cd.logger = mock.Mock()
+        ctx = cd.CephadmContext()
+        ctx.fsid = '00000000-0000-0000-0000-0000deadbeef'
+        ctx.container_engine = mock_podman()
+        with mock.patch('cephadm.list_daemons', return_value=daemon_list):
+            with mock.patch('cephadm.get_container_stats', return_value=container_stats):
+                assert cd.get_container_info(ctx, daemon_filter, by_name) == output
 
     def test_should_log_to_journald(self):
         ctx = cd.CephadmContext()
@@ -1796,8 +1985,8 @@ class TestPull:
 
     @mock.patch('cephadm.logger')
     @mock.patch('cephadm.get_image_info_from_inspect', return_value={})
-    @mock.patch('cephadm.get_last_local_ceph_image', return_value='last_local_ceph_image')
+    @mock.patch('cephadm.infer_local_ceph_image', return_value='last_local_ceph_image')
-    def test_image(self, get_last_local_ceph_image, get_image_info_from_inspect, logger):
+    def test_image(self, infer_local_ceph_image, get_image_info_from_inspect, logger):
         cmd = ['pull']
         with with_cephadm_ctx(cmd) as ctx:
             retval = cd.command_pull(ctx)