Merge pull request #1516 from sudhansu7/CLOUDSTACK-9366

CLOUDSTACK-9366: Capacity of one zone-wide primary storage ignoredDisable 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. CPU(1) and MEMORY(0) capacity in op_host_capacity for above host is disabled
4. 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 disabling 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:
```
Before 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      |
+---------+----------------+---------------+----------------+-------------------+

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

* pr/1516:
  CLOUDSTACK-9366: Capacity of one zone-wide primary storage ignored

Signed-off-by: Will Stevens <williamstevens@gmail.com>
This commit is contained in:
Will Stevens 2016-05-20 08:31:45 -04:00
commit 3f5b3a16dd
4 changed files with 245 additions and 11 deletions

View File

@ -50,8 +50,7 @@ public interface CapacityDao extends GenericDao<CapacityVO, Long> {
List<SummedCapacity> listCapacitiesGroupedByLevelAndType(Integer capacityType, Long zoneId, Long podId, Long clusterId, int level, Long limit);
void updateCapacityState(Long dcId, Long podId, Long clusterId,
Long hostId, String capacityState);
void updateCapacityState(Long dcId, Long podId, Long clusterId, Long hostId, String capacityState, short[] capacityType);
List<Long> listClustersCrossingThreshold(short capacityType, Long zoneId, String configName, long computeRequested);

View File

@ -962,35 +962,59 @@ public class CapacityDaoImpl extends GenericDaoBase<CapacityVO, Long> implements
}
@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(" (? ");
}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++) {
pstmt.setLong(2 + i, resourceIdList.get(i));
int i = 1;
pstmt.setString(i, capacityState);
i = i + 1;
for (int j=0 ; j < resourceIdList.size(); j++, i++) {
pstmt.setLong(i, resourceIdList.get(j));
}
for(int j=0; j < capacityType.length; i++, j++ ) {
pstmt.setShort(i, capacityType[j]);
}
pstmt.executeUpdate();
} catch (Exception e) {
s_logger.warn("Error updating CapacityVO", e);

View File

@ -1177,7 +1177,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
// TO DO - Make it more granular and have better conversion into capacity type
if(host.getType() == Type.Routing){
final CapacityState capacityState = nextState == ResourceState.Enabled ? CapacityState.Enabled : CapacityState.Disabled;
_capacityDao.updateCapacityState(null, null, null, host.getId(), capacityState.toString());
final short[] capacityTypes = {Capacity.CAPACITY_TYPE_CPU, Capacity.CAPACITY_TYPE_MEMORY};
_capacityDao.updateCapacityState(null, null, null, host.getId(), capacityState.toString(), capacityTypes);
}
return _hostDao.updateResourceState(currentState, event, nextState, host);
}

View File

@ -0,0 +1,210 @@
# 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 = None
if self.host_db_id and self.host_db_id[0]:
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])
if capacity_state and len(capacity_state)==2:
if capacity_state[0]:
self.assertEqual(
capacity_state[0][0],
host_state +
"d",
"Invalid db query response for capacity_state %s" %
capacity_state[0][0])
if capacity_state[1]:
self.assertEqual(
capacity_state[1][0],
host_state +
"d",
"Invalid db query response for capacity_state %s" %
capacity_state[1][0])
else:
self.logger.debug("Could not find capacities of type 1 and 0. Does not have necessary data to run this test")
capacity_state = None
if self.host_db_id and self.host_db_id[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])
if capacity_state and capacity_state[0]:
self.assertNotEqual(
capacity_state[0][0],
host_state +
"d",
"Invalid db query response for capacity_state %s" %
capacity_state[0][0])
else:
self.logger.debug("Could not find capacities of type 3. Does not have necessary data to run this test")
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)
cls.storage_pool_db_id = None
# 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]:
cls.logger.debug("Check capacity count : %s" % capacity_state[0][0])
if capacity_state[0][0] == 3:
cls.logger.debug("found host id : %s, can be used for this test" % host_id)
cls.my_host_id = host_id
cls.host_db_id = host_db_id
cls.storage_pool_db_id = storage_pool_db_id
break
if not cls.storage_pool_db_id:
i = i + 1
if cls.storage_pool_db_id is None:
raise unittest.SkipTest("There is no host and storage pool available in the setup to run this test")
@classmethod
def tearDownClass(cls):
cleanup_resources(cls.apiclient, cls._cleanup)
return
def setUp(self):
self.logger.debug("Capacity check for Disable host")
self.cleanup = []
return
def tearDown(self):
# Clean up
cleanup_resources(self.apiclient, self.cleanup)
return
@attr(tags=["advanced", "basic"], required_hardware="false")
def test_01_op_host_capacity_disable_host(self):
host_state = "Disable"
host_resourcestate = update_host(
self.apiclient,
host_state,
self.my_host_id)
self.assertEqual(
host_resourcestate,
host_state + "d",
"Host state not correct"
)
check_db(self, host_state)
return