cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hugo Trippaers" <htrippa...@schubergphilis.com>
Subject Re: Review Request: Nicira NVP integration for CloudStack
Date Fri, 06 Jul 2012 06:50:15 GMT


> On July 5, 2012, 11:36 p.m., Murali Reddy wrote:
> > Hugo, code in general looks good. Below are some minor comments. Is there is trial
version of NVP controller that can used for trying out integration or for testing?
> > 
> > - In CitrixResourceBase it seems to me that Nicira config params are added for every
VIF that is created. Should it be done only for the network's using STT isolation?
> > 
> > - In GuestNetworkGuru:
> > 
> >    + In isMyIsolationMethod(), if no isolation method is configured, its treated
as acceptable isolation by GuestNetworkGuru. I suggest a guru check only for what isolation
it can support.
> > 
> >    + Whats is the rational for canHanlde() being implemented in each of the implementation
of GuestNetworkGuru? I think single abstract implementations should be fine. Do you see that
logic will change across the implementations?
> > 
> > - CloudStack Account names are not unique. So please use a combination of domain
id + account name or some other unique tag when creating a logical switch.
> > 
> > - I see that it is permitted to configure multiple NVP controllers per physical
network?  But always use the first controller available for the physical network when implementing
the network. Is it intended behavior?
> > 
> > - Not sure you mentioned this in the your earlier mails. Does this implementation
work only with Xen? or works with Kvm, Vmware as well?

Hey Murali,

Thanks for the review!

I'll have to pass the question about the trial version of the NVP Controller on to Nicira.
I've been testing with a Nicira setup at our office and with a setup at their office. Of course
you are more than welcome to spend some time at our office to test it here, but it might not
be feasible as i am located in the Netherlands. What i can do is provide access to my test
servers, would that be helpful? 

Regarding your comments, i tried to answer them point by point below:

1. The other-config in the VIF should only be set for interfaces connected to networks that
are controlled by a Nicira NVP. My problem is that i don't seem to have this information available
in the createVif call. The option is see if to add a field with other options to the nic template
that gets passed to createvif during creation. Would that be alright or is there a better
way to do this?

2a. This is for backwards compatibility, the current ExternalGuestNetworkGuru also supports
empty isolation method. But i think this is fixed in the UI now, you can no longer select
empty isolation when creating an advanced network. I think is should be that ExternalGuestNetworkGuru
supports VLAN, OvsGuestNetorkGuru supports GRE and NiciraNVPGuestNetorkGuru supports STT.

2b. My reasoning was that should the guru's become pluggable in the future, the guru should
be able to decide what it can and will handle based on its internal logic, which can be more
than checking supported isolation type. In fact in there might be overlap if more SDN solutions
become available. Technically there is already some overlap as the Nicira solution also supports
GRE (i decided with Somik to ignore this and focus on STT)

3. Ok

4. Should not be possible, will fix.

5. For now it only works with Xen, but i will make sure support is extended to other hypervisors.
I'll be working with Nicira to see what is possible to support after this bit is done.


I'll take care of the fixes and provide a new patch base on the current master, is that ok?
That patch will also include the move of the nicira code to the plugins folder.

Cheers,

Hugo


- Hugo


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


On June 26, 2012, 4:56 p.m., Hugo Trippaers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5590/
> -----------------------------------------------------------
> 
> (Updated June 26, 2012, 4:56 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Patch to add Nicira NVP support to CloudStack. As discussed this patch is related to
phase 1, which is basic L2 connectivity. L3 connectivity and integration with the network
offering for SNAT will be in phase2.
> 
> 
> Diffs
> -----
> 
>   README.NiciraIntegration PRE-CREATION 
>   api/src/com/cloud/agent/api/CreateLogicalSwitchAnswer.java PRE-CREATION 
>   api/src/com/cloud/agent/api/CreateLogicalSwitchCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/CreateLogicalSwitchPortAnswer.java PRE-CREATION 
>   api/src/com/cloud/agent/api/CreateLogicalSwitchPortCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DeleteLogicalSwitchAnswer.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DeleteLogicalSwitchCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DeleteLogicalSwitchPortAnswer.java PRE-CREATION 
>   api/src/com/cloud/agent/api/DeleteLogicalSwitchPortCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/StartupNiciraNvpCommand.java PRE-CREATION 
>   api/src/com/cloud/agent/api/to/NicTO.java b65c61ee47c03bf33bcbf2ee10a2f8d6405538f0

>   api/src/com/cloud/agent/api/to/VirtualMachineTO.java 42d91626e35d20c960561ca1101aad17dc19febb

>   api/src/com/cloud/api/ApiConstants.java 00ec392d7b930a1ec0d2d77c2cccd035bc9f3321 
>   api/src/com/cloud/host/Host.java 0c9d06d48bfdaf1f491c71cda6e1d878214a2dd1 
>   api/src/com/cloud/network/Network.java 0443a0f41cc112105b48bb80dae58f6f3adc4b8d 
>   api/src/com/cloud/network/Networks.java 84135b8ee45489fb6c284c6cf3ae0356596cbc83 
>   api/src/com/cloud/network/PhysicalNetwork.java e54fe00bef0846ec96a3b3dde43ba433bab6f1ac

>   client/tomcatconf/components.xml.in 58541a59606aebb69c1e9566934736b1a6f86b21 
>   client/tomcatconf/nicira-nvp_commands.properties.in PRE-CREATION 
>   core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 39172423e36f06f3721a9b108f0f05f5008f5613

>   core/src/com/cloud/network/nicira/Attachment.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/LogicalSwitch.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/LogicalSwitchPort.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/LogicalSwitchPortList.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/NiciraNvpApi.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/NiciraNvpApiException.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/NiciraNvpTag.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/TransportZoneBinding.java PRE-CREATION 
>   core/src/com/cloud/network/nicira/VifAttachment.java PRE-CREATION 
>   core/src/com/cloud/network/resource/NiciraNvpResource.java PRE-CREATION 
>   server/src/com/cloud/api/commands/AddNiciraNvpDeviceCmd.java PRE-CREATION 
>   server/src/com/cloud/api/commands/DeleteNiciraNvpDeviceCmd.java PRE-CREATION 
>   server/src/com/cloud/api/commands/ListNiciraNvpDeviceNetworksCmd.java PRE-CREATION

>   server/src/com/cloud/api/commands/ListNiciraNvpDevicesCmd.java PRE-CREATION 
>   server/src/com/cloud/api/response/NiciraNvpDeviceResponse.java PRE-CREATION 
>   server/src/com/cloud/configuration/DefaultComponentLibrary.java 3bfe84df25dd4027fdaad48b5ea2c24fff2ce432

>   server/src/com/cloud/host/dao/HostDaoImpl.java 745567dd7d260de5fab0945d72f89a8133cca4c4

>   server/src/com/cloud/hypervisor/HypervisorGuruBase.java d62e3800542e0394203506b25f5eb44c5fc90308

>   server/src/com/cloud/network/ExternalNetworkDeviceManager.java e09ff8e9580612ae010f550fc6f0a24b9cfd971d

>   server/src/com/cloud/network/NetworkManagerImpl.java 1f306ab07d136a785b9b433122fcd21418da12eb

>   server/src/com/cloud/network/NiciraNvpDeviceVO.java PRE-CREATION 
>   server/src/com/cloud/network/NiciraNvpNicMappingVO.java PRE-CREATION 
>   server/src/com/cloud/network/dao/NiciraNvpDao.java PRE-CREATION 
>   server/src/com/cloud/network/dao/NiciraNvpDaoImpl.java PRE-CREATION 
>   server/src/com/cloud/network/dao/NiciraNvpNicMappingDao.java PRE-CREATION 
>   server/src/com/cloud/network/dao/NiciraNvpNicMappingDaoImpl.java PRE-CREATION 
>   server/src/com/cloud/network/element/NiciraNvpElement.java PRE-CREATION 
>   server/src/com/cloud/network/element/NiciraNvpElementService.java PRE-CREATION 
>   server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 6c381f2b18eb559e34b3d4d51329df0b7d8f5afb

>   server/src/com/cloud/network/guru/GuestNetworkGuru.java f9977102b1f4b7652a87f3b6c7e04e44aae6a2d8

>   server/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java PRE-CREATION 
>   server/src/com/cloud/network/guru/OvsGuestNetworkGuru.java d031feeb6ebb40155234d6b856746c8a5eab80de

>   setup/db/create-schema.sql afcee3f9f6f48d1e0da42f9f952018aec0904929 
>   ui/scripts/ui-custom/zoneWizard.js d46bad9800bac5b671fbe40c8456ab2da19e5531 
> 
> Diff: https://reviews.apache.org/r/5590/diff/
> 
> 
> Testing
> -------
> 
> Simple build check
>   clean-all build-all
> 
> Testing of all api calls
>  * addNiciraNvpDevice
>  * deleteNiciraNvpDevice		
>  * listNiciraNvpDevices
>  * listNiciraNvpDeviceNetwork
> 
> Functional testing using the following procedure
> * start from clean db and create zone (with guest traffic on a physical network with
stt isolation type)
> * add NiciraNvp network service provider and enable
> * add NiciraNcpDevice to physical network and configure using api
> * create guestnetwork
> * create instance linked to guest network
> * check existence of logical switch and logical ports for routervm and instance
> * check connectivity between routervm and instance
> * destroy host
> * after shutdown of routervm and network check logical switch and ports on nicira (should
be gone)
> 
> 
> Thanks,
> 
> Hugo Trippaers
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message