GitHub user wilderrodrigues opened a pull request:
https://github.com/apache/cloudstack/pull/14
Pull request of changes in the "cloud-server" module
In the last 10 weeks we have been working 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 started doing some integration tests
o Deployment of Basic and Advanced Zones
o Test Create Account and user for that account
o Test Sub domain allowed to launch VM when a Domain level zone
is created
o Test delete domain without force option
o Test delete domain with force option
o Test update admin details
o Test update domain admin details
o Test user update API
o Test login API with domain
o Test if Login API does not return UUID's
⢠VPC related tests were still done manually.
o Will have it automated as well.
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 and Antonio
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/schubergphilis/cloudstack visitor-rebase
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/14.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 #14
----
commit 14c86a5675c294a73d75d4d54e7c3a78e1b43dc2
Author: Antonio Fornie <[email protected]>
Date: 2014-07-03T17:17:26Z
Rules and visitors for Load Balance Rules
commit f9d89f54840b78feec2a2d77e5dfa79da3e206a7
Author: Wilder Rodrigues <[email protected]>
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 6b6da56522b2e8938e9668654d38b277e655f12e
Author: Antonio Fornie <[email protected]>
Date: 2014-07-10T14:54:01Z
Extract general behavior to Router and Vpc delegates
commit 9e4ce1985184aa3325fa8719be3bbe80b9664cdb
Author: Antonio Fornie <[email protected]>
Date: 2014-07-11T13:07:35Z
Fix dependency problem. Extract and unify router deployment stuff
commit e6675e554b5a03a30ab07910571887b92fb76fca
Author: Wilder Rodrigues <[email protected]>
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 3d9a3c3f7728b41babfd1e732dc72bb5135f2070
Author: Wilder Rodrigues <[email protected]>
Date: 2014-07-14T09:34:48Z
changing accessor modifier in instance variables
commit 8075ebeb97232895ac7afc181ba219b3aa22f798
Author: Wilder Rodrigues <[email protected]>
Date: 2014-07-14T09:52:51Z
fixing checkstyles
commit c44f177245127d29cd4e7497f8144e459e5dad93
Author: Wilder Rodrigues <[email protected]>
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 113e7faa04198249f144040ffbf19305f725ddfd
Author: Daan Hoogland <[email protected]>
Date: 2014-07-14T15:50:27Z
TODO
commit 852cc916e99aa297e254a488a6604d11454e6ad8
Author: Wilder Rodrigues <[email protected]>
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 b101ce07a3ee5037714c6e28a414dbd55dbac5a2
Author: Antonio Fornie <[email protected]>
Date: 2014-07-15T09:06:44Z
Fix offering setup
commit 2826d7c510b065ddd18d4dfb379f12a9532da4d4
Author: Wilder Rodrigues <[email protected]>
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 164026959d7c58524e5ba6215055b62b255d060d
Author: Wilder Rodrigues <[email protected]>
Date: 2014-07-15T07:38:21Z
adding apache license headers
commit 1b5570e7e4f5bc23363ef66600c4d264a5698246
Author: Wilder Rodrigues <[email protected]>
Date: 2014-07-15T08:59:42Z
adding Ip Association and VPN Rules
commit 8aae26d1d98c0850db8a00111985e1fb4019250d
Author: Antonio Fornie <[email protected]>
Date: 2014-07-15T09:54:58Z
Temporary put state info in a state object
commit 6e35db3b11299259b5d03292fa834e6b16ba8d39
Author: Daan Hoogland <[email protected]>
Date: 2014-07-15T09:28:49Z
package rename
commit d790f2d7a0096b40e4e2636c5095c6f82c9ecee7
Author: Wilder Rodrigues <[email protected]>
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 8f252ec06721b5b0330652f94fb7d8eb796247d9
Author: Antonio Fornie <[email protected]>
Date: 2014-07-15T16:52:54Z
Unify and encapsulate deployment flow methods and params
commit 239fbe670a9295a5838557e92a3ede581385e54f
Author: Wilder Rodrigues <[email protected]>
Date: 2014-07-15T13:56:02Z
adding password to router rules; moving the advance code to the advance net
topology.
commit 7e34dc8bc816f31020c08044a8cd287003364a32
Author: Wilder Rodrigues <[email protected]>
Date: 2014-07-16T05:38:33Z
adding userdata to router and ssh pub key to router rules.
commit 6b4cdab86ee3c10002737efeaaceff3e5235725a
Author: Wilder Rodrigues <[email protected]>
Date: 2014-07-16T06:20:52Z
making instance variables compliant with ACS convention
commit 427e9202131d3261ff96c99a73656928cb97f0f0
Author: Wilder Rodrigues <[email protected]>
Date: 2014-07-16T07:30:10Z
adding user data pwd rules
commit ff7fe0fe1fb85e7ae5d0f6ebebde4dfde574d531
Author: wrodrigues <[email protected]>
Date: 2014-07-16T12:24:48Z
adding DHCP entry rules
commit f0778b3a4e87daee5316fd8b07aaff719400294b
Author: wrodrigues <[email protected]>
Date: 2014-07-16T13:01:59Z
fixing injection of beans with a relationship
commit 04530cee2eeed197a0e7141697cb4cfb8f6fa035
Author: Daan Hoogland <[email protected]>
Date: 2014-07-16T14:57:54Z
whitespace
commit 228c6528180e70e8e1350f07473f62435b61a376
Author: wrodrigues <[email protected]>
Date: 2014-07-16T15:43:09Z
fixing the injection of the networkDao
commit 470c05a88eb45911f9c458307f90d2597fd5e081
Author: Daan Hoogland <[email protected]>
Date: 2014-07-16T15:43:34Z
mv to better name
oversimplified test added
commit e71848852a8232b7b06f5904271ca4612cc13f1b
Author: Antonio Fornie <[email protected]>
Date: 2014-07-16T17:11:47Z
Deployment more OO - Objects with data and behavior
commit b44253b70c4f524849766a4c084a880907349dc0
Author: Wilder Rodrigues <[email protected]>
Date: 2014-07-17T05:41:59Z
replacing my IP by localhost to avoid problems with my environment
commit b75d3f5272355d90efc21e23f1e65247c2a8619d
Author: Wilder Rodrigues <[email protected]>
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 [email protected] or file a JIRA ticket
with INFRA.
---
