From 3b92bc18317b41e0cc699c88f02cefb77ffd2161 Mon Sep 17 00:00:00 2001 From: Shawn Walton Date: Mon, 15 Apr 2024 15:43:33 -0700 Subject: [PATCH 1/5] Add autotune min/max support to service_shard_update --- paasta_tools/contrib/service_shard_update.py | 71 +++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/paasta_tools/contrib/service_shard_update.py b/paasta_tools/contrib/service_shard_update.py index e0491e0c1f..051b948187 100644 --- a/paasta_tools/contrib/service_shard_update.py +++ b/paasta_tools/contrib/service_shard_update.py @@ -39,6 +39,13 @@ def parse_args(): action="store_true", dest="verbose", ) + parser.add_argument( + "-d", + "--dry-run", + help="Do not commit changes to git", + action="store_true", + dest="dry_run", + ) parser.add_argument( "--source-id", help="String to attribute the changes in the commit message.", @@ -115,6 +122,48 @@ def parse_args(): type=int, dest="timeout_server_ms", ) + parser.add_argument( + "--autotune-min-cpus", + help="Minimum number of CPUs Autotune should give the shard", + required=False, + type=float, + dest="autotune_min_cpus", + ) + parser.add_argument( + "--autotune-max-cpus", + help="Maximum number of CPUs Autotune should give the shard", + required=False, + type=float, + dest="autotune_max_cpus", + ) + parser.add_argument( + "--autotune-min-mem", + help="Minimum amount of memory Autotune should give the shard", + required=False, + type=int, + dest="autotune_min_mem", + ) + parser.add_argument( + "--autotune-max-mem", + help="Maximum amount of memory Autotune should give the shard", + required=False, + type=int, + dest="autotune_max_mem", + ) + parser.add_argument( + "--autotune-min-disk", + help="Minimum amount of disk Autotune should give the shard", + required=False, + type=int, + dest="autotune_min_disk", + ) + parser.add_argument( + "--autotune-max-disk", + help="Maximum amount of disk Autotune should give the shard", + required=False, + type=int, + dest="autotune_max_disk", + ) return parser.parse_args() @@ -214,6 +263,26 @@ def main(args): instance_config["cpus"] = args.cpus if args.mem is not None: instance_config["mem"] = args.mem + if any((args.autotune_min_cpus, args.autotune_max_cpus, args.autotune_min_mem, args.autotune_max_mem, args.autotune_min_disk, args.autotune_max_disk)): + instance_config["autotune"] = {} + if args.autotune_min_cpus is not None or args.autotune_max_cpus is not None: + instance_config["autotune"]["cpus"] = {} + if args.autotune_min_cpus is not None: + instance_config["autotune"]["cpus"]["min"] = args.autotune_min_cpus + if args.autotune_max_cpus is not None: + instance_config["autotune"]["cpus"]["max"] = args.autotune_max_cpus + if args.autotune_min_mem is not None or args.autotune_max_mem is not None: + instance_config["autotune"]["mem"] = {} + if args.autotune_min_mem is not None: + instance_config["autotune"]["mem"]["min"] = args.autotune_min_mem + if args.autotune_max_mem is not None: + instance_config["autotune"]["mem"]["max"] = args.autotune_max_mem + if args.autotune_min_disk is not None or args.autotune_max_disk is not None: + instance_config["autotune"]["disk"] = {} + if args.autotune_min_disk is not None: + instance_config["autotune"]["disk"]["min"] = args.autotune_min_disk + if args.autotune_max_disk is not None: + instance_config["autotune"]["disk"]["max"] = args.autotune_max_disk # If the service config does not contain definitions for the shard in each ecosystem # Add the missing definition and write to the corresponding config if args.shard_name not in config_file.keys(): @@ -244,7 +313,7 @@ def main(args): log.info(f"{args.shard_name} is in smartstack config already, skipping.") # Only commit to remote if changes were made - if changes_made: + if changes_made and not args.dry_run: updater.commit_to_remote() trigger_deploys(args.service) else: From 0a0e67cc35ccbc6ede5bfa3532a626c6e0845aa2 Mon Sep 17 00:00:00 2001 From: Kedar Vaidya Date: Mon, 15 Apr 2024 10:38:26 -0700 Subject: [PATCH 2/5] optional min/max instance values instead of defaults --- paasta_tools/contrib/service_shard_update.py | 25 ++++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/paasta_tools/contrib/service_shard_update.py b/paasta_tools/contrib/service_shard_update.py index 051b948187..e1eefced29 100644 --- a/paasta_tools/contrib/service_shard_update.py +++ b/paasta_tools/contrib/service_shard_update.py @@ -63,14 +63,12 @@ def parse_args(): "--min-instance-count", help="If a deploy group is added, the min_instance count to create it with", required=False, - default=1, dest="min_instance_count", ) parser.add_argument( "--prod-max-instance-count", help="If a deploy group is added, the prod max_instance count to create it with", required=False, - default=100, type=int, dest="prod_max_instance_count", ) @@ -78,7 +76,6 @@ def parse_args(): "--non-prod-max-instance-count", help="If a deploy group is added, the non-prod max_instance count to create it with", required=False, - default=5, type=int, dest="non_prod_max_instance_count", ) @@ -243,14 +240,28 @@ def main(args): instance_config = { "deploy_group": f"{deploy_prefix}.{args.shard_name}", - "min_instances": args.min_instance_count, - "max_instances": args.prod_max_instance_count - if deploy_prefix == "prod" - else args.non_prod_max_instance_count, "env": { "PAASTA_SECRET_BUGSNAG_API_KEY": "SECRET(bugsnag_api_key)", }, } + + if args.min_instance_count is not None: + instance_config["min_instances"] = args.min_instance_count + + if ( + args.prod_max_instance_count is not None + and deploy_prefix == "prod" + ): + instance_config["max_instances"] = args.prod_max_instance_count + + if ( + args.non_prod_max_instance_count is not None + and deploy_prefix != "prod" + ): + instance_config[ + "max_instances" + ] = args.non_prod_max_instance_count + if args.metrics_provider is not None or args.setpoint is not None: instance_config["autoscaling"] = {} if args.metrics_provider is not None: From f7712481f51f05265fcd81c6f864c5ba91284418 Mon Sep 17 00:00:00 2001 From: Shawn Walton Date: Mon, 15 Apr 2024 15:58:31 -0700 Subject: [PATCH 3/5] whoops pre-commit didn't install --- paasta_tools/contrib/service_shard_update.py | 50 ++++++++++++++++---- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/paasta_tools/contrib/service_shard_update.py b/paasta_tools/contrib/service_shard_update.py index e1eefced29..b8c43e096d 100644 --- a/paasta_tools/contrib/service_shard_update.py +++ b/paasta_tools/contrib/service_shard_update.py @@ -274,26 +274,56 @@ def main(args): instance_config["cpus"] = args.cpus if args.mem is not None: instance_config["mem"] = args.mem - if any((args.autotune_min_cpus, args.autotune_max_cpus, args.autotune_min_mem, args.autotune_max_mem, args.autotune_min_disk, args.autotune_max_disk)): + if any( + ( + args.autotune_min_cpus, + args.autotune_max_cpus, + args.autotune_min_mem, + args.autotune_max_mem, + args.autotune_min_disk, + args.autotune_max_disk, + ) + ): instance_config["autotune"] = {} - if args.autotune_min_cpus is not None or args.autotune_max_cpus is not None: + if ( + args.autotune_min_cpus is not None + or args.autotune_max_cpus is not None + ): instance_config["autotune"]["cpus"] = {} if args.autotune_min_cpus is not None: - instance_config["autotune"]["cpus"]["min"] = args.autotune_min_cpus + instance_config["autotune"]["cpus"][ + "min" + ] = args.autotune_min_cpus if args.autotune_max_cpus is not None: - instance_config["autotune"]["cpus"]["max"] = args.autotune_max_cpus - if args.autotune_min_mem is not None or args.autotune_max_mem is not None: + instance_config["autotune"]["cpus"][ + "max" + ] = args.autotune_max_cpus + if ( + args.autotune_min_mem is not None + or args.autotune_max_mem is not None + ): instance_config["autotune"]["mem"] = {} if args.autotune_min_mem is not None: - instance_config["autotune"]["mem"]["min"] = args.autotune_min_mem + instance_config["autotune"]["mem"][ + "min" + ] = args.autotune_min_mem if args.autotune_max_mem is not None: - instance_config["autotune"]["mem"]["max"] = args.autotune_max_mem - if args.autotune_min_disk is not None or args.autotune_max_disk is not None: + instance_config["autotune"]["mem"][ + "max" + ] = args.autotune_max_mem + if ( + args.autotune_min_disk is not None + or args.autotune_max_disk is not None + ): instance_config["autotune"]["disk"] = {} if args.autotune_min_disk is not None: - instance_config["autotune"]["disk"]["min"] = args.autotune_min_disk + instance_config["autotune"]["disk"][ + "min" + ] = args.autotune_min_disk if args.autotune_max_disk is not None: - instance_config["autotune"]["disk"]["max"] = args.autotune_max_disk + instance_config["autotune"]["disk"][ + "max" + ] = args.autotune_max_disk # If the service config does not contain definitions for the shard in each ecosystem # Add the missing definition and write to the corresponding config if args.shard_name not in config_file.keys(): From 08b222aa568dfd356c849fde0263d5b7b6fcdf78 Mon Sep 17 00:00:00 2001 From: Shawn Walton Date: Fri, 11 Oct 2024 13:46:09 -0700 Subject: [PATCH 4/5] revert changes to min/max instances, clean up autotune limits code --- paasta_tools/contrib/service_shard_update.py | 91 +++++++------------- 1 file changed, 33 insertions(+), 58 deletions(-) diff --git a/paasta_tools/contrib/service_shard_update.py b/paasta_tools/contrib/service_shard_update.py index 72d713e2f4..4f07063fd6 100644 --- a/paasta_tools/contrib/service_shard_update.py +++ b/paasta_tools/contrib/service_shard_update.py @@ -63,12 +63,14 @@ def parse_args(): "--min-instance-count", help="If a deploy group is added, the min_instance count to create it with", required=False, + default=1, dest="min_instance_count", ) parser.add_argument( "--prod-max-instance-count", help="If a deploy group is added, the prod max_instance count to create it with", required=False, + default=100, type=int, dest="prod_max_instance_count", ) @@ -76,6 +78,7 @@ def parse_args(): "--non-prod-max-instance-count", help="If a deploy group is added, the non-prod max_instance count to create it with", required=False, + default=5, type=int, dest="non_prod_max_instance_count", ) @@ -240,28 +243,16 @@ def main(args): instance_config = { "deploy_group": f"{deploy_prefix}.{args.shard_name}", + "min_instances": args.min_instance_count, + "max_instances": ( + args.prod_max_instance_count + if deploy_prefix == "prod" + else args.non_prod_max_instance_count + ), "env": { "PAASTA_SECRET_BUGSNAG_API_KEY": "SECRET(bugsnag_api_key)", }, } - - if args.min_instance_count is not None: - instance_config["min_instances"] = args.min_instance_count - - if ( - args.prod_max_instance_count is not None - and deploy_prefix == "prod" - ): - instance_config["max_instances"] = args.prod_max_instance_count - - if ( - args.non_prod_max_instance_count is not None - and deploy_prefix != "prod" - ): - instance_config[ - "max_instances" - ] = args.non_prod_max_instance_count - if args.metrics_provider is not None or args.setpoint is not None: instance_config["autoscaling"] = {"metrics_providers": []} metrics_provider_config = {} @@ -287,46 +278,30 @@ def main(args): args.autotune_max_disk, ) ): - instance_config["autotune"] = {} - if ( - args.autotune_min_cpus is not None - or args.autotune_max_cpus is not None - ): - instance_config["autotune"]["cpus"] = {} - if args.autotune_min_cpus is not None: - instance_config["autotune"]["cpus"][ - "min" - ] = args.autotune_min_cpus - if args.autotune_max_cpus is not None: - instance_config["autotune"]["cpus"][ - "max" - ] = args.autotune_max_cpus - if ( - args.autotune_min_mem is not None - or args.autotune_max_mem is not None - ): - instance_config["autotune"]["mem"] = {} - if args.autotune_min_mem is not None: - instance_config["autotune"]["mem"][ - "min" - ] = args.autotune_min_mem - if args.autotune_max_mem is not None: - instance_config["autotune"]["mem"][ - "max" - ] = args.autotune_max_mem - if ( - args.autotune_min_disk is not None - or args.autotune_max_disk is not None - ): - instance_config["autotune"]["disk"] = {} - if args.autotune_min_disk is not None: - instance_config["autotune"]["disk"][ - "min" - ] = args.autotune_min_disk - if args.autotune_max_disk is not None: - instance_config["autotune"]["disk"][ - "max" - ] = args.autotune_max_disk + limit_config = {} + limit_config["cpus"] = { + "min": args.autotune_min_cpus, + "max": args.autotune_max_cpus, + } + limit_config["mem"] = { + "min": args.autotune_min_mem, + "max": args.autotune_max_mem, + } + limit_config["disk"] = { + "min": args.autotune_min_disk, + "max": args.autotune_max_disk, + } + + # remove any None values to keep the config clean + for resource in limit_config: + for key in limit_config[resource]: + if limit_config[resource][key] is None: + del limit_config[resource][key] + if len(limit_config[resource]) == 0: + del limit_config[resource] + + if len(limit_config) > 0: + instance_config["autotune_limits"] = limit_config # If the service config does not contain definitions for the shard in each ecosystem # Add the missing definition and write to the corresponding config if args.shard_name not in config_file.keys(): From ef3161758e7a60138eaf9d42d6ee4fc5feddbbeb Mon Sep 17 00:00:00 2001 From: Shawn Walton Date: Fri, 11 Oct 2024 13:57:18 -0700 Subject: [PATCH 5/5] fix iteration --- paasta_tools/contrib/service_shard_update.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/paasta_tools/contrib/service_shard_update.py b/paasta_tools/contrib/service_shard_update.py index 4f07063fd6..a48031c68d 100644 --- a/paasta_tools/contrib/service_shard_update.py +++ b/paasta_tools/contrib/service_shard_update.py @@ -293,8 +293,8 @@ def main(args): } # remove any None values to keep the config clean - for resource in limit_config: - for key in limit_config[resource]: + for resource in list(limit_config): + for key in list(limit_config[resource]): if limit_config[resource][key] is None: del limit_config[resource][key] if len(limit_config[resource]) == 0: