I've browsed the code, and I don't really see anything hypervisor-specific for KVM aside from a change to VirtualRoutingResource to handle the new AbstractConfigItemFacade, which looks like it's used to pass commands to the virtual router. Assuming the VR can be configured, there aren't any changes here that should keep VMs from starting or storage from working as-is.
As far as code review, this branch contains a lot of noise that makes it a bit more difficult to review. A large percentage of the diff was formatting and adding 'final' to variables, etc. Ideally those commits would be applied individually to master rather than confusing a functional change with them. On Tue, Feb 17, 2015 at 8:07 AM, Marcus <shadow...@gmail.com> wrote: > Wido, those were my thoughts as well. I haven't looked at the code > yet, but I'd be surprised if they had to change storage code (or even > hypervisor-specific code outside of passing along new VR script > Commands) in any way to accommodate a VR feature, so I hope this is > just caution. But enough guessing, perhaps I should just go look. > > On Tue, Feb 17, 2015 at 6:15 AM, Wido den Hollander <w...@widodh.nl> wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> >> >> On 02/17/2015 02:19 PM, Daan Hoogland wrote: >>> People, >>> >>> We are having some internal discussions about this code at >>> Schuberg Philis. We are (most specifically I am) convinced that we >>> cannot foresee the ways in which this can break master when merged. >>> We had only one issue coming back from the community and this was >>> functional more then an issue. By the number of issues that we find >>> our selves this can't be right. We had no -1 either so the only >>> logical conclusion is that no one else is testing this part of the >>> code. >>> >>> We do not test all the code and our quality improvement process is >>> not up to spec [1] yet. Things are going to stop working in the >>> master branch when we merge this. I don't like this but I see no >>> way to prevent this in our present state of affairs. We are adding >>> as much tests as we can without hurting Schuberg Philis business >>> and that has a limitation. >>> >>> You can read how Wilder's env is set up and Ian's isn't very >>> different. So let me use a negative formulation this time: If we >>> merge now the following things will not have been tested: most >>> hypervisors: kvm vmware hyperv >> >> I don't *think* it's a major problem since you changed the VR itself, >> but the way it is deployed is still the same. In the end it's just a >> disk for the hypervisor. >> >>> storage: all non nfs things >> >> I'm not so afraid for this since the storage is handled by the >> hypervisor and the VR doesn't have any knowledge there. >> >>> ipv6 functionality This is not a complete list, I suspect. >>> >>> Can I get a lot of +1 in spite of these uncertainties? Can you all >>> please test as much as possible? >>> >> >> As we discussed on the phone, my test cluster is broken at the moment >> and it will take me 2~3 weeks to get it up and running again. >> >> Wido >> >>> I called a merge for this morning but will give it a few more >>> rebase efforts because of those considerations. >>> >>> [1] >>> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Quality+and+Process+Improvement+Initiative >>> >>> On Tue, Feb 17, 2015 at 12:46 AM, Wilder Rodrigues >>> <wrodrig...@schubergphilis.com> wrote: >>>> Hi all, >>>> >>>> I have been some tests on the branch in order to give you all >>>> some confidence. >>>> >>>> During the tests I found 1 bug related to communication from VM A >>>> on Tier 1 to VM B on Tier 2 in a Single VPC. I can reproduce the >>>> bug and it disappears when I convert the Single VPC to a >>>> redundant one. I already talked to Ian and he is on it. >>>> >>>> Results follow below. >>>> >>>> Cheers, Wilder >>>> >>>> Environment: >>>> >>>> Xen 6.2 running on VMware zone within our Betacloud (ACS 4.4.2) >>>> MySQL running on MacBook Pro Management Server on MacBook Pro >>>> >>>> ::: Manual Tests::: >>>> >>>> Isolated Networks >>>> >>>> * Create Network * Create 2 VMs using new Network * Create FW >>>> rules * Create PF rules * SSH to the VMs * SSH from one VM onto >>>> the other in the same isolated network * Destroy Master router * >>>> Restart the Network * Restart the Network with Clean-up option * >>>> Repeat steps above >>>> >>>> Redundant Isolated Networks >>>> >>>> * Create Redundant Network Offering * Create 2 VMs using new >>>> offering * Create FW rules * Create PF rules * SSH to the VMs * >>>> SSH from one VM onto the other in the same redundant isolated >>>> network * Destroy Master router * Restart the Network * Stop the >>>> Master Router >>>> >>>> Single VPC >>>> >>>> * Create VPC * Create 2 Tiers * Create ACLS * Create 1 Vm for >>>> each Tier * Associate 2 IP address * Add PF rules * SSH onto VMs >>>> * SSH from 1 VM onto another * Restart VPC - Make it redundant * >>>> Repeat steps above >>>> >>>> Redundant VPC >>>> >>>> * Create VPC * Create 2 Tiers * Create ACLS * Create 1 Vm for >>>> each Tier * Associate 2 IP address * Add PF rules * SSH onto VMs >>>> * SSH from 1 VM onto another * Stop/Destroy the Master Router * >>>> Observe the Backup router became Master * SSH again onto the VMs >>>> * Restart VPC (without clean-up) * Observer only 1 new router is >>>> created * New router is started as Backup * SSH onto VMs * >>>> Restart VPC (with clean-up) * Observer only 2 new routers are >>>> created * SSH onto VMs >>>> >>>> ::: Automated Tests ::: >>>> >>>> Test Create Account and user for that account ... === TestName: >>>> test_01_create_account | Status : SUCCESS === ok Test Sub domain >>>> allowed to launch VM when a Domain level zone is created ... === >>>> TestName: test_01_add_vm_to_subdomain | Status : SUCCESS === ok >>>> Test delete domain without force option ... === TestName: >>>> test_DeleteDomain | Status : SUCCESS === ok Test delete domain >>>> with force option ... === TestName: test_forceDeleteDomain | >>>> Status : SUCCESS === ok Test update admin details ... === >>>> TestName: test_updateAdminDetails | Status : SUCCESS === ok Test >>>> update domain admin details ... === TestName: >>>> test_updateDomainAdminDetails | Status : SUCCESS === ok Test user >>>> update API ... === TestName: test_updateUserDetails | Status : >>>> SUCCESS === ok Test login API with domain ... === TestName: >>>> test_LoginApiDomain | Status : SUCCESS === ok Test if Login API >>>> does not return UUID's ... === TestName: >>>> test_LoginApiUuidResponse | Status : SUCCESS === ok >>>> >>>> ---------------------------------------------------------------------- >>>> >>>> >> Ran 9 tests in 1140.977s >>>> >>>> OK >>>> >>>> Test reset virtual machine on reboot ... === TestName: >>>> test_01_reset_vm_on_reboot | Status : SUCCESS === ok >>>> >>>> ---------------------------------------------------------------------- >>>> >>>> >> Ran 1 test in 216.907s >>>> >>>> OK >>>> >>>> Test advanced zone virtual router ... === TestName: >>>> test_advZoneVirtualRouter | Status : SUCCESS === ok Test Deploy >>>> Virtual Machine ... === TestName: test_deploy_vm | Status : >>>> SUCCESS === ok Test Multiple Deploy Virtual Machine ... === >>>> TestName: test_deploy_vm_multiple | Status : SUCCESS === ok Test >>>> Stop Virtual Machine ... === TestName: test_01_stop_vm | Status : >>>> SUCCESS === ok Test Start Virtual Machine ... === TestName: >>>> test_02_start_vm | Status : SUCCESS === ok Test Reboot Virtual >>>> Machine ... === TestName: test_03_reboot_vm | Status : SUCCESS >>>> === ok Test destroy Virtual Machine ... === TestName: >>>> test_06_destroy_vm | Status : SUCCESS === ok Test recover Virtual >>>> Machine ... === TestName: test_07_restore_vm | Status : SUCCESS >>>> === ok Test migrate VM ... SKIP: At least two hosts should be >>>> present in the zone for migration Test destroy(expunge) Virtual >>>> Machine ... === TestName: test_09_expunge_vm | Status : SUCCESS >>>> === ok >>>> >>>> ---------------------------------------------------------------------- >>>> >>>> >> Ran 10 tests in 851.022s >>>> >>>> OK (SKIP=1) >>>> >>>> Test router internal advanced zone ... SKIP: Marvin configuration >>>> has no host credentials to check router services Test restart >>>> network ... === TestName: test_03_restart_network_cleanup | >>>> Status : SUCCESS === ok Test router basic setup ... === TestName: >>>> test_05_router_basic | Status : SUCCESS === ok Test router >>>> advanced setup ... === TestName: test_06_router_advanced | Status >>>> : SUCCESS === ok Test stop router ... === TestName: >>>> test_07_stop_router | Status : SUCCESS === ok Test start router >>>> ... === TestName: test_08_start_router | Status : SUCCESS === ok >>>> Test reboot router ... === TestName: test_09_reboot_router | >>>> Status : SUCCESS === ok >>>> >>>> ---------------------------------------------------------------------- >>>> >>>> >> Ran 7 tests in 454.519s >>>> >>>> OK (SKIP=1) >>>> >>>> Test to create service offering ... === TestName: >>>> test_01_create_service_offering | Status : SUCCESS === ok Test to >>>> update existing service offering ... === TestName: >>>> test_02_edit_service_offering | Status : SUCCESS === ok Test to >>>> delete service offering ... === TestName: >>>> test_03_delete_service_offering | Status : SUCCESS === ok >>>> >>>> ---------------------------------------------------------------------- >>>> >>>> >> Ran 3 tests in 206.916s >>>> >>>> OK >>>> >>>> Test VPN in VPC ... === TestName: test_vpc_remote_access_vpn | >>>> Status : SUCCESS === ok Test VPN in VPC ... === TestName: >>>> test_vpc_site2site_vpn | Status : SUCCESS === ok >>>> >>>> ---------------------------------------------------------------------- >>>> >>>> >> Ran 2 tests in 373.908s >>>> >>>> OK >>>> >>>> Test create VPC offering ... === TestName: >>>> test_01_create_vpc_offering | Status : SUCCESS === ok Test VPC >>>> offering without load balancing service ... === TestName: >>>> test_03_vpc_off_without_lb | Status : SUCCESS === ok Test VPC >>>> offering without static NAT service ... === TestName: >>>> test_04_vpc_off_without_static_nat | Status : SUCCESS === ok Test >>>> VPC offering without port forwarding service ... === TestName: >>>> test_05_vpc_off_without_pf | Status : SUCCESS === ok Test VPC >>>> offering with invalid services ... === TestName: >>>> test_06_vpc_off_invalid_services | Status : SUCCESS === ok Test >>>> update VPC offering ... === TestName: test_07_update_vpc_off | >>>> Status : SUCCESS === ok Test list VPC offering ... === TestName: >>>> test_08_list_vpc_off | Status : SUCCESS === ok >>>> test_09_create_redundant_vpc_offering >>>> (integration.acs.tests.test_vpc_offerings.TestVPCOffering) ... >>>> === TestName: test_09_create_redundant_vpc_offering | Status : >>>> SUCCESS === ok >>>> >>>> ---------------------------------------------------------------------- >>>> >>>> >> Ran 8 tests in 672.382s >>>> >>>> OK >>>> >>>> test_privategw_acl >>>> (integration.acs.tests.test_privategw_acl.TestPrivateGwACL) ... >>>> === TestName: test_privategw_acl | Status : SUCCESS === ok >>>> >>>> ---------------------------------------------------------------------- >>>> >>>> >> Ran 1 test in 90.756s >>>> >>>> OK >>>> >>>> Test to change service offering of router after addition of one >>>> guest network ... === TestName: >>>> test_04_chg_srv_off_router_after_addition_of_one_guest_network | >>>> Status : SUCCESS === ok Test destroy of router after addition of >>>> one guest network ... === TestName: >>>> test_05_destroy_router_after_addition_of_one_guest_network | >>>> Status : SUCCESS === ok Test to stop and start router after >>>> creation of VPC ... === TestName: >>>> test_01_stop_start_router_after_creating_vpc | Status : SUCCESS >>>> === ok Test to reboot the router after creating a VPC ... === >>>> TestName: test_02_reboot_router_after_creating_vpc | Status : >>>> SUCCESS === ok Tests to change service offering of the Router >>>> after ... === TestName: test_04_change_service_offerring_vpc | >>>> Status : SUCCESS === ok Test to destroy the router after creating >>>> a VPC ... === TestName: test_05_destroy_router_after_creating_vpc >>>> | Status : SUCCESS === ok >>>> >>>> ---------------------------------------------------------------------- >>>> >>>> >> Ran 6 tests in 665.965s >>>> >>>> OK >>>> >>>> On 16 Feb 2015, at 17:53, Daan Hoogland <daan.hoogl...@gmail.com> >>>> wrote: >>>> >>>>> H, >>>>> >>>>> I will merge our feature/systemvm-persistent-config into >>>>> master. If you have objections please let me know before >>>>> tomorrow. >>>>> >>>>> @john: your comment was addressed in the present day version. >>>>> >>>>> -- Daan >>>> >>> >>> >>> >> -----BEGIN PGP SIGNATURE----- >> Version: GnuPG v1 >> >> iQIcBAEBAgAGBQJU40zmAAoJEAGbWC3bPspCVHgP/ReiqputXvndXpIy1q9dFBqF >> wXXtFlqLKAwm7XmtmOrqUdOWPB7fCGPo6G6Tka/divg0BHLxU/Mzfx9vQRXXzGwK >> 4WROFyOTl0pTLbksJhbTR0pzT4FEfK6EkfIz2S+zd1xaFGlc/puWSjGHs45RS5vm >> SqNM5knZVo+4nTTMepJFQFEBN4GND8wbTuI3EEjcaodQ1uZM6+3COPaNjj74cXtr >> K+k7MxSJq0tBBKlCO30Ou0oK/pPn4Az9zYkNgfBoyoJfe8W2PCHS0/f3La3O19Cr >> lIOjvVoA1zNfC+E8WRhhZC/HNOzme0MiAAPHHkPykWL9xVBObSihkhiu9XehYjKK >> aK614+Hte6bLD+gra9H01QA1z8Y8ylFxFRTXb+e6fcV6mVReICaQ5tkL50fuTuJ2 >> HRk+1Ybul35/J3ZW3qQ7rikpF4sQ9XTC6MrCpl3Ix0XopYxWtvkLTHVtH5iLWhzy >> hwiS2kAcc9F8zsLIl01tTbUMxAJUR6cms2AxInPadYUlNu3FmCUwTaFpV8GIr1VK >> kdcsEyQdXwXTPCcelCraJ214GVpH3IESqtkeqNDB1F7fPNhJQMyQgVT6fzCVXe4G >> I9Fo82+2X8x7XEwGZL81nt3D0p+ndG0zd+awWlrR4C7rQ3UJl+SSBIkKtqT8G8XS >> AA3KCgF8a43lyTNCk1x/ >> =19tY >> -----END PGP SIGNATURE-----
