cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Suresh Balineni" <sbalin...@juniper.net>
Subject Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation
Date Sun, 23 Mar 2014 22:48:06 GMT


> On March 18, 2014, 6:19 p.m., Alena Prokharchyk wrote:
> > 1) Why you've decided to introduce a new provider for the Internal LB? What different
does it make from the class its extending? I can't find anything different, neither in capabilities
terms, nor in other behavior. Why your feature can't just use existing InternalLbProvider?
Please update
> > 
> > 2) If you introduce a new provider element, how are you planning to add/configure/stop-start
it? I dont see you've introduced any new APIs for that matter. Adding a provider is much more
than just adding the class for the element. Its also adding the API logic for handling all
the operations. Look at api package, commands/internallb folder. Each element is gotta have
its own set of APIs.
> > 
> > 
> >

Hi Alena,

>>1) Why you've decided to introduce a new provider for the Internal LB? What different
does it make from the class its >.extending? I can't find anything different, neither in
capabilities terms, nor in other behavior. Why your feature >>can't just use existing
InternalLbProvider? Please update

Contrail plugin has its own Network offering for VPC Networks. Please take a look at ContrailManagerImpl
class. Existing Internal LB Provider uses Default Network Offering, default network offering
does not work with Contrail VPC. My intention was to use the Internal LB functionality as
it is except this LB VM nics should be created by contrail vrouter.  

>>2) If you introduce a new provider element, how are you planning to add/configure/stop-start
it? I dont see you've >>introduced any new APIs for that matter. Adding a provider is
much more than just adding the class for the element. Its >>also adding the API logic
for handling all the operations. Look at api package, commands/internallb folder. Each >>element
is gotta have its own set of APIs.

As per CS implementation Provider<->Network Element must have one-one mapping. Since
I introduced new Provider, there must be a new Network Element.
Since I extended InternalLBElement, all the functionality it provides is re-used(like internal
lb vm start/start/configure) in the derived class(except the Provider). When this VM needs
Nic resources, contrail network element will get invoked. 

Each element need not have its own set of APIs say for example, if we add a new Network Element
for Network creation/deletion, we don't need to add a new set of APIs. This new network element
can get invoked only if we set the associated Network offering used by new network element
while creating a network. Another example is, we added a new ContrailVPCElement without any
new set of APIs. This VPC Element uses existing CS VPC APIs. It is just that, based on the
network offering chosen the new Contrail VPC Element can get invoked.


- Suresh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/#review37581
-----------------------------------------------------------


On March 12, 2014, 6:19 p.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 6:19 p.m.)
> 
> 
> Review request for cloudstack and Alena Prokharchyk.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Internal LB support for Juniper contrail VPC implementation.
> 
> - This implementation just extends the existing implementation of internal lb.
> - New element  uses juniper contrail network offering so that nics are impelemented by
contrail element. 
> - LB VM deployment functionality is reused (new element is extended from existing Internal
LB element implementation).
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/network/Network.java 6dc6752 
>   api/src/org/apache/cloudstack/api/BaseCmd.java 0e83cee 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/resources/META-INF/cloudstack/contrail/spring-contrail-context.xml
99ab02e 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailInternalLbElementImpl.java
PRE-CREATION 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java
01be7db 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java 2157eac 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message