From c393ae46e39ca56ee6d68b50295b0ad8ef227184 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 (cherry picked from commit 925e09a46437bf9a8de0d7b853cdc3126ffe4260) Conflicts: src/python-common/ceph/deployment/service_spec.py --- .../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 cf207a3dcc0..ffa7ea66ece 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] @@ -937,8 +947,8 @@ class NFSServiceSpec(ServiceSpec): networks: Optional[List[str]] = None, port: Optional[int] = None, virtual_ip: 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 == 'nfs' @@ -1008,8 +1018,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, @@ -1102,8 +1112,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' @@ -1180,8 +1190,8 @@ class IngressSpec(ServiceSpec): unmanaged: bool = False, ssl: bool = False, keepalive_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 == 'ingress' @@ -1257,11 +1267,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, @@ -1324,8 +1335,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', @@ -1372,8 +1383,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' @@ -1428,8 +1439,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' @@ -1472,8 +1483,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' @@ -1546,8 +1557,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' @@ -1669,8 +1680,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' @@ -1703,7 +1714,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, ): @@ -1863,7 +1874,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