> On March 14, 2013, 8:15 a.m., Hugo Trippaers wrote: > >
Hi Hugo, thanks for the quick review reply. > On March 14, 2013, 8:15 a.m., Hugo Trippaers wrote: > > deps/install-non-oss.sh, line 19 > > <https://reviews.apache.org/r/9898/diff/1/?file=270113#file270113line19> > > > > I'm not really thrilled by adding components to the non-oss build. > > > > What are the possibilities of making the the midonet-client.jar > > publicly available? > > > > If that is not possible it might be worthwhile to discuss if this > > plugin should be maintained in a separate repository. Making the client jar public is definitely possible - we were making it nonoss for the initial commit to avoid the jar distribution logistics, but we can certainly tackle it now if needed. How would it work exactly? Could we upload the jar to a maven repo and have the build download it as a dependency, and if so, which maven repo could we upload to? Is there a precedent in the code base we can refer to? > On March 14, 2013, 8:15 a.m., Hugo Trippaers wrote: > > plugins/hypervisors/kvm/pom.xml, line 37 > > <https://reviews.apache.org/r/9898/diff/1/?file=270114#file270114line37> > > > > This dependency should only be activated during the nonoss build. Nice catch. Let's see how the nonoss discussion above shakes out before addressing this. > On March 14, 2013, 8:15 a.m., Hugo Trippaers wrote: > > plugins/network-elements/midonet/src/com/cloud/network/resource/MidoNetVifDriver.java, > > line 55 > > <https://reviews.apache.org/r/9898/diff/1/?file=270123#file270123line55> > > > > Is this always fixed to this host/port or should this be configurable? This is configurable - see the "// Load Midonet API server location" section of the configure() method. - Dave ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9898/#review17865 ----------------------------------------------------------- On March 13, 2013, 9:32 a.m., Dave Cahill wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9898/ > ----------------------------------------------------------- > > (Updated March 13, 2013, 9:32 a.m.) > > > Review request for cloudstack, Hugo Trippaers and Chiradeep Vittal. > > > Description > ------- > > Feature spec: > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Midokura+Networking+Plugin > > Jira ticket: > https://issues.apache.org/jira/browse/CLOUDSTACK-996 > > Notes: > > * Moved plugin to nonoss since the MidoNet client jar is not publicly > available > > * Documentation will follow as a separate commit > > * One main difference from existing networking plugins is the lack of a > Resource class; we didn't feel it was necessary in this case. As mentioned in > Extending CloudStack Networking [1]: > "Just like managers, resources are not strictly necessary. In theory a > Network Element could implement a client for the API of the new controller > and therefore be completely self-contained." > > * We allow overriding Public traffic via the MidoNetPublicNetworkGuru. We > checked this approach with the list [2] and received no comments, so we're > going with it for now. > > [1] https://cwiki.apache.org/CLOUDSTACK/extending-cloudstack-networking.html > [2] http://markmail.org/message/k5qse63eyylszm3i > > > This addresses bug CLOUDSTACK-996. > > > Diffs > ----- > > api/src/com/cloud/network/Network.java efed5cd > api/src/com/cloud/network/Networks.java e3d2158 > api/src/com/cloud/network/PhysicalNetwork.java 343a2b1 > api/src/org/apache/cloudstack/network/ExternalNetworkDeviceManager.java > bc22804 > client/pom.xml cda6ab8 > client/tomcatconf/nonossComponentContext.xml.in 20e0c32 > deps/install-non-oss.sh 74575a8 > plugins/hypervisors/kvm/pom.xml 013a58d > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java > b622b6d > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java > c93aeeb > plugins/network-elements/midokura-midonet/pom.xml 7f2e2d3 > plugins/network-elements/midonet/pom.xml PRE-CREATION > > plugins/network-elements/midonet/src/com/cloud/network/element/MidoNetElement.java > 48833b3 > > plugins/network-elements/midonet/src/com/cloud/network/element/SimpleFirewallRule.java > PRE-CREATION > > plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetGuestNetworkGuru.java > ed0eb3c > > plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetPublicNetworkGuru.java > PRE-CREATION > > plugins/network-elements/midonet/src/com/cloud/network/resource/MidoNetVifDriver.java > PRE-CREATION > > plugins/network-elements/midonet/test/com/cloud/network/element/MidoNetElementTest.java > PRE-CREATION > plugins/pom.xml 88f617b > server/src/com/cloud/configuration/Config.java 64465a2 > server/src/com/cloud/network/NetworkManagerImpl.java 3220c91 > ui/scripts/system.js 4d529ae > > Diff: https://reviews.apache.org/r/9898/diff/ > > > Testing > ------- > > Built and deployed, spun up Advanced Isolated network with two VMs, verified > internal and external connectivity via MidoNet. > > > Thanks, > > Dave Cahill > >
