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