You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Suresh Balineni <sb...@juniper.net> on 2014/02/27 01:47:06 UTC

Review Request 18552: Internal LB support for Juniper contrail VPC implementation

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

Review request for cloudstack.


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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Alena Prokharchyk <al...@citrix.com>.

> 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.
> > 
> > 
> >
> 
> Suresh Balineni wrote:
>     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.
>

1) "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."

You can never base a decision whether to create a new provider, just base on the offering presence. You should create a new provider only if its functionality is different. The reason why internal lb provider was implemented, is - it has diff set of services/capabilities supported, and the logic for it deployment is different for other routers. 

2) "is except this LB VM nics should be created by contrail vrouter. ". I don't see the code in ContrailManagerImpl creating nics fot the Contrail Internal lb. Can you please point out to that. Or are you saying, that the ContrailVpcElementImpl and ContrailInternalLbElementImpl.java, refer to the same VR element in Contrail network? If your ContrailVpcElementImpl and ContrailInternalLbElementImpl refer to the same physical provider (contrail VR), they should be combined into the same provider/networkElement. If its a separate VR managed just the way INternal LB vm is managed, I would like to see what is diff in the process of nic creating for your internal lb provider as opposed to VPC internal lb provider. 


- Alena


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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.

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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Alena Prokharchyk <al...@citrix.com>.

> 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.
> > 
> > 
> >
> 
> Suresh Balineni wrote:
>     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.
>
> 
> Alena Prokharchyk wrote:
>     1) "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."
>     
>     You can never base a decision whether to create a new provider, just base on the offering presence. You should create a new provider only if its functionality is different. The reason why internal lb provider was implemented, is - it has diff set of services/capabilities supported, and the logic for it deployment is different for other routers. 
>     
>     2) "is except this LB VM nics should be created by contrail vrouter. ". I don't see the code in ContrailManagerImpl creating nics fot the Contrail Internal lb. Can you please point out to that. Or are you saying, that the ContrailVpcElementImpl and ContrailInternalLbElementImpl.java, refer to the same VR element in Contrail network? If your ContrailVpcElementImpl and ContrailInternalLbElementImpl refer to the same physical provider (contrail VR), they should be combined into the same provider/networkElement. If its a separate VR managed just the way INternal LB vm is managed, I would like to see what is diff in the process of nic creating for your internal lb provider as opposed to VPC internal lb provider. 
>     
>     
>
> 
> Suresh Balineni wrote:
>     Contrail VPC Implementation has its own VPC Network Offering name. Please take a look at ContrailManagerImpl: locateNetworkOffering(for networks - vpcRouterOfferingName), locateVpcOffering(for vpc - juniperVPCOfferingName). When VPC is created using this vpc offering, networks created under this vpc should have "vpcRouterOfferingName". This VPC network offering should support "Service.Lb", but I wanted to re-use the existing Internal Lb implementation as is. 
>     
>     >>If your ContrailVpcElementImpl and ContrailInternalLbElementImpl refer to the same physical provider (contrail VR), they should be combined into the same provider/networkElement.
>     
>     That is correct, but I can not combine them since I wanted to re-use existing Internal Lb implementation.
>     
>     
>      
>     
>

Suresh, as I said, network offering DOESTN'T matter for making a decision whether to create a new provider. In your case - one single VR instance - all you have to do is - add LB service with capability scheme=Internal, to your existing contrail implementation. And add related LB methods to it.

You can't have 2 CS Providers mapped to the same physical provider. Look at the NetscalerElement. It also implementes internal lb functionality.


- Alena


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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.

> 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.
> > 
> > 
> >
> 
> Suresh Balineni wrote:
>     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.
>
> 
> Alena Prokharchyk wrote:
>     1) "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."
>     
>     You can never base a decision whether to create a new provider, just base on the offering presence. You should create a new provider only if its functionality is different. The reason why internal lb provider was implemented, is - it has diff set of services/capabilities supported, and the logic for it deployment is different for other routers. 
>     
>     2) "is except this LB VM nics should be created by contrail vrouter. ". I don't see the code in ContrailManagerImpl creating nics fot the Contrail Internal lb. Can you please point out to that. Or are you saying, that the ContrailVpcElementImpl and ContrailInternalLbElementImpl.java, refer to the same VR element in Contrail network? If your ContrailVpcElementImpl and ContrailInternalLbElementImpl refer to the same physical provider (contrail VR), they should be combined into the same provider/networkElement. If its a separate VR managed just the way INternal LB vm is managed, I would like to see what is diff in the process of nic creating for your internal lb provider as opposed to VPC internal lb provider. 
>     
>     
>

Contrail VPC Implementation has its own VPC Network Offering name. Please take a look at ContrailManagerImpl: locateNetworkOffering(for networks - vpcRouterOfferingName), locateVpcOffering(for vpc - juniperVPCOfferingName). When VPC is created using this vpc offering, networks created under this vpc should have "vpcRouterOfferingName". This VPC network offering should support "Service.Lb", but I wanted to re-use the existing Internal Lb implementation as is. 

>>If your ContrailVpcElementImpl and ContrailInternalLbElementImpl refer to the same physical provider (contrail VR), they should be combined into the same provider/networkElement.

That is correct, but I can not combine them since I wanted to re-use existing Internal Lb implementation.


 


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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/#review37581
-----------------------------------------------------------


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.




- Alena Prokharchyk


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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/#review39594
-----------------------------------------------------------


Suresh, the patch fails to apply on the latest 4.4 branch, can you please check whats going on, and upload a fresh one?

alena@alenas-air: [4.4]~/cloudstack-oss$ git am --signoff < ~/Desktop/internal_lb_per_alena_comments.patch
Applying: Per Alena comments, it is not required to introduce a new Provider for Internal LB
error: patch failed: plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java:277
error: plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java: patch does not apply
Patch failed at 0001 Per Alena comments, it is not required to introduce a new Provider for Internal LB
The copy of the patch that failed is found in:
   /Users/alena/cloudstack-oss/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

- Alena Prokharchyk


On March 27, 2014, 12:42 a.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 12:42 a.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
> -----
> 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java 01be7db 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java fe49981 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Alena Prokharchyk <al...@citrix.com>.

> On April 4, 2014, 10:45 p.m., Alena Prokharchyk wrote:
> > Ship It!

Fixed in 4.4 and master branches


- Alena


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


On April 4, 2014, 10:27 p.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated April 4, 2014, 10:27 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
> -----
> 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java bf083fd 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java 7e1ba96 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/#review39608
-----------------------------------------------------------

Ship it!


Ship It!

- Alena Prokharchyk


On April 4, 2014, 10:27 p.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated April 4, 2014, 10:27 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
> -----
> 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java bf083fd 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java 7e1ba96 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/
-----------------------------------------------------------

(Updated April 4, 2014, 10:27 p.m.)


Review request for cloudstack and Alena Prokharchyk.


Changes
-------

Thanks Alena,

Please find latest 4.4 based diff.


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 (updated)
-----

  plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
  plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java bf083fd 
  server/src/com/cloud/network/vpc/VpcManagerImpl.java 7e1ba96 

Diff: https://reviews.apache.org/r/18552/diff/


Testing
-------

Tested LB VM deployment. Traffic tests are done.


Thanks,

Suresh Balineni


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Alena Prokharchyk <al...@citrix.com>.

> On March 27, 2014, 5:02 p.m., Alena Prokharchyk wrote:
> > Suresh, your fix contradicts with whatever you said in your previous comment. You said that your contrail element will play the role of the internal load Balancer. In that case, you should have added the Service LB with capability LbSchemas="internal" to your contrail provider implementation.
> > 
> > The current fix though implies that the separate Internal LB vm will be started? Please elaborate.
> 
> Suresh Balineni wrote:
>     I wanted to re-use the existing Internal LB element as is except required Nic resources should be implemented by Contrail VRouter. 
>     
>

Suresh, InternalLbElement is being managed by InternalLBElementManager. The nics are created there. If you want the rules to be programmed on your VPC contrail element, use your VPC contrail element as a provider for internal lb. With the current code in the patch, the internal lb vm will be created as a separate vm from your contrail vm, with the nics configured the standard way.


- Alena


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


On March 27, 2014, 12:42 a.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 12:42 a.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
> -----
> 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java 01be7db 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java fe49981 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.

> On March 27, 2014, 5:02 p.m., Alena Prokharchyk wrote:
> > Suresh, your fix contradicts with whatever you said in your previous comment. You said that your contrail element will play the role of the internal load Balancer. In that case, you should have added the Service LB with capability LbSchemas="internal" to your contrail provider implementation.
> > 
> > The current fix though implies that the separate Internal LB vm will be started? Please elaborate.

I wanted to re-use the existing Internal LB element as is except required Nic resources should be implemented by Contrail VRouter. 

  


- Suresh


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


On March 27, 2014, 12:42 a.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 12:42 a.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
> -----
> 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java 01be7db 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java fe49981 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.

> On March 27, 2014, 5:02 p.m., Alena Prokharchyk wrote:
> > Suresh, your fix contradicts with whatever you said in your previous comment. You said that your contrail element will play the role of the internal load Balancer. In that case, you should have added the Service LB with capability LbSchemas="internal" to your contrail provider implementation.
> > 
> > The current fix though implies that the separate Internal LB vm will be started? Please elaborate.
> 
> Suresh Balineni wrote:
>     I wanted to re-use the existing Internal LB element as is except required Nic resources should be implemented by Contrail VRouter. 
>     
>
> 
> Alena Prokharchyk wrote:
>     Suresh, InternalLbElement is being managed by InternalLBElementManager. The nics are created there. If you want the rules to be programmed on your VPC contrail element, use your VPC contrail element as a provider for internal lb. With the current code in the patch, the internal lb vm will be created as a separate vm from your contrail vm, with the nics configured the standard way.

Hi Alena,

1. Contrail does not launch any VM for VPC. Routing support is provided by separate kernel module called contrail-vrouter. It does not support internal lb. I want to leverage CS Virtual-Router load balancing capabilities for supporting internal lb in contrail vpc implementation. I just have to create Nics in contrail vrouter for internal lb vm.

2. Since Contrail Network Offering is configured for Internal LB VM, "contrail guru" gets invoked when Internal LB VM is getting launched. 

           for (Service svc : services) {
            if (svc == Service.Lb) {
                if(offeringName.equals(vpcRouterOfferingName)) {
                    Set<Provider> lbProviderSet = new HashSet<Provider>();
                    lbProviderSet.add(Provider.InternalLbVm);
                    serviceProviderMap.put(svc, lbProviderSet);
                }
                continue;
            }
            serviceProviderMap.put(svc, providerSet);
        }
        ConfigurationManager configMgr = (ConfigurationManager)_configService;
        NetworkOfferingVO voffer =
                configMgr.createNetworkOffering(offeringName, offeringDisplayText, TrafficType.Guest, null, false,         Availability.Optional, null, serviceProviderMap, true,
                        Network.GuestType.Isolated, false, null, false, null, false, true, null, true, null, false);

3. I verified this implementation, It works perfectly fine.


- Suresh


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


On March 27, 2014, 12:42 a.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 12:42 a.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
> -----
> 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java 01be7db 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java fe49981 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.

> On March 27, 2014, 5:02 p.m., Alena Prokharchyk wrote:
> > Suresh, your fix contradicts with whatever you said in your previous comment. You said that your contrail element will play the role of the internal load Balancer. In that case, you should have added the Service LB with capability LbSchemas="internal" to your contrail provider implementation.
> > 
> > The current fix though implies that the separate Internal LB vm will be started? Please elaborate.
> 
> Suresh Balineni wrote:
>     I wanted to re-use the existing Internal LB element as is except required Nic resources should be implemented by Contrail VRouter. 
>     
>
> 
> Alena Prokharchyk wrote:
>     Suresh, InternalLbElement is being managed by InternalLBElementManager. The nics are created there. If you want the rules to be programmed on your VPC contrail element, use your VPC contrail element as a provider for internal lb. With the current code in the patch, the internal lb vm will be created as a separate vm from your contrail vm, with the nics configured the standard way.
> 
> Suresh Balineni wrote:
>     Hi Alena,
>     
>     1. Contrail does not launch any VM for VPC. Routing support is provided by separate kernel module called contrail-vrouter. It does not support internal lb. I want to leverage CS Virtual-Router load balancing capabilities for supporting internal lb in contrail vpc implementation. I just have to create Nics in contrail vrouter for internal lb vm.
>     
>     2. Since Contrail Network Offering is configured for Internal LB VM, "contrail guru" gets invoked when Internal LB VM is getting launched. 
>     
>                for (Service svc : services) {
>                 if (svc == Service.Lb) {
>                     if(offeringName.equals(vpcRouterOfferingName)) {
>                         Set<Provider> lbProviderSet = new HashSet<Provider>();
>                         lbProviderSet.add(Provider.InternalLbVm);
>                         serviceProviderMap.put(svc, lbProviderSet);
>                     }
>                     continue;
>                 }
>                 serviceProviderMap.put(svc, providerSet);
>             }
>             ConfigurationManager configMgr = (ConfigurationManager)_configService;
>             NetworkOfferingVO voffer =
>                     configMgr.createNetworkOffering(offeringName, offeringDisplayText, TrafficType.Guest, null, false,         Availability.Optional, null, serviceProviderMap, true,
>                             Network.GuestType.Isolated, false, null, false, null, false, true, null, true, null, false);
>     
>     3. I verified this implementation, It works perfectly fine.
> 
> Alena Prokharchyk wrote:
>     Suresh, ok, please confirm that the following is correct.
>     
>     When internalLb is used in contrail network, internalLB VM will be launched in CS to handle the service. So you do expect CS to invoke internal lb vm, and this vm will have nics configured by your guru.
>     
>     
>     If the above is correct, I'm going to ship the fix.
>

Yes, That's correct.


- Suresh


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


On March 27, 2014, 12:42 a.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 12:42 a.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
> -----
> 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java 01be7db 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java fe49981 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Alena Prokharchyk <al...@citrix.com>.

> On March 27, 2014, 5:02 p.m., Alena Prokharchyk wrote:
> > Suresh, your fix contradicts with whatever you said in your previous comment. You said that your contrail element will play the role of the internal load Balancer. In that case, you should have added the Service LB with capability LbSchemas="internal" to your contrail provider implementation.
> > 
> > The current fix though implies that the separate Internal LB vm will be started? Please elaborate.
> 
> Suresh Balineni wrote:
>     I wanted to re-use the existing Internal LB element as is except required Nic resources should be implemented by Contrail VRouter. 
>     
>
> 
> Alena Prokharchyk wrote:
>     Suresh, InternalLbElement is being managed by InternalLBElementManager. The nics are created there. If you want the rules to be programmed on your VPC contrail element, use your VPC contrail element as a provider for internal lb. With the current code in the patch, the internal lb vm will be created as a separate vm from your contrail vm, with the nics configured the standard way.
> 
> Suresh Balineni wrote:
>     Hi Alena,
>     
>     1. Contrail does not launch any VM for VPC. Routing support is provided by separate kernel module called contrail-vrouter. It does not support internal lb. I want to leverage CS Virtual-Router load balancing capabilities for supporting internal lb in contrail vpc implementation. I just have to create Nics in contrail vrouter for internal lb vm.
>     
>     2. Since Contrail Network Offering is configured for Internal LB VM, "contrail guru" gets invoked when Internal LB VM is getting launched. 
>     
>                for (Service svc : services) {
>                 if (svc == Service.Lb) {
>                     if(offeringName.equals(vpcRouterOfferingName)) {
>                         Set<Provider> lbProviderSet = new HashSet<Provider>();
>                         lbProviderSet.add(Provider.InternalLbVm);
>                         serviceProviderMap.put(svc, lbProviderSet);
>                     }
>                     continue;
>                 }
>                 serviceProviderMap.put(svc, providerSet);
>             }
>             ConfigurationManager configMgr = (ConfigurationManager)_configService;
>             NetworkOfferingVO voffer =
>                     configMgr.createNetworkOffering(offeringName, offeringDisplayText, TrafficType.Guest, null, false,         Availability.Optional, null, serviceProviderMap, true,
>                             Network.GuestType.Isolated, false, null, false, null, false, true, null, true, null, false);
>     
>     3. I verified this implementation, It works perfectly fine.

Suresh, ok, please confirm that the following is correct.

When internalLb is used in contrail network, internalLB VM will be launched in CS to handle the service. So you do expect CS to invoke internal lb vm, and this vm will have nics configured by your guru.


If the above is correct, I'm going to ship the fix.


- Alena


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


On March 27, 2014, 12:42 a.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 12:42 a.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
> -----
> 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java 01be7db 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java fe49981 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/#review38754
-----------------------------------------------------------


Suresh, your fix contradicts with whatever you said in your previous comment. You said that your contrail element will play the role of the internal load Balancer. In that case, you should have added the Service LB with capability LbSchemas="internal" to your contrail provider implementation.

The current fix though implies that the separate Internal LB vm will be started? Please elaborate.

- Alena Prokharchyk


On March 27, 2014, 12:42 a.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 12:42 a.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
> -----
> 
>   plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
>   plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java 01be7db 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java fe49981 
> 
> Diff: https://reviews.apache.org/r/18552/diff/
> 
> 
> Testing
> -------
> 
> Tested LB VM deployment. Traffic tests are done.
> 
> 
> Thanks,
> 
> Suresh Balineni
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/
-----------------------------------------------------------

(Updated March 27, 2014, 12:42 a.m.)


Review request for cloudstack and Alena Prokharchyk.


Changes
-------

Thanks Alena. You are right. I don't need to introduce a new provider/element in this case.
Please find the new diff attached.


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 (updated)
-----

  plugins/network-elements/juniper-contrail/pom.xml 8c6877d 
  plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java 01be7db 
  server/src/com/cloud/network/vpc/VpcManagerImpl.java fe49981 

Diff: https://reviews.apache.org/r/18552/diff/


Testing
-------

Tested LB VM deployment. Traffic tests are done.


Thanks,

Suresh Balineni


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.
-----------------------------------------------------------
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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/
-----------------------------------------------------------

(Updated March 4, 2014, 10:11 p.m.)


Review request for cloudstack.


Changes
-------

Hi Alena,

Could you review the change in VpcManagerImpl.java?


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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/
-----------------------------------------------------------

(Updated March 4, 2014, 10:10 p.m.)


Review request for cloudstack.


Changes
-------

Hi Alena,

Could you review the change in VpcManagerImpl.java?


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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/
-----------------------------------------------------------

(Updated March 1, 2014, 10:02 p.m.)


Review request for cloudstack.


Changes
-------

Fixed spell and added re-throw exception.


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 (updated)
-----

  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


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Pedro Marques <pe...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/#review35757
-----------------------------------------------------------



plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailInternalLbElementImpl.java
<https://reviews.apache.org/r/18552/#comment66489>

    typo: hanling vs handling.



plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java
<https://reviews.apache.org/r/18552/#comment66491>

    Shouldn't the exception be re-throwed ?


- Pedro Marques


On Feb. 27, 2014, 12:56 a.m., Suresh Balineni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18552/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2014, 12:56 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> 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
> 
>


Re: Review Request 18552: Internal LB support for Juniper contrail VPC implementation

Posted by Suresh Balineni <sb...@juniper.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18552/
-----------------------------------------------------------

(Updated Feb. 27, 2014, 12:56 a.m.)


Review request for cloudstack.


Changes
-------

added a new file (ContrailInternalLbElementImpl).


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 (updated)
-----

  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