GitHub user wilderrodrigues opened a pull request:

    https://github.com/apache/cloudstack/pull/19

    VPC and Virtual Network Managers refactor. Part of the VPC Redundant Router 
work

    This is a new Pull Request in order to fix the conflicts we found in the 
old pull request #14 . I will put the most update version of the description 
here:
    
    Pull request of changes in the "cloud-server" module
    
    In the last 10 weeks we have worked in the cloud-server, focusing our time 
in the refactor of the [Vpc]VirtualNetworkApplianceManagerImpl. We had a mains 
goals increase of Maintainability, Extensibility, Readability and test 
coverage. That was just a first step towards the development, still in 
progress, of the Redundant Virtual Routers for VPC.
    
    == What has been done so far:
    
    • The VirtualNetworkApplianceManagerImpl class line numbers dropped from 
4440 to 2558
    • The VpcVirtualNetworkApplianceImpl class line numbers dropped from 1484 
to 749
    • We created 35 new classes in order to split the code/responsibility
    • We added 97.8% unit test coverage for com.cloud.network.element/router 
and org.cloud.network.router.deployment packages
    o The most complex classes we changed are in those packages
    o About 1700 lines of unit tests
    • We executed many Marvin tests that we got from ACS and made compliant 
with our domain:
    o test_01_create_account
    o test_01_add_vm_to_subdomain
    o test_DeleteDomain
    o test_forceDeleteDomain
    o test_updateAdminDetails
    o test_updateDomainAdminDetails
    o test_updateUserDetails
    o test_LoginApiDomain
    o test_LoginApiUuidResponse
    o test_privategw_acl
    o test_01_reset_vm_on_reboot
    o test_03_restart_network_cleanup
    o test_05_router_basic
    o test_06_router_advanced
    o test_07_stop_router
    o test_08_start_router
    o test_09_reboot_router
    o test_01_create_service_offering
    o test_02_edit_service_offering
    o test_03_delete_service_offering
    o test_01_start_stop_router_after_addition_of_one_guest_network
    o test_02_reboot_router_after_addition_of_one_guest_network
    o test_04_chg_srv_off_router_after_addition_of_one_guest_network
    o test_05_destroy_router_after_addition_of_one_guest_network
    o test_01_stop_start_router_after_creating_vpc
    o test_02_reboot_router_after_creating_vpc
    o test_04_change_service_offerring_vpc
    o test_05_destroy_router_after_creating_vpc
    o test_vpc_remote_access_vpn
    o test_vpc_site2site_vpn
    
    We started the changes in the network area, trying to identify the 
differences in the 2 types of network we have. For that we created Basic and 
Advanced Network Topology classes. The network topology classes are responsible 
by invoking the Apply/Setup/Create/Save rules that were previously done by the 
[Vpc]VirtualNetworkAppliance. A topology instance is retrieved via a context 
object that is injected in the [Vpc]VirtualElement. The context object will 
return the most appropriate topology instance based on the Network Type, which 
is defined in the Data Centre. That was the first step towards the refactor.
    
    From the topology class we reach the Rule Applier implementation that will 
be used to do all the rule setup preparation (i.e. invoke DAOs and prepare the 
command object). The RuleApplier interface was extracted from the 
VirtualNetworkApplianceManagerImpl, where it use to be an inner interface. For 
each anonymous implementation of the RuleApplier we created a concrete class. 
The rules are used as elements of a Visitor class, which will perform some 
extra logic, depending on the rule it's visiting, and call the send commands to 
router method. The latter has also been extracted from the 
VirtualNetworkApplianceManagerImpl and is now in a new helper class: 
NetworkHelperImpl.
    
    The visitor has been used because we were aiming to split the 
responsibility and also because the way the RuleApplier was implemented before, 
it was clear that every command sent to the router was following a 2-steps 
approach: gather information to create the commands, apply some logic to send 
to the router. For those reason we implemented the visitor pattern. Since we 
already had the Basic/Advanced Network Topology classes, we created 2 concrete 
classes to visit the rules: Basic/Advanced Network Visitors. Both classes 
extend the abstract class NetworkTopologyVisitor, which defines all the visit 
methods per type of rule. By doing so, we can use the same rule and separate 
the logic based on the type of visitor that we have - Basic or Advanced.
    
    Continuing on the refactor, we also added some helper classes for the 
"getSomething" related methods. Following this approach we ended up having the 
following classes:
    
    • NetworkHelper (interface)
    • NetworkHelperImpl
    • VpcNetworkHelperImpl
    • CommandSetupHelper
    • NicProfileHelper
    • RouterControlHelper
    
    Last, but not least - and actually the most crucial part of the code - 
there was also a huge refactor in terms of how the routers are deployed. The 
previous deployeRouter and deployVpcInrouter methods do not exist any more. 
Instead of having the logics spread, or sometime tangled, in the 
[Vpc]VirtualNetworkApplianceManagerImpl, we have created a Router Deployment 
Definition mechanism, with classes that follow the same naming convention. The 
deployment definition has 2 implementations, Router and Vpc router, which are 
created with the aid of a Builder class. Most of the work, which is common to 
both implementation, is being done by the RouterDeploymentDefinition class. The 
specific bits are done by their implementation. for example, when 
findOrDeployVirtualrouter() method is called, it will make sure that 
precondition are checked, deployment plan is done and generated and executed. 
The implementation will vary according to the Deployment Definition instance we 
have: Router or VpcR
 outer.
    
    Although it looks like a huge change in the ACS cloud-server core, we kept 
most of the original code. Ou mains focus in this first step was to restructure 
it and make it better to understand. We have excessively tested our tested via 
Unit Tests, integration tests and also manually in order to have the 100% 
confidence to push the code towards the upstream branch.
    
    Please, if you have doubts/suggestions/change requests, do not hesitate to 
contact us. Also feel free to improve the code we change in any aspect you 
think it's necessary, but do not forget to share with the community your 
reasons for doing so.
    
    The Redundant VPC subject has been discussed in a few threads in the last 
months:
    
    Working on CloudStack Jira-764:nTier Apps 2.0 : Redundant Virtual Router 
for VPC email 2 of 2 http://markmail.org/message/56xrscvnmdweoxf5
    redundant virtual routers for VPCs: 
http://markmail.org/message/w4ow3ddcpxsic7g6
    Adding Redundant Routers to VPCs: 
http://markmail.org/message/hcay37lvfaev6wqw
    Look to hear your feedback.
    
    With kind regards,
    Wilder Rodrigues

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/schubergphilis/cloudstack 
vpc-refactor-rvr-final

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/19.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 #19
    
----
commit f7a85d2ab741b8d12a95ba08505b634789ef38fa
Author: Antonio Fornie <afor...@schubergphilis.com>
Date:   2014-07-03T17:17:26Z

    Rules and visitors for Load Balance Rules

commit cdd30b75c8f1dfd4274cf7156141a68eaf7f0171
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-04T13:11:29Z

    Adding new behaviour to the apply load balancing rules method. The whole 
chain has been implemented and we will now test it.

commit 2229d999146b77f864bc7adad8528b6cff7a5cfe
Author: Antonio Fornie <afor...@schubergphilis.com>
Date:   2014-07-10T14:54:01Z

    Extract general behavior to Router and Vpc delegates

commit a4a8232ba586860a2a6e5f8285fd728ea58901e3
Author: Antonio Fornie <afor...@schubergphilis.com>
Date:   2014-07-11T13:07:35Z

    Fix dependency problem. Extract and unify router deployment stuff

commit fdbf1a26480e8f294ea1f576230ea983f0458056
Author: Wilder Rodrigues <wilder@ekhos-01.ekhos-01>
Date:   2014-07-13T12:34:23Z

    Adding Firewall Rules to comply with the Visitor pattern implementation; 
refactoring the applyRules so we can reuse it.

commit f89f3b37144e946a43d7bd352e5a5cd350b4701e
Author: Wilder Rodrigues <wilder@ekhos-01.ekhos-01>
Date:   2014-07-14T09:34:48Z

    changing accessor modifier in instance variables

commit 0e9e109fa1e74d95c37d9712044daef71274474e
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-14T09:52:51Z

    fixing checkstyles

commit 3fbb2a17b9ae8106aa7badb16653d1d20fdaf3c1
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-14T14:20:28Z

    finished firewall rules and load balancing rules; fixed all the injection 
problems; added VirtualMachineManager to the appliance factory to be injected.

commit f44def9335e165c31fafcb09f54c1c3424a8ab31
Author: Daan Hoogland <d...@onecht.net>
Date:   2014-07-14T15:50:27Z

    TODO

commit 492249dbefbae6c9a50e931a0be79392fc5eea00
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-14T17:36:29Z

    adding static nat rules. Deploying new VMs is not working due to the 
appliance refactory, will check the changes with Antonio tomorrow.

commit 8979acbe096e7f0ae5e4216686495588edcbc600
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-15T07:23:26Z

    we have to check if VPC is null bfore calling it. VPC is not used in gest 
networks, so deploying a new VM was broken.

commit 327cd3faa7ef3f48444d5d7a4262359a6a408aa1
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-15T07:38:21Z

    adding apache license headers

commit 70af2e79ce3c9bf6154d5c547d7c67e6496fecca
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-15T08:59:42Z

    adding Ip Association and VPN Rules

commit 8a4a74dea3570417f448f615aa07fc7bd78ac303
Author: Antonio Fornie <afor...@schubergphilis.com>
Date:   2014-07-15T09:06:44Z

    Fix offering setup

commit 948b9c6d4d18808bea8d2f64b68e06e3c011d486
Author: Daan Hoogland <d...@onecht.net>
Date:   2014-07-15T09:28:49Z

    package rename

commit 224ef7600c19676a01815eafce5bd995387011d2
Author: Antonio Fornie <afor...@schubergphilis.com>
Date:   2014-07-15T09:54:58Z

    Temporary put state info in a state object

commit 865abacbe80be3da8384eb9367f1c21c86ff15b4
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-15T11:50:37Z

    fixing the classes relationship; adding beans properly in the spring 
context; using the right basic/advance stuff; testing ip and port forwarding 
rules

commit e2071f94a989ea6e75c25fd69257a24c1f059cb6
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-15T13:56:02Z

    adding password to router rules; moving the advance code to the advance net 
topology.

commit 1bfd40ec997227401dcaf924b6cd11e89c77e978
Author: Antonio Fornie <afor...@schubergphilis.com>
Date:   2014-07-15T16:52:54Z

    Unify and encapsulate deployment flow methods and params

commit c571aa2bdb8f75a86fb6bac802b461ad17f35335
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-16T05:38:33Z

    adding userdata to router and ssh pub key to router rules.

commit c40ecf2def361496bbe7671d60e0e02a05e78f63
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-16T06:20:52Z

    making instance variables compliant with ACS convention

commit ee8b02715643c42539b715962d0189eb0913796e
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-16T07:30:10Z

    adding user data pwd rules

commit 770c12b26f89017be3866e989b3c1fa0a35b1066
Author: wrodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-16T12:24:48Z

    adding DHCP entry rules

commit 2abf343bdf049f75c7e8bfabd28e3c96b34e584a
Author: wrodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-16T13:01:59Z

    fixing injection of beans with a relationship

commit 565f77fb22f6df3ebe8d01297e031befd91da706
Author: Daan Hoogland <d...@onecht.net>
Date:   2014-07-16T14:57:54Z

    whitespace

commit 1cdcb1c98adac2141feb3c86f4b2a7c31db13248
Author: wrodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-16T15:43:09Z

    fixing the injection of the networkDao

commit d84929e216a2b89846accbff1d2f674b4c6f1f32
Author: Daan Hoogland <d...@onecht.net>
Date:   2014-07-16T15:43:34Z

    mv to better name
    
    oversimplified test added

commit bdef2f8d3939267ea91961a83c0c56ed739ff1ce
Author: Antonio Fornie <afor...@schubergphilis.com>
Date:   2014-07-16T17:11:47Z

    Deployment more OO - Objects with data and behavior

commit 4f93ef255c5370d78786a31525ee28ec54bf4b63
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-17T05:41:59Z

    replacing my IP by localhost to avoid problems with my environment

commit ee0389bb9a7ff8856b6fcb7274414ce4c57ddb5c
Author: Wilder Rodrigues <wrodrig...@schubergphilis.com>
Date:   2014-07-17T05:57:29Z

    fixing import in virtual router element and checkstyle in dhcp entry 
related changes

----


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to