cephadm: _extract_host_info_from_*() refactor

The current implementation doesn't take into account the format is Yaml

This can lead to issue when oob details are passed to host service spec.

For instance, with the following host spec:

```
---
service_type: host
addr: 1.2.3.4
hostname: node1
oob:
  username: root
  password: passw0rd
  addr: 127.0.0.1
```

it is converted to a list like the following:

```
['service_type: host', 'addr: 1.2.3.4', 'hostname: node1',
 'oob:', 'username: root', 'password: passw0rd', 'addr: 127.0.0.1']
```

It was (probably) assumed that the pattern `addr:` would be present only
once. With the introduction of node-proxy, this isn't true anymore.

Now that the cephadm binary can embed some external libraries we can leverage pyyaml.
The idea is to use proper yaml format instead so it is easier to process the data.

Fixes: https://tracker.ceph.com/issues/66165
Signed-off-by: Guillaume Abrioux <gabrioux@ibm.com>
(cherry picked from commit f4ddb2750eb04d7563356a41c7433a3c5d6ded57)

diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py
index 0a8292c5f00bc7c3828143e1c8999e023d0f9fda..7dbceb1c891585e3a77a776dacdb0b72d18d2d1c 100755 (executable)
--- a/src/cephadm/cephadm.py
+++ b/src/cephadm/cephadm.py
@@ -18,7 +18,7 @@ import tempfile
 import time
 import errno
 import ssl
-from typing import Dict, List, Tuple, Optional, Union, Any, Callable, Sequence, TypeVar, cast, Iterable
+from typing import Dict, List, Tuple, Optional, Union, Any, Callable, Sequence, TypeVar, cast
 
 import re
 import uuid
@@ -96,6 +96,7 @@ from cephadmlib.data_utils import (
     try_convert_datetime,
     read_config,
     with_units_to_int,
+    _extract_host_info_from_applied_spec,
 )
 from cephadmlib.file_utils import (
     get_file_timestamp,
@@ -2563,88 +2564,6 @@ def finish_bootstrap_config(
     pass
 
 
-def _extract_host_info_from_applied_spec(f: Iterable[str]) -> List[Dict[str, str]]:
-    # overall goal of this function is to go through an applied spec and find
-    # the hostname (and addr is provided) for each host spec in the applied spec.
-    # Generally, we should be able to just pass the spec to the mgr module where
-    # proper yaml parsing can happen, but for host specs in particular we want to
-    # be able to distribute ssh keys, which requires finding the hostname (and addr
-    # if possible) for each potential host spec in the applied spec.
-
-    specs: List[List[str]] = []
-    current_spec: List[str] = []
-    for line in f:
-        if re.search(r'^---\s+', line):
-            if current_spec:
-                specs.append(current_spec)
-            current_spec = []
-        else:
-            line = line.strip()
-            if line:
-                current_spec.append(line)
-    if current_spec:
-        specs.append(current_spec)
-
-    host_specs: List[List[str]] = []
-    for spec in specs:
-        for line in spec:
-            if 'service_type' in line:
-                try:
-                    _, type = line.split(':')
-                    type = type.strip()
-                    if type == 'host':
-                        host_specs.append(spec)
-                except ValueError as e:
-                    spec_str = '\n'.join(spec)
-                    logger.error(f'Failed to pull service_type from spec:\n{spec_str}. Got error: {e}')
-                break
-            spec_str = '\n'.join(spec)
-            logger.error(f'Failed to find service_type within spec:\n{spec_str}')
-
-    host_dicts = []
-    for s in host_specs:
-        host_dict = _extract_host_info_from_spec(s)
-        # if host_dict is empty here, we failed to pull the hostname
-        # for the host from the spec. This should have already been logged
-        # so at this point we just don't want to include it in our output
-        if host_dict:
-            host_dicts.append(host_dict)
-
-    return host_dicts
-
-
-def _extract_host_info_from_spec(host_spec: List[str]) -> Dict[str, str]:
-    # note:for our purposes here, we only really want the hostname
-    # and address of the host from each of these specs in order to
-    # be able to distribute ssh keys. We will later apply the spec
-    # through the mgr module where proper yaml parsing can be done
-    # The returned dicts from this function should only contain
-    # one or two entries, one (required) for hostname, one (optional) for addr
-    # {
-    #   hostname: <hostname>
-    #   addr: <ip-addr>
-    # }
-    # if we fail to find the hostname, an empty dict is returned
-
-    host_dict = {}  # type: Dict[str, str]
-    for line in host_spec:
-        for field in ['hostname', 'addr']:
-            if field in line:
-                try:
-                    _, field_value = line.split(':')
-                    field_value = field_value.strip()
-                    host_dict[field] = field_value
-                except ValueError as e:
-                    spec_str = '\n'.join(host_spec)
-                    logger.error(f'Error trying to pull {field} from host spec:\n{spec_str}. Got error: {e}')
-
-    if 'hostname' not in host_dict:
-        spec_str = '\n'.join(host_spec)
-        logger.error(f'Could not find hostname in host spec:\n{spec_str}')
-        return {}
-    return host_dict
-
-
 def _distribute_ssh_keys(ctx: CephadmContext, host_info: Dict[str, str], bootstrap_hostname: str) -> int:
     # copy ssh key to hosts in host spec (used for apply spec)
     ssh_key = CEPH_DEFAULT_PUBKEY

diff --git a/src/cephadm/cephadmlib/data_utils.py b/src/cephadm/cephadmlib/data_utils.py
index 9493a37d00f26ef4897f8e570a7969e29fd37dcb..2f4674752cc17d10ccc8064565a633720722e4d0 100644 (file)
--- a/src/cephadm/cephadmlib/data_utils.py
+++ b/src/cephadm/cephadmlib/data_utils.py
@@ -4,15 +4,20 @@ import datetime
 import os
 import re
 import uuid
+import yaml
+import logging
 
 from configparser import ConfigParser
 
-from typing import Dict, Any, Optional
+from typing import Dict, Any, Optional, Iterable, List
 
 from .constants import DATEFMT, DEFAULT_REGISTRY
 from .exceptions import Error
 
 
+logger = logging.getLogger()
+
+
 def dict_get(
     d: Dict, key: str, default: Any = None, require: bool = False
 ) -> Any:
@@ -197,3 +202,78 @@ def get_legacy_config_fsid(cluster, legacy_dir=None):
         ):
             return config.get('global', 'fsid')
     return None
+
+
+def _extract_host_info_from_applied_spec(
+    f: Iterable[str],
+) -> List[Dict[str, str]]:
+    # overall goal of this function is to go through an applied spec and find
+    # the hostname (and addr is provided) for each host spec in the applied spec.
+    # Generally, we should be able to just pass the spec to the mgr module where
+    # proper yaml parsing can happen, but for host specs in particular we want to
+    # be able to distribute ssh keys, which requires finding the hostname (and addr
+    # if possible) for each potential host spec in the applied spec.
+
+    specs: List[str] = []
+    current_spec: str = ''
+    for line in f:
+        if re.search(r'^---\s+', line):
+            if current_spec:
+                specs.append(current_spec)
+            current_spec = ''
+        else:
+            if line:
+                current_spec += line
+    if current_spec:
+        specs.append(current_spec)
+
+    host_specs: List[Dict[str, Any]] = []
+    for spec in specs:
+        yaml_data = yaml.safe_load(spec)
+        if 'service_type' in yaml_data.keys():
+            if yaml_data['service_type'] == 'host':
+                host_specs.append(yaml_data)
+        else:
+            spec_str = yaml.safe_dump(yaml_data)
+            logger.error(
+                f'Failed to pull service_type from spec:\n{spec_str}.'
+            )
+
+    host_dicts = []
+    for s in host_specs:
+        host_dict = _extract_host_info_from_spec(s)
+        # if host_dict is empty here, we failed to pull the hostname
+        # for the host from the spec. This should have already been logged
+        # so at this point we just don't want to include it in our output
+        if host_dict:
+            host_dicts.append(host_dict)
+
+    return host_dicts
+
+
+def _extract_host_info_from_spec(host_spec: Dict[str, Any]) -> Dict[str, str]:
+    # note:for our purposes here, we only really want the hostname
+    # and address of the host from each of these specs in order to
+    # be able to distribute ssh keys. We will later apply the spec
+    # through the mgr module where proper yaml parsing can be done
+    # The returned dicts from this function should only contain
+    # one or two entries, one (required) for hostname, one (optional) for addr
+    # {
+    #   hostname: <hostname>
+    #   addr: <ip-addr>
+    # }
+    # if we fail to find the hostname, an empty dict is returned
+
+    host_dict = {}  # type: Dict[str, str]
+    for field in ['hostname', 'addr']:
+        try:
+            host_dict[field] = host_spec[field]
+        except KeyError as e:
+            logger.error(
+                f'Error trying to pull {field} from host spec:\n{host_spec}. Got error: {e}'
+            )
+
+    if 'hostname' not in host_dict:
+        logger.error(f'Could not find hostname in host spec:\n{host_spec}')
+        return {}
+    return host_dict