----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23257/#review47287 -----------------------------------------------------------
Thanks for submitting this review, great to see this happening. Besides a few points line by line there is a general remark i want to make. This plugin doesn't follow the agent resource model. Plugin generally follow this pattern to ensure that the code calling the external device is registered as an agent (this also gives you access to configuration parameters) and the orchestration bits are happening in the network guru layer. As i can see it this plugin should also follow that pattern, but if you have a reason not to work that way i'm interested to discuss this. Also configuration should be available in the API so administrators can programmatically use this plugin. It should not be something that is based on a configuration outside CloudStack. Hope you can address these items. If you want to discuss catch me on IRC or on the dev list. Cheers, Hugo debian/cloudstack-management.install <https://reviews.apache.org/r/23257/#comment82954> Why do you need a separate configuration file? This is typically done using the cloudstack configuration mechanism. Either by adding a network service provider or using configuration properties. Using separate configuration files means cloudstack administrators can't configure the system from within the UI or with APIs. Breaking a fundamental design principle. deps/install-non-oss.sh <https://reviews.apache.org/r/23257/#comment82955> Any possibility of getting this publicly available from a maven repository? What are the licensing restrictions that require this to be in the noredist build? plugins/network-elements/juniper-networkguru/pom.xml <https://reviews.apache.org/r/23257/#comment82956> It would be best to avoid repository statements in the pom files. This will break compilation for people using proxys and other maven management tools. Which dependency is not available on the regular maven systems? plugins/network-elements/juniper-networkguru/src/com/cloud/network/JuniperNDAPINaasServiceNetworkMapVO.java <https://reviews.apache.org/r/23257/#comment82957> Possible to fix this TODO before submitting? - Hugo Trippaers On July 3, 2014, 8:19 a.m., Pradeep HK wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23257/ > ----------------------------------------------------------- > > (Updated July 3, 2014, 8:19 a.m.) > > > Review request for cloudstack. > > > Bugs: CLOUDSTACK-5398 > https://issues.apache.org/jira/browse/CLOUDSTACK-5398 > > > Repository: cloudstack-git > > > Description > ------- > > Pls refer to the Design document available @ > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Cloudstack+network-element+plugin+to+orchestrate+Juniper%27s+switches+%28for+L2+services%29 > > This feature is about a Cloudstack network-element plugin to orchestrate > Juniper's switches. > > As a first-cut, we are focussing on L2 services and we will write a > NetworkGuru. As part of it's implement() method we will: > (1)Create the required logical interfaces on the Juniper switches (EX,QFX) > (2)Create the required VLANs on the Juniper switches (EX,QFX). > (3)Configure VLAN membership on the interfaces > > Our customers need this plugin in Cloudstack deployments to automatically > orchestrate the Juniper switches to create Virtual Networks. > Without this plugin, there will be a manual intervention needed to configure > the switches (after figuring out the > current configuration of the switch). > > We have a Network Management Platform (called JUNOS SPACE) which is heavily > used by customers to orchestrate Juniper's networking devices. > It also exposes REST-ful APIs for integration with 3rdParty tools. > The proposed Juniper's Cloudstack network-element plugin leverages these > REST-ful APIs to appropriately orchestrate Juniper's switches to > create Virtual Networks > > > Diffs > ----- > > client/pom.xml 29fef4f > debian/cloudstack-management.install ea3f93b > debian/rules 197e243 > deps/install-non-oss.sh 940bd32 > plugins/network-elements/juniper-networkguru/pom.xml PRE-CREATION > > plugins/network-elements/juniper-networkguru/resources/META-INF/cloudstack/junipernetworkguru/module.properties > PRE-CREATION > > plugins/network-elements/juniper-networkguru/resources/META-INF/cloudstack/junipernetworkguru/spring-junipernetworkguru-context.xml > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/JuniperNetworkGuru.properties > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/JuniperNDAPINaasServiceNetworkMapVO.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/guru/JuniperNetworkGuru.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/naas/dao/JuniperNDAPINaasServiceNetworkMapDao.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/naas/dao/JuniperNDAPINaasServiceNetworkMapDaoImpl.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/utils/DeviceInterfacesMap.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/utils/ELSJunosL2XMLConfigs.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/utils/JuniperNDAPIConstants.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/utils/JunosL2XMLConfigs.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/utils/LLDPNeighbors.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/utils/ShowHardwareModel.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/utils/Utils.java > PRE-CREATION > > plugins/network-elements/juniper-networkguru/src/com/cloud/network/utils/netconfOperations/NetconfOperations.java > PRE-CREATION > plugins/pom.xml b5e6a61 > > Diff: https://reviews.apache.org/r/23257/diff/ > > > Testing > ------- > > When a Network created from Cloudstack UI is assigned a vlan-id, the same > gets orchestrated on the Juniper switches. > > We have tested the above scenarios : > (1)Various Hypervisors like KVM,VMWare,Xen > (2)LLDP is enabled on switches and hosts (to get info abt the switch-port > connected to hosts) > (3)Integration with Network Director API 1.6 > > > Thanks, > > Pradeep HK > >