Fix azure terminate to allow for downscaling specific instances #621
Conversation
appscale/tools/agents/azure_agent.py
Outdated
| for vm in vm_list: | ||
| vm_names.append(vm.name) | ||
|
|
||
| if not vm_names: |
There was a problem hiding this comment.
Could we just check the size of vm_list instead of creating a new array?
There was a problem hiding this comment.
The VM list we get from Azure is a Paged object and we don't have any methods we can use on it to get its size and it can only be iterated over to check the size by creating a new array.
| result = compute_client.virtual_machine_scale_set_vms.delete(resource_group, | ||
| vmss_name, | ||
| instance_id) | ||
| resource_name = 'Virtual Machine Instance ' + instance_id |
There was a problem hiding this comment.
Should Virtual Machine Instance be a constant? I assume it's a fixed construct from Azure?
There was a problem hiding this comment.
Not really, so I have a common method for waiting until the resources are created/deleted. So I pass the resource name so that I can print in the logs which is the resource we are waiting on. I don't see any benefit in making it a constant because we would not really use it anywhere else. Unless you see it as neater?
appscale/tools/agents/azure_agent.py
Outdated
| for vmss in vmss_list: | ||
| vm_list = compute_client.virtual_machine_scale_set_vms.list( | ||
| resource_group, vmss.name) | ||
| vm_names = [] |
There was a problem hiding this comment.
If you want to just check if there are no elements in an iterable without iterating through the whole thing, you can do:
if not any(True for _ in vm_list):
[delete_scale_set]
any will stop on the first element, so it would prevent paging through a large scale set.
In cases where you really do need to exhaust the iterable, the list() function can be a nice shortcut.
There was a problem hiding this comment.
Thats awesome, I did not know about that! Thanks, I will make that change :)
| thread = threading.Thread( | ||
| target=self.delete_virtual_machine_scale_set, args=( | ||
| compute_client, resource_group, verbose, vmss.name)) | ||
| delete_ss_instances.append(vm.name) |
There was a problem hiding this comment.
If the entire scale set is being deleted, what is this list for?
There was a problem hiding this comment.
The list of instances to delete consists of both the scale set vms and the regular (load balancer) vms. I need this list of scale set vms so that I only give the remaining vms to delete to the delete_virtual_machine method. In this line over here: https://github.com/AppScale/appscale-tools/pull/621/files#diff-6b911255ca065691da775e67ef91b1e2R768
|
Latest build: https://ocd.appscale.com:8080/job/Daily%20Build/3259/ |
https://ocd.appscale.com:8080/job/Daily%20Build/3243/