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

[asfgit]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[swill]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[swill]

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 
--
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 
--
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 
--
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)
* 

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

[swill]

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 

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

[rhtyd]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[rhtyd]

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 listByType(Host.Type type) {
+SearchCriteria 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[kishankavala]

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 listByType(Host.Type type) {
+SearchCriteria 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[kishankavala]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[swill]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[rhtyd]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[rhtyd]

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 listByType(Host.Type type) {
+SearchCriteria 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[swill]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GabrielBrascher]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[swill]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[rajesh-battala]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GabrielBrascher]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[wilderrodrigues]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[kishankavala]

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 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---