> 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
> 
>

Reply via email to