From 925e09a46437bf9a8de0d7b853cdc3126ffe4260 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 17 May 2023 13:50:19 -0400 Subject: [PATCH] python-common: replace types for extra_*_args with ArgSpec based types Use the ArgSpec, ArgumentList, and GeneralArgList types throughout the deployment module to support extra_container_args and extra_entrypoint_args. The GeneralArgList type supports all possible input forms while ArgumentList reflects the internal representation of the argument lists. Signed-off-by: John Mulligan --- .../ceph/deployment/drive_group.py | 5 +- .../ceph/deployment/service_spec.py | 71 +++++++++++-------- .../ceph/tests/test_service_spec.py | 5 +- 3 files changed, 47 insertions(+), 34 deletions(-) diff --git a/src/python-common/ceph/deployment/drive_group.py b/src/python-common/ceph/deployment/drive_group.py index 5ab38b2108b..cf24fc0efa7 100644 --- a/src/python-common/ceph/deployment/drive_group.py +++ b/src/python-common/ceph/deployment/drive_group.py @@ -4,6 +4,7 @@ import yaml from ceph.deployment.inventory import Device from ceph.deployment.service_spec import ( CustomConfig, + GeneralArgList, PlacementSpec, ServiceSpec, ) @@ -194,8 +195,8 @@ class DriveGroupSpec(ServiceSpec): unmanaged=False, # type: bool filter_logic='AND', # type: str preview_only=False, # type: bool - extra_container_args=None, # type: Optional[List[str]] - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, data_allocate_fraction=None, # type: Optional[float] method=None, # type: Optional[OSDMethod] config=None, # type: Optional[Dict[str, str]] diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index 1211fd8ba4a..3c9e0483b04 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -686,8 +686,8 @@ class ServiceSpec(object): unmanaged: bool = False, preview_only: bool = False, networks: Optional[List[str]] = None, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): @@ -727,8 +727,14 @@ class ServiceSpec(object): if config: self.config = {k.replace(' ', '_'): v for k, v in config.items()} - self.extra_container_args: Optional[List[str]] = extra_container_args - self.extra_entrypoint_args: Optional[List[str]] = extra_entrypoint_args + self.extra_container_args: Optional[ArgumentList] = None + self.extra_entrypoint_args: Optional[ArgumentList] = None + if extra_container_args: + self.extra_container_args = ArgumentSpec.from_general_args( + extra_container_args) + if extra_entrypoint_args: + self.extra_entrypoint_args = ArgumentSpec.from_general_args( + extra_entrypoint_args) self.custom_configs: Optional[List[CustomConfig]] = custom_configs @classmethod @@ -856,9 +862,13 @@ class ServiceSpec(object): if self.networks: ret['networks'] = self.networks if self.extra_container_args: - ret['extra_container_args'] = self.extra_container_args + ret['extra_container_args'] = ArgumentSpec.map_json( + self.extra_container_args + ) if self.extra_entrypoint_args: - ret['extra_entrypoint_args'] = self.extra_entrypoint_args + ret['extra_entrypoint_args'] = ArgumentSpec.map_json( + self.extra_entrypoint_args + ) if self.custom_configs: ret['custom_configs'] = [c.to_json() for c in self.custom_configs] @@ -938,8 +948,8 @@ class NFSServiceSpec(ServiceSpec): port: Optional[int] = None, virtual_ip: Optional[str] = None, enable_haproxy_protocol: bool = False, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): assert service_type == 'nfs' @@ -1010,8 +1020,8 @@ class RGWSpec(ServiceSpec): config: Optional[Dict[str, str]] = None, networks: Optional[List[str]] = None, subcluster: Optional[str] = None, # legacy, only for from_json on upgrade - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, rgw_realm_token: Optional[str] = None, update_endpoints: Optional[bool] = False, @@ -1104,8 +1114,8 @@ class IscsiServiceSpec(ServiceSpec): preview_only: bool = False, config: Optional[Dict[str, str]] = None, networks: Optional[List[str]] = None, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): assert service_type == 'iscsi' @@ -1183,8 +1193,8 @@ class IngressSpec(ServiceSpec): ssl: bool = False, keepalive_only: bool = False, enable_haproxy_protocol: bool = False, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): assert service_type == 'ingress' @@ -1261,11 +1271,12 @@ class CustomContainerSpec(ServiceSpec): preview_only: bool = False, image: Optional[str] = None, entrypoint: Optional[str] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, uid: Optional[int] = None, gid: Optional[int] = None, volume_mounts: Optional[Dict[str, str]] = {}, - args: Optional[List[str]] = [], # args for the container runtime, not entrypoint + # args are for the container runtime, not entrypoint + args: Optional[GeneralArgList] = [], envs: Optional[List[str]] = [], privileged: Optional[bool] = False, bind_mounts: Optional[List[List[str]]] = None, @@ -1328,8 +1339,8 @@ class MonitoringSpec(ServiceSpec): unmanaged: bool = False, preview_only: bool = False, port: Optional[int] = None, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): assert service_type in ['grafana', 'node-exporter', 'prometheus', 'alertmanager', @@ -1376,8 +1387,8 @@ class AlertManagerSpec(MonitoringSpec): networks: Optional[List[str]] = None, port: Optional[int] = None, secure: bool = False, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): assert service_type == 'alertmanager' @@ -1432,8 +1443,8 @@ class GrafanaSpec(MonitoringSpec): protocol: Optional[str] = 'https', initial_admin_password: Optional[str] = None, anonymous_access: Optional[bool] = True, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): assert service_type == 'grafana' @@ -1476,8 +1487,8 @@ class PrometheusSpec(MonitoringSpec): port: Optional[int] = None, retention_time: Optional[str] = None, retention_size: Optional[str] = None, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): assert service_type == 'prometheus' @@ -1550,8 +1561,8 @@ class SNMPGatewaySpec(ServiceSpec): unmanaged: bool = False, preview_only: bool = False, port: Optional[int] = None, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): assert service_type == 'snmp-gateway' @@ -1673,8 +1684,8 @@ class MDSSpec(ServiceSpec): config: Optional[Dict[str, str]] = None, unmanaged: bool = False, preview_only: bool = False, - extra_container_args: Optional[List[str]] = None, - extra_entrypoint_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, + extra_entrypoint_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, ): assert service_type == 'mds' @@ -1707,7 +1718,7 @@ class MONSpec(ServiceSpec): unmanaged: bool = False, preview_only: bool = False, networks: Optional[List[str]] = None, - extra_container_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, custom_configs: Optional[List[CustomConfig]] = None, crush_locations: Optional[Dict[str, List[str]]] = None, ): @@ -1867,7 +1878,7 @@ class CephExporterSpec(ServiceSpec): placement: Optional[PlacementSpec] = None, unmanaged: bool = False, preview_only: bool = False, - extra_container_args: Optional[List[str]] = None, + extra_container_args: Optional[GeneralArgList] = None, ): assert service_type == 'ceph-exporter' diff --git a/src/python-common/ceph/tests/test_service_spec.py b/src/python-common/ceph/tests/test_service_spec.py index c28151ad652..5f8af847768 100644 --- a/src/python-common/ceph/tests/test_service_spec.py +++ b/src/python-common/ceph/tests/test_service_spec.py @@ -8,6 +8,7 @@ import pytest from ceph.deployment.service_spec import ( AlertManagerSpec, + ArgumentSpec, CustomContainerSpec, GrafanaSpec, HostPlacementSpec, @@ -1114,5 +1115,5 @@ def test_extra_args_handling(y, ec_args, ee_args): data = yaml.safe_load(y) spec_obj = ServiceSpec.from_json(data) - assert spec_obj.extra_container_args == ec_args - assert spec_obj.extra_entrypoint_args == ee_args + assert ArgumentSpec.map_json(spec_obj.extra_container_args) == ec_args + assert ArgumentSpec.map_json(spec_obj.extra_entrypoint_args) == ee_args -- 2.39.5