Allow different instance type per role #675
Conversation
…nto type-per-role
…nto type-per-role
…ot at the higher level.
appscale/tools/remote_helper.py
Outdated
| 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: |
appscale/tools/remote_helper.py
Outdated
| 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': {}} |
There was a problem hiding this comment.
I'm having trouble figuring out the reason for this separation.
There was a problem hiding this comment.
@cdonati azure cannot attach existing disks to scaleset machines so they would have to be created as regular machines, it was added in a little early with the expectation of managed disks.
There was a problem hiding this comment.
Okay. That sounds like an implementation detail that belongs in the azure agent instead of here.
|
@jeanleonov I did test the autoscaling on Azure, euca and GCE. I can record those for a demo or do them live if you prefer that. Let me know if there's any specific test that you would like to see to confirm it works as it should. |
|
Latest build: https://ocd.appscale.com:8080/job/Daily%20Build/4745/ |
|
@tmarballi Did you see my question about the caller performing the terminate? I think it would be unexpected for a spawn function to terminate other instances as a side effect. |
|
@cdonati Oops, I completely missed that. Looking into it now. |
appscale/tools/remote_helper.py
Outdated
| instance_type_params = params.copy() | ||
| instance_type_params['instance_type'] = instance_type | ||
|
|
||
| instance_ids, public_ips, private_ips = cls.spawn_nodes_in_cloud( |
There was a problem hiding this comment.
Does this need to clean up after itself if it encounters an exception?
There was a problem hiding this comment.
Yes it does, It left my mind that we could technically have different instance types for LBs and it would need clean up if the 2nd LB faced a problem after the 1st one was started successfully. I have added the same try except as for non load balancer nodes.
…lem while spawning.
|
Latest build: https://ocd.appscale.com:8080/job/Daily%20Build/4763/ and Tools test rerun: https://ocd.appscale.com:8080/job/Tools-test/3115/ |
appscale/tools/remote_helper.py
Outdated
| """ | ||
| terminate_params = params.copy() | ||
| terminate_params[agent.PARAM_INSTANCE_IDS] = spawned_instance_ids | ||
| for _ in range(len(spawned_instance_ids)): |
There was a problem hiding this comment.
Why is this running the terminate call multiple times (based on the number of started machines)?
There was a problem hiding this comment.
You are right, it does not need a loop because the terminate instances method uses a list of instances to delete anyways.
|
Latest all blue build: https://ocd.appscale.com:8080/job/Daily%20Build/4767/ |
Needs AppScale/gts#2672