From 327e38703fa8414a730c4c50e2081819b5fbbdc9 Mon Sep 17 00:00:00 2001 From: piglei Date: Thu, 21 Nov 2024 17:00:05 +0800 Subject: [PATCH 1/2] fix: always reset overlay data when the base value was set on the process obj --- .../entities_syncer/proc_env_overlays.py | 52 +++++++++++++++---- .../paasng/platform/bkapp_model/importer.py | 6 +-- .../platform/bkapp_model/test_importer.py | 34 ++++++------ 3 files changed, 60 insertions(+), 32 deletions(-) diff --git a/apiserver/paasng/paasng/platform/bkapp_model/entities_syncer/proc_env_overlays.py b/apiserver/paasng/paasng/platform/bkapp_model/entities_syncer/proc_env_overlays.py index 574335f57f..c602a73a03 100644 --- a/apiserver/paasng/paasng/platform/bkapp_model/entities_syncer/proc_env_overlays.py +++ b/apiserver/paasng/paasng/platform/bkapp_model/entities_syncer/proc_env_overlays.py @@ -20,7 +20,7 @@ from typing import Dict, Iterable, List, Set, Tuple, Union from paasng.platform.bkapp_model import fieldmgr -from paasng.platform.bkapp_model.entities import AutoscalingOverlay, ReplicasOverlay, ResQuotaOverlay +from paasng.platform.bkapp_model.entities import AutoscalingOverlay, Process, ReplicasOverlay, ResQuotaOverlay from paasng.platform.bkapp_model.models import ModuleProcessSpec, ProcessSpecEnvOverlay from paasng.platform.modules.models import Module from paasng.utils.structure import NotSetType @@ -31,27 +31,48 @@ def sync_env_overlays_replicas( - module: Module, overlay_replicas: List[ReplicasOverlay] | NotSetType, manager: fieldmgr.FieldMgrName + module: Module, + overlay_replicas: List[ReplicasOverlay] | NotSetType, + manager: fieldmgr.FieldMgrName, + processes: List[Process] | None = None, ) -> CommonSyncResult: """Sync replicas overlay data to db.""" syncer = OverlayDataSyncer(algo=ReplicasSyncerFieldAlgo()) - return syncer.sync(module, overlay_replicas, manager) + if processes is None: + proc_value_is_set = None + else: + proc_value_is_set = {p.name: not isinstance(p.replicas, NotSetType) for p in processes} + return syncer.sync(module, overlay_replicas, manager, proc_value_is_set) def sync_env_overlays_res_quotas( - module: Module, overlay_res_quotas: List[ResQuotaOverlay] | NotSetType, manager: fieldmgr.FieldMgrName + module: Module, + overlay_res_quotas: List[ResQuotaOverlay] | NotSetType, + manager: fieldmgr.FieldMgrName, + processes: List[Process] | None = None, ) -> CommonSyncResult: """Sync res_quota overlay data to db.""" syncer = OverlayDataSyncer(algo=ResQuotasSyncerFieldAlgo()) - return syncer.sync(module, overlay_res_quotas, manager) + if processes is None: + proc_value_is_set = None + else: + proc_value_is_set = {p.name: not isinstance(p.res_quota_plan, NotSetType) for p in processes} + return syncer.sync(module, overlay_res_quotas, manager, proc_value_is_set) def sync_env_overlays_autoscalings( - module: Module, overlay_autoscalings: List[AutoscalingOverlay] | NotSetType, manager: fieldmgr.FieldMgrName + module: Module, + overlay_autoscalings: List[AutoscalingOverlay] | NotSetType, + manager: fieldmgr.FieldMgrName, + processes: List[Process] | None = None, ) -> CommonSyncResult: """Sync autoscaling overlay data to db model""" syncer = OverlayDataSyncer(algo=AutoscalingSyncerFieldAlgo()) - return syncer.sync(module, overlay_autoscalings, manager) + if processes is None: + proc_value_is_set = None + else: + proc_value_is_set = {p.name: not isinstance(p.autoscaling, NotSetType) for p in processes} + return syncer.sync(module, overlay_autoscalings, manager, proc_value_is_set) def clean_empty_overlays(module): @@ -83,11 +104,17 @@ def sync( module: Module, items: Iterable[Union[ReplicasOverlay, ResQuotaOverlay, AutoscalingOverlay]] | NotSetType, manager: fieldmgr.FieldMgrName, + proc_value_is_set: dict[str, bool] | None, ) -> CommonSyncResult: - """Sync overlay data to the db.""" + """Sync overlay data to the db. + + :param proc_value_is_set: A optional dict to indicate if the base value was + set on the process object. When the `items` is not set and the value of + the process object was set, the overlay data will always be reset. + """ ret = CommonSyncResult() if isinstance(items, NotSetType): - return self._sync_notset(module, manager) + return self._sync_notset(module, manager, proc_value_is_set) # Build the index of existing data first to clean data later. existing_specs, existing_index = self._build_specs_and_index(module) @@ -113,7 +140,9 @@ def sync( ret.deleted_num += 1 return ret - def _sync_notset(self, module: Module, manager: fieldmgr.FieldMgrName) -> CommonSyncResult: + def _sync_notset( + self, module: Module, manager: fieldmgr.FieldMgrName, proc_value_is_set: dict[str, bool] | None + ) -> CommonSyncResult: """Sync when the given data is not set.""" ret = CommonSyncResult() @@ -123,8 +152,9 @@ def _sync_notset(self, module: Module, manager: fieldmgr.FieldMgrName) -> Common # Reset existing data for (proc, env), pk in existing_index.items(): + base_value_is_set = proc_value_is_set and proc_value_is_set.get(proc) # Do not reset the data if the environment is not managed by the current manager. - if (proc, env) in not_managed_envs: + if (proc, env) in not_managed_envs and not base_value_is_set: continue ProcessSpecEnvOverlay.objects.update_or_create(pk=pk, defaults=self.algo.get_empty_values()) diff --git a/apiserver/paasng/paasng/platform/bkapp_model/importer.py b/apiserver/paasng/paasng/platform/bkapp_model/importer.py index 95011b3739..a9e0a6c358 100644 --- a/apiserver/paasng/paasng/platform/bkapp_model/importer.py +++ b/apiserver/paasng/paasng/platform/bkapp_model/importer.py @@ -116,9 +116,9 @@ def import_bkapp_spec_entity(module: Module, spec_entity: v1alpha2_entity.BkAppS sync_observability(module, spec_entity.observability) # NOTE: Must import the processes first to create the ModuleProcessSpec objs - sync_env_overlays_replicas(module, overlay_replicas, manager) - sync_env_overlays_res_quotas(module, overlay_res_quotas, manager) - sync_env_overlays_autoscalings(module, overlay_autoscaling, manager) + sync_env_overlays_replicas(module, overlay_replicas, manager, spec_entity.processes) + sync_env_overlays_res_quotas(module, overlay_res_quotas, manager, spec_entity.processes) + sync_env_overlays_autoscalings(module, overlay_autoscaling, manager, spec_entity.processes) clean_empty_overlays(module) diff --git a/apiserver/paasng/tests/paasng/platform/bkapp_model/test_importer.py b/apiserver/paasng/tests/paasng/platform/bkapp_model/test_importer.py index 11bddb1f52..48f2e5a41d 100644 --- a/apiserver/paasng/tests/paasng/platform/bkapp_model/test_importer.py +++ b/apiserver/paasng/tests/paasng/platform/bkapp_model/test_importer.py @@ -240,26 +240,12 @@ class TestReplicasOverlay: @pytest.fixture() def base_manifest_with_overlay(self, base_manifest): d = copy.deepcopy(base_manifest) - d["spec"]["envOverlay"] = { - "replicas": [ - { - "envName": "stag", - "process": "web", - "count": "3", - } - ] - } + d["spec"]["envOverlay"] = {"replicas": [{"envName": "stag", "process": "web", "count": "3"}]} return d def test_invalid_replicas_input(self, bk_module, base_manifest): base_manifest["spec"]["envOverlay"] = { - "replicas": [ - { - "envName": "stag", - "process": "web", - "count": "not_a_number", - } - ] + "replicas": [{"envName": "stag", "process": "web", "count": "not_a_number"}] } with pytest.raises(ManifestImportError) as e: import_manifest_app_desc(bk_module, base_manifest) @@ -278,13 +264,25 @@ def test_reset_by_not_providing_value(self, bk_module, base_manifest, base_manif proc_spec = ModuleProcessSpec.objects.get(module=bk_module, name="web") assert proc_spec.get_target_replicas("stag") == 1, "The overlay data should be reset" - def test_not_reset_when_manager_different(self, bk_module, base_manifest, base_manifest_with_overlay): + def test_not_reset_when_manager_different(self, bk_module, manifest_no_replicas, base_manifest_with_overlay): import_manifest_app_desc(bk_module, base_manifest_with_overlay) - import_manifest(bk_module, base_manifest, manager=fieldmgr.FieldMgrName.WEB_FORM) + import_manifest(bk_module, manifest_no_replicas, manager=fieldmgr.FieldMgrName.WEB_FORM) proc_spec = ModuleProcessSpec.objects.get(module=bk_module, name="web") assert proc_spec.get_target_replicas("stag") == 3, "The overlay data should remain as it is" + def test_proc_replicas_reset_overlay_managed_by_other(self, bk_module, base_manifest, base_manifest_with_overlay): + """When the replicas field is set on the process object, the overlay should + always be reset even if no overlay data is provided. + """ + import_manifest(bk_module, base_manifest_with_overlay, manager=fieldmgr.FieldMgrName.WEB_FORM) + proc_spec = ModuleProcessSpec.objects.get(module=bk_module, name="web") + assert proc_spec.get_target_replicas("stag") == 3 + + import_manifest_app_desc(bk_module, base_manifest) + proc_spec.refresh_from_db() + assert proc_spec.get_target_replicas("stag") == 1 + class TestAutoscaling: def test_normal(self, bk_module, base_manifest): From 1991f3d8174a36304bbd7832f47d5eff4fa59c01 Mon Sep 17 00:00:00 2001 From: piglei Date: Thu, 21 Nov 2024 17:02:44 +0800 Subject: [PATCH 2/2] doc: fix typo --- .../platform/bkapp_model/entities_syncer/proc_env_overlays.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apiserver/paasng/paasng/platform/bkapp_model/entities_syncer/proc_env_overlays.py b/apiserver/paasng/paasng/platform/bkapp_model/entities_syncer/proc_env_overlays.py index c602a73a03..f790639ffc 100644 --- a/apiserver/paasng/paasng/platform/bkapp_model/entities_syncer/proc_env_overlays.py +++ b/apiserver/paasng/paasng/platform/bkapp_model/entities_syncer/proc_env_overlays.py @@ -108,7 +108,7 @@ def sync( ) -> CommonSyncResult: """Sync overlay data to the db. - :param proc_value_is_set: A optional dict to indicate if the base value was + :param proc_value_is_set: An optional dict to indicate if the base value was set on the process object. When the `items` is not set and the value of the process object was set, the overlay data will always be reset. """