You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by nvazquez <gi...@git.apache.org> on 2015/10/15 21:11:19 UTC

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

GitHub user nvazquez opened a pull request:

    https://github.com/apache/cloudstack/pull/935

    From4.5.1: NSX/Nicira Plugin does not support NSX v4.2.1

    JIRA Ticket: https://issues.apache.org/jira/browse/CLOUDSTACK-8956

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nvazquez/cloudstack from4.5.1

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/935.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #935
    
----
commit 82e786ff430276390d4b50d5f154ca1493a88e76
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-23T20:23:19Z

    Add log before portgroup creation

commit 68a5bb07f8596779da0f885a08a33ddd7b212d2f
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-23T20:36:09Z

    Logger fix

commit 847048b2721b4aeb67983bdadd1d984732577d3a
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-23T21:03:23Z

    Json portgroup

commit 9d6a53cc4730c3cf1a597ee7b11b1b710e9948e8
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-24T14:05:03Z

    Logs before create port

commit 9c4645e571a389f15294247142e4db66caf409c2
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-24T16:09:31Z

    Logging opaqueNetwork

commit 6b6431767f684e02f139b00e9987594dad1d2dfb
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-24T18:03:12Z

    Remove retrieve opaquenetwork

commit 9a287ac001d65b8b74977767e049bb8dd47872ee
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-24T19:44:14Z

    Only logging networks

commit da5eb1a4b17d4e6b245161e5e5c69b927864dede
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-24T20:02:21Z

    Add summary network to properties

commit 0dfd291f597152b806a836659cc41ce2c736e27e
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-24T21:02:15Z

    Retrieve opaque networks

commit 328c6808e38afddf7126441f328901f119eaa513
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-25T12:26:16Z

    Extending host network info with opaque networks

commit ac71af39b6a0818cfd3eff41267ad37c0ea460bd
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-25T12:33:51Z

    Fixes opaque network

commit 6f8b9b683f95ebc3c1cd4c28056622081947544e
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-25T12:38:46Z

    Fix checkstyle

commit a313247407195d779a49b38570206ca333914f27
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-25T14:50:01Z

    vpshere api 5.5

commit 240de4c486418e549c4d13385729af6b23354e66
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-25T15:21:15Z

    Fix

commit 661a80af49f6ccb0a7e304c92e6690f77ea52558
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-25T20:17:43Z

    Trying opaqueNetwork search

commit 93bb3c21af1c0b14f3a492ec02696ffc8e57409b
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-28T13:45:58Z

    Fix iter

commit 962eb72339707465046834affb2e3ad9ab3ee80a
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-28T15:36:10Z

    Opaque netowrks

commit 85b8ec2880118349395621cdee53f9c7f5d494c9
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-28T16:00:29Z

    Opaque network class

commit a431593119e77ff77e20e03d216ec89540502da3
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-28T18:09:16Z

    Opaque networks

commit 30583786852cfabc245db76d5fa808547478789c
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-28T18:41:06Z

    Opaque networks filter

commit 68a10b59766866664a1b114bb52db17be62c85fc
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-28T19:44:35Z

    Opaque networks filters

commit 03b0aab89b8a3e2cb9b6bc418a184e26a0dc2b2e
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-28T20:15:58Z

    Opaque networks

commit 93bad52c43d20942289e101f6bbdac1a77602a58
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-28T21:38:47Z

    MorNetwork

commit 015b84e2e41426a15d64a4afa88c639006a35a68
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-29T15:34:18Z

    vm92

commit f3efb69073a280bf90fa98e29367044759b6a2b9
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-29T15:43:25Z

    vm92

commit 92b5149548f733b47cadec72878b05cfcf807a68
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-29T15:58:59Z

    vm92

commit c3c2db7bfb2fd559c4466aa373b4cb1600d1cf05
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-29T16:02:11Z

    opaque

commit be835c027393ce9bc1fee22a4231b2f850f4edc9
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-29T16:25:47Z

    opaque

commit 8dfca735bc5f85f7c8b9a143a74b6c54618c5034
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-29T19:48:55Z

    Changes

commit 663b17d97e133b5b8c43bd884967dd4c3d09c14a
Author: nvazquez <ni...@gmail.com>
Date:   2015-09-29T20:21:41Z

    GuestNic

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-151509780
  
    Awesome to see the changes, @nvazquez ! Thanks for being openminded and also for your contributions!
    
    Cheers,
    Wilder


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r42740897
  
    --- Diff: utils/src/main/java/com/cloud/utils/nicira/nvp/plugin/NiciraNvpApiVersion.java ---
    @@ -0,0 +1,60 @@
    +//
    +// Licensed to the Apache Software Foundation (ASF) under one
    +// or more contributor license agreements.  See the NOTICE file
    +// distributed with this work for additional information
    +// regarding copyright ownership.  The ASF licenses this file
    +// to you under the Apache License, Version 2.0 (the
    +// "License"); you may not use this file except in compliance
    +// with the License.  You may obtain a copy of the License at
    +//
    +//   http://www.apache.org/licenses/LICENSE-2.0
    +//
    +// Unless required by applicable law or agreed to in writing,
    +// software distributed under the License is distributed on an
    +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +// KIND, either express or implied.  See the License for the
    +// specific language governing permissions and limitations
    +// under the License.
    +//
    +
    +package com.cloud.utils.nicira.nvp.plugin;
    +
    +import org.apache.log4j.Logger;
    +
    +import com.cloud.maint.Version;
    +
    +public class NiciraNvpApiVersion {
    +    private static final Logger s_logger = Logger.getLogger(NiciraNvpApiVersion.class);
    +
    +    private static String niciraApiVersion = null;
    +
    +    public static void pingNiciraApiVersion(String apiVersion){
    --- End diff --
    
    What is the whole point of this method? You already have a static method to set the version that accepts null. You also have another static method that checks if the a given API version is lower than the existing one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157321576
  
    Couple of minor things that can go through now and be fixed after this is merged. @miguelaferreira has already tested the PR and showed that it works just fine. So, it has my LGTM as well :+1: 
    
    Thanks a lot for your contribution, @nvazquez !
    
    Cheers,
    Wilder


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157731551
  
    @DaanHoogland No, it's not.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157311867
  
    Would be great to include this in 4.7, please ping me when you're ready!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157739765
  
    @DaanHoogland Ok, sure


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r45038824
  
    --- Diff: plugins/network-elements/nicira-nvp/src/main/java/com/cloud/network/nicira/ControlClusterStatus.java ---
    @@ -84,4 +89,17 @@ public int getActiveCount() {
             }
     
         }
    +
    +    public class ClusterRoleConfig {
    --- End diff --
    
    Do those inner classes need to be public?
    
    It's also something we can fix after the merge, unless there is a reason for that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157727789
  
    @nvazquez I tried to mv and copy the jar in the job but it doesn't work. now trying to see if your link yields a newer version then our job has


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157700801
  
    @nvazquez Can you look at this build failure: http://jenkins.buildacloud.org/job/build-master-noredist/4746/console
    
    Does the slave need the 5.5 jar or is something else wrong?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-148929405
  
    @nvazquez Thanks for your reply. If you made this from 4.5.1, be aware that you submitted it against current master so this will for sure cause problems. That may be why Jenkins failed and you have all these conflicts. 
    
    Please make your patch against current master, as it is the only way to get it in. Otherwise, it will would be in 4.5.x and not in any newer versions causing regression when upgrading from 4.5 to 4.6 etc. 
    
    Master is currently frozen and almost stable as we're heading to a 4.6 release so developing against it works great.
    
    You will also find the test in the master branch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-150012697
  
    ### VMware vSphere API version from 5.1 to 5.5:
    Since vSphere API version 5.5, [OpaqueNetworks](https://www.vmware.com/support/developer/converter-sdk/conv55_apireference/vim.OpaqueNetwork.html) are introduced. 
    Its description says: 
    > This interface defines an opaque network, in the sense that the detail and configuration of the network is unknown to vShpere and is managed by a management plane outside of vSphere. However, the identifier and name of these networks is made available to vSphere so that host and virtual machine virtual ethernet device can connect to them.
    
    In order to connect a vm's virtual ethernet device to the proper opaque network when deploying a vm into a NSX managed network, we first need to look for a particular opaque network on hosts. This opaque network's id has to be **"br-int"** and its type **"nsx.network"**.
    
    Since vSphere API version 5.5 [HostNetworkInfo](https://www.vmware.com/support/developer/converter-sdk/conv55_apireference/vim.host.NetworkInfo.html#opaqueNetwork) introduces a list of available opaque networks for each host. 
    If NSX API version >= 4.2 we look for a [OpaqueNetworkInfo](https://www.vmware.com/support/developer/converter-sdk/conv55_apireference/vim.host.OpaqueNetworkInfo.html) which satisfies:
    * opaqueNetworkId = "br-int"
    * opaqueNetworkType = "nsx.netork"
    
    If that opaque network is found, then we need to attach vm's NIC to a virtual ethernet device which support this, so we use [VirtualEthernetCardOpaqueNetworkBackingInfo](https://www.vmware.com/support/developer/converter-sdk/conv55_apireference/vim.vm.device.VirtualEthernetCard.OpaqueNetworkBackingInfo.html) setting:
    * opaqueNetworkId = "br-int"
    * opaqueNetworkType = "nsx.netork"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-148704543
  
    Hi @nvazquez Please reorganise your commits, as we need them to be atomic. It cannot go in like this.
    
    I'm also with @miguelaferreira, please let us know what problem you fix here and what testing you have done (also to make sure it still works with current setups).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-155418071
  
    @wilderrodrigues Thanks a lot Wilder!
    
    @miguelaferreira Thanks for following this PR and testing it!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157724852
  
    @nvazquez Do you mean to say that we can just rename the old jar to contain the new version in the name and the sun shines again?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157736873
  
    Ok, shouldn't it be removed from the install script and then?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-148639781
  
    Hi @nvazquez 
    
    Thanks for your contribution. I'm not quite sure I understand the problem you are trying to solve, but that likely my own lack of NXS knowledge.
    The last time I changed this plugin (NiciraNvp), I've added an integration tests for checking is redirects within a cluster were being followed. You change probably doesn't impact that, but it would be great to make sure. Can you please run the test (`test/integration/smoke/test_nicira_controller.py`, let me know if you need help in getting the test running) in your setup and report on the results?
    In addition, it would be great of you document the problem you are trying to fix by adding a new integration tests (that fails before your change is applied and succeeds afterwards).
    
    A final remark about the code and the commits, please see my comments on certain lines, and consider rearranging your commits in such a way that what you did and/or the intention behind each commit become clear.
    
    Thanks again, and do let me know if I can hep you further.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r42216002
  
    --- Diff: vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java ---
    @@ -1079,8 +1088,10 @@ private static void createNvpPortGroup(HostMO hostMo, HostVirtualSwitch vSwitch,
             List<Integer> usedVlans = new ArrayList<Integer>();
             for (HostPortGroup pg : hostMo.getHostNetworkInfo().getPortgroup()) {
                 HostPortGroupSpec hpgs = pg.getSpec();
    -            if (vSwitchName.equals(hpgs.getVswitchName()))
    +            if (vSwitchName.equals(hpgs.getVswitchName())){
                     usedVlans.add(hpgs.getVlanId());
    +                //s_logger.info("[NSX_PLUGIN_LOG] vSwitchName=" + vSwitchName + " pg: " + new Gson().toJson(pg));
    --- End diff --
    
    Commented code left behind. Was the intention to delete it or to uncomment it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157718592
  
    @remibergsma It looks like it is not finding 5.5 jar. These are the steps I followed:
    * Download VMware-vSphere-SDK-5.5.0-1284541.zip file from https://my.vmware.com/group/vmware/get-download?downloadGroup=WEBSDK550
    * Extract zip file, cd SDK/vsphere-ws/java/JAXWS/lib/ and rename vim25_51.jar to vim25_55.jar
    * Copy SDK/vsphere-ws/java/JAXWS/lib/vim25_55.jar to CS deps folder
    * In CS deps folder: ./install-non-oss.sh



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r42216007
  
    --- Diff: vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java ---
    @@ -1101,6 +1112,12 @@ private static void createNvpPortGroup(HostMO hostMo, HostVirtualSwitch vSwitch,
             secPolicy.setMacChanges(Boolean.FALSE);
     
             // Create a portgroup with the uuid of the nic and the vlanid found above
    +        //s_logger.info("[NSX_PLUGIN_LOG] createPortGroup: vSwitch=[key=" + vSwitch.getDynamicType() + ", name=" + vSwitch.getName() +
    --- End diff --
    
    Commented code left behind. Was the intention to delete it or to uncomment it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-149582907
  
    @miguelaferreira  @remibergsma 
    Thanks again for your help! As you suggested I rebased master branch. I also added a more detailed description of the patch on this PR description and JIRA ticket.
    
    Thanks,
    Nicolas


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157154339
  
    @miguelaferreira Great, I rebased my branch to master and pushed again


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157782057
  
    @DaanHoogland those warnings are in files that don't belong to my pull request. 
    I could see in http://jenkins.buildacloud.org/job/build-master-slowbuild/2660/changes#detail0 that it compiled 5 commits, but the first one is commited by other contributor.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-148715124
  
    Hi @miguelaferreira @remibergsma 
    
    Thanks a lot for your help. I will try to make a better description of the problem and work on commits and code as you suggested. Sorry for unorganized commits, this is my first contribution to CS, I will try to make it better.
    
    @miguelaferreira I don't have that test file (test/integration/smoke/test_nicira_controller.py) on my branch. I've created this branch from tag 4.5.1, maybe it wasn't available yet. How can I do for testing it?
    
    Thanks,
    Nicolas



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/935


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157805785
  
    sorry, @nlivens can you have a look?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157701917
  
    According to Jenkins the build was fine:
    
    ```
    Maven:  -B -f /home/jenkins/jenkins-slave/workspace/cloudstack-pull-analysis/pom.xml -Dmaven.repo.local=/home/jenkins/jenkins-slave/maven-repositories/0 -Penablefindbugs clean install cobertura:cobertura
    ```
    although it doesn't do the `-Pnoredist` profile.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-152197682
  
    @nvazquez I havent' forgotten your PR. I'm having some trouble with the automation to re-create a NSX cluster with a KVM host in our test environment. As soon as it is fixed, I will test.
    In any case, this PR can only be merged after 4.6 because of feature freeze.
    
    I'll keep you posted.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157748781
  
    @nvazquez can you please look at http://jenkins.buildacloud.org/job/build-master-slowbuild/2660/findbugsResult/new/
    I think these are trivial, you just have to decide on which encoding to use and call the right util method (getPrefferedCharset() or getDefaultCharset()) from StringUtils.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157727519
  
    @DaanHoogland Hi! No, just renaming the old jar (version 5.1) will fail because it doesn't contain opaque network classes. Those ones are introduced in the 5.5 version jar.
    I added a line in deps/install-non-oss.sh which includes 5.5 jar to maven repo and also replaced cs.vmware.api.version property in pom.xml file from 5.1 to 5.5


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r45038549
  
    --- Diff: plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java ---
    @@ -2076,6 +2087,9 @@ private static void postNvpConfigBeforeStart(VirtualMachineMO vmMo, VirtualMachi
                     } else if (backing instanceof VirtualEthernetCardNetworkBackingInfo) {
                         // This NIC is connected to a Virtual Switch
                         // Nothing to do
    +                } else if (backing instanceof VirtualEthernetCardOpaqueNetworkBackingInfo) {
    +                    //if NSX API VERSION >= 4.2, connect to br-int (nsx.network), do not create portgroup else previous behaviour
    +                    //OK, connected to OpaqueNetwork
    --- End diff --
    
    Those 2 else/if blocks should contain some logging in order to inform sys admins what is now comments in the code.
    
    You can keep it for now, because it doesn't affect the feature that @miguelaferreira has already tested. Once this PR is merged, we can fix it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r42215505
  
    --- Diff: plugins/network-elements/nicira-nvp/src/com/cloud/network/resource/NiciraNvpResource.java ---
    @@ -180,6 +182,17 @@ public Type getType() {
         public PingCommand getCurrentStatus(final long id) {
             try {
                 ControlClusterStatus ccs = niciraNvpApi.getControlClusterStatus();
    +            //------------------------------------------------GET API_PROVIDER MAJORITY VERSION
    --- End diff --
    
    if this block is wrapped by a comment, wouldn't it be better to refactor it into a new method? (giving the method a meaningful name that would render the comment redundant, say `getApiProviderMajorityVersion(...)`)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-149018797
  
    @nvazquez Thanks for your work on the commits, they are much better now. I would say it can easily be improved even further, but given the current practice in the project, it is already good!
    
    As @remibergsma pointed out, it would be best if you apply your changes to master. However if you want to use the functionality you added in a new minor version of an older release, then make a PR for the branch of that release, and we will help you port your changes all the way up to master. We have worked on a workflow that helps with that (https://cwiki.apache.org/confluence/display/CLOUDSTACK/Release+principles+for+Apache+CloudStack+4.6+and+up#ReleaseprinciplesforApacheCloudStack4.6andup-Whymergingforwardovercherry-pickingfrommaster?)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r42215862
  
    --- Diff: services/console-proxy-rdp/rdpconsole/src/test/java/rdpclient/MockServerTest.java ---
    @@ -161,7 +161,8 @@ public void testIsMockServerCanUpgradeConnectionToSsl() throws Exception {
     
                     final SSLSocketFactory sslSocketFactory = (SSLSocketFactory)SSLSocketFactory.getDefault();
                     SSLSocket sslSocket = (SSLSocket)sslSocketFactory.createSocket(socket, address.getHostName(), address.getPort(), true);
    -                sslSocket.setEnabledCipherSuites(sslSocket.getSupportedCipherSuites());
    +                //sslSocket.setEnabledCipherSuites(sslSocket.getSupportedCipherSuites());
    --- End diff --
    
    Commented code left behind. Was the intention to delete it or to uncomment it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r42740931
  
    --- Diff: utils/src/main/java/com/cloud/utils/nicira/nvp/plugin/NiciraNvpApiVersion.java ---
    @@ -0,0 +1,60 @@
    +//
    +// Licensed to the Apache Software Foundation (ASF) under one
    +// or more contributor license agreements.  See the NOTICE file
    +// distributed with this work for additional information
    +// regarding copyright ownership.  The ASF licenses this file
    +// to you under the Apache License, Version 2.0 (the
    +// "License"); you may not use this file except in compliance
    +// with the License.  You may obtain a copy of the License at
    +//
    +//   http://www.apache.org/licenses/LICENSE-2.0
    +//
    +// Unless required by applicable law or agreed to in writing,
    +// software distributed under the License is distributed on an
    +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +// KIND, either express or implied.  See the License for the
    +// specific language governing permissions and limitations
    +// under the License.
    +//
    +
    +package com.cloud.utils.nicira.nvp.plugin;
    +
    +import org.apache.log4j.Logger;
    +
    +import com.cloud.maint.Version;
    +
    +public class NiciraNvpApiVersion {
    +    private static final Logger s_logger = Logger.getLogger(NiciraNvpApiVersion.class);
    +
    +    private static String niciraApiVersion = null;
    --- End diff --
    
    No need to initialise as null. By default it will be null.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-156985757
  
    @nvazquez last Friday 4.6.0 was released so current master is now 4.7.0. Could you please rebase your branch on latest master?
    
    Then @wilderrodrigues will do one final review of the code, and if all is well (as it looks now) then he will give another LGTM and this feature can be merged.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-150145949
  
    @nvazquez PR looks awesome now. Great work!
    
    I will pull in your PR and run the marvin test we already have against it to see if that is still good. I will post the results in here.
    However, I do not have an environment with vSphere to test one. Could you please run the tests we have already on you environment and post the results here?
    
    In addition, to make this PR super awesome, we would also need at least one new marvin test that exercises the new logic you have introduced. That is, a test where the Nicira plugin is configured for vSphere, and we can at least create te a VM on a Nicira backed network.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-157729881
  
    @nvazquez is the older one still needed?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r42215621
  
    --- Diff: plugins/network-elements/nicira-nvp/src/com/cloud/network/resource/NiciraNvpResource.java ---
    @@ -191,6 +204,19 @@ public PingCommand getCurrentStatus(final long id) {
             return new PingCommand(Host.Type.L2Networking, id);
         }
     
    +    private int searchApiProvider(ClusterRoleConfig[] configuredRoles) {
    +        int length = configuredRoles.length;
    +        for (int i = 0; i < length; i++) {
    +            if (! configuredRoles[i].getRole().equals("api_provider")){
    --- End diff --
    
    This conditional could be simplified:
    ```java
    if (configuredRoles[i].getRole().equals("api_provider")) {
      return i
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r42740183
  
    --- Diff: vmware-base/src/com/cloud/hypervisor/vmware/util/VmwareHelper.java ---
    @@ -79,6 +79,52 @@ public static boolean isReservedScsiDeviceNumber(int deviceNumber) {
             return deviceNumber == 7;
         }
     
    +    public static VirtualDevice prepareNicOpaque(VirtualMachineMO vmMo, VirtualEthernetCardType deviceType, String portGroupName,
    +            String macAddress, int deviceNumber, int contextNumber, boolean conntected, boolean connectOnStart) throws Exception {
    +
    +        assert(vmMo.getRunningHost().hasOpaqueNSXNetwork());
    +
    +        VirtualEthernetCard nic;
    +        switch (deviceType) {
    +        case E1000:
    +            nic = new VirtualE1000();
    +            break;
    +
    +        case PCNet32:
    +            nic = new VirtualPCNet32();
    +            break;
    +
    +        case Vmxnet2:
    +            nic = new VirtualVmxnet2();
    +            break;
    +
    +        case Vmxnet3:
    +            nic = new VirtualVmxnet3();
    +            break;
    +
    +        default:
    +            assert (false);
    --- End diff --
    
    Why this "assert (false);" here? Asserting as false might cause problems.
    ```
    Each assertion contains a boolean expression that you believe will be true when the assertion executes. If it is not true, the system will throw an error.
    ```
    
    Source: http://docs.oracle.com/javase/7/docs/technotes/guides/language/assert.html
    
    The user os assertion in this way is irrelevant and error-prone. Could you please remove it and push a new commit?
    
    Cheers,
    Wilder


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-151511065
  
    btw, I'm testing your PR in our setup


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-155396401
  
    @nvazquez I've built and tested your PR in our environment and the functionality we have tests for remains stable. :+1: 
    
    What I did:
    
    1. build and unit-test with developer and systemvm profiles
    
    2. package centos7 KVM agent RPMs
    
    3. deploy infra with
    
       - 1 mangement server (war installed from step 1)
       - 2 NSX controllers (cluster)
       - 1 NSX manger node
       - 1 NSX service node
       - 1 KVM host (agent installed from step 2)
    
    4. deploy data center using [this](https://github.com/schubergphilis/MCT-shared/blob/master/marvin/mct-zone1-kvm1-NVP.cfg) config
    
    5. run marvin tests from `test/integration/smoke/test_nicira_controller.py` with flags `tags=advanced,required_hardware=true`
    
    Result
    ```
    [root@cs1 cloudstack]# nosetests --with-marvin --marvin-config=/data/shared/marvin/mct-zone1-kvm1-NVP.cfg -s -a tags=advanced,required_hardware=true test/integration/smoke/test_nicira_controller.py
    
    ==== Marvin Init Started ====
    
    === Marvin Parse Config Successful ===
    
    === Marvin Setting TestData Successful===
    
    ==== Log Folder Path: /tmp//MarvinLogs//Nov_10_2015_10_27_55_46U497. All logs will be available here ====
    
    === Marvin Init Logging Successful===
    
    ==== Marvin Init Successful ====
    /usr/lib/python2.7/site-packages/requests/packages/urllib3/util/ssl_.py:100: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
      InsecurePlatformWarning
    (previous warning repeated a couple times)
    === TestName: test_01_nicira_controller | Status : SUCCESS ===
    
    === TestName: test_02_nicira_controller_redirect | Status : SUCCESS ===
    
    ===final results are now copied to: /tmp//MarvinLogs/test_nicira_controller_RV74VA===
    
    ```
    
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/935#discussion_r42215794
  
    --- Diff: plugins/network-elements/nicira-nvp/src/com/cloud/network/resource/NiciraNvpResource.java ---
    @@ -191,6 +204,19 @@ public PingCommand getCurrentStatus(final long id) {
             return new PingCommand(Host.Type.L2Networking, id);
         }
     
    +    private int searchApiProvider(ClusterRoleConfig[] configuredRoles) {
    +        int length = configuredRoles.length;
    +        for (int i = 0; i < length; i++) {
    +            if (! configuredRoles[i].getRole().equals("api_provider")){
    --- End diff --
    
    The searchApiProvider returns an index (if the element is found), and that index is then used to obtain the element from the array (Line 190). Why not return the element right away?
    This would simplify the code around line 190


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...

Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-150676496
  
    @miguelaferreira thanks a lot for your help and advices! As suggested I post test_nicira_controller.py results:
    
    $ cat /tmp/MarvinLogs/test_nicira_controller_77FOV9/results.txt
    test_01_nicira_controller (integration.smoke.test_nicira_controller.TestNiciraContoller) ... === TestName: test_01_nicira_controller | Status : SUCCESS ===
    ok
    Nicira clusters will redirect clients (in this case ACS) to the master node. ... === TestName: test_02_nicira_controller_redirect | Status : SUCCESS ===
    ok
    
    ----------------------------------------------------------------------
    Ran 2 tests in 398.132s
    
    OK



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---