> On July 8, 2014, 10:07 a.m., Hugo Trippaers wrote:
> > client/tomcatconf/log4j-cloud.xml.in, line 66
> > <https://reviews.apache.org/r/23282/diff/1/?file=624288#file624288line66>
> >
> >     Why do you introduce a new logfile specific for Nuage? Isn't is easier 
> > for admins if all cloudstack related messages appear in the already 
> > existing logfile that admins are familiar with?
> 
> Suresh Ramamurthy wrote:
>     Sure, I will remove our logger.

Fixed this comment. I removed NuageVsp logger.


> On July 8, 2014, 10:07 a.m., Hugo Trippaers wrote:
> > plugins/network-elements/nuage-vsp/resources/META-INF/cloudstack/vsp/vsp-defaults.ini,
> >  line 1
> > <https://reviews.apache.org/r/23282/diff/1/?file=624296#file624296line1>
> >
> >     Missing Apache License Header
> 
> Suresh Ramamurthy wrote:
>     Accepted, I will add the header. Sorry, I missed it.

This file is no longer needed as I am using global configuration


> On July 8, 2014, 10:07 a.m., Hugo Trippaers wrote:
> > plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java,
> >  line 103
> > <https://reviews.apache.org/r/23282/diff/1/?file=624341#file624341line103>
> >
> >     copy-paste error ;-)
> 
> Suresh Ramamurthy wrote:
>     :) I will fix it

Fixed this...


> On July 8, 2014, 10:07 a.m., Hugo Trippaers wrote:
> > plugins/network-elements/nuage-vsp/src/com/cloud/network/resource/NuageVspResource.java,
> >  line 196
> > <https://reviews.apache.org/r/23282/diff/1/?file=624333#file624333line196>
> >
> >     I'm not really agreeing with this method of loading the client. The 
> > client should be loaded using the classloader.
> >     
> >     Is the library available from maven central or is distribution 
> > restricted? If the latter case we need to move this plugin and the 
> > dependency on the library to the noredist build.
> 
> Suresh Ramamurthy wrote:
>     Yes, NuageVsp plugin client library is distribution restricted. I had 
> some design question regarding dropping the library and loading it 
> dynamically. Please find below our requirement'
>     
>     1) We wanted to drop our NuageVsp plugin client after installing 
> CloudStack. 
>     2) We wanted to create an NuageVsp plugin Client RPM and then install the 
> RPM that copies in a location. The location could be some other location that 
> NuageVsp Plugin client chooses.
>        In this case I thought of copying it under 
> /usr/share/cloudstack-management/webapps/client/WEB-INF/lib but it could be 
> different location
>     3) Then restart CloudStack.
>     4) When NuageVsp Device is added, then the class files will be 
> dynamically loaded and things will work seamlessly
>     
>     Advantages of above approach:
>     
>     1) Cloudstack build does not have any dependency on our jar files. This 
> is because, we do not have any reference of NuageVsp Plugin client code 
>        and the client classes are loaded dynamically
>     2) There is no maven central dependency
>     3) Version Upgrade of NuageVsp Plugin client does not depend on 
> Cloudstack version. We will just uninstall our RPM and re-install the new 
> version of NuageVsp Plugin client code and restart CloudStack
>     
>     Hugo, above was our suggestion. Is it okay to follow the above approach 
> of loading the Plugin Client files? 
>     
>     If the above approach is not acceptable then we are open to follow the 
> noredist build approach. If we are following the noredist build approach then 
> I just want to clarify the procedure to be
>     followed to use NuageVsp Plugin
>     
>     1) User needs to download NuageVsp Plugin client jar file
>     2) Copy the jar under deps folder
>     3) Run install-non-oss.sh
>     4) Build RPM using -p NOREDIST option
>     
>     Could you please let me know details of the forum where we can have 
> design discussions?
>     
>     I know, I am asking too many question :)
>     
>     Thanks reviewing and supporting us.
>
> 
> Hugo Trippaers wrote:
>     That makes sense, thanks for the clarification. I think we can work that 
> way, actually our plugin system is designed to work in such a way that the 
> entire plugin could technically be separated into an rpm package. 
>     
>     Probably the best way to go forward is to agree on a file location. 
> Probably something like /usr/share/nuage/lib for unix alike systems is a 
> better place to store the library. The actual location would need to be 
> configured as a global setting in CloudStack and passed to the resource 
> during initialization. 
>     
>     However to use this type of loading i would like to make sure that a user 
> will receive a clear error message explaining that the correct library is not 
> installed when it tries to use it. 
>     
>     For functional testing this introduces number of problems as only people 
> with access to those libraries can do any kind of testing on the library or 
> are you delivering a stub together with the functional tests so we can 
> integrate it in the functional test stream.
>     
>     In short, i'm ok with this type of loading, provided:
>       * we have a way of testing the plugin or execute functional tests 
> against something that has access to the libraries.
>       * the cloudstack plugin documentation makes it clear that the plugin 
> requires an additional library.
>       * the user receives an error message when he tries to configure the 
> nauge plugin in cloudstack without the library.
>     
>     I'll take the question whether or not we need to put this plugin in the 
> noredist build to the dev-list as i would like some more input on that.
> 
> Suresh Ramamurthy wrote:
>     Hi Hugo,
>     
>     Thanks for quick response.
>     
>     Please find my response below for you comments
>     
>       * we have a way of testing the plugin or execute functional tests 
> against something that has access to the libraries.
>     
>     <Suresh> We are doing functional testing here at Nuage Networks. Will 
> community also perform functional test. Could you 
>              please clarify me regarding what kind of functional testing does 
> the community perform. Could you please point me
>              to any document? As our plugin will need additional library and 
> an SDN setup, how will community test it. Let me 
>              know so that I can work on that ASAP
>     
>       * the cloudstack plugin documentation makes it clear that the plugin 
> requires an additional library.
>     
>     <Suresh> Yes, I will update the NuageVsp plugin design spec regarding the 
> additional library once the process is approved by you 
>              and others in community
>     
>       * the user receives an error message when he tries to configure the 
> nauge plugin in cloudstack without the library.
>     
>     <Suresh> The way I have written the logic in NuageVspResource is if the 
> client library is not configured, we throw error message and also log the 
> error 
>              message in log files.
>     
>     I am perfectly fine with the location that you mentioned and it makes 
> sense. And, I can make is configurable too in global properties.
>     May be in future, as an enhancement, we can also make this location as a 
> common location for all the network plugins to drop their client libraries.
>

As per our discussion, I am using a different location and it is configurable 
in global configuration. I have updated the design spec to mention about this 
client plugin to be installed


> On July 8, 2014, 10:07 a.m., Hugo Trippaers wrote:
> > plugins/network-elements/nuage-vsp/resources/META-INF/cloudstack/vsp/vsp-defaults.ini,
> >  line 10
> > <https://reviews.apache.org/r/23282/diff/1/?file=624296#file624296line10>
> >
> >     Please do not make any assumptions on the locations of jar files. That 
> > is up to the people doing the packaging and might vary from distribution to 
> > distribution. Expect everything to be available using the web app class 
> > loader.
> 
> Suresh Ramamurthy wrote:
>     Could you please at my comments for dynamic class loader...

I am using a different location that configurable in global configuration


> On July 8, 2014, 10:07 a.m., Hugo Trippaers wrote:
> > plugins/network-elements/nuage-vsp/src/com/cloud/network/resource/NuageVspResource.java,
> >  line 95
> > <https://reviews.apache.org/r/23282/diff/1/?file=624333#file624333line95>
> >
> >     Don't depend on file locations as they will change. Use the java class 
> > loader to load any libraries.
> 
> Suresh Ramamurthy wrote:
>     Could you please at my comments for dynamic class loader...

Fixed this. I am using a different location as per our discussion..


> On July 8, 2014, 10:07 a.m., Hugo Trippaers wrote:
> > plugins/network-elements/nuage-vsp/resources/META-INF/cloudstack/vsp/vsp-defaults.ini,
> >  line 4
> > <https://reviews.apache.org/r/23282/diff/1/?file=624296#file624296line4>
> >
> >     Why introduce another configuration file? CloudStack provides options 
> > to set configuration using either network service provides or global 
> > configuration.
> 
> Suresh Ramamurthy wrote:
>     noOfSyncThreads=5
>     syncUpIntervalInMinutes=480
>     
>     Both the above configurations are for NuageVsp plugin sync behavior. So, 
> I will add them to Global configuration.

This file is no longer needed as I am using global configuration


- Suresh


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


On July 14, 2014, 7:29 a.m., Suresh Ramamurthy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23282/
> -----------------------------------------------------------
> 
> (Updated July 14, 2014, 7:29 a.m.)
> 
> 
> Review request for cloudstack and Hugo Trippaers.
> 
> 
> Bugs: CLOUDSTACK-6845
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6845
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> This is first code drop for NuageVsp Network plugin support that will bring 
> the Nuage VSP network virtualization technology to CloudStack.
> 
> We need a new branch to checkin the fixes once the review is done. Please 
> create a new branch for NuageVsp plugin.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/event/EventTypes.java 71bfdb6 
>   api/src/com/cloud/network/Network.java 885bffe 
>   api/src/com/cloud/network/Networks.java 1e4d229 
>   api/src/com/cloud/network/PhysicalNetwork.java 8cc214e 
>   client/WEB-INF/classes/resources/messages.properties b192cb0 
>   client/WEB-INF/classes/resources/messages_zh_CN.properties 1ec4e95 
>   client/pom.xml 46933d9 
>   client/tomcatconf/commands.properties.in b9ac27b 
>   
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
>  8e4c710 
>   
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
>  15eeb13 
>   
> plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
>  a9840bd 
>   plugins/network-elements/nuage-vsp/pom.xml PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/resources/META-INF/cloudstack/vsp/module.properties
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/resources/META-INF/cloudstack/vsp/spring-vsp-context.xml
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/StartupVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/VspResourceAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/VspResourceCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/element/ApplyAclRuleVspAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/element/ApplyAclRuleVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/element/ApplyStaticNatVspAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/element/ApplyStaticNatVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/element/ShutDownVpcVspAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/element/ShutDownVpcVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/DeallocateVmVspAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/DeallocateVmVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/ImplementNetworkVspAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/ImplementNetworkVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/ReleaseVmVspAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/ReleaseVmVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/ReserveVmInterfaceVspAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/ReserveVmInterfaceVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/TrashNetworkVspAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/guru/TrashNetworkVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/sync/SyncVspAnswer.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/agent/api/sync/SyncVspCommand.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/AddNuageVspDeviceCmd.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/DeleteNuageVspDeviceCmd.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/IssueNuageVspResourceRequestCmd.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/ListNuageVspDevicesCmd.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/VspConstants.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/api/response/NuageVspDeviceResponse.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/api/response/NuageVspResourceResponse.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/NuageVspDeviceVO.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/dao/NuageVspDao.java 
> PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/dao/NuageVspDaoImpl.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/element/NuageVspElement.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/element/NuageVspVpcElement.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/guru/NuageVspGuestNetworkGuru.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManager.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManagerImpl.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/resource/NuageVspResource.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/sync/NuageVspSync.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/com/cloud/network/sync/NuageVspSyncImpl.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/net/nuage/vsp/acs/NuageVspPluginClientLoader.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/net/nuage/vsp/acs/client/NuageVspApiClient.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/net/nuage/vsp/acs/client/NuageVspElementClient.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/net/nuage/vsp/acs/client/NuageVspGuruClient.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/src/net/nuage/vsp/acs/client/NuageVspSyncClient.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java
>  PRE-CREATION 
>   
> plugins/network-elements/nuage-vsp/test/com/cloud/network/guru/NuageVspGuestNetworkGuruTest.java
>  PRE-CREATION 
>   plugins/pom.xml 802e2ea 
>   server/src/com/cloud/api/ApiResponseHelper.java 51122e0 
>   server/src/com/cloud/configuration/Config.java fbcb1f4 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java 1940f48 
>   server/src/com/cloud/network/NetworkServiceImpl.java c8105e8 
>   server/src/com/cloud/network/vpc/VpcManagerImpl.java c7237c1 
>   setup/db/db/schema-440to450.sql d047060 
>   tools/apidoc/gen_toc.py 827d6bf 
>   ui/dictionary.jsp e9d84de 
>   ui/scripts/configuration.js 9311e37 
>   ui/scripts/docs.js 74a08bc 
>   ui/scripts/system.js 9012580 
>   ui/scripts/ui-custom/zoneWizard.js 4091c03 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 
> dd55439 
> 
> Diff: https://reviews.apache.org/r/23282/diff/
> 
> 
> Testing
> -------
> 
> Nuage VSP plugin depends on following components of Nuage SDN solution
> a) Nuage VSD
> b) Nuage VSC
> c) Nuage VRS, this needs installed on the Hypervisor
> All the above components needs to be provisioned for the plugin to function 
> properly. Also, Nuage VSP plugin directly talks with Nuage VSD using Rest 
> API. So, all the components needs to be running to test the plugin 
> functionality.
> 
> The following tests are tested
> 
> Isolated Network Test Cases
> 
> a) Create a network offering with default egress deny rule and select 
> services supported by Nuage VSP plugin. Choose NuageVsp as the service 
> provider for DHCP, SourceNAT, StaticNAT, Firewall and Virtual Networking 
> services.
>    Choose VirtualRouter as the service provider for UserData service.
> b) Create an isolated Network with network offering created above
> c) Spawn 2 VMs. Verify that VMs should each get an IP address. They should 
> ping each other. Verify that SSH to a box on the external network should fail
> b) Create a Static NAT and associate it one of the VM. Add an Egress rule for 
> the network with source CIDR as 0.0.0.0/0, protocol as TCP and ssh port number
> d) Verify that SSH to box that is in the external network should work
> e) Verify that Password reset for the VM should work
> 
> VPC Test Cases
> 
> a) Create a network offering for VPC with default deny all rule and select 
> services supported by Nuage VSP plugin for VPC. Choose NuageVsp as the 
> service provider for DHCP, SourceNAT, StaticNAT and Virtual Networking 
> services. Choose NuageVspVpc for NerworkACL service. 
> b) Create an VPC and select "Default VPC offering with NuageVsp" as the VPC 
> offering
> c) Create a tier and select the network offering created above
> c) Spawn 2 VMs. Verify that VMs should each get an IP address. They should 
> ping each other. SSH to a box on the external network should fail
> d) Create a Static NAT and associate it one of the VM
> e) Add an Network ACL Egress rule for the network with source CIDR as 
> 0.0.0.0/0, protocol as TCP and ssh port number
> f) Verify that SSH to box that is in the external network should work
> 
> 
> Thanks,
> 
> Suresh Ramamurthy
> 
>

Reply via email to