CLOUDSTACK-9951: Add support for batch/bulk VM deployment option in CS #2140
Conversation
Added a new API to support bulk deployment of VMs. There is also an option to specify pod or cluster to deploy the VMs. VMs will be appropriately placed based on selected deployment planner and the existing VM distribution based on count. For details refer the FS @ https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=70258861
|
Test bulk deploy of VM ... === TestName: test_bulk_deploy_vm | Status : SUCCESS === Ran 2 tests in 63.674s OK |
|
@koushik-das awesome. This is what we need. |
|
@ustcweizhou Thanks! Please try it out and provide your feedback. |
|
Could you check the conflict @koushik-das ? Awesome feature though! |
|
@koushik-das can you rebase against latest master and fix the conflict? |
There was a problem hiding this comment.
Sounds like a good feature to have, the implementation can be improved and simplified if we restrict the options the bulk API accepted limited to deploying of bulk VMs under the same set of common rules such as network, affinity rules, etc. I would advise extending over the existing deploy VM API and avoid putting hacks in the implementation, and the response may return an array of jobs instead of a single job.
| import com.cloud.offering.DiskOffering; | ||
| import com.cloud.offering.ServiceOffering; | ||
|
|
||
| @APICommand(name = "bulkDeployVirtualMachine", description = "Creates and automatically start multiple virtual machines of similar configuration based on virtualmachinecount.", responseObject = AsyncJobResponse.class, |
There was a problem hiding this comment.
how about rename this to, deployBulkVirtualMachine?
| authorized = { RoleType.Admin }, since = "4.11") | ||
| public class BulkDeployVMCmd extends BaseCmd { | ||
| public static final Logger s_logger = Logger.getLogger(BulkDeployVMCmd.class.getName()); | ||
| private static final String s_name = "bulkdeployvirtualmachineresponse"; |
There was a problem hiding this comment.
Can you consider declaring the APINAME as a static var and use that for both declaration of name and response strings, see this for example: https://github.com/apache/cloudstack/blob/master/api/src/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java#L46
| @APICommand(name = "bulkDeployVirtualMachine", description = "Creates and automatically start multiple virtual machines of similar configuration based on virtualmachinecount.", responseObject = AsyncJobResponse.class, | ||
| requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, | ||
| authorized = { RoleType.Admin }, since = "4.11") | ||
| public class BulkDeployVMCmd extends BaseCmd { |
There was a problem hiding this comment.
Is it possible to extend the DeployVMCmd class, this has several options same as the deployVM api.
| } | ||
|
|
||
| private void verifyInputs() { | ||
| if (count == null || count <= 0) { |
There was a problem hiding this comment.
Can you consider using Preconditions check, or at least throw ServerApiException(ApiErrorCode.PARAM_ERROR, ... ?
| @@ -184,10 +184,18 @@ | |||
| @Parameter(name = ApiConstants.DEPLOYMENT_PLANNER, type = CommandType.STRING, description = "Deployment planner to use for vm allocation. Available to ROOT admin only", since = "4.4", authorized = { RoleType.Admin }) | |||
| private String deploymentPlanner; | |||
|
|
|||
| protected Long podId = null; | |||
| protected Long clusterId = null; | |||
| protected List<Long> vmIds = new ArrayList<Long>(); | |||
There was a problem hiding this comment.
Consider removing this hack, consider a different way of implementation.
| @@ -39,11 +40,37 @@ | |||
| public class DeployVMCmdByAdmin extends DeployVMCmd { | |||
| public static final Logger s_logger = Logger.getLogger(DeployVMCmdByAdmin.class.getName()); | |||
|
|
|||
| private void processContextParameters() { | |||
There was a problem hiding this comment.
Consider removing this hack, consider a different way of implementation.
| if (vmIds != null && !vmIds.isEmpty()) { | ||
| boolean wait = true; | ||
| int count = 0; | ||
| while (wait && count++ < 30) { |
There was a problem hiding this comment.
Should this be configurable?
|
@koushik-das please rebase and re-open if still relevant |
Added a new API to support bulk deployment of VMs. There is also an option to specify pod or cluster to deploy the VMs.
VMs will be appropriately placed based on selected deployment planner and the existing VM distribution based on count.
For details refer the FS @ https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=70258861