From 7b35c7c8dd7c7ecef478c09f5c2336752e733788 Mon Sep 17 00:00:00 2001 From: whoarethebritons Date: Fri, 20 Oct 2017 10:59:23 -0700 Subject: [PATCH 01/15] different instance types per role --- appscale/tools/node_layout.py | 23 +++++++++---- appscale/tools/remote_helper.py | 59 ++++++++++++++++++++++----------- 2 files changed, 56 insertions(+), 26 deletions(-) diff --git a/appscale/tools/node_layout.py b/appscale/tools/node_layout.py index dd98917e..2427c7a4 100644 --- a/appscale/tools/node_layout.py +++ b/appscale/tools/node_layout.py @@ -88,7 +88,7 @@ class NodeLayout(): # deployment. USED_SIMPLE_AND_ADVANCED_KEYS = "Check your node layout and make sure not " \ "to mix simple and advanced deployment methods." - + def __init__(self, options): """Creates a new NodeLayout from the given YAML file. @@ -127,6 +127,7 @@ def __init__(self, options): self.replication = options.get('replication') self.database_type = options.get('table', 'cassandra') self.add_to_existing = options.get('add_to_existing') + self.default_instance_type = options.get('instance_type') if 'login_host' in options and options['login_host'] is not None: self.login_host = options['login_host'] @@ -336,7 +337,7 @@ def is_valid_simple_format(self): def is_valid_node_format(self): """Checks to see if this NodeLayout represents an acceptable (new) advanced deployment strategy, and if so, constructs self.nodes from it. - + Returns: True if the deployment strategy is valid. Raises: @@ -419,7 +420,7 @@ def is_valid_node_format(self): def is_valid_advanced_format(self): """Checks to see if this NodeLayout represents an acceptable (new) advanced deployment strategy, and if so, constructs self.nodes from it. - + Returns: True if the deployment strategy is valid. Raises: @@ -489,10 +490,17 @@ def is_valid_advanced_format(self): for node, disk in zip(nodes, disks): node.disk = disk + instance_type = node_set.get('instance_type', self.default_instance_type) + + if not instance_type: + self.invalid("Must set a default instance type or specify instance " + "type per role.") + # Add the defined roles to the nodes. for node in nodes: for role in roles: node.add_role(role) + node.instance_type = instance_type for node in nodes: if not node.is_valid(): @@ -585,7 +593,7 @@ def validate_database_replication(self, nodes): def distribute_unassigned_roles(self, nodes, role_count): """ Distributes roles that were not defined by user. - + Args: nodes: The list of nodes. role_count: A dict containing roles mapped to their count. @@ -780,7 +788,7 @@ def from_locations_json_list(self, locations_nodes_list): def invalid(self, message): """ Wrapper that NodeLayout validation aspects call when the given layout is invalid. - + Raises: BadConfigurationException with the given message. """ raise BadConfigurationException(message) @@ -794,7 +802,7 @@ class Node(): DUMMY_INSTANCE_ID = "i-APPSCALE" - def __init__(self, public_ip, cloud, roles=[], disk=None): + def __init__(self, public_ip, cloud, roles=[], disk=None, instance_type=None): """Creates a new Node, representing the given id in the specified cloud. @@ -811,6 +819,7 @@ def __init__(self, public_ip, cloud, roles=[], disk=None): self.cloud = cloud self.roles = roles self.disk = disk + self.instance_type = instance_type self.expand_roles() @@ -866,7 +875,7 @@ def is_role(self, role): else: return False - + def is_valid(self): """Checks to see if this Node's roles can be used together in an AppScale deployment. diff --git a/appscale/tools/remote_helper.py b/appscale/tools/remote_helper.py index f779c88f..857a4080 100644 --- a/appscale/tools/remote_helper.py +++ b/appscale/tools/remote_helper.py @@ -151,31 +151,52 @@ def start_all_nodes(cls, options, node_layout): agent.configure_instance_security(params) - load_balancer_nodes = node_layout.get_nodes('load_balancer', True) - instance_ids, public_ips, private_ips = cls.spawn_load_balancers_in_cloud( - options, agent, params, - len(load_balancer_nodes)) + load_balancer_roles = {} - for node_index, node in enumerate(load_balancer_nodes): - index = node_layout.nodes.index(node) - node_layout.nodes[index].public_ip = public_ips[node_index] - node_layout.nodes[index].private_ip = private_ips[node_index] - node_layout.nodes[index].instance_id = instance_ids[node_index] + instance_type_roles = {'with_disks':{}, 'without_disks': {}} + + for node in node_layout.get_nodes('load_balancer', True): + load_balancer_roles.setdefault(node.instance_type, []).append(node) + + for node in node_layout.get_nodes('load_balancer', False): + instance_type = instance_type_roles['with_disks'] if node.disk else \ + instance_type_roles['without_disks'] + instance_type.setdefault(node.instance_type, []).append(node) + + for instance_type, load_balancer_nodes in load_balancer_roles.items(): + + instance_type_params = params.copy() + instance_type_params['instance_type'] = instance_type + + instance_ids, public_ips, private_ips = cls.spawn_load_balancers_in_cloud( + options, agent, instance_type_params, len(load_balancer_nodes)) + + for node_index, node in enumerate(load_balancer_nodes): + index = node_layout.nodes.index(node) + node_layout.nodes[index].public_ip = public_ips[node_index] + node_layout.nodes[index].private_ip = private_ips[node_index] + node_layout.nodes[index].instance_id = instance_ids[node_index] AppScaleLogger.log("\nPlease wait for AppScale to prepare your machines " "for use. This can take few minutes.") - other_nodes = node_layout.get_nodes('load_balancer', False) - if len(other_nodes) > 0: - _instance_ids, _public_ips, _private_ips = cls.spawn_other_nodes_in_cloud( - agent, params, - len(other_nodes)) + for _, nodes in instance_type_roles.items(): + for instance_type, other_nodes in nodes.items(): + if len(other_nodes) <= 0: + break - for node_index, node in enumerate(other_nodes): - index = node_layout.nodes.index(node) - node_layout.nodes[index].public_ip = _public_ips[node_index] - node_layout.nodes[index].private_ip = _private_ips[node_index] - node_layout.nodes[index].instance_id = _instance_ids[node_index] + instance_type_params = params.copy() + instance_type_params['instance_type'] = instance_type + + _instance_ids, _public_ips, _private_ips =\ + cls.spawn_other_nodes_in_cloud(agent, instance_type_params, + len(other_nodes)) + + for node_index, node in enumerate(other_nodes): + index = node_layout.nodes.index(node) + node_layout.nodes[index].public_ip = _public_ips[node_index] + node_layout.nodes[index].private_ip = _private_ips[node_index] + node_layout.nodes[index].instance_id = _instance_ids[node_index] return node_layout From f137ca5210b9ff9600106cf2bfac14d3380cf7ee Mon Sep 17 00:00:00 2001 From: whoarethebritons Date: Thu, 26 Oct 2017 14:09:44 -0700 Subject: [PATCH 02/15] verify instance type --- appscale/tools/node_layout.py | 14 +++++++++++++- appscale/tools/parse_args.py | 11 +++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/appscale/tools/node_layout.py b/appscale/tools/node_layout.py index 2427c7a4..ed3f41a1 100644 --- a/appscale/tools/node_layout.py +++ b/appscale/tools/node_layout.py @@ -10,6 +10,8 @@ from agents.factory import InfrastructureAgentFactory from appscale_logger import AppScaleLogger from custom_exceptions import BadConfigurationException +from local_state import LocalState +from parse_args import ParseArgs class NodeLayout(): @@ -128,6 +130,8 @@ def __init__(self, options): self.database_type = options.get('table', 'cassandra') self.add_to_existing = options.get('add_to_existing') self.default_instance_type = options.get('instance_type') + self.test = options.get('test') + self.force = options.get('force') if 'login_host' in options and options['login_host'] is not None: self.login_host = options['login_host'] @@ -495,7 +499,15 @@ def is_valid_advanced_format(self): if not instance_type: self.invalid("Must set a default instance type or specify instance " "type per role.") - + # Check if this is an allowed instance type. + if instance_type in ParseArgs.DISALLOWED_INSTANCE_TYPES and \ + not (self.force or self.test): + reason = "the suggested 4GB of RAM" + if 'database' in roles: + reason += " to run Cassandra" + LocalState.confirm_or_abort("The {0} instance type does not have {1}." + "Please consider using a larger instance " + "type.".format(instance_type, reason)) # Add the defined roles to the nodes. for node in nodes: for role in roles: diff --git a/appscale/tools/parse_args.py b/appscale/tools/parse_args.py index 501b7dca..ede9bbd2 100644 --- a/appscale/tools/parse_args.py +++ b/appscale/tools/parse_args.py @@ -649,16 +649,11 @@ def validate_infrastructure_flags(self): raise BadConfigurationException("--disks must be a dict, but was a " \ "{0}".format(type(self.args.disks))) - if not self.args.instance_type: - raise BadConfigurationException("Cannot start a cloud instance without " \ - "the instance type.") - if self.args.instance_type in self.DISALLOWED_INSTANCE_TYPES and \ not (self.args.force or self.args.test): - LocalState.confirm_or_abort("The {0} instance type does not have " \ - "enough RAM to run Cassandra in a production setting. Please " \ - "consider using a larger instance type.".format( - self.args.instance_type)) + LocalState.confirm_or_abort("The {0} instance type does not have " + "the suggested 4GB of RAM. Please consider using a larger instance " + "type.".format(self.args.instance_type)) if self.args.infrastructure == 'azure': if not self.args.azure_subscription_id: From 97e2fd6450d75816ce7ee629600aa05981455e48 Mon Sep 17 00:00:00 2001 From: whoarethebritons Date: Thu, 26 Oct 2017 08:35:09 -0700 Subject: [PATCH 03/15] cleanup instances if we fail to start --- appscale/tools/remote_helper.py | 80 ++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 12 deletions(-) diff --git a/appscale/tools/remote_helper.py b/appscale/tools/remote_helper.py index 857a4080..bbe64d32 100644 --- a/appscale/tools/remote_helper.py +++ b/appscale/tools/remote_helper.py @@ -13,6 +13,7 @@ import uuid import yaml +from boto.exception import BotoServerError # AppScale-specific imports from agents.factory import InfrastructureAgentFactory @@ -24,6 +25,7 @@ from custom_exceptions import BadConfigurationException from custom_exceptions import ShellException from custom_exceptions import TimeoutException +from agents.base_agent import AgentRuntimeException from agents.gce_agent import CredentialTypes from agents.gce_agent import GCEAgent from local_state import APPSCALE_VERSION @@ -163,13 +165,19 @@ def start_all_nodes(cls, options, node_layout): instance_type_roles['without_disks'] instance_type.setdefault(node.instance_type, []).append(node) - for instance_type, load_balancer_nodes in load_balancer_roles.items(): + spawned_instance_ids = [] + for instance_type, load_balancer_nodes in load_balancer_roles.items(): + # Copy parameters so we can modify the instance type. instance_type_params = params.copy() instance_type_params['instance_type'] = instance_type instance_ids, public_ips, private_ips = cls.spawn_load_balancers_in_cloud( - options, agent, instance_type_params, len(load_balancer_nodes)) + options, agent, instance_type_params, spawned_instance_ids, + len(load_balancer_nodes)) + + # Keep track of instances we have started. + spawned_instance_ids.extend(instance_ids) for node_index, node in enumerate(load_balancer_nodes): index = node_layout.nodes.index(node) @@ -184,13 +192,16 @@ def start_all_nodes(cls, options, node_layout): for instance_type, other_nodes in nodes.items(): if len(other_nodes) <= 0: break - + # Copy parameters so we can modify the instance type. instance_type_params = params.copy() instance_type_params['instance_type'] = instance_type _instance_ids, _public_ips, _private_ips =\ cls.spawn_other_nodes_in_cloud(agent, instance_type_params, - len(other_nodes)) + spawned_instance_ids, len(other_nodes)) + + # Keep track of instances we have started. + spawned_instance_ids.extend(_instance_ids) for node_index, node in enumerate(other_nodes): index = node_layout.nodes.index(node) @@ -297,7 +308,8 @@ def start_head_node(cls, options, my_id, node_layout): @classmethod - def spawn_load_balancers_in_cloud(cls, options, agent, params, count=1): + def spawn_load_balancers_in_cloud(cls, options, agent, params, + spawned_instance_ids, count=1): """Starts count number of virtual machines in a cloud infrastructure with public ips. @@ -309,14 +321,25 @@ def spawn_load_balancers_in_cloud(cls, options, agent, params, count=1): agent: The agent to start VMs with, must be passed as an argument because agents cannot be made twice. params: The parameters to be sent to the agent. + spawned_instance_ids: Ids of instances that AppScale has started. count: A int, the number of instances to start. Returns: The instance ID, public IP address, and private IP address of the machine that was started. """ - instance_ids, public_ips, private_ips = agent.run_instances( - count=count, parameters=params, security_configured=True, - public_ip_needed=True) + try: + instance_ids, public_ips, private_ips = agent.run_instances( + count=count, parameters=params, security_configured=True, + public_ip_needed=True) + except (AgentRuntimeException, BotoServerError): + AppScaleLogger.warn("AppScale was unable to start the requested number " + "of instances, attempting to terminate those that " + "were started.") + if len(spawned_instance_ids) > 0: + AppScaleLogger.warn("Attempting to terminate those that were started.") + cls.terminate_spawned_instances(spawned_instance_ids, agent, params) + # Re-raise the original exception. + raise if options.static_ip: agent.associate_static_ip(params, instance_ids[0], options.static_ip) @@ -326,7 +349,8 @@ def spawn_load_balancers_in_cloud(cls, options, agent, params, count=1): @classmethod - def spawn_other_nodes_in_cloud(cls, agent, params, count=1): + def spawn_other_nodes_in_cloud(cls, agent, params, spawned_instance_ids, + count=1): """Starts count number of virtual machines in a cloud infrastructure. This method also prepares the virtual machine for use by the AppScale Tools. @@ -335,14 +359,24 @@ def spawn_other_nodes_in_cloud(cls, agent, params, count=1): agent: The agent to start VMs with, must be passed as an argument because agents cannot be made twice. params: The parameters to be sent to the agent. + spawned_instance_ids: Ids of instances that AppScale has started. count: A int, the number of instances to start. Returns: The instance ID, public IP address, and private IP address of the machine that was started. """ - instance_ids, public_ips, private_ips = agent.run_instances( - count=count, parameters=params, security_configured=True, - public_ip_needed=False) + try: + instance_ids, public_ips, private_ips = agent.run_instances( + count=count, parameters=params, security_configured=True, + public_ip_needed=False) + except (AgentRuntimeException, BotoServerError): + AppScaleLogger.warn("AppScale was unable to start the requested number " + "of instances.") + if len(spawned_instance_ids) > 0: + AppScaleLogger.warn("Attempting to terminate those that were started.") + cls.terminate_spawned_instances(spawned_instance_ids, agent, params) + # Re-raise the original exception. + raise return instance_ids, public_ips, private_ips @classmethod @@ -869,6 +903,28 @@ def wait_for_machines_to_finish_loading(cls, host, keyname): time.sleep(cls.WAIT_TIME) + @classmethod + def terminate_spawned_instances(cls, spawned_instance_ids, agent, params): + """ Shuts down instances specified. For use when AppScale has failed to + start all the instances for the deployment since we do not check or clean + any local files. + + Args: + spawned_instance_ids: A list of instance ids we have started that + should be terminated. + agent: The agent to call terminate instance with. + params: Agent parameters. + """ + terminate_params = params.copy() + terminate_params[agent.PARAM_INSTANCE_IDS] = spawned_instance_ids + for _ in range(len(spawned_instance_ids)): + try: + agent.terminate_instances(params) + except (AgentRuntimeException, BotoServerError): + AppScaleLogger.warn("AppScale failed to terminate instance(s) with " + "id(s): {}".format(spawned_instance_ids)) + + @classmethod def terminate_cloud_instance(cls, instance_id, options): """ Powers off a single instance in the currently AppScale deployment and From e70b62a23674d1f61bf3e33a7d2139a0bd293e55 Mon Sep 17 00:00:00 2001 From: whoarethebritons Date: Thu, 26 Oct 2017 09:27:23 -0700 Subject: [PATCH 04/15] cleanup keyname and merge functions that are too similar --- appscale/tools/remote_helper.py | 61 ++++++++++----------------------- 1 file changed, 19 insertions(+), 42 deletions(-) diff --git a/appscale/tools/remote_helper.py b/appscale/tools/remote_helper.py index bbe64d32..520c638e 100644 --- a/appscale/tools/remote_helper.py +++ b/appscale/tools/remote_helper.py @@ -172,9 +172,9 @@ def start_all_nodes(cls, options, node_layout): instance_type_params = params.copy() instance_type_params['instance_type'] = instance_type - instance_ids, public_ips, private_ips = cls.spawn_load_balancers_in_cloud( + instance_ids, public_ips, private_ips = cls.spawn_nodes_in_cloud( options, agent, instance_type_params, spawned_instance_ids, - len(load_balancer_nodes)) + count=len(load_balancer_nodes), load_balancer=True) # Keep track of instances we have started. spawned_instance_ids.extend(instance_ids) @@ -185,6 +185,13 @@ def start_all_nodes(cls, options, node_layout): node_layout.nodes[index].private_ip = private_ips[node_index] node_layout.nodes[index].instance_id = instance_ids[node_index] + if options.static_ip: + node = node_layout.head_node() + agent.associate_static_ip(params, node.instance_id, + options.static_ip) + node.public_ip = options.static_ip + AppScaleLogger.log("Static IP associated with head node.") + AppScaleLogger.log("\nPlease wait for AppScale to prepare your machines " "for use. This can take few minutes.") @@ -197,8 +204,8 @@ def start_all_nodes(cls, options, node_layout): instance_type_params['instance_type'] = instance_type _instance_ids, _public_ips, _private_ips =\ - cls.spawn_other_nodes_in_cloud(agent, instance_type_params, - spawned_instance_ids, len(other_nodes)) + cls.spawn_nodes_in_cloud(options, agent, instance_type_params, + spawned_instance_ids, count=len(other_nodes)) # Keep track of instances we have started. spawned_instance_ids.extend(_instance_ids) @@ -308,8 +315,8 @@ def start_head_node(cls, options, my_id, node_layout): @classmethod - def spawn_load_balancers_in_cloud(cls, options, agent, params, - spawned_instance_ids, count=1): + def spawn_nodes_in_cloud(cls, options, agent, params, spawned_instance_ids, + count=1, load_balancer=False): """Starts count number of virtual machines in a cloud infrastructure with public ips. @@ -323,6 +330,8 @@ def spawn_load_balancers_in_cloud(cls, options, agent, params, params: The parameters to be sent to the agent. spawned_instance_ids: Ids of instances that AppScale has started. count: A int, the number of instances to start. + load_balancer: A boolean indicating whether the spawned instance should + have a public ip or not. Returns: The instance ID, public IP address, and private IP address of the machine that was started. @@ -330,7 +339,7 @@ def spawn_load_balancers_in_cloud(cls, options, agent, params, try: instance_ids, public_ips, private_ips = agent.run_instances( count=count, parameters=params, security_configured=True, - public_ip_needed=True) + public_ip_needed=load_balancer) except (AgentRuntimeException, BotoServerError): AppScaleLogger.warn("AppScale was unable to start the requested number " "of instances, attempting to terminate those that " @@ -338,45 +347,13 @@ def spawn_load_balancers_in_cloud(cls, options, agent, params, if len(spawned_instance_ids) > 0: AppScaleLogger.warn("Attempting to terminate those that were started.") cls.terminate_spawned_instances(spawned_instance_ids, agent, params) - # Re-raise the original exception. - raise - - if options.static_ip: - agent.associate_static_ip(params, instance_ids[0], options.static_ip) - public_ips[0] = options.static_ip - AppScaleLogger.log("Static IP associated with head node.") - return instance_ids, public_ips, private_ips - - - @classmethod - def spawn_other_nodes_in_cloud(cls, agent, params, spawned_instance_ids, - count=1): - """Starts count number of virtual machines in a cloud infrastructure. - This method also prepares the virtual machine for use by the AppScale Tools. + # Cleanup the keyname since it failed. + LocalState.cleanup_keyname(options.keyname) - Args: - agent: The agent to start VMs with, must be passed as an argument - because agents cannot be made twice. - params: The parameters to be sent to the agent. - spawned_instance_ids: Ids of instances that AppScale has started. - count: A int, the number of instances to start. - Returns: - The instance ID, public IP address, and private IP address of the machine - that was started. - """ - try: - instance_ids, public_ips, private_ips = agent.run_instances( - count=count, parameters=params, security_configured=True, - public_ip_needed=False) - except (AgentRuntimeException, BotoServerError): - AppScaleLogger.warn("AppScale was unable to start the requested number " - "of instances.") - if len(spawned_instance_ids) > 0: - AppScaleLogger.warn("Attempting to terminate those that were started.") - cls.terminate_spawned_instances(spawned_instance_ids, agent, params) # Re-raise the original exception. raise + return instance_ids, public_ips, private_ips @classmethod From ac72c54eb278cf1e0e62c134c7e6eb604c5b691b Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Tue, 6 Feb 2018 16:46:56 -0800 Subject: [PATCH 05/15] Check instance type while adding an autoscaled instance to a scaleset --- appscale/tools/agents/azure_agent.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/appscale/tools/agents/azure_agent.py b/appscale/tools/agents/azure_agent.py index 0b5f7fe5..6ad821e8 100644 --- a/appscale/tools/agents/azure_agent.py +++ b/appscale/tools/agents/azure_agent.py @@ -123,7 +123,6 @@ class AzureAgent(BaseAgent): PARAM_APP_SECRET, PARAM_APP_ID, PARAM_IMAGE_ID, - PARAM_INSTANCE_TYPE, PARAM_KEYNAME, PARAM_SUBSCRIBER_ID, PARAM_TENANT_ID, @@ -491,6 +490,7 @@ def add_instances_to_existing_ss(self, count, parameters): credentials = self.open_connection(parameters) subscription_id = str(parameters[self.PARAM_SUBSCRIBER_ID]) resource_group = parameters[self.PARAM_RESOURCE_GROUP] + instance_type = parameters[self.PARAM_INSTANCE_TYPE] compute_client = ComputeManagementClient(credentials, subscription_id) num_instances_added = 0 @@ -505,6 +505,9 @@ def add_instances_to_existing_ss(self, count, parameters): if ss_instance_count >= self.MAX_VMSS_CAPACITY: continue + if not vmss.sku.name == instance_type: + continue + scaleset = compute_client.virtual_machine_scale_sets.get( resource_group, vmss.name) ss_upgrade_policy = scaleset.upgrade_policy From 400e885a6290ff1e27f6747f29a118511a9b89a3 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Tue, 6 Feb 2018 17:02:34 -0800 Subject: [PATCH 06/15] Fix unit tests to allow instance types per role --- test/test_ip_layouts.py | 33 +++++++++++++++++++-------------- test/test_local_state.py | 6 ++++-- test/test_node_layout.py | 11 ++++++----- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/test/test_ip_layouts.py b/test/test_ip_layouts.py index 0424f3f2..3095ac30 100644 --- a/test/test_ip_layouts.py +++ b/test/test_ip_layouts.py @@ -12,6 +12,8 @@ DISK_ONE = 'disk_number_1' DISK_TWO = 'disk_number_2' +INSTANCE_TYPE_1 = "instance_type_1" +INSTANCE_TYPE_2 = "instance_type_2" ONE_NODE_CLOUD = [ { @@ -23,34 +25,35 @@ ONE_NODE_CLUSTER = [ { 'roles': ['master', 'database', 'appengine'], - 'nodes': IP_1 + 'nodes': IP_1, + 'instance_type': INSTANCE_TYPE_1 } ] -OPEN_NODE_CLOUD = [{'roles': ['master', 'database', 'appengine'], 'nodes': 1}, - {'roles': 'open', 'nodes': 1}] +OPEN_NODE_CLOUD = [{'roles': ['master', 'database', 'appengine'], 'nodes': 1, 'instance_type': INSTANCE_TYPE_1}, + {'roles': 'open', 'nodes': 1, 'instance_type': INSTANCE_TYPE_1}] -LOGIN_NODE_CLOUD = [{'roles': ['master', 'database', 'appengine'], 'nodes': 1}, - {'roles': 'login', 'nodes': 1}] +LOGIN_NODE_CLOUD = [{'roles': ['master', 'database', 'appengine'], 'nodes': 1, 'instance_type': INSTANCE_TYPE_1}, + {'roles': 'login', 'nodes': 1, 'instance_type': INSTANCE_TYPE_2}] FOUR_NODE_CLOUD = [{'roles': 'master', 'nodes': 1}, {'roles': 'appengine', 'nodes': 1}, {'roles': 'database', 'nodes': 1}, {'roles': 'zookeeper', 'nodes': 1}] -FOUR_NODE_CLUSTER = [{'roles': 'master', 'nodes': IP_1}, - {'roles': 'appengine', 'nodes': IP_2}, - {'roles': 'database', 'nodes': IP_3}, - {'roles': 'zookeeper', 'nodes': IP_4}] +FOUR_NODE_CLUSTER = [{'roles': 'master', 'nodes': IP_1, 'instance_type': INSTANCE_TYPE_1}, + {'roles': 'appengine', 'nodes': IP_2, 'instance_type': INSTANCE_TYPE_2}, + {'roles': 'database', 'nodes': IP_3, 'instance_type': INSTANCE_TYPE_1}, + {'roles': 'zookeeper', 'nodes': IP_4, 'instance_type': INSTANCE_TYPE_2}] THREE_NODE_CLOUD = [{'roles': 'master', 'nodes': 1}, {'roles': 'zookeeper', 'nodes': 1}, {'roles': ['database', 'appengine'], 'nodes': 1}] -TWO_NODES_TWO_DISKS_CLOUD = [{'roles': ['master', 'database'], 'nodes': 1, - 'disks': DISK_ONE}, +TWO_NODES_TWO_DISKS_CLOUD = [{'roles': ['master', 'database'], 'nodes': 1, + 'instance_type': INSTANCE_TYPE_1, 'disks': DISK_ONE}, {'roles': ['appengine'], 'nodes': 1, - 'disks': DISK_TWO}] + 'instance_type': INSTANCE_TYPE_2, 'disks': DISK_TWO}] TWO_NODES_ONE_NOT_UNIQUE_DISK_CLOUD = [ {'roles': ['master', 'database'], 'nodes': 1, 'disks': DISK_ONE}, @@ -61,5 +64,7 @@ {'roles': ['appengine'], 'nodes': 2, 'disks': DISK_TWO}] THREE_NODES_TWO_DISKS_FOR_NODESET_CLOUD = [ - {'roles': ['master', 'database'], 'nodes': 1}, - {'roles': ['appengine'], 'nodes': 2, 'disks': [DISK_ONE, DISK_TWO]}] + {'roles': ['master', 'database'], 'nodes': 1, 'instance_type': INSTANCE_TYPE_1}, + {'roles': ['appengine'], 'nodes': 2, + 'instance_type': INSTANCE_TYPE_2, 'disks': [DISK_ONE, DISK_TWO]}] + diff --git a/test/test_local_state.py b/test/test_local_state.py index 005052e5..e3a0cd37 100644 --- a/test/test_local_state.py +++ b/test/test_local_state.py @@ -105,7 +105,8 @@ def test_generate_deployment_params(self): 'table' : 'cassandra', 'infrastructure' : "ec2", 'min_machines' : 1, - 'max_machines' : 1 + 'max_machines' : 1, + 'instance_type': 'm1.large' }) flexmock(NodeLayout).should_receive("head_node").and_return(Node( @@ -247,7 +248,8 @@ def test_update_local_metadata(self): 'min_machines' : 1, 'max_machines' : 1, 'infrastructure' : 'ec2', - 'table' : 'cassandra' + 'table' : 'cassandra', + 'instance_type': 'm1.large' }) LocalState.update_local_metadata(options, 'public1', 'public1') diff --git a/test/test_node_layout.py b/test/test_node_layout.py index 1c119650..7d1ef490 100644 --- a/test/test_node_layout.py +++ b/test/test_node_layout.py @@ -92,7 +92,8 @@ def test_login_override_login_node(self): def test_is_database_replication_valid_with_db_slave(self): - input_yaml = [{'roles': ['master', 'database', 'appengine'], 'nodes': 1}] + input_yaml = [{'roles': ['master', 'database', 'appengine'], 'nodes': 1, + 'instance_type': 'm1.large'}] options = self.default_options.copy() options['ips'] = input_yaml fake_node = flexmock() @@ -178,9 +179,9 @@ def test_new_with_right_number_of_unique_disks_both_nodes(self): # suppose that the user has specified two nodes, and two EBS / PD disks # with different names. This is the desired user behavior. input_yaml = [{'roles': ['master', 'database'], 'nodes': 1, - 'disks': self.DISK_ONE}, + 'instance_type': 'm1.large', 'disks': self.DISK_ONE}, {'roles': ['appengine'], 'nodes': 1, - 'disks': self.DISK_TWO}] + 'instance_type': 'm1.large', 'disks': self.DISK_TWO}] options = self.default_options.copy() options['ips'] = input_yaml layout = NodeLayout(options) @@ -193,9 +194,9 @@ def test_new_with_right_number_of_unique_disks_one_node(self): # suppose that the user has specified two nodes, and two EBS / PD disks # with different names. This is the desired user behavior. input_yaml = [ - {'roles': ['master', 'database'], 'nodes': 1}, + {'roles': ['master', 'database'], 'nodes': 1, 'instance_type': 'm1.large'}, {'roles': ['appengine'], 'nodes': 2, - 'disks': [self.DISK_ONE, self.DISK_TWO]}] + 'instance_type': 'm1.large', 'disks': [self.DISK_ONE, self.DISK_TWO]}] options = self.default_options.copy() options['ips'] = input_yaml layout = NodeLayout(options) From 5ce242f62b3c09afc906bea41a3149d8260dcb7e Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Tue, 6 Feb 2018 17:06:50 -0800 Subject: [PATCH 07/15] Add instance type parameter to Node Layout methods --- appscale/tools/node_layout.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/appscale/tools/node_layout.py b/appscale/tools/node_layout.py index 839638b1..0ad2ecde 100644 --- a/appscale/tools/node_layout.py +++ b/appscale/tools/node_layout.py @@ -733,7 +733,8 @@ def to_json(self): 'private_ip': self.private_ip, 'instance_id': self.instance_id, 'jobs': self.roles, - 'disk': self.disk + 'disk': self.disk, + 'instance_type' : self.instance_type } @@ -756,3 +757,4 @@ def from_json(self, node_dict): self.instance_id = node_dict.get('instance_id') self.roles = node_dict.get('jobs') self.disk = node_dict.get('disk') + self.instance_type = node_dict.get('instance_type') From 4a9e843a157951794de7c74a93f5a0e997d57bee Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Wed, 14 Feb 2018 10:29:32 -0800 Subject: [PATCH 08/15] Make instance type mandatory only at the node/role layout level and not at the higher level. --- appscale/tools/agents/ec2_agent.py | 1 - appscale/tools/agents/gce_agent.py | 1 - 2 files changed, 2 deletions(-) diff --git a/appscale/tools/agents/ec2_agent.py b/appscale/tools/agents/ec2_agent.py index e4d0399a..c9d6412f 100644 --- a/appscale/tools/agents/ec2_agent.py +++ b/appscale/tools/agents/ec2_agent.py @@ -56,7 +56,6 @@ class EC2Agent(BaseAgent): PARAM_CREDENTIALS, PARAM_GROUP, PARAM_IMAGE_ID, - PARAM_INSTANCE_TYPE, PARAM_KEYNAME, PARAM_SPOT ) diff --git a/appscale/tools/agents/gce_agent.py b/appscale/tools/agents/gce_agent.py index 3493ab22..098a7fb7 100644 --- a/appscale/tools/agents/gce_agent.py +++ b/appscale/tools/agents/gce_agent.py @@ -123,7 +123,6 @@ class GCEAgent(BaseAgent): REQUIRED_CREDENTIALS = ( PARAM_GROUP, PARAM_IMAGE_ID, - PARAM_INSTANCE_TYPE, PARAM_KEYNAME, PARAM_PROJECT, PARAM_ZONE From fa1783d590a06c4afd776f7b5fb142730782b7cf Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Wed, 21 Feb 2018 16:54:49 -0800 Subject: [PATCH 09/15] To only mandate instance type for cloud ASFs --- appscale/tools/node_layout.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/appscale/tools/node_layout.py b/appscale/tools/node_layout.py index 0ad2ecde..44fa007a 100644 --- a/appscale/tools/node_layout.py +++ b/appscale/tools/node_layout.py @@ -268,9 +268,11 @@ def validate_node_layout(self): instance_type = node_set.get('instance_type', self.default_instance_type) - if not instance_type: - self.invalid("Must set a default instance type or specify instance " - "type per role.") + if self.infrastructure: + if not instance_type: + self.invalid("Must set a default instance type or specify instance " + "type per role.") + # Check if this is an allowed instance type. if instance_type in ParseArgs.DISALLOWED_INSTANCE_TYPES and \ not (self.force or self.test): From 3694d133f33d2e4d656bc2df4645d42ec31e5991 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Fri, 23 Feb 2018 13:59:36 -0800 Subject: [PATCH 10/15] Remove the distinction between nodes with disks and without --- appscale/tools/remote_helper.py | 43 +++++++++++++++------------------ 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/appscale/tools/remote_helper.py b/appscale/tools/remote_helper.py index d25b67a2..22346343 100644 --- a/appscale/tools/remote_helper.py +++ b/appscale/tools/remote_helper.py @@ -154,15 +154,13 @@ def start_all_nodes(cls, options, node_layout): agent.configure_instance_security(params) load_balancer_roles = {} - - instance_type_roles = {'with_disks':{}, 'without_disks': {}} + instance_type_roles = {} for node in node_layout.get_nodes('load_balancer', True): load_balancer_roles.setdefault(node.instance_type, []).append(node) for node in node_layout.get_nodes('load_balancer', False): - instance_type = instance_type_roles['with_disks'] if node.disk else \ - instance_type_roles['without_disks'] + instance_type = instance_type_roles instance_type.setdefault(node.instance_type, []).append(node) spawned_instance_ids = [] @@ -195,26 +193,23 @@ def start_all_nodes(cls, options, node_layout): AppScaleLogger.log("\nPlease wait for AppScale to prepare your machines " "for use. This can take few minutes.") - for _, nodes in instance_type_roles.items(): - for instance_type, other_nodes in nodes.items(): - if len(other_nodes) <= 0: - break - # Copy parameters so we can modify the instance type. - instance_type_params = params.copy() - instance_type_params['instance_type'] = instance_type - - _instance_ids, _public_ips, _private_ips =\ - cls.spawn_nodes_in_cloud(options, agent, instance_type_params, - spawned_instance_ids, count=len(other_nodes)) - - # Keep track of instances we have started. - spawned_instance_ids.extend(_instance_ids) - - for node_index, node in enumerate(other_nodes): - index = node_layout.nodes.index(node) - node_layout.nodes[index].public_ip = _public_ips[node_index] - node_layout.nodes[index].private_ip = _private_ips[node_index] - node_layout.nodes[index].instance_id = _instance_ids[node_index] + for instance_type, nodes in instance_type_roles.items(): + # Copy parameters so we can modify the instance type. + instance_type_params = params.copy() + instance_type_params['instance_type'] = instance_type + + _instance_ids, _public_ips, _private_ips = cls.spawn_nodes_in_cloud( + options, agent, instance_type_params, spawned_instance_ids, + count=len(nodes)) + + # Keep track of instances we have started. + spawned_instance_ids.extend(_instance_ids) + + for node_index, node in enumerate(nodes): + index = node_layout.nodes.index(node) + node_layout.nodes[index].public_ip = _public_ips[node_index] + node_layout.nodes[index].private_ip = _private_ips[node_index] + node_layout.nodes[index].instance_id = _instance_ids[node_index] return node_layout From 945d05fc8e2011c7580bd7d05ab23acb66b63019 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Fri, 23 Feb 2018 15:20:02 -0800 Subject: [PATCH 11/15] Allow checking of instance types for the nodes being reused after a down --clean --- appscale/tools/node_layout.py | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/appscale/tools/node_layout.py b/appscale/tools/node_layout.py index 44fa007a..968e6af5 100644 --- a/appscale/tools/node_layout.py +++ b/appscale/tools/node_layout.py @@ -548,8 +548,9 @@ def from_locations_json_list(self, locations_nodes_list): open_nodes.append(old_node) continue for _, node in enumerate(nodes_copy): - # Match nodes based on jobs/roles. - if set(old_node_roles) == set(node.roles): + # Match nodes based on jobs/roles and the instance type specified. + if set(old_node_roles) == set(node.roles) \ + and old_node.get('instance_type') == node.instance_type: nodes_copy.remove(node) node.from_json(old_node) if node.is_valid(): @@ -559,19 +560,19 @@ def from_locations_json_list(self, locations_nodes_list): return None break for open_node in open_nodes: - try: - node = nodes_copy.pop() - except IndexError: - return None - # Match nodes based on jobs/roles. - roles = node.roles - node.from_json(open_node) - node.roles = roles - if node.is_valid(): - nodes.append(node) - else: - # Locations JSON is incorrect if we get here. - return None + for node in nodes_copy: + # Match nodes based on jobs/roles and the instance type specified. + if node.instance_type == open_node.get('instance_type'): + roles = node.roles + node.from_json(open_node) + node.roles = roles + if node.is_valid(): + nodes.append(node) + else: + # Locations JSON is incorrect if we get here. + return None + else: + continue # If these lengths are equal all nodes were matched. if len(nodes) == len(self.nodes): From 667a85c3df9fd4111403c3c9d8ea7baa1837808f Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Fri, 23 Feb 2018 16:39:14 -0800 Subject: [PATCH 12/15] Fix broken unit tests for reattach nodes --- test/test_ip_layouts.py | 8 ++++---- test/test_node_layout.py | 12 ++++++++---- test/test_remote_helper.py | 12 ++++++++---- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/test/test_ip_layouts.py b/test/test_ip_layouts.py index 3095ac30..f1a1f77a 100644 --- a/test/test_ip_layouts.py +++ b/test/test_ip_layouts.py @@ -36,10 +36,10 @@ LOGIN_NODE_CLOUD = [{'roles': ['master', 'database', 'appengine'], 'nodes': 1, 'instance_type': INSTANCE_TYPE_1}, {'roles': 'login', 'nodes': 1, 'instance_type': INSTANCE_TYPE_2}] -FOUR_NODE_CLOUD = [{'roles': 'master', 'nodes': 1}, - {'roles': 'appengine', 'nodes': 1}, - {'roles': 'database', 'nodes': 1}, - {'roles': 'zookeeper', 'nodes': 1}] +FOUR_NODE_CLOUD = [{'roles': 'master', 'nodes': 1, 'instance_type': INSTANCE_TYPE_1}, + {'roles': 'appengine', 'nodes': 1, 'instance_type': INSTANCE_TYPE_1}, + {'roles': 'database', 'nodes': 1, 'instance_type': INSTANCE_TYPE_1}, + {'roles': 'zookeeper', 'nodes': 1, 'instance_type': INSTANCE_TYPE_1}] FOUR_NODE_CLUSTER = [{'roles': 'master', 'nodes': IP_1, 'instance_type': INSTANCE_TYPE_1}, {'roles': 'appengine', 'nodes': IP_2, 'instance_type': INSTANCE_TYPE_2}, diff --git a/test/test_node_layout.py b/test/test_node_layout.py index 7d1ef490..16fe209a 100644 --- a/test/test_node_layout.py +++ b/test/test_node_layout.py @@ -230,19 +230,23 @@ def test_new_with_right_number_of_unique_disks_one_node(self): "private_ip": "0.0.0.0", "instance_id": "i-APPSCALE1", "jobs": ['load_balancer', 'taskqueue', 'shadow', 'login', - 'taskqueue_master'] }, + 'taskqueue_master'], + "instance_type": "instance_type_1"}, { "public_ip": "0.0.0.0", "private_ip": "0.0.0.0", "instance_id": "i-APPSCALE2", - "jobs": ['memcache', 'appengine'] }, + "jobs": ['memcache', 'appengine'], + "instance_type": "instance_type_1"}, { "public_ip": "0.0.0.0", "private_ip": "0.0.0.0", "instance_id": "i-APPSCALE3", - "jobs": ['zookeeper'] }, + "jobs": ['zookeeper'], + "instance_type": "instance_type_1"}, { "public_ip": "0.0.0.0", "private_ip": "0.0.0.0", "instance_id": "i-APPSCALE4", - "jobs": ['database', 'db_master'] } + "jobs": ['database', 'db_master'], + "instance_type": "instance_type_1"} ] diff --git a/test/test_remote_helper.py b/test/test_remote_helper.py index b2c60732..4baa8480 100644 --- a/test/test_remote_helper.py +++ b/test/test_remote_helper.py @@ -529,19 +529,23 @@ def test_wait_for_machines_to_finish_loading(self): "private_ip": "0.0.0.0", "instance_id": "i-APPSCALE1", "jobs": ['load_balancer', 'taskqueue', 'shadow', 'login', - 'taskqueue_master'] }, + 'taskqueue_master'], + "instance_type" : "instance_type_1"}, { "public_ip": "0.0.0.0", "private_ip": "0.0.0.0", "instance_id": "i-APPSCALE2", - "jobs": ['memcache', 'appengine'] }, + "jobs": ['memcache', 'appengine'], + "instance_type": "instance_type_1"}, { "public_ip": "0.0.0.0", "private_ip": "0.0.0.0", "instance_id": "i-APPSCALE3", - "jobs": ['zookeeper'] }, + "jobs": ['zookeeper'], + "instance_type": "instance_type_1"}, { "public_ip": "0.0.0.0", "private_ip": "0.0.0.0", "instance_id": "i-APPSCALE4", - "jobs": ['database', 'db_master'] } + "jobs": ['database', 'db_master'], + "instance_type": "instance_type_1"} ] def test_start_all_nodes_reattach(self): From 87fbf18624e952ae06e70487f003ab5a956daf78 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Tue, 27 Feb 2018 12:31:59 -0800 Subject: [PATCH 13/15] Addressing code review comments. --- appscale/tools/remote_helper.py | 49 +++++++++++++++------------------ 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/appscale/tools/remote_helper.py b/appscale/tools/remote_helper.py index f09ea062..0b5ca5ef 100644 --- a/appscale/tools/remote_helper.py +++ b/appscale/tools/remote_helper.py @@ -171,8 +171,8 @@ def start_all_nodes(cls, options, node_layout): instance_type_params['instance_type'] = instance_type instance_ids, public_ips, private_ips = cls.spawn_nodes_in_cloud( - options, agent, instance_type_params, spawned_instance_ids, - count=len(load_balancer_nodes), load_balancer=True) + agent, instance_type_params, count=len(load_balancer_nodes), + load_balancer=True) # Keep track of instances we have started. spawned_instance_ids.extend(instance_ids) @@ -198,9 +198,22 @@ def start_all_nodes(cls, options, node_layout): instance_type_params = params.copy() instance_type_params['instance_type'] = instance_type - _instance_ids, _public_ips, _private_ips = cls.spawn_nodes_in_cloud( - options, agent, instance_type_params, spawned_instance_ids, - count=len(nodes)) + try: + _instance_ids, _public_ips, _private_ips = cls.spawn_nodes_in_cloud( + agent, instance_type_params, count=len(nodes)) + except (AgentRuntimeException, BotoServerError): + AppScaleLogger.warn("AppScale was unable to start the requested number " + "of instances, attempting to terminate those that " + "were started.") + if len(spawned_instance_ids) > 0: + AppScaleLogger.warn("Attempting to terminate those that were started.") + cls.terminate_spawned_instances(spawned_instance_ids, agent, params) + + # Cleanup the keyname since it failed. + LocalState.cleanup_keyname(options.keyname) + + # Re-raise the original exception. + raise # Keep track of instances we have started. spawned_instance_ids.extend(_instance_ids) @@ -310,20 +323,16 @@ def start_head_node(cls, options, my_id, node_layout): @classmethod - def spawn_nodes_in_cloud(cls, options, agent, params, spawned_instance_ids, - count=1, load_balancer=False): + def spawn_nodes_in_cloud(cls, agent, params, count=1, load_balancer=False): """Starts count number of virtual machines in a cloud infrastructure with public ips. This method also prepares the virtual machine for use by the AppScale Tools. Args: - options: A Namespace that specifies the cloud infrastructure to use, as - well as how to interact with that cloud. agent: The agent to start VMs with, must be passed as an argument because agents cannot be made twice. params: The parameters to be sent to the agent. - spawned_instance_ids: Ids of instances that AppScale has started. count: A int, the number of instances to start. load_balancer: A boolean indicating whether the spawned instance should have a public ip or not. @@ -331,23 +340,9 @@ def spawn_nodes_in_cloud(cls, options, agent, params, spawned_instance_ids, The instance ID, public IP address, and private IP address of the machine that was started. """ - try: - instance_ids, public_ips, private_ips = agent.run_instances( - count=count, parameters=params, security_configured=True, - public_ip_needed=load_balancer) - except (AgentRuntimeException, BotoServerError): - AppScaleLogger.warn("AppScale was unable to start the requested number " - "of instances, attempting to terminate those that " - "were started.") - if len(spawned_instance_ids) > 0: - AppScaleLogger.warn("Attempting to terminate those that were started.") - cls.terminate_spawned_instances(spawned_instance_ids, agent, params) - - # Cleanup the keyname since it failed. - LocalState.cleanup_keyname(options.keyname) - - # Re-raise the original exception. - raise + instance_ids, public_ips, private_ips = agent.run_instances( + count=count, parameters=params, security_configured=True, + public_ip_needed=load_balancer) return instance_ids, public_ips, private_ips From 35cb0aec23fc82a6aab50ab20159dd5964b0d23a Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Wed, 28 Feb 2018 11:30:46 -0800 Subject: [PATCH 14/15] Clean up after the load balancer nodes in case one of them has a problem while spawning. --- appscale/tools/remote_helper.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/appscale/tools/remote_helper.py b/appscale/tools/remote_helper.py index 0b5ca5ef..26044418 100644 --- a/appscale/tools/remote_helper.py +++ b/appscale/tools/remote_helper.py @@ -170,9 +170,23 @@ def start_all_nodes(cls, options, node_layout): instance_type_params = params.copy() instance_type_params['instance_type'] = instance_type - instance_ids, public_ips, private_ips = cls.spawn_nodes_in_cloud( - agent, instance_type_params, count=len(load_balancer_nodes), - load_balancer=True) + try: + instance_ids, public_ips, private_ips = cls.spawn_nodes_in_cloud( + agent, instance_type_params, count=len(load_balancer_nodes), + load_balancer=True) + except (AgentRuntimeException, BotoServerError): + AppScaleLogger.warn("AppScale was unable to start the requested number " + "of instances, attempting to terminate those that " + "were started.") + if len(spawned_instance_ids) > 0: + AppScaleLogger.warn("Attempting to terminate those that were started.") + cls.terminate_spawned_instances(spawned_instance_ids, agent, params) + + # Cleanup the keyname since it failed. + LocalState.cleanup_keyname(options.keyname) + + # Re-raise the original exception. + raise # Keep track of instances we have started. spawned_instance_ids.extend(instance_ids) From 331c1640a4377b38d9d0f9dbc711b82a8bddd0b4 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Thu, 1 Mar 2018 09:22:35 -0800 Subject: [PATCH 15/15] Remove the incorrect looping of terminate instances. --- appscale/tools/remote_helper.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/appscale/tools/remote_helper.py b/appscale/tools/remote_helper.py index 26044418..06654068 100644 --- a/appscale/tools/remote_helper.py +++ b/appscale/tools/remote_helper.py @@ -900,12 +900,11 @@ def terminate_spawned_instances(cls, spawned_instance_ids, agent, params): """ terminate_params = params.copy() terminate_params[agent.PARAM_INSTANCE_IDS] = spawned_instance_ids - for _ in range(len(spawned_instance_ids)): - try: - agent.terminate_instances(params) - except (AgentRuntimeException, BotoServerError): - AppScaleLogger.warn("AppScale failed to terminate instance(s) with " - "id(s): {}".format(spawned_instance_ids)) + try: + agent.terminate_instances(terminate_params) + except (AgentRuntimeException, BotoServerError): + AppScaleLogger.warn("AppScale failed to terminate instance(s) with " + "id(s): {}".format(spawned_instance_ids)) @classmethod