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

kishankavala · 2015-09-18T04:14:17Z

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

asfbot · 2015-09-18T04:22:26Z

cloudstack-pull-rats #648 SUCCESS
This pull request looks good

asfbot · 2015-09-18T05:49:30Z

cloudstack-pull-analysis #596 SUCCESS
This pull request looks good

wilderrodrigues · 2016-01-20T13:09:44Z

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

GabrielBrascher · 2016-02-07T00:51:43Z

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


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

That typo is just a detail, your code seems ok.

rajesh-battala · 2016-04-21T07:21:38Z

I have gone through the code. patch LGTM.

swill · 2016-04-21T15:52:27Z

I need one more LGTM. I will add this to my CI queue. Thx...

GabrielBrascher · 2016-04-21T16:31:38Z

Although I pointed a typo in a comment line (nothing serious), the code LGTM.

swill · 2016-04-21T17:45:28Z

Thank you @GabrielBrascher. @kishankavala would you mind fixing that typo? I will try to get this run though CI soon...

rohityadavcloud · 2016-05-02T10:25:15Z

engine/schema/src/com/cloud/host/dao/HostDaoImpl.java

+    public List<HostVO> listByType(Host.Type type) {
+        SearchCriteria<HostVO> sc = TypeSearch.create();
+        sc.setParameters("type", type);
+        return listBy(sc);


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

@rhtyd
listBy(sc) in GenericDao base returns an empty ArrayList if there is no matching data.
listByType doesn't return null

Fair enough

rohityadavcloud · 2016-05-02T10:25:45Z

@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

swill · 2016-05-20T04:16:57Z

@kishankavala please rebase this PR as we have merge conflicts. Thanks...

kishankavala · 2016-05-23T09:47:26Z

@swill rebased with latest master
@GabrielBrascher fixed the typo

…external devices exist

rohityadavcloud · 2016-05-23T10:04:20Z

LGTM

swill · 2016-05-25T16:38:24Z

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:

/tmp/MarvinLogs/test_network_F00C21:

/tmp/MarvinLogs/test_vpc_routers_J2J0PP:

Uploads will be available until 2016-07-25 02:00:00 +0200 CEST

Comment created by upr comment.

swill · 2016-05-26T01:57:29Z

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:

/tmp/MarvinLogs/test_network_VKMDK0:

/tmp/MarvinLogs/test_vpc_routers_PXO5U1:

Uploads will be available until 2016-07-26 02:00:00 +0200 CEST

Comment created by upr comment.

swill · 2016-05-26T02:00:05Z

This is a cleaner run because this issue is a known issue. I think this one is ready now...

Bug-ID: CLOUDSTACK-8870: Skip external device usage collection if no external devices existexternal 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 * pr/846: Bug-ID: CLOUDSTACK-8870: Skip external device usage collection if no external devices exist Signed-off-by: Will Stevens <williamstevens@gmail.com>

rajesh-battala · 2016-11-07T14:32:54Z

code/patch LGTM.

GabrielBrascher reviewed Feb 7, 2016
View reviewed changes

rohityadavcloud reviewed May 2, 2016
View reviewed changes

kishankavala force-pushed the CLOUDSTACK-8870 branch from a77dc78 to c12d836 Compare May 23, 2016 09:45

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

c12d836

…external devices exist

asfgit merged commit c12d836 into apache:master May 26, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

kishankavala commented Sep 18, 2015

asfbot commented Sep 18, 2015

asfbot commented Sep 18, 2015

wilderrodrigues commented Jan 20, 2016

GabrielBrascher Feb 7, 2016

rajesh-battala commented Apr 21, 2016

swill commented Apr 21, 2016

GabrielBrascher commented Apr 21, 2016 •

edited

swill commented Apr 21, 2016

rohityadavcloud May 2, 2016

kishankavala May 23, 2016

rohityadavcloud May 23, 2016

rohityadavcloud commented May 2, 2016

swill commented May 20, 2016

kishankavala commented May 23, 2016

rohityadavcloud commented May 23, 2016

swill commented May 25, 2016

swill commented May 26, 2016

swill commented May 26, 2016

rajesh-battala commented Nov 7, 2016

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

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

Conversation

kishankavala commented Sep 18, 2015

asfbot commented Sep 18, 2015

asfbot commented Sep 18, 2015

wilderrodrigues commented Jan 20, 2016

GabrielBrascher Feb 7, 2016

Choose a reason for hiding this comment

rajesh-battala commented Apr 21, 2016

swill commented Apr 21, 2016

GabrielBrascher commented Apr 21, 2016 • edited

swill commented Apr 21, 2016

rohityadavcloud May 2, 2016

Choose a reason for hiding this comment

kishankavala May 23, 2016

Choose a reason for hiding this comment

rohityadavcloud May 23, 2016

Choose a reason for hiding this comment

rohityadavcloud commented May 2, 2016

swill commented May 20, 2016

kishankavala commented May 23, 2016

rohityadavcloud commented May 23, 2016

swill commented May 25, 2016

CI RESULTS

swill commented May 26, 2016

CI RESULTS

swill commented May 26, 2016

rajesh-battala commented Nov 7, 2016

GabrielBrascher commented Apr 21, 2016 •

edited