You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by sudhansu7 <gi...@git.apache.org> on 2016/04/22 19:36:45 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-9366: Capacity of one zone-wid...
GitHub user sudhansu7 opened a pull request:
https://github.com/apache/cloudstack/pull/1516
CLOUDSTACK-9366: Capacity of one zone-wide primary storage ignored
Disable and Remove Host operation disables the primary storage capacity.
Steps to replicate:
Base Condition: There exists a host and storage pool with same id
Steps:
1. Find a host and storage pool having same id
2. Disable the host
3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host is disabled
4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id same as above host is also disabled
RCA:
'host_id' column in 'op_host_capacity' table used for storing both storage pool id (for STORAGE capacity) and host id (MEMORY and CPU). While removing a HOST we also disable the capacity associated with host.
Ideally while disabling capacity we should only disable MEMORY and CPU capacity, but we are not doing so.
Code Path:
ResourceManagerImpl.doDeleteHost() -> ResourceManagerImpl.resourceStateTransitTo() -> CapacityDaoImpl.updateCapacityState(null, null, null, host.getId(), capacityState.toString())
updateCapacityState is updating disabling all entries which matches the host_id. This will also disable a entry having storage pool id same as that of host id.
Changes:
introduced new capacityType parameter in updateCapacityState method and necessary changes to add capacity_type clause in sql
also fixed incorrect sql builder logic (unused code path for which it is never surfaced )
Added marvin test to check host and storagepool capacity when host is disabled
Test Result:
```
mysql> select ohc.host_id, ohc.`capacity_state`, case capacity_type when 0 then 'MEMORY' when 1 then 'CPU' ELSE 'STORAGE' END as 'capacity_type' , total_capacity, case capacity_type when 0 then 'HOST' when 1 then 'HOST' ELSE 'STORAGE POOL' END as 'HOST/STORAGE POOL' from op_host_capacity ohc where host_id=3;
+---------+----------------+---------------+----------------+-------------------+
| host_id | capacity_state | capacity_type | total_capacity | HOST/STORAGE POOL |
+---------+----------------+---------------+----------------+-------------------+
| 3 | Enabled | MEMORY | 8589934592 | HOST |
| 3 | Enabled | CPU | 32000 | HOST |
| 3 | Enabled | STORAGE | 2199023255552 | STORAGE POOL |
+---------+----------------+---------------+----------------+-------------------+
9 rows in set (0.00 sec)
Disable Host 3 from UI.
mysql> select ohc.host_id, ohc.`capacity_state`, case capacity_type when 0 then 'MEMORY' when 1 then 'CPU' ELSE 'STORAGE' END as 'capacity_type' , total_capacity, case capacity_type when 0 then 'HOST' when 1 then 'HOST' ELSE 'STORAGE POOL' END as 'HOST/STORAGE POOL' from op_host_capacity ohc where host_id=3;
+---------+----------------+---------------+----------------+-------------------+
| host_id | capacity_state | capacity_type | total_capacity | HOST/STORAGE POOL |
+---------+----------------+---------------+----------------+-------------------+
| 3 | Disabled | MEMORY | 8589934592 | HOST |
| 3 | Disabled | CPU | 32000 | HOST |
| 3 | Disabled | STORAGE | 2199023255552 | STORAGE POOL |
+---------+----------------+---------------+----------------+-------------------+
After Fix:
mysql> select ohc.host_id, ohc.`capacity_state`, case capacity_type when 0 then 'MEMORY' when 1 then 'CPU' ELSE 'STORAGE' END as 'capacity_type' , total_capacity, case capacity_type when 0 then 'HOST' when 1 then 'HOST' ELSE 'STORAGE POOL' END as 'HOST/STORAGE POOL' from op_host_capacity ohc where host_id=3;
+---------+----------------+---------------+----------------+-------------------+
| host_id | capacity_state | capacity_type | total_capacity | HOST/STORAGE POOL |
+---------+----------------+---------------+----------------+-------------------+
| 3 | Enabled | MEMORY | 8589934592 | HOST |
| 3 | Enabled | CPU | 32000 | HOST |
| 3 | Enabled | STORAGE | 2199023255552 | STORAGE POOL |
+---------+----------------+---------------+----------------+-------------------+
3 rows in set (0.01 sec)
Disable Host 3 from UI.
mysql> select ohc.host_id, ohc.`capacity_state`, case capacity_type when 0 then 'MEMORY' when 1 then 'CPU' ELSE 'STORAGE' END as 'capacity_type' , total_capacity, case capacity_type when 0 then 'HOST' when 1 then 'HOST' ELSE 'STORAGE POOL' END as 'HOST/STORAGE POOL' from op_host_capacity ohc where host_id=3;
+---------+----------------+---------------+----------------+-------------------+
| host_id | capacity_state | capacity_type | total_capacity | HOST/STORAGE POOL |
+---------+----------------+---------------+----------------+-------------------+
| 3 | Disabled | MEMORY | 8589934592 | HOST |
| 3 | Disabled | CPU | 32000 | HOST |
| 3 | Enabled | STORAGE | 2199023255552 | STORAGE POOL |
+---------+----------------+---------------+----------------+-------------------+
3 rows in set (0.00 sec)
Sudhansus-MAC:cloudstack sudhansu$ nosetests-2.7 --with-marvin --marvin-config=setup/dev/advanced.cfg test/integration/component/maint/test_capacity_host_delete.py
==== Marvin Init Started ====
=== Marvin Parse Config Successful ===
=== Marvin Setting TestData Successful===
==== Log Folder Path: /tmp//MarvinLogs//Apr_22_2016_22_42_27_X4VBWD. All logs will be available here ====
=== Marvin Init Logging Successful===
==== Marvin Init Successful ====
===final results are now copied to: /tmp//MarvinLogs/test_capacity_host_delete_9RHSNB===
Sudhansus-MAC:cloudstack sudhansu$ cat /tmp//MarvinLogs/test_capacity_host_delete_9RHSNB/results.txt
test_01_op_host_capacity_disable_host (integration.component.maint.test_capacity_host_delete.TestHosts) ... === TestName: test_01_op_host_capacity_disable_host | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 1 test in 0.168s
OK
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sudhansu7/cloudstack CLOUDSTACK-9366
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1516.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 #1516
----
commit ebeb26a351dc95f2bef877fe469a05952821b97a
Author: Sudhansu <su...@accelerite.com>
Date: 2016-04-22T10:06:51Z
CLOUDSTACK-9366: Capacity of one zone-wide primary storage ignored
introduced new capacityType parameter in updateCapacityState method and necessary changes to add capacity_type clause in sql
also fixed incorrect sql builder logic (unused code path for which it is never surfaced )
Added marvin test to check host and storagepool capacity when host is disabled
----
---
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-9366: Capacity of one zone-wid...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-216303934
LGTM, considdered it implied by the test report but here it is.
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-219706136
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 0
Errors: 2
Duration: 8h 54m 46s
```
**Summary of the problem(s):**
```
ERROR: test suite for <class 'integration.smoke.test_internal_lb.TestInternalLb'>
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 228, in run
self.tearDown()
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 351, in tearDown
self.teardownContext(ancestor)
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 367, in teardownContext
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_internal_lb.py", line 847, in tearDownClass
raise Exception("Cleanup failed with %s" % e)
Exception: Cleanup failed with Job failed: {jobprocstatus : 0, created : u'2016-05-17T06:38:54+0200', jobresult : {errorcode : 530, errortext : u'Failed to delete template'}, cmd : u'org.apache.cloudstack.api.command.user.template.DeleteTemplateCmd', userid : u'024b32e3-1bc2-11e6-9386-5254001daa61', jobstatus : 2, jobid : u'f821c138-df6b-4ce2-bac4-b660a3f9e4fd', jobresultcode : 530, jobinstanceid : u'71ec6930-d516-400d-8296-679d88cb2d98', jobresulttype : u'object', jobinstancetype : u'Template', accountid : u'024b19df-1bc2-11e6-9386-5254001daa61'}
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_1E239G/results.txt
```
```
ERROR: test suite for <class 'integration.component.maint.test_capacity_host_delete.TestHosts'>
----------------------------------------------------------------------
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/component/maint/test_capacity_host_delete.py", line 138, in setUpClass
cls.logger.debug("Found storage_pool_db_id : %s" % storage_pool_db_id[0][0])
IndexError: list index out of range
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_vpc_routers_VMPIZ9/results.txt
```
**Associated Uploads**
**`/tmp/MarvinLogs/DeployDataCenter__May_17_2016_02_00_55_GYUDUS:`**
* [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/DeployDataCenter__May_17_2016_02_00_55_GYUDUS/dc_entries.obj)
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/DeployDataCenter__May_17_2016_02_00_55_GYUDUS/failed_plus_exceptions.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/DeployDataCenter__May_17_2016_02_00_55_GYUDUS/runinfo.txt)
**`/tmp/MarvinLogs/test_network_1E239G:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_network_1E239G/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_network_1E239G/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_network_1E239G/runinfo.txt)
**`/tmp/MarvinLogs/test_vpc_routers_VMPIZ9:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_vpc_routers_VMPIZ9/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_vpc_routers_VMPIZ9/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_vpc_routers_VMPIZ9/runinfo.txt)
Uploads will be available until `2016-07-17 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-219538895
@sudhansu7 can you rebase and push to see if we can get this green before the freeze? 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-217105794
@rhtyd @swill
The branch has been re-based against master and also squashed multiple commits to a single.
---
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-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63894357
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
--- End diff --
This check is not required. The check_db will only be invoked if there is a host_db_id, storage_pool_db_id and capacity check =3.
In my last check in I have added checks for this.
---
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-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63893791
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id and host_db_id[0]:
+ cls.logger.debug("found host db id : %s" % host_db_id)
+ storage_pool_db_id = cls.dbclient.execute(
+ "select id from storage_pool where id='%s' and removed is null;" %
+ host_db_id[0][0])
+
+ if storage_pool_db_id and storage_pool_db_id[0]:
--- End diff --
Fixed 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-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63893826
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id and host_db_id[0]:
+ cls.logger.debug("found host db id : %s" % host_db_id)
+ storage_pool_db_id = cls.dbclient.execute(
+ "select id from storage_pool where id='%s' and removed is null;" %
+ host_db_id[0][0])
+
+ if storage_pool_db_id and storage_pool_db_id[0]:
+ cls.logger.debug("Found storage_pool_db_id : %s" % storage_pool_db_id[0][0])
+ capacity_state = cls.dbclient.execute(
+ "select count(capacity_state) from op_host_capacity where host_id='%s' and capacity_type in (0,1,3) and capacity_state = 'Enabled'" %
+ host_db_id[0][0])
+
+ if capacity_state and capacity_state[0]:
--- End diff --
Fixed 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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-219707593
@sudhansu7 there are problems with the tests added in this PR. We need to be programming a lot more defensively. There are a LOT of places where something like this is called without any verification that it exists: `some_var[0][0]`
---
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-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-219995275
@swill Thanks for the review. I have updated the test. Now its green. finally.
---
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-9366: Capacity of one zone-wid...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-214143155
LGTM. Code Reviewed.
---
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-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r60844257
--- Diff: engine/schema/src/com/cloud/capacity/dao/CapacityDaoImpl.java ---
@@ -962,35 +962,58 @@ public boolean removeBy(Short capacityType, Long zoneId, Long podId, Long cluste
}
@Override
- public void updateCapacityState(Long dcId, Long podId, Long clusterId, Long hostId, String capacityState) {
+ public void updateCapacityState(Long dcId, Long podId, Long clusterId, Long hostId, String capacityState, short[] capacityType) {
TransactionLegacy txn = TransactionLegacy.currentTxn();
StringBuilder sql = new StringBuilder(UPDATE_CAPACITY_STATE);
List<Long> resourceIdList = new ArrayList<Long>();
+ StringBuilder where = new StringBuilder();
if (dcId != null) {
- sql.append(" data_center_id = ?");
+ where.append(" data_center_id = ? ");
resourceIdList.add(dcId);
}
if (podId != null) {
- sql.append(" pod_id = ?");
+ where.append((where.length() > 0) ? " and pod_id = ? " : " pod_id = ? ");
resourceIdList.add(podId);
}
if (clusterId != null) {
- sql.append(" cluster_id = ?");
+ where.append((where.length() > 0) ? " and cluster_id = ? " : " cluster_id = ? ");
resourceIdList.add(clusterId);
}
if (hostId != null) {
- sql.append(" host_id = ?");
+ where.append((where.length() > 0) ? " and host_id = ? " : " host_id = ? ");
resourceIdList.add(hostId);
}
+ if (capacityType != null) {
+ where.append((where.length() > 0) ? " and capacity_type in " : " capacity_type in ");
+
+ StringBuilder builder = new StringBuilder();
+ for( int i = 0 ; i < capacityType.length; i++ ) {
+ if(i==0){
+ builder.append(" (? ");
+ }else{
+ builder.append(" ,? ");
+ }
+ }
+ builder.append(" ) ");
--- End diff --
Fixed.
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63721020
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id:
--- End diff --
So you can probably get away with:
```
if host_db_id and host_db_id[0]:
```
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63722337
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id:
--- End diff --
Here is an example to illustrate that you need to be checking both the outer and inner lists:
```
>>> some_list = [['one', 'two'], ['three', 'four']]
>>> if some_list and some_list[0]:
... print("has nested list")
... else:
... print("failed check")
...
has nested list
>>>
>>> some_list = [[], []]
>>> if some_list and some_list[0]:
... print("has nested list")
... else:
... print("failed check")
...
failed check
>>>
>>>
>>> some_list = [[], []]
>>> if some_list:
... print("try and access nested list")
... print(some_list[0][0])
... else:
... print("failed check")
...
try and access nested list
Traceback (most recent call last):
File "<stdin>", line 3, in <module>
IndexError: list index out of range
```
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63720420
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id:
--- End diff --
Yes, that works if you only reference `some_list[0]`, but you are referencing nested lists with `some_list[0][0]` so you need to validate that the inside list is also there and not empty. The first check to `len(some_list)` is probably not needed because you did `if some_list`, but you definitely need to be checking that the list inside `some_list[0]` is also not empty...
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-220393200
Thank you for the clean up. It looks much better now. I will rerun CI on this...
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63739140
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
--- End diff --
`self.host_db_id[0][0])` may not be set because any number of conditionals in `setUpClass` could fail. You should be checking both lists exist before using 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-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63715139
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id:
--- End diff --
@swill
I am not an expert in python, but as per [PEP 8](https://www.python.org/dev/peps/pep-0008/)
For sequences, (strings, lists, tuples), use the fact that empty sequences are false.
Yes: if not seq:
if seq:
No: if len(seq):
if not len(seq):
As I am selecting only one column , so if the array is not empty then it has at least one row and column.
Please suggest if you think otherwise.
---
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-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-213954430
@DaanHoogland
Thanks for the review. I have made necessary changes. I think in t=your test setup op_host_capacity does not have capacity details, for which the test is failing. In my latest commit , I have added checks to ensure op_host_capacity is having required data before picking the host for test.
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-216305760
@DaanHoogland thanks...
@sudhansu7 can you do a force push to kick off the travis run again so we can get this PR green. 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63707528
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id:
--- End diff --
This applies to all similar situations.
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-218807140
@sudhansu7 I pushed fixes to Jenkins and Travis this morning. Can you please force push again so we can get this green. 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r60826329
--- Diff: engine/schema/src/com/cloud/capacity/dao/CapacityDaoImpl.java ---
@@ -962,35 +962,58 @@ public boolean removeBy(Short capacityType, Long zoneId, Long podId, Long cluste
}
@Override
- public void updateCapacityState(Long dcId, Long podId, Long clusterId, Long hostId, String capacityState) {
+ public void updateCapacityState(Long dcId, Long podId, Long clusterId, Long hostId, String capacityState, short[] capacityType) {
TransactionLegacy txn = TransactionLegacy.currentTxn();
StringBuilder sql = new StringBuilder(UPDATE_CAPACITY_STATE);
List<Long> resourceIdList = new ArrayList<Long>();
+ StringBuilder where = new StringBuilder();
if (dcId != null) {
- sql.append(" data_center_id = ?");
+ where.append(" data_center_id = ? ");
resourceIdList.add(dcId);
}
if (podId != null) {
- sql.append(" pod_id = ?");
+ where.append((where.length() > 0) ? " and pod_id = ? " : " pod_id = ? ");
resourceIdList.add(podId);
}
if (clusterId != null) {
- sql.append(" cluster_id = ?");
+ where.append((where.length() > 0) ? " and cluster_id = ? " : " cluster_id = ? ");
resourceIdList.add(clusterId);
}
if (hostId != null) {
- sql.append(" host_id = ?");
+ where.append((where.length() > 0) ? " and host_id = ? " : " host_id = ? ");
resourceIdList.add(hostId);
}
+ if (capacityType != null) {
+ where.append((where.length() > 0) ? " and capacity_type in " : " capacity_type in ");
+
+ StringBuilder builder = new StringBuilder();
+ for( int i = 0 ; i < capacityType.length; i++ ) {
+ if(i==0){
+ builder.append(" (? ");
+ }else{
+ builder.append(" ,? ");
+ }
+ }
+ builder.append(" ) ");
--- End diff --
gets appended even if capacityTypes.length == 0. please guard 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-9366: Capacity of one zone-wid...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-218962171
@sudhansu7 yeah, my bad -- I've fixed them here: https://github.com/apache/cloudstack/pull/1543
---
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-9366: Capacity of one zone-wid...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-216229873
@sudhansu7 rebase against master and push -f, squash changes to a single commit, 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63738053
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id and host_db_id[0]:
+ cls.logger.debug("found host db id : %s" % host_db_id)
+ storage_pool_db_id = cls.dbclient.execute(
+ "select id from storage_pool where id='%s' and removed is null;" %
+ host_db_id[0][0])
+
+ if storage_pool_db_id and storage_pool_db_id[0]:
--- End diff --
If the condition `if host_db_id and host_db_id[0]:` evaluates to false, then `storage_pool_db_id` will not be defined and I think this will cause a problem because the variable will not be defined.
For example:
```
NameError: name 'storage_pool_db_id' is not defined
```
You should be making sure the variable is defined outside the conditional otherwise you can run into this...
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-217472184
Can you force push again to get the jobs to kick off again. 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-216435537
@sudhansu7 can you rebase against latest master, squash the changes to a single commit and push -f to re-kick travis. 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-216286395
We are missing one code review on this 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-9366: Capacity of one zone-wid...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r60826448
--- Diff: engine/schema/src/com/cloud/capacity/dao/CapacityDaoImpl.java ---
@@ -962,35 +962,58 @@ public boolean removeBy(Short capacityType, Long zoneId, Long podId, Long cluste
}
@Override
- public void updateCapacityState(Long dcId, Long podId, Long clusterId, Long hostId, String capacityState) {
+ public void updateCapacityState(Long dcId, Long podId, Long clusterId, Long hostId, String capacityState, short[] capacityType) {
TransactionLegacy txn = TransactionLegacy.currentTxn();
StringBuilder sql = new StringBuilder(UPDATE_CAPACITY_STATE);
List<Long> resourceIdList = new ArrayList<Long>();
+ StringBuilder where = new StringBuilder();
if (dcId != null) {
- sql.append(" data_center_id = ?");
+ where.append(" data_center_id = ? ");
resourceIdList.add(dcId);
}
if (podId != null) {
- sql.append(" pod_id = ?");
+ where.append((where.length() > 0) ? " and pod_id = ? " : " pod_id = ? ");
resourceIdList.add(podId);
}
if (clusterId != null) {
- sql.append(" cluster_id = ?");
+ where.append((where.length() > 0) ? " and cluster_id = ? " : " cluster_id = ? ");
resourceIdList.add(clusterId);
}
if (hostId != null) {
- sql.append(" host_id = ?");
+ where.append((where.length() > 0) ? " and host_id = ? " : " host_id = ? ");
resourceIdList.add(hostId);
}
+ if (capacityType != null) {
+ where.append((where.length() > 0) ? " and capacity_type in " : " capacity_type in ");
+
+ StringBuilder builder = new StringBuilder();
+ for( int i = 0 ; i < capacityType.length; i++ ) {
+ if(i==0){
+ builder.append(" (? ");
+ }else{
+ builder.append(" ,? ");
+ }
+ }
+ builder.append(" ) ");
+
+ where.append(builder);
+ }
+ sql.append(where);
+
PreparedStatement pstmt = null;
try {
pstmt = txn.prepareAutoCloseStatement(sql.toString());
pstmt.setString(1, capacityState);
- for (int i = 0; i < resourceIdList.size(); i++) {
+ int i = 0;
+ for (; i < resourceIdList.size(); i++) {
pstmt.setLong(2 + i, resourceIdList.get(i));
}
+ for(int j=0; j < capacityType.length; i++, j++ ) {
+ pstmt.setShort(2 + i, capacityType[j]);
--- End diff --
from the code above you can not guarantee capacity types are starting at parameter no.3. The 2 above should be a variable kept track off since the start of updateCapacityState()
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63707401
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id:
--- End diff --
These checks should probably be more extensive since we are still making assumptions about what is inside that object which we are not checking.
For example:
```
if host_db_id and len(host_db_id) > 0 and len(host_db_id[0]) > 0:
```
---
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-9366: Capacity of one zone-wid...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1516
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-217269248
Can you do a force push to kick of Jenkins again. 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-218366572
@sudhansu7 can you force push again to kick off Jenkins and Travis again? 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-220512874
### CI RESULTS
```
Tests Run: 84
Skipped: 0
Failed: 0
Errors: 2
Duration: 8h 36m 08s
```
**Summary of the problem(s):**
```
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_OMDZZF/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_OMDZZF/results.txt
```
**Associated Uploads**
**`/tmp/MarvinLogs/DeployDataCenter__May_19_2016_19_49_54_JIUQTZ:`**
* [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/DeployDataCenter__May_19_2016_19_49_54_JIUQTZ/dc_entries.obj)
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/DeployDataCenter__May_19_2016_19_49_54_JIUQTZ/failed_plus_exceptions.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/DeployDataCenter__May_19_2016_19_49_54_JIUQTZ/runinfo.txt)
**`/tmp/MarvinLogs/test_capacity_host_delete_B4HOBT:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_capacity_host_delete_B4HOBT/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_capacity_host_delete_B4HOBT/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_capacity_host_delete_B4HOBT/runinfo.txt)
**`/tmp/MarvinLogs/test_network_OMDZZF:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_network_OMDZZF/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_network_OMDZZF/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1516/tmp/MarvinLogs/test_network_OMDZZF/runinfo.txt)
Uploads will be available until `2016-07-20 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: CLOUDSTACK-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63731204
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id:
--- End diff --
Got 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-9366: Capacity of one zone-wid...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-213825554
regression suite passed:
[1516.results.network.txt](https://github.com/apache/cloudstack/files/233136/1516.results.network.txt)
[1516.results.vpc_routers.txt](https://github.com/apache/cloudstack/files/233137/1516.results.vpc_routers.txt)
$ ssh root@192.168.23.5
Warning: Permanently added '192.168.23.5' (ECDSA) to the list of known hosts.
root@192.168.23.5's password:
# ping -c 3 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: seq=0 ttl=46 time=24.794 ms
64 bytes from 8.8.8.8: seq=1 ttl=46 time=22.653 ms
64 bytes from 8.8.8.8: seq=2 ttl=46 time=24.178 ms
--- 8.8.8.8 ping statistics ---
3 packets transmitted, 3 packets received, 0% packet loss
round-trip min/avg/max = 22.653/23.875/24.794 ms
but the added test did not pass. did investigate yet:
[1516.results.capacity_host_delete.txt](https://github.com/apache/cloudstack/files/233138/1516.results.capacity_host_delete.txt)
---
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-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on the pull request:
https://github.com/apache/cloudstack/pull/1516#issuecomment-218960848
@rhtyd
Jenkin build is failing for cloud-util tests.
Below is the snippet. Could you please check this.
-------------------------------------------------------
T E S T S
-------------------------------------------------------
Running org.apache.cloudstack.utils.volume.VirtualMachineDiskInfoTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.51 sec - in org.apache.cloudstack.utils.volume.VirtualMachineDiskInfoTest
Running org.apache.cloudstack.utils.imagestore.ImageStoreUtilTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.345 sec - in org.apache.cloudstack.utils.imagestore.ImageStoreUtilTest
Running org.apache.cloudstack.utils.process.ProcessTest
Tests run: 4, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 301.191 sec <<< FAILURE! - in org.apache.cloudstack.utils.process.ProcessTest
testProcessRunner(org.apache.cloudstack.utils.process.ProcessTest) Time elapsed: 300.103 sec <<< FAILURE!
java.lang.AssertionError: expected:<-1> but was:<0>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at org.junit.Assert.assertEquals(Assert.java:631)
at org.apache.cloudstack.utils.process.ProcessTest.testProcessRunner(ProcessTest.java:43)
---
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-9366: Capacity of one zone-wid...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63895181
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
--- End diff --
In my last check in I have added checks for this.
Actually we don't have to add these checks for inner list. The array is generated from mysql cursor. So either its empty (if no rows found) or it has a value in [0][0].
---
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-9366: Capacity of one zone-wid...
Posted by alexandrelimassantana <gi...@git.apache.org>.
Github user alexandrelimassantana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r60852850
--- Diff: engine/schema/src/com/cloud/capacity/dao/CapacityDaoImpl.java ---
@@ -962,35 +962,59 @@ public boolean removeBy(Short capacityType, Long zoneId, Long podId, Long cluste
}
@Override
- public void updateCapacityState(Long dcId, Long podId, Long clusterId, Long hostId, String capacityState) {
+ public void updateCapacityState(Long dcId, Long podId, Long clusterId, Long hostId, String capacityState, short[] capacityType) {
TransactionLegacy txn = TransactionLegacy.currentTxn();
StringBuilder sql = new StringBuilder(UPDATE_CAPACITY_STATE);
List<Long> resourceIdList = new ArrayList<Long>();
+ StringBuilder where = new StringBuilder();
if (dcId != null) {
- sql.append(" data_center_id = ?");
+ where.append(" data_center_id = ? ");
resourceIdList.add(dcId);
}
if (podId != null) {
- sql.append(" pod_id = ?");
+ where.append((where.length() > 0) ? " and pod_id = ? " : " pod_id = ? ");
resourceIdList.add(podId);
}
if (clusterId != null) {
- sql.append(" cluster_id = ?");
+ where.append((where.length() > 0) ? " and cluster_id = ? " : " cluster_id = ? ");
resourceIdList.add(clusterId);
}
if (hostId != null) {
- sql.append(" host_id = ?");
+ where.append((where.length() > 0) ? " and host_id = ? " : " host_id = ? ");
resourceIdList.add(hostId);
}
+ if (capacityType != null && capacityType.length > 0) {
+ where.append((where.length() > 0) ? " and capacity_type in " : " capacity_type in ");
+
+ StringBuilder builder = new StringBuilder();
+ for( int i = 0 ; i < capacityType.length; i++ ) {
+ if(i==0){
+ builder.append(" (? ");
--- End diff --
You can extract the if(i==0) code from the for and write it before the loop. That way you don`t need to check anything inside the loop
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63738462
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ self.assertEqual(
+ capacity_state[1][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[1][0])
+
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type = 3 order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertNotEqual(
+ capacity_state[0][0],
+ host_state +
+ "d",
+ "Invalid db query response for capacity_state %s" %
+ capacity_state[0][0])
+ return capacity_state[0][0]
+
+
+class TestHosts(cloudstackTestCase):
+
+ """
+ Testing Hosts
+ """
+ @classmethod
+ def setUpClass(cls):
+ cls.testClient = super(TestHosts, cls).getClsTestClient()
+ cls.testdata = cls.testClient.getParsedTestDataConfig()
+ cls.apiclient = cls.testClient.getApiClient()
+ cls.dbclient = cls.testClient.getDbConnection()
+ cls._cleanup = []
+
+ # get zone, domain etc
+ cls.zone = Zone(get_zone(cls.apiclient, cls.testClient.getZoneForTests()).__dict__)
+ cls.domain = get_domain(cls.apiclient)
+ cls.pod = get_pod(cls.apiclient, cls.zone.id)
+
+ cls.logger = logging.getLogger('TestHosts')
+ cls.stream_handler = logging.StreamHandler()
+ cls.logger.setLevel(logging.DEBUG)
+ cls.logger.addHandler(cls.stream_handler)
+
+ # list hosts
+ hosts = list_hosts(cls.apiclient, type="Routing")
+ i = 0
+ while (i < len(hosts)):
+ host_id = hosts[i].id
+ cls.logger.debug("Trying host id : %s" % host_id)
+ host_db_id = cls.dbclient.execute(
+ "select id from host where uuid='%s';" %
+ host_id)
+
+ if host_db_id and host_db_id[0]:
+ cls.logger.debug("found host db id : %s" % host_db_id)
+ storage_pool_db_id = cls.dbclient.execute(
+ "select id from storage_pool where id='%s' and removed is null;" %
+ host_db_id[0][0])
+
+ if storage_pool_db_id and storage_pool_db_id[0]:
+ cls.logger.debug("Found storage_pool_db_id : %s" % storage_pool_db_id[0][0])
+ capacity_state = cls.dbclient.execute(
+ "select count(capacity_state) from op_host_capacity where host_id='%s' and capacity_type in (0,1,3) and capacity_state = 'Enabled'" %
+ host_db_id[0][0])
+
+ if capacity_state and capacity_state[0]:
--- End diff --
Again `capacity_state` is not defined in the scope it is being used and the definition is dependant on a conditional that could fail.
---
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-9366: Capacity of one zone-wid...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1516#discussion_r63739326
--- Diff: test/integration/component/maint/test_capacity_host_delete.py ---
@@ -0,0 +1,195 @@
+# 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 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.
+
+# Test from the Marvin - Testing in Python wiki
+
+# All tests inherit from cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+
+# Import Integration Libraries
+
+# base - contains all resources as entities and defines create, delete,
+# list operations on them
+from marvin.lib.base import Host, Cluster, Zone, Pod
+
+# utils - utility classes for common cleanup, external library wrappers etc
+from marvin.lib.utils import cleanup_resources
+
+# common - commonly used methods for all tests are listed here
+from marvin.lib.common import get_zone, get_domain, list_hosts, get_pod
+
+from nose.plugins.attrib import attr
+
+import time
+import logging
+# These tests need to be run separately and not in parallel with other tests.
+# Because it disables the host
+# host_id column of op_host_capacity refers to host_id or a storage pool id
+#
+# This test is to make sure that Disable host only disables the capacities of type
+# CPU and MEMORY
+#
+# TEST:
+# Base Condition: There exists a host and storage pool with same id
+#
+# Steps:
+# 1. Find a host and storage pool having same id
+# 2. Disable the host
+# 3. verify that the CPU(1) and MEMORY(0) capacity in op_host_capacity for above host
+# is disabled
+# 4. verify that the STORAGE(3) capacity in op_host_capacity for storage pool with id
+# same as above host is not disabled
+#
+
+def update_host(apiclient, state, host_id):
+ """
+ Function to Enable/Disable Host
+ """
+ host_status = Host.update(
+ apiclient,
+ id=host_id,
+ allocationstate=state
+ )
+ return host_status.resourcestate
+
+
+def check_db(self, host_state):
+ """
+ Function to check capacity_state in op_host_capacity table
+ """
+ capacity_state = self.dbclient.execute(
+ "select capacity_state from op_host_capacity where host_id='%s' and capacity_type in (0,1) order by capacity_type asc;" %
+ self.host_db_id[0][0])
+ self.assertEqual(
+ capacity_state[0][0],
--- End diff --
`capacity_state[0][0]` has not been verified to be a valid multidimensional array at this point.
---
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.
---