openstack/cinder
Code Issues Proposed changes

Fix resource count for os-host show

Browse Source 
os-host show API should only return the resouce count on the
specified host.
The logic in Cinder which counts the project's resouce on all host
is wrong.

Change-Id: I96cd285a82b44af8818514692818e739443dcc45
Closes-bug: #1699936
This commit is contained in:
wangxiyuan 2017-06-23 10:41:08 +08:00 committed by TommyLike
parent 10a3f4e1c6
commit f50b355577
8 changed files with 148 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
cinder/api/contrib/hosts.py
View File

@ -135,12 +135,12 @@ class HostController(wsgi.Controller):
def show(self, req, id):
"""Shows the volume usage info given by hosts.
:param context: security context
:param host: hostname
:returns: expected to use HostShowTemplate.
:param req: security context
:param id: hostname
:returns: dict -- the host resources dictionary.
ex.::
{'host': {'resource':D},..}
{'host': [{'resource': D},..]}
D: {'host': 'hostname','project': 'admin',
'volume_count': 1, 'total_volume_gb': 2048}
"""
@ -154,33 +154,33 @@ class HostController(wsgi.Controller):
host_ref = objects.Service.get_by_host_and_topic(
context, host, constants.VOLUME_TOPIC)
# Getting total available/used resource
# TODO(jdg): Add summary info for Snapshots
# Getting total available/used resource on a host.
volume_refs = db.volume_get_all_by_host(context, host_ref.host)
(count, sum) = db.volume_data_get_for_host(context,
host_ref.host)
(count, vol_sum) = db.volume_data_get_for_host(context,
host_ref.host)
snap_count_total = 0
snap_sum_total = 0
resources = [{'resource': {'host': host, 'project': '(total)',
'volume_count': str(count),
'total_volume_gb': str(sum),
'total_volume_gb': str(vol_sum),
'snapshot_count': str(snap_count_total),
'total_snapshot_gb': str(snap_sum_total)}}]
project_ids = [v['project_id'] for v in volume_refs]
project_ids = list(set(project_ids))
for project_id in project_ids:
(count, sum) = db.volume_data_get_for_project(context, project_id)
(count, vol_sum) = db.volume_data_get_for_project(
context, project_id, host=host_ref.host)
(snap_count, snap_sum) = (
objects.Snapshot.snapshot_data_get_for_project(context,
project_id))
objects.Snapshot.snapshot_data_get_for_project(
context, project_id, host=host_ref.host))
resources.append(
{'resource':
{'host': host,
'project': project_id,
'volume_count': str(count),
'total_volume_gb': str(sum),
'total_volume_gb': str(vol_sum),
'snapshot_count': str(snap_count),
'total_snapshot_gb': str(snap_sum)}})
snap_count_total += int(snap_count)

10
cinder/db/api.py
View File

@ -253,9 +253,9 @@ def volume_data_get_for_host(context, host, count_only=False):
count_only)
def volume_data_get_for_project(context, project_id):
def volume_data_get_for_project(context, project_id, host=None):
"""Get (volume_count, gigabytes) for project."""
return IMPL.volume_data_get_for_project(context, project_id)
return IMPL.volume_data_get_for_project(context, project_id, host=host)
def volume_destroy(context, volume_id):
@ -501,11 +501,13 @@ def snapshot_update(context, snapshot_id, values):
return IMPL.snapshot_update(context, snapshot_id, values)
def snapshot_data_get_for_project(context, project_id, volume_type_id=None):
def snapshot_data_get_for_project(context, project_id, volume_type_id=None,
host=None):
"""Get count and gigabytes used for snapshots for specified project."""
return IMPL.snapshot_data_get_for_project(context,
project_id,
volume_type_id)
volume_type_id,
host=host)
def snapshot_get_all_active_by_window(context, begin, end=None,

34
cinder/db/sqlalchemy/api.py
View File

@ -1592,13 +1592,15 @@ def volume_data_get_for_host(context, host, count_only=False):
@require_admin_context
def _volume_data_get_for_project(context, project_id, volume_type_id=None,
session=None):
session=None, host=None):
query = model_query(context,
func.count(models.Volume.id),
func.sum(models.Volume.size),
read_deleted="no",
session=session).\
filter_by(project_id=project_id)
if host:
query = query.filter(_filter_host(models.Volume.host, host))
if volume_type_id:
query = query.filter_by(volume_type_id=volume_type_id)
@ -1629,8 +1631,10 @@ def _backup_data_get_for_project(context, project_id, volume_type_id=None,
@require_admin_context
def volume_data_get_for_project(context, project_id, volume_type_id=None):
return _volume_data_get_for_project(context, project_id, volume_type_id)
def volume_data_get_for_project(context, project_id,
volume_type_id=None, host=None):
return _volume_data_get_for_project(context, project_id,
volume_type_id, host=host)
@require_admin_context
@ -3225,27 +3229,31 @@ def snapshot_get_all_by_project(context, project_id, filters=None, marker=None,
@require_context
def _snapshot_data_get_for_project(context, project_id, volume_type_id=None,
session=None):
session=None, host=None):
authorize_project_context(context, project_id)
query = model_query(context,
func.count(models.Snapshot.id),
func.sum(models.Snapshot.volume_size),
read_deleted="no",
session=session).\
filter_by(project_id=project_id)
if volume_type_id:
query = query.join('volume').filter_by(volume_type_id=volume_type_id)
result = query.first()
session=session)
if volume_type_id or host:
query = query.join('volume')
if volume_type_id:
query = query.filter(
models.Volume.volume_type_id == volume_type_id)
if host:
query = query.filter(_filter_host(models.Volume.host, host))
result = query.filter(models.Snapshot.project_id == project_id).first()
# NOTE(vish): convert None to 0
return (result[0] or 0, result[1] or 0)
@require_context
def snapshot_data_get_for_project(context, project_id, volume_type_id=None):
return _snapshot_data_get_for_project(context, project_id, volume_type_id)
def snapshot_data_get_for_project(context, project_id,
volume_type_id=None, host=None):
return _snapshot_data_get_for_project(context, project_id, volume_type_id,
host=host)
@require_context

5
cinder/objects/snapshot.py
View File

@ -273,9 +273,10 @@ class Snapshot(cleanable.CinderCleanableObject, base.CinderObject,
@classmethod
def snapshot_data_get_for_project(cls, context, project_id,
volume_type_id=None):
volume_type_id=None, host=None):
return db.snapshot_data_get_for_project(context, project_id,
volume_type_id)
volume_type_id,
host=host)
@staticmethod
def _is_cleanable(status, obj_version):

61
cinder/tests/unit/api/contrib/test_hosts.py
View File

@ -14,6 +14,7 @@
# under the License.
import datetime
import mock
import iso8601
from oslo_utils import timeutils
@ -22,7 +23,10 @@ import webob.exc
from cinder.api.contrib import hosts as os_hosts
from cinder import context
from cinder import exception
from cinder.objects import service
from cinder import test
from cinder.tests.unit import fake_constants
from cinder.tests.unit import utils as test_utils
created_time = datetime.datetime(2012, 11, 14, 1, 20, 41, 95099)
@ -150,6 +154,63 @@ class HostTestCase(test.TestCase):
'bogus_host_name',
body={'disabled': 0})
@mock.patch.object(service.Service, 'get_by_host_and_topic')
def test_show_host(self, mock_get_host):
host = 'test_host'
test_service = service.Service(id=1, host=host,
binary='cinder-volume',
topic='cinder-volume')
mock_get_host.return_value = test_service
ctxt1 = context.RequestContext(project_id=fake_constants.PROJECT_ID,
is_admin=True)
ctxt2 = context.RequestContext(project_id=fake_constants.PROJECT2_ID,
is_admin=True)
# Create two volumes with different project.
volume1 = test_utils.create_volume(ctxt1,
host=host, size=1)
test_utils.create_volume(ctxt2, host=host, size=1)
# This volume is not on the same host. It should not be counted.
test_utils.create_volume(ctxt2, host='fake_host', size=1)
test_utils.create_snapshot(ctxt1, volume_id=volume1.id)
resp = self.controller.show(self.req, host)
host_resp = resp['host']
# There are 3 resource list: total, project1, project2
self.assertEqual(3, len(host_resp))
expected = [
{
"resource": {
"volume_count": "2",
"total_volume_gb": "2",
"host": "test_host",
"total_snapshot_gb": "1",
"project": "(total)",
"snapshot_count": "1"}
},
{
"resource": {
"volume_count": "1",
"total_volume_gb": "1",
"host": "test_host",
"project": fake_constants.PROJECT2_ID,
"total_snapshot_gb": "0",
"snapshot_count": "0"}
},
{
"resource": {
"volume_count": "1",
"total_volume_gb": "1",
"host": "test_host",
"total_snapshot_gb": "1",
"project": fake_constants.PROJECT_ID,
"snapshot_count": "1"}
}
]
self.assertListEqual(expected, sorted(
host_resp, key=lambda h: h['resource']['project']))
def test_show_forbidden(self):
self.req.environ['cinder.context'].is_admin = False
dest = 'dummydest'

3
cinder/tests/unit/objects/test_snapshot.py
View File

@ -201,7 +201,8 @@ class TestSnapshot(test_objects.BaseObjectsTestCase):
volume_type_id)
snapshot_data_get.assert_called_once_with(self.context,
self.project_id,
volume_type_id)
volume_type_id,
host=None)
@mock.patch('cinder.db.sqlalchemy.api.snapshot_get')
def test_refresh(self, snapshot_get):

37
cinder/tests/unit/test_db_api.py
View File

@ -549,6 +549,22 @@ class DBAPIVolumeTestCase(BaseTest):
db.volume_data_get_for_project(
self.ctxt, 'p%d' % i))
def test_volume_data_get_for_project_with_host(self):
db.volume_create(self.ctxt, {'project_id': fake.PROJECT_ID,
'size': 100,
'host': 'host1'})
db.volume_create(self.ctxt, {'project_id': fake.PROJECT2_ID,
'size': 200,
'host': 'host1'})
db.volume_create(self.ctxt, {'project_id': fake.PROJECT2_ID,
'size': 300,
'host': 'host2'})
resp = db.volume_data_get_for_project(self.ctxt,
fake.PROJECT2_ID,
host='host2')
self.assertEqual((1, 300), resp)
def test_volume_detached_from_instance(self):
volume = db.volume_create(self.ctxt, {})
instance_uuid = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
@ -1820,6 +1836,27 @@ class DBAPISnapshotTestCase(BaseTest):
{'fake_key': 'fake'}),
ignored_keys='volume')
def test_snapshot_get_all_by_project_with_host(self):
db.volume_create(self.ctxt, {'id': 1, 'host': 'host1', 'size': 1,
'project_id': fake.PROJECT_ID})
db.volume_create(self.ctxt, {'id': 2, 'host': 'host1', 'size': 2,
'project_id': fake.PROJECT2_ID})
db.volume_create(self.ctxt, {'id': 3, 'host': 'host2', 'size': 3,
'project_id': fake.PROJECT2_ID})
db.snapshot_create(self.ctxt, {'id': 1, 'volume_id': 1,
'project_id': fake.PROJECT_ID,
'volume_size': 1})
db.snapshot_create(self.ctxt, {'id': 2, 'volume_id': 2,
'project_id': fake.PROJECT2_ID,
'volume_size': 2})
db.snapshot_create(self.ctxt, {'id': 3, 'volume_id': 3,
'project_id': fake.PROJECT2_ID,
'volume_size': 3})
resp = db.snapshot_data_get_for_project(self.ctxt,
fake.PROJECT2_ID,
host='host2')
self.assertEqual((1, 3), resp)
def test_snapshot_metadata_get(self):
metadata = {'a': 'b', 'c': 'd'}
db.volume_create(self.ctxt, {'id': 1})

5
releasenotes/notes/bug-1699936-fix-host-show-incorrect-fg8698gu7y6r7d15.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- |
Now the ``os-host show`` API will count project's
resource correctly.