You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by anshul1886 <gi...@git.apache.org> on 2015/12/23 07:53:54 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
GitHub user anshul1886 opened a pull request:
https://github.com/apache/cloudstack/pull/1280
CLOUDSTACK-9199: Fixed deployVirtualMachine API does not throw an error when cpunumber is specified for static compute offering
https://issues.apache.org/jira/browse/CLOUDSTACK-9199
To test:
-------------
Deploy VM by providing cpuNumber, cpuSpeed or memory for static/ non dynamic service offering
Deployment should fail.
API example:
------------------
http://10.220.135.6/client/api?command=deployVirtualMachine&name=olotwo&response=&zoneid=ab6e4154-62a3-42a8-9627-3cbdc66bcbb6&templateid=3aaaace6-91b4-11e5-b6fc-e26c2aa1d1d0&hypervisor=XenServer&serviceofferingid=39643075-4b45-489d-afac-88f09d536bdd&details[0].cpuNumber=1&details[0].cpuSpeed=1000&details[0].memory=1000&securitygroupids=60844698-91b4-11e5-b6fc-e26c2aa1d1d0&_=1448277187743
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anshul1886/cloudstack-1 CLOUDSTACK-9199
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1280.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1280
----
commit 149f6c05147a07abe845b53ed4d9bc159b68b0ff
Author: Anshul Gangwar <an...@citrix.com>
Date: 2015-12-23T06:42:44Z
CLOUDSTACK-9199: Fixed deployVirtualMachine API does not throw an error when cpunumber is specified for static compute offering
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-218959384
LGTM (just code review), based on what @anshul1886 says there should not be backward compatibility issue though I've not verified this by performing manual tests
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by bvbharatk <gi...@git.apache.org>.
Github user bvbharatk commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-202622942
### ACS CI BVT Run
**Sumarry:**
Build Number 141
Hypervisor xenserver
NetworkType Advanced
Passed=101
Failed=5
Skipped=4
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* integration.smoke.test_loadbalance.TestLoadBalance
* test_01_create_lb_rule_src_nat Failed
* test_02_create_lb_rule_non_nat Failed
* test_assign_and_removal_lb Failed
* integration.smoke.test_volumes.TestCreateVolume
* test_01_create_volume Failed
* test_06_download_detached_volume Failed
**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_deploy_vgpu_enabled_vm
test_06_copy_template
test_06_copy_iso
**Passed test suits:**
integration.smoke.test_deploy_vm_with_userdata.TestDeployVmWithUserData
integration.smoke.test_affinity_groups_projects.TestDeployVmWithAffinityGroup
integration.smoke.test_portable_publicip.TestPortablePublicIPAcquire
integration.smoke.test_over_provisioning.TestUpdateOverProvision
integration.smoke.test_global_settings.TestUpdateConfigWithScope
integration.smoke.test_guest_vlan_range.TestDedicateGuestVlanRange
integration.smoke.test_scale_vm.TestScaleVm
integration.smoke.test_service_offerings.TestCreateServiceOffering
integration.smoke.test_routers.TestRouterServices
integration.smoke.test_reset_vm_on_reboot.TestResetVmOnReboot
integration.smoke.test_snapshots.TestSnapshotRootDisk
integration.smoke.test_deploy_vms_with_varied_deploymentplanners.TestDeployVmWithVariedPlanners
integration.smoke.test_network.TestDeleteAccount
integration.smoke.test_non_contigiousvlan.TestUpdatePhysicalNetwork
integration.smoke.test_deploy_vm_iso.TestDeployVMFromISO
integration.smoke.test_public_ip_range.TestDedicatePublicIPRange
integration.smoke.test_multipleips_per_nic.TestDeployVM
integration.smoke.test_regions.TestRegions
integration.smoke.test_affinity_groups.TestDeployVmWithAffinityGroup
integration.smoke.test_network_acl.TestNetworkACL
integration.smoke.test_pvlan.TestPVLAN
integration.smoke.test_ssvm.TestSSVMs
integration.smoke.test_nic.TestNic
integration.smoke.test_deploy_vm_root_resize.TestDeployVM
integration.smoke.test_resource_detail.TestResourceDetail
integration.smoke.test_secondary_storage.TestSecStorageServices
integration.smoke.test_vm_life_cycle.TestDeployVM
integration.smoke.test_disk_offerings.TestCreateDiskOffering
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-217610090
@rhtyd @swill There will not be backward compatibility issues as with static offering those parameters were not taken into consideration. They were wrongly giving the impression that they are being used.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-217480884
@anshul1886 can you answer @rhtyd's question so we can get his code review and get this moving forward. Thanks...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-215070344
LGTM. Verified on a simulator setup. Also the test failures from CI run are unrelated to this change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-216219348
@anshul1886 please rebase against latest master, can you explain if this can cause backward compatiblity issue
tag:easypr
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by alexandrelimassantana <gi...@git.apache.org>.
Github user alexandrelimassantana commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-217842674
as @pedro-martins stated, it seems to be fitting that this method is extracted to a class to be documented/tested. The code looks good but if the exception is to be launched I see no room for backward compatibility. If that's an issue, this needs to be rethinked as the raised exception would have to be treated and at that point we would have to think if raising the exception was really necessary. Overall the code looks good, simple and justified.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-219141565
I think we are ok on this one. thanks @rhtyd. I will merge this one...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1280
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by pedro-martins <gi...@git.apache.org>.
Github user pedro-martins commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-170409736
Nice :)
Can you extract the code to a method with a javadoc explaining the code?
If you can do a testcase for the method too, it will be apreciated :)
Thx :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-215080256
Looking for one more LGTM for this one... Thx...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-218381704
I don't find much value here in adding documentation as code seems to be self explanatory. Regrading backward compatibility I have already answered in my previous comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-218367282
@anshul1886 can you please review and address the comments in this PR? Thanks...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9199: Fixed deployVirtualMachi...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-218462063
@anshul1886 I think the ask is that it get extracted as a method and a test be written for it. @pedro-martins and @alexandrelimassantana, I feel that this might be a bit overkill. Can you explain what the test would be validating?
As for the backwards compatibility issue, I am not entirely sure I understand the concern. Can someone help me understand what that concern is? Thx...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---