You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by kishankavala <gi...@git.apache.org> on 2015/09/18 06:14:19 UTC

[GitHub] cloudstack pull request: Bug-ID: CLOUDSTACK-8870: Skip external de...

GitHub user kishankavala opened a pull request:

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

    Bug-ID: CLOUDSTACK-8870: Skip external device usage collection if no external devices exist

    external network device usage monitor thread that runs every 5mins by default (based on global config external.network.stats.interval) and runs coalesce query to acquire a lock. When there are no external devices exist, there is no need to run usage collection.
    Added test case to verify that usage collection task is not run when there are no External LB or External FW

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

    $ git pull https://github.com/kishankavala/cloudstack CLOUDSTACK-8870

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

    https://github.com/apache/cloudstack/pull/846.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 #846
    
----
commit a77dc78bfead71d0c9b5afa5813575ac67880f11
Author: Kishan Kavala <ki...@apache.org>
Date:   2015-09-16T15:15:06Z

    Bug-ID: CLOUDSTACK-8870: Skip external device usage collection if no external devices exist.

----


---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-213034173
  
    Thank you @GabrielBrascher.  @kishankavala would you mind fixing that typo?  I will try to get this run though CI soon...


---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-212999101
  
    Although I had pointed a typo in a comment line (nothing serious), the code 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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-212983883
  
    I need one more LGTM.  I will add this to my CI queue.  Thx...


---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-220935383
  
    @swill rebased with latest master
    @GabrielBrascher fixed the typo


---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-212782054
  
    I have gone through the code. patch 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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-220939007
  
    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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#discussion_r64197154
  
    --- Diff: engine/schema/src/com/cloud/host/dao/HostDaoImpl.java ---
    @@ -1087,4 +1087,11 @@ public HostVO findByPublicIp(String publicIp) {
             sc.addAnd("dataCenterId", SearchCriteria.Op.EQ, zoneId);
             return customSearch(sc, null);
         }
    +
    +    @Override
    +    public List<HostVO> listByType(Host.Type type) {
    +        SearchCriteria<HostVO> sc = TypeSearch.create();
    +        sc.setParameters("type", type);
    +        return listBy(sc);
    --- End diff --
    
    Fair enough


---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#discussion_r61724005
  
    --- Diff: engine/schema/src/com/cloud/host/dao/HostDaoImpl.java ---
    @@ -1087,4 +1087,11 @@ public HostVO findByPublicIp(String publicIp) {
             sc.addAnd("dataCenterId", SearchCriteria.Op.EQ, zoneId);
             return customSearch(sc, null);
         }
    +
    +    @Override
    +    public List<HostVO> listByType(Host.Type type) {
    +        SearchCriteria<HostVO> sc = TypeSearch.create();
    +        sc.setParameters("type", type);
    +        return listBy(sc);
    --- End diff --
    
    the listByType consumers are not doing != null checks, we can fix it by doing it here and returning a Collections.emptyList(); this way the methods ensures that no null is returned
    
    Please fix the NPE case, otherwise 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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#discussion_r64195431
  
    --- Diff: engine/schema/src/com/cloud/host/dao/HostDaoImpl.java ---
    @@ -1087,4 +1087,11 @@ public HostVO findByPublicIp(String publicIp) {
             sc.addAnd("dataCenterId", SearchCriteria.Op.EQ, zoneId);
             return customSearch(sc, null);
         }
    +
    +    @Override
    +    public List<HostVO> listByType(Host.Type type) {
    +        SearchCriteria<HostVO> sc = TypeSearch.create();
    +        sc.setParameters("type", type);
    +        return listBy(sc);
    --- End diff --
    
    @rhtyd 
    listBy(sc) in GenericDao base returns an empty ArrayList if there is no matching data. 
    listByType doesn't return 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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#discussion_r52110260
  
    --- Diff: server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java ---
    @@ -342,6 +342,12 @@ public ExternalDeviceNetworkUsageTask() {
     
             @Override
             protected void runInContext() {
    +            //Check if there are any external deivces
    +            //Skip external device usage collection if none exist
    --- End diff --
    
    @kishankavala Could you please change from "deivces" to "devices"?
    
    Also, you can use this commented lines as a Javadoc block describing the runInContext() method (if you think it would improve your code).
    
    Except that typo, your code seems 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.
---

[GitHub] cloudstack issue #846: Bug-ID: CLOUDSTACK-8870: Skip external device usage c...

Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on the issue:

    https://github.com/apache/cloudstack/pull/846
  
    code/patch 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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-221758314
  
    This is a cleaner run because this issue is a known issue.  I think this one is ready now...


---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-221757982
  
    
    
    ### CI RESULTS
    
    ```
    Tests Run: 82
      Skipped: 0
       Failed: 0
       Errors: 3
     Duration: 8h 33m 42s
    ```
    
    **Summary of the problem(s):**
    ```
    ERROR: test suite for <class 'integration.smoke.test_vpc_vpn.TestRVPCSite2SiteVpn'>
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 209, in run
        self.setUp()
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 292, in setUp
        self.setupContext(ancestor)
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 315, in setupContext
        try_run(context, names)
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/util.py", line 471, in try_run
        return func()
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_vpc_vpn.py", line 835, in setUpClass
        cls.template.download(cls.apiclient)
      File "/usr/lib/python2.7/site-packages/marvin/lib/base.py", line 1350, in download
        elif 'Downloaded' in template.status:
    TypeError: argument of type 'NoneType' is not iterable
    ----------------------------------------------------------------------
    Additional details in: /tmp/MarvinLogs/test_network_VKMDK0/results.txt
    ```
    
    ```
    ERROR: test suite for <class 'integration.smoke.test_vpc_vpn.TestVpcRemoteAccessVpn'>
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 209, in run
        self.setUp()
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 292, in setUp
        self.setupContext(ancestor)
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 315, in setupContext
        try_run(context, names)
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/util.py", line 471, in try_run
        return func()
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_vpc_vpn.py", line 293, in setUpClass
        cls.template.download(cls.apiclient)
      File "/usr/lib/python2.7/site-packages/marvin/lib/base.py", line 1350, in download
        elif 'Downloaded' in template.status:
    TypeError: argument of type 'NoneType' is not iterable
    ----------------------------------------------------------------------
    Additional details in: /tmp/MarvinLogs/test_network_VKMDK0/results.txt
    ```
    
    ```
    ERROR: test suite for <class 'integration.smoke.test_vpc_vpn.TestVpcSite2SiteVpn'>
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 209, in run
        self.setUp()
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 292, in setUp
        self.setupContext(ancestor)
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 315, in setupContext
        try_run(context, names)
      File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/util.py", line 471, in try_run
        return func()
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_vpc_vpn.py", line 472, in setUpClass
        cls.template.download(cls.apiclient)
      File "/usr/lib/python2.7/site-packages/marvin/lib/base.py", line 1350, in download
        elif 'Downloaded' in template.status:
    TypeError: argument of type 'NoneType' is not iterable
    ----------------------------------------------------------------------
    Additional details in: /tmp/MarvinLogs/test_network_VKMDK0/results.txt
    ```
    
    
    
    **Associated Uploads**
    
    **`/tmp/MarvinLogs/DeployDataCenter__May_25_2016_19_10_57_8DI3FD:`**
    * [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/DeployDataCenter__May_25_2016_19_10_57_8DI3FD/dc_entries.obj)
    * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/DeployDataCenter__May_25_2016_19_10_57_8DI3FD/failed_plus_exceptions.txt)
    * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/DeployDataCenter__May_25_2016_19_10_57_8DI3FD/runinfo.txt)
    
    **`/tmp/MarvinLogs/test_network_VKMDK0:`**
    * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_network_VKMDK0/failed_plus_exceptions.txt)
    * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_network_VKMDK0/results.txt)
    * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_network_VKMDK0/runinfo.txt)
    
    **`/tmp/MarvinLogs/test_vpc_routers_PXO5U1:`**
    * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_vpc_routers_PXO5U1/failed_plus_exceptions.txt)
    * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_vpc_routers_PXO5U1/results.txt)
    * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_vpc_routers_PXO5U1/runinfo.txt)
    
    
    Uploads will be available until `2016-07-26 02:00:00 +0200 CEST`
    
    *Comment created by [`upr comment`](https://github.com/cloudops/upr).*



---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-221633655
  
    
    
    ### CI RESULTS
    
    ```
    Tests Run: 85
      Skipped: 0
       Failed: 2
       Errors: 1
     Duration: 10h 41m 55s
    ```
    
    **Summary of the problem(s):**
    ```
    ERROR: Test to verify access to loadbalancer haproxy admin stats page
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_internal_lb.py", line 854, in tearDown
        raise Exception("Cleanup failed with %s" % e)
    Exception: Cleanup failed with Job failed: {jobprocstatus : 0, created : u'2016-05-24T12:24:25+0200', jobresult : {errorcode : 530, errortext : u'Failed to delete network'}, cmd : u'org.apache.cloudstack.api.command.user.network.DeleteNetworkCmd', userid : u'a5538db6-2168-11e6-932f-5254001daa61', jobstatus : 2, jobid : u'8e7ebb93-fb36-4842-8e2d-5cefea2ff80a', jobresultcode : 530, jobresulttype : u'object', jobinstancetype : u'Network', accountid : u'a5537331-2168-11e6-932f-5254001daa61'}
    ----------------------------------------------------------------------
    Additional details in: /tmp/MarvinLogs/test_network_F00C21/results.txt
    ```
    
    ```
    FAIL: test_02_vpc_privategw_static_routes (integration.smoke.test_privategw_acl.TestPrivateGwACL)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 262, in test_02_vpc_privategw_static_routes
        self.performVPCTests(vpc_off)
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 325, in performVPCTests
        privateGw_1 = self.createPvtGw(vpc_1, "10.0.3.100", "10.0.3.101", acl1.id, vlan_1)
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 595, in createPvtGw
        self.fail("Failed to create Private Gateway ==> %s" % e)
    AssertionError: Failed to create Private Gateway ==> Execute cmd: createprivategateway failed, due to: errorCode: 431, errorText:Network with vlan vlan://100 already exists in zone 1
    ----------------------------------------------------------------------
    Additional details in: /tmp/MarvinLogs/test_network_F00C21/results.txt
    ```
    
    ```
    FAIL: Test destroy(expunge) Virtual Machine
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_vm_life_cycle.py", line 646, in test_09_expunge_vm
        self.assertEqual(list_vm_response,None,"Check Expunged virtual machine is in listVirtualMachines response")
    AssertionError: Check Expunged virtual machine is in listVirtualMachines response
    ----------------------------------------------------------------------
    Additional details in: /tmp/MarvinLogs/test_vpc_routers_J2J0PP/results.txt
    ```
    
    
    
    **Associated Uploads**
    
    **`/tmp/MarvinLogs/DeployDataCenter__May_24_2016_06_36_18_ATGDYK:`**
    * [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/DeployDataCenter__May_24_2016_06_36_18_ATGDYK/dc_entries.obj)
    * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/DeployDataCenter__May_24_2016_06_36_18_ATGDYK/failed_plus_exceptions.txt)
    * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/DeployDataCenter__May_24_2016_06_36_18_ATGDYK/runinfo.txt)
    
    **`/tmp/MarvinLogs/test_network_F00C21:`**
    * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_network_F00C21/failed_plus_exceptions.txt)
    * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_network_F00C21/results.txt)
    * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_network_F00C21/runinfo.txt)
    
    **`/tmp/MarvinLogs/test_vpc_routers_J2J0PP:`**
    * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_vpc_routers_J2J0PP/failed_plus_exceptions.txt)
    * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_vpc_routers_J2J0PP/results.txt)
    * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR846/tmp/MarvinLogs/test_vpc_routers_J2J0PP/runinfo.txt)
    
    
    Uploads will be available until `2016-07-25 02:00:00 +0200 CEST`
    
    *Comment created by [`upr comment`](https://github.com/cloudops/upr).*



---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-173198529
  
    Ping @remibergsma @borisroman 
    
    @kishankavala added a unit test to cover his change. I went through the code and haven't found any issue.
    
    The if block ```if(_hostDao.listByType(Host.Type.ExternalFirewall).isEmpty() && _hostDao.listByType(Host.Type.ExternalLoadBalancer).isEmpty()){``` added to ExternalDeviceUsageManagerImpl make sense and are covered.
    
    Please give it a go with the integration tests we have.
    
    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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-220513570
  
    @kishankavala please rebase this PR as we have merge conflicts.  Thanks...


---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

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


---
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: Bug-ID: CLOUDSTACK-8870: Skip external de...

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

    https://github.com/apache/cloudstack/pull/846#issuecomment-216191410
  
    @kishankavala please rebase against latest master and push -f, update on status of your PR
    
    I've left a NPE check, we should merge it once you fix that. Thanks.
    
    tag:easypr
    
    cc @swill 


---
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.
---