From 0c284f2026d62662c9f6e4a4214725ade22c47dd Mon Sep 17 00:00:00 2001 From: Sina Siadat Date: Tue, 27 Aug 2024 07:36:50 -0700 Subject: [PATCH 1/9] Autoscale VitessCell pods with HPA --- paasta_tools/setup_kubernetes_cr.py | 15 +++ paasta_tools/vitesscell_tools.py | 136 +++++++++++++++++++++++++++- paasta_tools/vitesscluster_tools.py | 2 + 3 files changed, 152 insertions(+), 1 deletion(-) diff --git a/paasta_tools/setup_kubernetes_cr.py b/paasta_tools/setup_kubernetes_cr.py index 6b8da97bb6..44639aa7de 100644 --- a/paasta_tools/setup_kubernetes_cr.py +++ b/paasta_tools/setup_kubernetes_cr.py @@ -50,6 +50,7 @@ from paasta_tools.utils import load_all_configs from paasta_tools.utils import load_system_paasta_config from paasta_tools.vitesscell_tools import load_vitess_cell_instance_configs +from paasta_tools.vitesscell_tools import update_vitess_cell_related_api_objects from paasta_tools.vitesscell_tools import VITESSCELL_KUBERNETES_NAMESPACE from paasta_tools.vitesscluster_tools import load_vitess_cluster_instance_configs from paasta_tools.vitesscluster_tools import VITESSCLUSTER_KUBERNETES_NAMESPACE @@ -67,6 +68,11 @@ } +INSTANCE_TYPE_TO_RELATED_OBJECTS_UPDATER = { + "vitesscell": update_vitess_cell_related_api_objects, +} + + INSTANCE_TYPE_TO_NAMESPACE_LOADER = { "vitesscluster": VITESSCLUSTER_KUBERNETES_NAMESPACE, "vitesscell": VITESSCELL_KUBERNETES_NAMESPACE, @@ -444,6 +450,15 @@ def reconcile_kubernetes_resource( ) else: log.info(f"{desired_resource} is up to date, no action taken") + + if crd.file_prefix in INSTANCE_TYPE_TO_RELATED_OBJECTS_UPDATER: + INSTANCE_TYPE_TO_RELATED_OBJECTS_UPDATER[crd.file_prefix]( + service=service, + instance=inst, + cluster=cluster, + kube_client=kube_client, + soa_dir=DEFAULT_SOA_DIR, + ) except Exception as e: log.error(str(e)) succeeded = False diff --git a/paasta_tools/vitesscell_tools.py b/paasta_tools/vitesscell_tools.py index 2b4e2fc2a2..c30425041d 100644 --- a/paasta_tools/vitesscell_tools.py +++ b/paasta_tools/vitesscell_tools.py @@ -8,9 +8,19 @@ from typing import Union import service_configuration_lib - +from kubernetes.client import V1ObjectMeta +from kubernetes.client import V1OwnerReference +from kubernetes.client import V2beta2CrossVersionObjectReference +from kubernetes.client import V2beta2HorizontalPodAutoscaler +from kubernetes.client import V2beta2HorizontalPodAutoscalerSpec + +from paasta_tools.kubernetes_tools import get_cr +from paasta_tools.kubernetes_tools import KubeClient from paasta_tools.kubernetes_tools import KubernetesDeploymentConfigDict +from paasta_tools.kubernetes_tools import paasta_prefixed from paasta_tools.kubernetes_tools import sanitised_cr_name +from paasta_tools.long_running_service_tools import DEFAULT_AUTOSCALING_SETPOINT +from paasta_tools.long_running_service_tools import METRICS_PROVIDER_CPU from paasta_tools.utils import BranchDictV2 from paasta_tools.utils import deep_merge_dictionaries from paasta_tools.utils import DEFAULT_SOA_DIR @@ -59,6 +69,7 @@ class GatewayConfigDict(TypedDict, total=False): extraFlags: Dict[str, str] extraLabels: Dict[str, str] replicas: int + scale_selector: str resources: Dict[str, Any] annotations: Mapping[str, Any] @@ -115,6 +126,7 @@ def get_cell_config( }, extraLabels=labels, replicas=replicas, + scale_selector=",".join([f"{k}={v}" for k, v in labels.items()]), resources={ "requests": requests, "limits": requests, @@ -175,6 +187,116 @@ def get_global_lock_server(self) -> Dict[str, str]: "rootPath": TOPO_GLOBAL_ROOT, } + def update_related_api_objects(self, kube_client: KubeClient): + vitesscell_cr = get_cr(kube_client, cr_id(self.service, self.instance)) + if not vitesscell_cr: + # Nothing to do, HPAs are deleted when its owner VitessCell is deleted. + return + + uid = vitesscell_cr["metadata"]["uid"] + log.info(f"Reconciling HPA for {self.service} {self.instance}") + self.reconcile_vtgate_hpa( + kube_client, + owner_uid=uid, + ) + + def get_desired_hpa( + self, + kube_client: KubeClient, + owner_uid: str, + min_replicas: Optional[int], + max_replicas: Optional[int], + ) -> Optional[V2beta2HorizontalPodAutoscaler]: + name = sanitised_cr_name(self.service, self.instance) + return V2beta2HorizontalPodAutoscaler( + kind="HorizontalPodAutoscaler", + metadata=V1ObjectMeta( + name=name, + namespace=self.get_namespace(), + annotations=dict(), + labels={ + paasta_prefixed("service"): self.service, + paasta_prefixed("instance"): self.instance, + paasta_prefixed("pool"): self.get_pool(), + paasta_prefixed("managed"): "true", + }, + owner_references=[ + V1OwnerReference( + api_version="planetscale.com/v2", + kind="VitessCell", + name=name, + uid=owner_uid, + controller=True, + block_owner_deletion=True, + ), + ], + ), + spec=V2beta2HorizontalPodAutoscalerSpec( + behavior=self.get_autoscaling_scaling_policy(max_replicas, {}), + max_replicas=max_replicas, + min_replicas=min_replicas, + metrics=[ + self.get_autoscaling_provider_spec( + name=name, + namespace=self.get_namespace(), + provider={ + "type": METRICS_PROVIDER_CPU, + "setpoint": DEFAULT_AUTOSCALING_SETPOINT, + }, + ), + ], + scale_target_ref=V2beta2CrossVersionObjectReference( + api_version="planetscale.com/v2", kind="VitessCell", name=name + ), + ), + ) + + def reconcile_vtgate_hpa( + self, + kube_client: KubeClient, + owner_uid: str, + ): + vtgate_resources = self.config_dict.get("vtgate_resources") + min_instances = vtgate_resources.get("min_instances", 1) + max_instances = vtgate_resources.get("max_instances") + should_exist = min_instances and max_instances + + name = sanitised_cr_name(self.service, self.instance) + exists = ( + len( + kube_client.autoscaling.list_namespaced_horizontal_pod_autoscaler( + field_selector=f"metadata.name={name}", + namespace=self.get_namespace(), + ).items + ) + > 0 + ) + + if should_exist: + hpa = self.get_desired_hpa( + kube_client=kube_client, + owner_uid=owner_uid, + min_replicas=min_instances, + max_replicas=max_instances, + ) + if exists: + kube_client.autoscaling.replace_namespaced_horizontal_pod_autoscaler( + name=name, + namespace=self.get_namespace(), + body=hpa, + ) + else: + log.info(f"Creating HPA for {name} in {self.get_namespace()}") + kube_client.autoscaling.create_namespaced_horizontal_pod_autoscaler( + namespace=self.get_namespace(), + body=hpa, + ) + elif exists: + kube_client.autoscaling.delete_namespaced_horizontal_pod_autoscaler( + name=name, + namespace=self.get_namespace(), + ) + def get_vitess_cell_config(self) -> VitessCellConfigDict: cell = self.config_dict.get("cell") all_cells = self.config_dict.get("cells") @@ -278,6 +400,18 @@ def load_vitess_cell_instance_configs( return vitess_cell_instance_configs +def update_vitess_cell_related_api_objects( + service: str, + instance: str, + cluster: str, + kube_client: KubeClient, + soa_dir: str = DEFAULT_SOA_DIR, +) -> VitessCellConfigDict: + load_vitess_cell_instance_config( + service, instance, cluster, soa_dir=soa_dir + ).update_related_api_objects(kube_client) + + # TODO: read this from CRD in service configs def cr_id(service: str, instance: str) -> Mapping[str, str]: return dict( diff --git a/paasta_tools/vitesscluster_tools.py b/paasta_tools/vitesscluster_tools.py index 0f1e578956..12c9cf4d8e 100644 --- a/paasta_tools/vitesscluster_tools.py +++ b/paasta_tools/vitesscluster_tools.py @@ -73,6 +73,8 @@ class RequestsDict(TypedDict, total=False): class ResourceConfigDict(TypedDict, total=False): replicas: int + min_instances: Optional[int] + max_instances: Optional[int] requests: Dict[str, RequestsDict] limits: Dict[str, RequestsDict] From 54e0489a8bd9ee516e9b1afc0cb2354d6a86cbcc Mon Sep 17 00:00:00 2001 From: Sina Siadat Date: Tue, 27 Aug 2024 07:49:58 -0700 Subject: [PATCH 2/9] Remove unnecessary method --- paasta_tools/vitesscell_tools.py | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/paasta_tools/vitesscell_tools.py b/paasta_tools/vitesscell_tools.py index c30425041d..a3d7f4ed77 100644 --- a/paasta_tools/vitesscell_tools.py +++ b/paasta_tools/vitesscell_tools.py @@ -187,19 +187,6 @@ def get_global_lock_server(self) -> Dict[str, str]: "rootPath": TOPO_GLOBAL_ROOT, } - def update_related_api_objects(self, kube_client: KubeClient): - vitesscell_cr = get_cr(kube_client, cr_id(self.service, self.instance)) - if not vitesscell_cr: - # Nothing to do, HPAs are deleted when its owner VitessCell is deleted. - return - - uid = vitesscell_cr["metadata"]["uid"] - log.info(f"Reconciling HPA for {self.service} {self.instance}") - self.reconcile_vtgate_hpa( - kube_client, - owner_uid=uid, - ) - def get_desired_hpa( self, kube_client: KubeClient, @@ -251,28 +238,35 @@ def get_desired_hpa( ), ) - def reconcile_vtgate_hpa( + def update_related_api_objects( self, kube_client: KubeClient, - owner_uid: str, ): + name = sanitised_cr_name(self.service, self.instance) + vtgate_resources = self.config_dict.get("vtgate_resources") min_instances = vtgate_resources.get("min_instances", 1) max_instances = vtgate_resources.get("max_instances") should_exist = min_instances and max_instances - name = sanitised_cr_name(self.service, self.instance) exists = ( len( kube_client.autoscaling.list_namespaced_horizontal_pod_autoscaler( field_selector=f"metadata.name={name}", namespace=self.get_namespace(), + limit=1, ).items ) > 0 ) if should_exist: + vitesscell_cr = get_cr(kube_client, cr_id(self.service, self.instance)) + if not vitesscell_cr: + # We need the uid of the VitessCell CR to set the owner reference on the HPA. + return + + owner_uid = vitesscell_cr["metadata"]["uid"] hpa = self.get_desired_hpa( kube_client=kube_client, owner_uid=owner_uid, From 74f50c394110b9835fa8e1c7d2b154bdb92666bb Mon Sep 17 00:00:00 2001 From: Sina Siadat Date: Tue, 27 Aug 2024 08:54:56 -0700 Subject: [PATCH 3/9] Fix mypy --- paasta_tools/vitesscell_tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paasta_tools/vitesscell_tools.py b/paasta_tools/vitesscell_tools.py index a3d7f4ed77..63372b769b 100644 --- a/paasta_tools/vitesscell_tools.py +++ b/paasta_tools/vitesscell_tools.py @@ -400,7 +400,7 @@ def update_vitess_cell_related_api_objects( cluster: str, kube_client: KubeClient, soa_dir: str = DEFAULT_SOA_DIR, -) -> VitessCellConfigDict: +) -> None: load_vitess_cell_instance_config( service, instance, cluster, soa_dir=soa_dir ).update_related_api_objects(kube_client) From 93b1efab7f56a28abdee63db5d9baf55dde74629 Mon Sep 17 00:00:00 2001 From: Sina Siadat Date: Tue, 27 Aug 2024 09:38:55 -0700 Subject: [PATCH 4/9] Use existing HPA methods --- paasta_tools/kubernetes_tools.py | 9 ++-- paasta_tools/vitesscell_tools.py | 85 +++++++------------------------- 2 files changed, 24 insertions(+), 70 deletions(-) diff --git a/paasta_tools/kubernetes_tools.py b/paasta_tools/kubernetes_tools.py index 1e99d3fc87..8621e318cb 100644 --- a/paasta_tools/kubernetes_tools.py +++ b/paasta_tools/kubernetes_tools.py @@ -871,6 +871,11 @@ def get_autoscaling_provider_spec( ) return None + def get_autoscaling_target(self, name: str) -> V2beta2CrossVersionObjectReference: + return V2beta2CrossVersionObjectReference( + api_version="apps/v1", kind="Deployment", name=name + ) + def get_autoscaling_metric_spec( self, name: str, @@ -926,9 +931,7 @@ def get_autoscaling_metric_spec( max_replicas=max_replicas, min_replicas=min_replicas, metrics=metrics, - scale_target_ref=V2beta2CrossVersionObjectReference( - api_version="apps/v1", kind="Deployment", name=name - ), + scale_target_ref=self.get_autoscaling_target(name), ), ) diff --git a/paasta_tools/vitesscell_tools.py b/paasta_tools/vitesscell_tools.py index 63372b769b..994e9a1934 100644 --- a/paasta_tools/vitesscell_tools.py +++ b/paasta_tools/vitesscell_tools.py @@ -8,19 +8,11 @@ from typing import Union import service_configuration_lib -from kubernetes.client import V1ObjectMeta -from kubernetes.client import V1OwnerReference from kubernetes.client import V2beta2CrossVersionObjectReference -from kubernetes.client import V2beta2HorizontalPodAutoscaler -from kubernetes.client import V2beta2HorizontalPodAutoscalerSpec -from paasta_tools.kubernetes_tools import get_cr from paasta_tools.kubernetes_tools import KubeClient from paasta_tools.kubernetes_tools import KubernetesDeploymentConfigDict -from paasta_tools.kubernetes_tools import paasta_prefixed from paasta_tools.kubernetes_tools import sanitised_cr_name -from paasta_tools.long_running_service_tools import DEFAULT_AUTOSCALING_SETPOINT -from paasta_tools.long_running_service_tools import METRICS_PROVIDER_CPU from paasta_tools.utils import BranchDictV2 from paasta_tools.utils import deep_merge_dictionaries from paasta_tools.utils import DEFAULT_SOA_DIR @@ -187,57 +179,19 @@ def get_global_lock_server(self) -> Dict[str, str]: "rootPath": TOPO_GLOBAL_ROOT, } - def get_desired_hpa( - self, - kube_client: KubeClient, - owner_uid: str, - min_replicas: Optional[int], - max_replicas: Optional[int], - ) -> Optional[V2beta2HorizontalPodAutoscaler]: - name = sanitised_cr_name(self.service, self.instance) - return V2beta2HorizontalPodAutoscaler( - kind="HorizontalPodAutoscaler", - metadata=V1ObjectMeta( - name=name, - namespace=self.get_namespace(), - annotations=dict(), - labels={ - paasta_prefixed("service"): self.service, - paasta_prefixed("instance"): self.instance, - paasta_prefixed("pool"): self.get_pool(), - paasta_prefixed("managed"): "true", - }, - owner_references=[ - V1OwnerReference( - api_version="planetscale.com/v2", - kind="VitessCell", - name=name, - uid=owner_uid, - controller=True, - block_owner_deletion=True, - ), - ], - ), - spec=V2beta2HorizontalPodAutoscalerSpec( - behavior=self.get_autoscaling_scaling_policy(max_replicas, {}), - max_replicas=max_replicas, - min_replicas=min_replicas, - metrics=[ - self.get_autoscaling_provider_spec( - name=name, - namespace=self.get_namespace(), - provider={ - "type": METRICS_PROVIDER_CPU, - "setpoint": DEFAULT_AUTOSCALING_SETPOINT, - }, - ), - ], - scale_target_ref=V2beta2CrossVersionObjectReference( - api_version="planetscale.com/v2", kind="VitessCell", name=name - ), - ), + def get_autoscaling_target(self, name: str) -> V2beta2CrossVersionObjectReference: + return V2beta2CrossVersionObjectReference( + api_version="planetscale.com/v2", kind="VitessCell", name=name ) + def get_min_instances(self) -> Optional[int]: + vtgate_resources = self.config_dict.get("vtgate_resources") + return vtgate_resources.get("min_instances", 1) + + def get_max_instances(self) -> Optional[int]: + vtgate_resources = self.config_dict.get("vtgate_resources") + return vtgate_resources.get("max_instances") + def update_related_api_objects( self, kube_client: KubeClient, @@ -261,18 +215,15 @@ def update_related_api_objects( ) if should_exist: - vitesscell_cr = get_cr(kube_client, cr_id(self.service, self.instance)) - if not vitesscell_cr: - # We need the uid of the VitessCell CR to set the owner reference on the HPA. - return - - owner_uid = vitesscell_cr["metadata"]["uid"] - hpa = self.get_desired_hpa( + hpa = self.get_autoscaling_metric_spec( + name=name, + cluster=self.get_cluster(), kube_client=kube_client, - owner_uid=owner_uid, - min_replicas=min_instances, - max_replicas=max_instances, + namespace=self.get_namespace(), ) + if not hpa: + return + if exists: kube_client.autoscaling.replace_namespaced_horizontal_pod_autoscaler( name=name, From caba4e5f9c9ee0e2c3de1caeb51d1dea50610221 Mon Sep 17 00:00:00 2001 From: Sina Siadat Date: Thu, 29 Aug 2024 04:17:12 -0700 Subject: [PATCH 5/9] Rename function to something more generic and create alias when importing --- paasta_tools/setup_kubernetes_cr.py | 4 +++- paasta_tools/vitesscell_tools.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/paasta_tools/setup_kubernetes_cr.py b/paasta_tools/setup_kubernetes_cr.py index 44639aa7de..3708d15d54 100644 --- a/paasta_tools/setup_kubernetes_cr.py +++ b/paasta_tools/setup_kubernetes_cr.py @@ -50,7 +50,9 @@ from paasta_tools.utils import load_all_configs from paasta_tools.utils import load_system_paasta_config from paasta_tools.vitesscell_tools import load_vitess_cell_instance_configs -from paasta_tools.vitesscell_tools import update_vitess_cell_related_api_objects +from paasta_tools.vitesscell_tools import ( + update_related_api_objects as update_vitess_cell_related_api_objects, +) from paasta_tools.vitesscell_tools import VITESSCELL_KUBERNETES_NAMESPACE from paasta_tools.vitesscluster_tools import load_vitess_cluster_instance_configs from paasta_tools.vitesscluster_tools import VITESSCLUSTER_KUBERNETES_NAMESPACE diff --git a/paasta_tools/vitesscell_tools.py b/paasta_tools/vitesscell_tools.py index 994e9a1934..2ff07c572a 100644 --- a/paasta_tools/vitesscell_tools.py +++ b/paasta_tools/vitesscell_tools.py @@ -345,7 +345,7 @@ def load_vitess_cell_instance_configs( return vitess_cell_instance_configs -def update_vitess_cell_related_api_objects( +def update_related_api_objects( service: str, instance: str, cluster: str, From d6596d4d3295067d186cc474138fcddd3b068c74 Mon Sep 17 00:00:00 2001 From: Sina Siadat Date: Thu, 29 Aug 2024 04:17:55 -0700 Subject: [PATCH 6/9] Revert "Use existing HPA methods" This reverts commit 93b1efab7f56a28abdee63db5d9baf55dde74629. --- paasta_tools/kubernetes_tools.py | 9 ++-- paasta_tools/vitesscell_tools.py | 85 +++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 24 deletions(-) diff --git a/paasta_tools/kubernetes_tools.py b/paasta_tools/kubernetes_tools.py index 8621e318cb..1e99d3fc87 100644 --- a/paasta_tools/kubernetes_tools.py +++ b/paasta_tools/kubernetes_tools.py @@ -871,11 +871,6 @@ def get_autoscaling_provider_spec( ) return None - def get_autoscaling_target(self, name: str) -> V2beta2CrossVersionObjectReference: - return V2beta2CrossVersionObjectReference( - api_version="apps/v1", kind="Deployment", name=name - ) - def get_autoscaling_metric_spec( self, name: str, @@ -931,7 +926,9 @@ def get_autoscaling_metric_spec( max_replicas=max_replicas, min_replicas=min_replicas, metrics=metrics, - scale_target_ref=self.get_autoscaling_target(name), + scale_target_ref=V2beta2CrossVersionObjectReference( + api_version="apps/v1", kind="Deployment", name=name + ), ), ) diff --git a/paasta_tools/vitesscell_tools.py b/paasta_tools/vitesscell_tools.py index 2ff07c572a..ee4f71ef26 100644 --- a/paasta_tools/vitesscell_tools.py +++ b/paasta_tools/vitesscell_tools.py @@ -8,11 +8,19 @@ from typing import Union import service_configuration_lib +from kubernetes.client import V1ObjectMeta +from kubernetes.client import V1OwnerReference from kubernetes.client import V2beta2CrossVersionObjectReference +from kubernetes.client import V2beta2HorizontalPodAutoscaler +from kubernetes.client import V2beta2HorizontalPodAutoscalerSpec +from paasta_tools.kubernetes_tools import get_cr from paasta_tools.kubernetes_tools import KubeClient from paasta_tools.kubernetes_tools import KubernetesDeploymentConfigDict +from paasta_tools.kubernetes_tools import paasta_prefixed from paasta_tools.kubernetes_tools import sanitised_cr_name +from paasta_tools.long_running_service_tools import DEFAULT_AUTOSCALING_SETPOINT +from paasta_tools.long_running_service_tools import METRICS_PROVIDER_CPU from paasta_tools.utils import BranchDictV2 from paasta_tools.utils import deep_merge_dictionaries from paasta_tools.utils import DEFAULT_SOA_DIR @@ -179,19 +187,57 @@ def get_global_lock_server(self) -> Dict[str, str]: "rootPath": TOPO_GLOBAL_ROOT, } - def get_autoscaling_target(self, name: str) -> V2beta2CrossVersionObjectReference: - return V2beta2CrossVersionObjectReference( - api_version="planetscale.com/v2", kind="VitessCell", name=name + def get_desired_hpa( + self, + kube_client: KubeClient, + owner_uid: str, + min_replicas: Optional[int], + max_replicas: Optional[int], + ) -> Optional[V2beta2HorizontalPodAutoscaler]: + name = sanitised_cr_name(self.service, self.instance) + return V2beta2HorizontalPodAutoscaler( + kind="HorizontalPodAutoscaler", + metadata=V1ObjectMeta( + name=name, + namespace=self.get_namespace(), + annotations=dict(), + labels={ + paasta_prefixed("service"): self.service, + paasta_prefixed("instance"): self.instance, + paasta_prefixed("pool"): self.get_pool(), + paasta_prefixed("managed"): "true", + }, + owner_references=[ + V1OwnerReference( + api_version="planetscale.com/v2", + kind="VitessCell", + name=name, + uid=owner_uid, + controller=True, + block_owner_deletion=True, + ), + ], + ), + spec=V2beta2HorizontalPodAutoscalerSpec( + behavior=self.get_autoscaling_scaling_policy(max_replicas, {}), + max_replicas=max_replicas, + min_replicas=min_replicas, + metrics=[ + self.get_autoscaling_provider_spec( + name=name, + namespace=self.get_namespace(), + provider={ + "type": METRICS_PROVIDER_CPU, + "setpoint": DEFAULT_AUTOSCALING_SETPOINT, + }, + ), + ], + scale_target_ref=V2beta2CrossVersionObjectReference( + api_version="planetscale.com/v2", kind="VitessCell", name=name + ), + ), ) - def get_min_instances(self) -> Optional[int]: - vtgate_resources = self.config_dict.get("vtgate_resources") - return vtgate_resources.get("min_instances", 1) - - def get_max_instances(self) -> Optional[int]: - vtgate_resources = self.config_dict.get("vtgate_resources") - return vtgate_resources.get("max_instances") - def update_related_api_objects( self, kube_client: KubeClient, @@ -215,15 +261,18 @@ def update_related_api_objects( ) if should_exist: - hpa = self.get_autoscaling_metric_spec( - name=name, - cluster=self.get_cluster(), - kube_client=kube_client, - namespace=self.get_namespace(), - ) - if not hpa: + vitesscell_cr = get_cr(kube_client, cr_id(self.service, self.instance)) + if not vitesscell_cr: + # We need the uid of the VitessCell CR to set the owner reference on the HPA. return + owner_uid = vitesscell_cr["metadata"]["uid"] + hpa = self.get_desired_hpa( + kube_client=kube_client, + owner_uid=owner_uid, + min_replicas=min_instances, + max_replicas=max_instances, + ) if exists: kube_client.autoscaling.replace_namespaced_horizontal_pod_autoscaler( name=name, From fe83870016e3981866f4215278baa1d11bec209d Mon Sep 17 00:00:00 2001 From: Sina Siadat Date: Thu, 29 Aug 2024 04:37:57 -0700 Subject: [PATCH 7/9] Address review: don't reuse existing HPA configuration method --- paasta_tools/vitesscell_tools.py | 81 ++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 35 deletions(-) diff --git a/paasta_tools/vitesscell_tools.py b/paasta_tools/vitesscell_tools.py index ee4f71ef26..16a907d90f 100644 --- a/paasta_tools/vitesscell_tools.py +++ b/paasta_tools/vitesscell_tools.py @@ -9,12 +9,10 @@ import service_configuration_lib from kubernetes.client import V1ObjectMeta -from kubernetes.client import V1OwnerReference from kubernetes.client import V2beta2CrossVersionObjectReference from kubernetes.client import V2beta2HorizontalPodAutoscaler from kubernetes.client import V2beta2HorizontalPodAutoscalerSpec -from paasta_tools.kubernetes_tools import get_cr from paasta_tools.kubernetes_tools import KubeClient from paasta_tools.kubernetes_tools import KubernetesDeploymentConfigDict from paasta_tools.kubernetes_tools import paasta_prefixed @@ -187,14 +185,35 @@ def get_global_lock_server(self) -> Dict[str, str]: "rootPath": TOPO_GLOBAL_ROOT, } - def get_desired_hpa( + def get_autoscaling_target(self, name: str) -> V2beta2CrossVersionObjectReference: + return V2beta2CrossVersionObjectReference( + api_version="planetscale.com/v2", kind="VitessCell", name=name + ) + + def get_autoscaling_metric_spec( self, + name: str, + cluster: str, kube_client: KubeClient, - owner_uid: str, - min_replicas: Optional[int], - max_replicas: Optional[int], + namespace: str, ) -> Optional[V2beta2HorizontalPodAutoscaler]: - name = sanitised_cr_name(self.service, self.instance) + # Returns None if an HPA should not be attached based on the config, + # or the config is invalid. + + if self.get_desired_state() == "stop": + return None + + if not self.is_autoscaling_enabled(): + return None + + min_replicas = self.get_min_instances() + max_replicas = self.get_max_instances() + if min_replicas == 0 or max_replicas == 0: + log.error( + f"Invalid value for min or max_instances on {name}: {min_replicas}, {max_replicas}" + ) + return None + return V2beta2HorizontalPodAutoscaler( kind="HorizontalPodAutoscaler", metadata=V1ObjectMeta( @@ -207,21 +226,11 @@ def get_desired_hpa( paasta_prefixed("pool"): self.get_pool(), paasta_prefixed("managed"): "true", }, - owner_references=[ - V1OwnerReference( - api_version="planetscale.com/v2", - kind="VitessCell", - name=name, - uid=owner_uid, - controller=True, - block_owner_deletion=True, - ), - ], ), spec=V2beta2HorizontalPodAutoscalerSpec( behavior=self.get_autoscaling_scaling_policy(max_replicas, {}), - max_replicas=max_replicas, - min_replicas=min_replicas, + max_replicas=self.get_max_instances(), + min_replicas=self.get_min_instances(), metrics=[ self.get_autoscaling_provider_spec( name=name, @@ -232,21 +241,26 @@ def get_desired_hpa( }, ), ], - scale_target_ref=V2beta2CrossVersionObjectReference( - api_version="planetscale.com/v2", kind="VitessCell", name=name - ), + scale_target_ref=self.get_autoscaling_target(name), ), ) + def get_min_instances(self) -> Optional[int]: + vtgate_resources = self.config_dict.get("vtgate_resources") + return vtgate_resources.get("min_instances", 1) + + def get_max_instances(self) -> Optional[int]: + vtgate_resources = self.config_dict.get("vtgate_resources") + return vtgate_resources.get("max_instances") + def update_related_api_objects( self, kube_client: KubeClient, ): name = sanitised_cr_name(self.service, self.instance) - vtgate_resources = self.config_dict.get("vtgate_resources") - min_instances = vtgate_resources.get("min_instances", 1) - max_instances = vtgate_resources.get("max_instances") + min_instances = self.get_min_instances() + max_instances = self.get_max_instances() should_exist = min_instances and max_instances exists = ( @@ -261,18 +275,15 @@ def update_related_api_objects( ) if should_exist: - vitesscell_cr = get_cr(kube_client, cr_id(self.service, self.instance)) - if not vitesscell_cr: - # We need the uid of the VitessCell CR to set the owner reference on the HPA. - return - - owner_uid = vitesscell_cr["metadata"]["uid"] - hpa = self.get_desired_hpa( + hpa = self.get_autoscaling_metric_spec( + name=sanitised_cr_name(self.service, self.instance), + cluster=self.get_cluster(), kube_client=kube_client, - owner_uid=owner_uid, - min_replicas=min_instances, - max_replicas=max_instances, + namespace=self.get_namespace(), ) + if not hpa: + return + if exists: kube_client.autoscaling.replace_namespaced_horizontal_pod_autoscaler( name=name, From 568f4fd98f7729f7725680a8ce96ffd7a57d16ce Mon Sep 17 00:00:00 2001 From: Sina Siadat Date: Thu, 29 Aug 2024 04:38:41 -0700 Subject: [PATCH 8/9] Address review: prefix yelp-specific spec with yelp_ --- paasta_tools/vitesscell_tools.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/paasta_tools/vitesscell_tools.py b/paasta_tools/vitesscell_tools.py index 16a907d90f..c58c0b9aa8 100644 --- a/paasta_tools/vitesscell_tools.py +++ b/paasta_tools/vitesscell_tools.py @@ -67,7 +67,7 @@ class GatewayConfigDict(TypedDict, total=False): extraFlags: Dict[str, str] extraLabels: Dict[str, str] replicas: int - scale_selector: str + yelp_selector: str resources: Dict[str, Any] annotations: Mapping[str, Any] @@ -124,7 +124,7 @@ def get_cell_config( }, extraLabels=labels, replicas=replicas, - scale_selector=",".join([f"{k}={v}" for k, v in labels.items()]), + yelp_selector=",".join([f"{k}={v}" for k, v in labels.items()]), resources={ "requests": requests, "limits": requests, From f1f462d6678c22d2cf1f8ee882bd13fde1af9ac3 Mon Sep 17 00:00:00 2001 From: Sina Siadat Date: Thu, 29 Aug 2024 04:55:38 -0700 Subject: [PATCH 9/9] Make it more like KubernetesDeploymentConfig.get_autoscaling_metric_spec --- paasta_tools/vitesscell_tools.py | 54 +++++++++++++++++--------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/paasta_tools/vitesscell_tools.py b/paasta_tools/vitesscell_tools.py index c58c0b9aa8..80b8f05391 100644 --- a/paasta_tools/vitesscell_tools.py +++ b/paasta_tools/vitesscell_tools.py @@ -17,8 +17,6 @@ from paasta_tools.kubernetes_tools import KubernetesDeploymentConfigDict from paasta_tools.kubernetes_tools import paasta_prefixed from paasta_tools.kubernetes_tools import sanitised_cr_name -from paasta_tools.long_running_service_tools import DEFAULT_AUTOSCALING_SETPOINT -from paasta_tools.long_running_service_tools import METRICS_PROVIDER_CPU from paasta_tools.utils import BranchDictV2 from paasta_tools.utils import deep_merge_dictionaries from paasta_tools.utils import DEFAULT_SOA_DIR @@ -206,6 +204,10 @@ def get_autoscaling_metric_spec( if not self.is_autoscaling_enabled(): return None + autoscaling_params = self.get_autoscaling_params() + if autoscaling_params["metrics_providers"][0]["decision_policy"] == "bespoke": + return None + min_replicas = self.get_min_instances() max_replicas = self.get_max_instances() if min_replicas == 0 or max_replicas == 0: @@ -214,37 +216,39 @@ def get_autoscaling_metric_spec( ) return None - return V2beta2HorizontalPodAutoscaler( + metrics = [] + for provider in autoscaling_params["metrics_providers"]: + spec = self.get_autoscaling_provider_spec(name, namespace, provider) + if spec is not None: + metrics.append(spec) + scaling_policy = self.get_autoscaling_scaling_policy( + max_replicas, + autoscaling_params, + ) + + labels = { + paasta_prefixed("service"): self.service, + paasta_prefixed("instance"): self.instance, + paasta_prefixed("pool"): self.get_pool(), + paasta_prefixed("managed"): "true", + } + + hpa = V2beta2HorizontalPodAutoscaler( kind="HorizontalPodAutoscaler", metadata=V1ObjectMeta( - name=name, - namespace=self.get_namespace(), - annotations=dict(), - labels={ - paasta_prefixed("service"): self.service, - paasta_prefixed("instance"): self.instance, - paasta_prefixed("pool"): self.get_pool(), - paasta_prefixed("managed"): "true", - }, + name=name, namespace=namespace, annotations=dict(), labels=labels ), spec=V2beta2HorizontalPodAutoscalerSpec( - behavior=self.get_autoscaling_scaling_policy(max_replicas, {}), - max_replicas=self.get_max_instances(), - min_replicas=self.get_min_instances(), - metrics=[ - self.get_autoscaling_provider_spec( - name=name, - namespace=self.get_namespace(), - provider={ - "type": METRICS_PROVIDER_CPU, - "setpoint": DEFAULT_AUTOSCALING_SETPOINT, - }, - ), - ], + behavior=scaling_policy, + max_replicas=max_replicas, + min_replicas=min_replicas, + metrics=metrics, scale_target_ref=self.get_autoscaling_target(name), ), ) + return hpa + def get_min_instances(self) -> Optional[int]: vtgate_resources = self.config_dict.get("vtgate_resources") return vtgate_resources.get("min_instances", 1)