From c26ef5ac11ef5192b8399c0131f3c225422d620e Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Tue, 21 May 2024 05:08:29 -0400 Subject: [PATCH] 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 (cherry picked from commit f4ddb2750eb04d7563356a41c7433a3c5d6ded57) --- src/cephadm/cephadm.py | 85 +--------------------------- src/cephadm/cephadmlib/data_utils.py | 82 ++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 84 deletions(-) diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index 0a8292c5f00bc..7dbceb1c89158 100755 --- 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: - # 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 9493a37d00f26..2f4674752cc17 100644 --- 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: + # 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 -- 2.39.5