Return-Path: X-Original-To: apmail-cloudstack-dev-archive@www.apache.org Delivered-To: apmail-cloudstack-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 8AEF110E8E for ; Tue, 15 Oct 2013 14:06:34 +0000 (UTC) Received: (qmail 54593 invoked by uid 500); 15 Oct 2013 14:06:33 -0000 Delivered-To: apmail-cloudstack-dev-archive@cloudstack.apache.org Received: (qmail 54548 invoked by uid 500); 15 Oct 2013 14:06:33 -0000 Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list dev@cloudstack.apache.org Received: (qmail 54538 invoked by uid 99); 15 Oct 2013 14:06:32 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 15 Oct 2013 14:06:32 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 1C93D1D3599; Tue, 15 Oct 2013 14:06:31 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============4626524901432742631==" MIME-Version: 1.0 Subject: Re: Review Request 14549: Rename net.juniper.contrail to org.apache.cloudstack.network.contrail From: "Murali Reddy" To: "Murali Reddy" , "Pedro Marques" , "cloudstack" Date: Tue, 15 Oct 2013 14:06:30 -0000 Message-ID: <20131015140630.7314.45022@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Murali Reddy" X-ReviewGroup: cloudstack X-ReviewRequest-URL: https://reviews.apache.org/r/14549/ X-Sender: "Murali Reddy" References: <20131009144950.20899.71351@reviews.apache.org> In-Reply-To: <20131009144950.20899.71351@reviews.apache.org> Reply-To: "Murali Reddy" --===============4626524901432742631== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit > On Oct. 9, 2013, 2:49 p.m., Murali Reddy wrote: > > - Is there a reason why no new isolation type was not added for 'contrail controller'. For other overlay technologies (STT, GRE, VXLAN) that CloudStack support there is an isolation type and corresponding Guru that handles isolation type. > > > > - There is a 'EventBus' on to which all events generated by CloudStack gets published. Right approach would be to plug-in to subscribe to interested events from event bus. But the current implementation of EventBus expects external AMQP server, so it may not be ideal. EventInterceptor approach implemented in the plug-in works fine to get the notification. But enabling it by default in ApplicationContext does not seem right thing to do. > > > > - If you can add some more details in to the FS on deployment model it will give more perspective to reviewers > > > > + is the VRouter mentioned in the FS is a appliance provisioned by CloudStack for each guest network by servicemanager? Or its a logical router on the dataplane that does the forwarding? > > + is BGP/MPLS required on the IP fabric and the Hypervisors > > > > Pedro Marques wrote: > Murali, > We didn't add a new isolation type to avoid changing the CloudStack code. The current define describes the encapsulation technology. Contrail can use several encapsulation technologies: MPLS/GRE, MPLS/UDP, MPLS over VXLAN header. I can add one if you find it helpful... from a technology perspective contrail focus on the control plane solution rather than the encapsulation. > > The event bus doesn't look like an attractive approach. If the ActionEvent listeners will cause a problem we can periodically poll from the plugin. > > The "VRouter" is a linux kernel module running on each Host. BGP/MPLS is a control plane technology. The fabric is required to be able to transport IP packets between Hosts... No other assumption is made; no need for VLANs or MPLS on the fabric. > Pedro, Thanks for clarification. Now I understand that Contrial being control plane solution and its ability support multiple encapsulation technologies. Actually my earlier comment was not so much about isolation method (CloudStack notion of overlay/encapsulation technologies). Currently there is no prescriptive way for CloudStack networking to know which NetworkGuru should be handling the network design, so it just loops through all network guru to implement & design the network. Unfortunately, its network guru to know if it should handle the design/implement call etc for a network. For e.g. VxlanGuestNetworkGuru thinks its authoritative to handle the isolation method 'VXLAN'. If ContrailGuru thinks it can handle VXLAN then there is conflict if both the plug-ins are enabled by default in the componentContext. It should not be plug-in developers problem, but i want to suggest to add a safe-guard. Either add new isolation method (like MIDO for e.g for Midokura) and let Contrail Guru and Ele ment to handle only if isolation method is for Contrail. I agree that event bus approach is not attractive with current RabbitMq implementation. What my intention was new interceptor (contrailEventInterceptor) be enabled only if some one wants to use Contrail plug-in. If Contrial plug-in is not used then interceptor is just adding additional overhead. You can make this step (of configuring interceptor in applicationContext as setup step). - Murali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14549/#review26819 ----------------------------------------------------------- On Oct. 8, 2013, 11:58 p.m., Pedro Marques wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14549/ > ----------------------------------------------------------- > > (Updated Oct. 8, 2013, 11:58 p.m.) > > > Review request for cloudstack. > > > Repository: cloudstack-git > > > Description > ------- > > Rename net.juniper.contrail to org.apache.cloudstack.network.contrail. > > > Diffs > ----- > > client/tomcatconf/applicationContext.xml.in 0ab2515 > client/tomcatconf/componentContext.xml.in 157ad5a > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/api/command/CreateServiceInstanceCmd.java 92f5eeb > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/api/response/ServiceInstanceResponse.java 1b7a7d8 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ContrailElement.java 885a60f > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ContrailElementImpl.java 3a38020 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ContrailGuru.java c655b0b > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ContrailManager.java 5195793 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ContrailManagerImpl.java 8a3ca1b > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/DBSyncGeneric.java d169b37 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/EventUtils.java acd1bed > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ManagementNetworkGuru.java bad2502 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ModelDatabase.java f9e7c24 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ServerDBSync.java 4c8c2e9 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ServerDBSyncImpl.java 06daf12 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ServerEventHandler.java 6f0ecf2 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ServerEventHandlerImpl.java aa4e9d5 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ServiceManager.java f3884fb > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ServiceManagerImpl.java b90792c > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/management/ServiceVirtualMachine.java 9c8b61d > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/FloatingIpModel.java ca90666 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/FloatingIpPoolModel.java 8e238fd > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/InstanceIpModel.java ff08560 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/ModelController.java 7abb40a > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/ModelObject.java 7cd420c > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/ModelObjectBase.java 4b05e96 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/ServiceInstanceModel.java f65bfc7 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/VMInterfaceModel.java 0ec7c9e > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/VirtualMachineModel.java df40025 > plugins/network-elements/juniper-contrail/src/net/juniper/contrail/model/VirtualNetworkModel.java 99ab944 > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/api/command/CreateServiceInstanceCmd.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/api/response/ServiceInstanceResponse.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailElement.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailElementImpl.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailGuru.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManager.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/DBSyncGeneric.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/EventUtils.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ManagementNetworkGuru.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ModelDatabase.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ServerDBSync.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ServerDBSyncImpl.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ServerEventHandler.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ServerEventHandlerImpl.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ServiceManager.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ServiceManagerImpl.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ServiceVirtualMachine.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/FloatingIpModel.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/FloatingIpPoolModel.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/InstanceIpModel.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelController.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelObject.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelObjectBase.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ServiceInstanceModel.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VMInterfaceModel.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualMachineModel.java PRE-CREATION > plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualNetworkModel.java PRE-CREATION > plugins/network-elements/juniper-contrail/test/net/juniper/contrail/management/MockAccountManager.java 029950a > plugins/network-elements/juniper-contrail/test/net/juniper/contrail/management/NetworkProviderTest.java c3e07d0 > plugins/network-elements/juniper-contrail/test/net/juniper/contrail/management/TestConfiguration.java 87feaa9 > plugins/network-elements/juniper-contrail/test/net/juniper/contrail/management/TestDbSetup.java 55cef7c > plugins/network-elements/juniper-contrail/test/net/juniper/contrail/management/VirtualNetworkModelTest.java a5767c6 > plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java PRE-CREATION > plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/NetworkProviderTest.java PRE-CREATION > plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/TestConfiguration.java PRE-CREATION > plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/TestDbSetup.java PRE-CREATION > plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/VirtualNetworkModelTest.java PRE-CREATION > plugins/network-elements/juniper-contrail/test/resources/serviceContext.xml 623d188 > > Diff: https://reviews.apache.org/r/14549/diff/ > > > Testing > ------- > > Integration test passes. > > > Thanks, > > Pedro Marques > > --===============4626524901432742631==--