From 94ab08577921e3accf7dcb21cf324dbf6838e10e Mon Sep 17 00:00:00 2001 From: Michal Dulko Date: Wed, 8 Jul 2015 11:27:30 +0200 Subject: [PATCH] Service object Add versionedobjects abstraction layer to services. Distinguish time zone aware DateTime fields. The object derives from CinderObjectDictCompat, so it supports both object (obj.prop) and dict (obj['prop']) syntax to access properties. Complete move to object notation will be made in a follow up clean up patch. Co-Authored-By: Michal Dulko Change-Id: I09f593f9f9aa8befa40d989b731159b78a429071 Partial-Implements: blueprint cinder-objects --- cinder/api/contrib/hosts.py | 30 ++-- cinder/api/contrib/services.py | 37 ++--- cinder/backup/api.py | 13 +- cinder/cmd/manage.py | 15 +- cinder/cmd/volume_usage_audit.py | 1 + cinder/objects/__init__.py | 1 + cinder/objects/base.py | 4 + cinder/objects/service.py | 128 ++++++++++++++++++ cinder/scheduler/host_manager.py | 12 +- cinder/service.py | 42 +++--- cinder/tests/unit/api/contrib/test_hosts.py | 6 +- .../tests/unit/api/contrib/test_services.py | 8 +- cinder/tests/unit/api/v1/stubs.py | 2 +- cinder/tests/unit/api/v2/stubs.py | 2 +- cinder/tests/unit/fake_service.py | 56 ++++++++ cinder/tests/unit/objects/test_service.py | 122 +++++++++++++++++ .../tests/unit/scheduler/test_host_manager.py | 50 +++++-- cinder/tests/unit/test_cmd.py | 7 +- cinder/tests/unit/test_service.py | 11 +- cinder/tests/unit/test_volume.py | 2 +- cinder/utils.py | 3 +- cinder/volume/api.py | 20 +-- tools/lintstack.py | 2 + 23 files changed, 458 insertions(+), 116 deletions(-) create mode 100644 cinder/objects/service.py create mode 100644 cinder/tests/unit/fake_service.py create mode 100644 cinder/tests/unit/objects/test_service.py diff --git a/cinder/api/contrib/hosts.py b/cinder/api/contrib/hosts.py index fcad9a2b3..df6a6e2ff 100644 --- a/cinder/api/contrib/hosts.py +++ b/cinder/api/contrib/hosts.py @@ -97,9 +97,9 @@ class HostDeserializer(wsgi.XMLDeserializer): def _list_hosts(req, service=None): """Returns a summary list of hosts.""" - curr_time = timeutils.utcnow() + curr_time = timeutils.utcnow(with_timezone=True) context = req.environ['cinder.context'] - services = db.service_get_all(context, False) + services = objects.ServiceList.get_all(context, False) zone = '' if 'zone' in req.GET: zone = req.GET['zone'] @@ -107,23 +107,24 @@ def _list_hosts(req, service=None): services = [s for s in services if s['availability_zone'] == zone] hosts = [] for host in services: - delta = curr_time - (host['updated_at'] or host['created_at']) + delta = curr_time - (host.updated_at or host.created_at) alive = abs(delta.total_seconds()) <= CONF.service_down_time status = (alive and "available") or "unavailable" active = 'enabled' - if host['disabled']: + if host.disabled: active = 'disabled' LOG.debug('status, active and update: %s, %s, %s', - status, active, host['updated_at']) - hosts.append({'host_name': host['host'], - 'service': host['topic'], - 'zone': host['availability_zone'], + status, active, host.updated_at) + hosts.append({'host_name': host.host, + 'service': host.topic, + 'zone': host.availability_zone, 'service-status': status, 'service-state': active, - 'last-update': host['updated_at']}) + 'last-update': timeutils.normalize_time(host.updated_at), + }) if service: hosts = [host for host in hosts - if host["service"] == service] + if host['service'] == service] return hosts @@ -209,17 +210,16 @@ class HostController(wsgi.Controller): raise webob.exc.HTTPForbidden(explanation=msg) try: - host_ref = db.service_get_by_host_and_topic(context, - host, - CONF.volume_topic) + host_ref = objects.Service.get_by_host_and_topic( + context, host, CONF.volume_topic) except exception.ServiceNotFound: raise webob.exc.HTTPNotFound(explanation=_("Host not found")) # Getting total available/used resource # TODO(jdg): Add summary info for Snapshots - volume_refs = db.volume_get_all_by_host(context, host_ref['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']) + host_ref.host) snap_count_total = 0 snap_sum_total = 0 diff --git a/cinder/api/contrib/services.py b/cinder/api/contrib/services.py index 6a84f04b5..f07402ff1 100644 --- a/cinder/api/contrib/services.py +++ b/cinder/api/contrib/services.py @@ -23,9 +23,9 @@ import webob.exc from cinder.api import extensions from cinder.api.openstack import wsgi from cinder.api import xmlutil -from cinder import db from cinder import exception from cinder.i18n import _ +from cinder import objects from cinder import utils @@ -81,8 +81,8 @@ class ServiceController(wsgi.Controller): context = req.environ['cinder.context'] authorize(context, action='index') detailed = self.ext_mgr.is_loaded('os-extended-services') - now = timeutils.utcnow() - services = db.service_get_all(context) + now = timeutils.utcnow(with_timezone=True) + services = objects.ServiceList.get_all(context) host = '' if 'host' in req.GET: @@ -98,32 +98,32 @@ class ServiceController(wsgi.Controller): binary = req.GET['binary'] if host: - services = [s for s in services if s['host'] == host] + services = [s for s in services if s.host == host] # NOTE(uni): deprecating service request key, binary takes precedence binary_key = binary or service if binary_key: - services = [s for s in services if s['binary'] == binary_key] + services = [s for s in services if s.binary == binary_key] svcs = [] for svc in services: - updated_at = svc['updated_at'] - delta = now - (svc['updated_at'] or svc['created_at']) + updated_at = svc.updated_at + delta = now - (svc.updated_at or svc.created_at) delta_sec = delta.total_seconds() - if svc['modified_at']: - delta_mod = now - svc['modified_at'] + if svc.modified_at: + delta_mod = now - svc.modified_at if abs(delta_sec) >= abs(delta_mod.total_seconds()): - updated_at = svc['modified_at'] + updated_at = svc.modified_at alive = abs(delta_sec) <= CONF.service_down_time art = (alive and "up") or "down" active = 'enabled' - if svc['disabled']: + if svc.disabled: active = 'disabled' - ret_fields = {'binary': svc['binary'], 'host': svc['host'], - 'zone': svc['availability_zone'], + ret_fields = {'binary': svc.binary, 'host': svc.host, + 'zone': svc.availability_zone, 'status': active, 'state': art, - 'updated_at': updated_at} + 'updated_at': timeutils.normalize_time(updated_at)} if detailed: - ret_fields['disabled_reason'] = svc['disabled_reason'] + ret_fields['disabled_reason'] = svc.disabled_reason svcs.append(ret_fields) return {'services': svcs} @@ -182,11 +182,14 @@ class ServiceController(wsgi.Controller): raise webob.exc.HTTPBadRequest() try: - svc = db.service_get_by_args(context, host, binary_key) + svc = objects.Service.get_by_args(context, host, binary_key) if not svc: raise webob.exc.HTTPNotFound(explanation=_('Unknown service')) - db.service_update(context, svc['id'], ret_val) + svc.disabled = ret_val['disabled'] + if 'disabled_reason' in ret_val: + svc.disabled_reason = ret_val['disabled_reason'] + svc.save() except exception.ServiceNotFound: raise webob.exc.HTTPNotFound(explanation=_("service not found")) diff --git a/cinder/backup/api.py b/cinder/backup/api.py index ca2349da8..11e7f2089 100644 --- a/cinder/backup/api.py +++ b/cinder/backup/api.py @@ -126,12 +126,11 @@ class API(base.Base): """Check if there is a backup service available.""" topic = CONF.backup_topic ctxt = context.get_admin_context() - services = self.db.service_get_all_by_topic(ctxt, - topic, - disabled=False) + services = objects.ServiceList.get_all_by_topic( + ctxt, topic, disabled=False) for srv in services: - if (srv['availability_zone'] == volume['availability_zone'] and - srv['host'] == volume_host and + if (srv.availability_zone == volume['availability_zone'] and + srv.host == volume_host and utils.service_is_up(srv)): return True return False @@ -143,8 +142,8 @@ class API(base.Base): """ topic = CONF.backup_topic ctxt = context.get_admin_context() - services = self.db.service_get_all_by_topic(ctxt, topic) - return [srv['host'] for srv in services if not srv['disabled']] + services = objects.ServiceList.get_all_by_topic(ctxt, topic) + return [srv.host for srv in services if not srv.disabled] def create(self, context, name, description, volume_id, container, incremental=False, availability_zone=None, diff --git a/cinder/cmd/manage.py b/cinder/cmd/manage.py index 05785a46a..05834b89c 100644 --- a/cinder/cmd/manage.py +++ b/cinder/cmd/manage.py @@ -62,6 +62,7 @@ from oslo_config import cfg from oslo_db.sqlalchemy import migration from oslo_log import log as logging import oslo_messaging as messaging +from oslo_utils import timeutils from oslo_utils import uuidutils from cinder import i18n @@ -204,9 +205,9 @@ class HostCommands(object): """ print(_("%(host)-25s\t%(zone)-15s") % {'host': 'host', 'zone': 'zone'}) ctxt = context.get_admin_context() - services = db.service_get_all(ctxt) + services = objects.ServiceList.get_all(ctxt) if zone: - services = [s for s in services if s['availability_zone'] == zone] + services = [s for s in services if s.availability_zone == zone] hosts = [] for srv in services: if not [h for h in hosts if h['host'] == srv['host']]: @@ -436,7 +437,7 @@ class ServiceCommands(object): def list(self): """Show a list of all cinder services.""" ctxt = context.get_admin_context() - services = db.service_get_all(ctxt) + services = objects.ServiceList.get_all(ctxt) print_format = "%-16s %-36s %-16s %-10s %-5s %-10s" print(print_format % (_('Binary'), _('Host'), @@ -448,11 +449,11 @@ class ServiceCommands(object): alive = utils.service_is_up(svc) art = ":-)" if alive else "XXX" status = 'enabled' - if svc['disabled']: + if svc.disabled: status = 'disabled' - print(print_format % (svc['binary'], svc['host'].partition('.')[0], - svc['availability_zone'], status, art, - svc['updated_at'])) + print(print_format % (svc.binary, svc.host.partition('.')[0], + svc.availability_zone, status, art, + timeutils.normalize_time(svc.updated_at))) @args('binary', type=str, help='Service to delete from the host.') diff --git a/cinder/cmd/volume_usage_audit.py b/cinder/cmd/volume_usage_audit.py index 5e30980a0..b5123e508 100644 --- a/cinder/cmd/volume_usage_audit.py +++ b/cinder/cmd/volume_usage_audit.py @@ -73,6 +73,7 @@ CONF.register_cli_opts(script_opts) def main(): + objects.register_all() admin_context = context.get_admin_context() CONF(sys.argv[1:], project='cinder', version=version.version_string()) diff --git a/cinder/objects/__init__.py b/cinder/objects/__init__.py index 2cdec1b55..28f1e9c55 100644 --- a/cinder/objects/__init__.py +++ b/cinder/objects/__init__.py @@ -25,6 +25,7 @@ def register_all(): # function in order for it to be registered by services that may # need to receive it via RPC. __import__('cinder.objects.volume') + __import__('cinder.objects.service') __import__('cinder.objects.snapshot') __import__('cinder.objects.backup') __import__('cinder.objects.consistencygroup') diff --git a/cinder/objects/base.py b/cinder/objects/base.py index 105aa915d..578129516 100644 --- a/cinder/objects/base.py +++ b/cinder/objects/base.py @@ -141,6 +141,10 @@ class CinderPersistentObject(object): self._context = original_context +class CinderComparableObject(base.ComparableVersionedObject): + pass + + class ObjectListBase(base.ObjectListBase): pass diff --git a/cinder/objects/service.py b/cinder/objects/service.py new file mode 100644 index 000000000..18e3d95bb --- /dev/null +++ b/cinder/objects/service.py @@ -0,0 +1,128 @@ +# Copyright 2015 Intel Corp. +# +# Licensed 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 required 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. + +from oslo_config import cfg +from oslo_log import log as logging +from oslo_versionedobjects import fields + +from cinder import db +from cinder import exception +from cinder.i18n import _ +from cinder import objects +from cinder.objects import base +from cinder import utils + +CONF = cfg.CONF +LOG = logging.getLogger(__name__) + + +@base.CinderObjectRegistry.register +class Service(base.CinderPersistentObject, base.CinderObject, + base.CinderObjectDictCompat, + base.CinderComparableObject): + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'id': fields.IntegerField(), + 'host': fields.StringField(nullable=True), + 'binary': fields.StringField(nullable=True), + 'topic': fields.StringField(nullable=True), + 'report_count': fields.IntegerField(default=0), + 'disabled': fields.BooleanField(default=False), + 'availability_zone': fields.StringField(nullable=True, + default='cinder'), + 'disabled_reason': fields.StringField(nullable=True), + + 'modified_at': fields.DateTimeField(nullable=True), + } + + def obj_make_compatible(self, primitive, target_version): + """Make an object representation compatible with a target version.""" + target_version = utils.convert_version_to_tuple(target_version) + + @staticmethod + def _from_db_object(context, service, db_service): + for name, field in service.fields.items(): + value = db_service.get(name) + if isinstance(field, fields.IntegerField): + value = value or 0 + elif isinstance(field, fields.DateTimeField): + value = value or None + service[name] = value + + service._context = context + service.obj_reset_changes() + return service + + @base.remotable_classmethod + def get_by_id(cls, context, id): + db_service = db.service_get(context, id) + return cls._from_db_object(context, cls(context), db_service) + + @base.remotable_classmethod + def get_by_host_and_topic(cls, context, host, topic): + db_service = db.service_get_by_host_and_topic(context, host, topic) + return cls._from_db_object(context, cls(context), db_service) + + @base.remotable_classmethod + def get_by_args(cls, context, host, binary_key): + db_service = db.service_get_by_args(context, host, binary_key) + return cls._from_db_object(context, cls(context), db_service) + + @base.remotable + def create(self): + if self.obj_attr_is_set('id'): + raise exception.ObjectActionError(action='create', + reason=_('already created')) + updates = self.cinder_obj_get_changes() + db_service = db.service_create(self._context, updates) + self._from_db_object(self._context, self, db_service) + + @base.remotable + def save(self): + updates = self.cinder_obj_get_changes() + if updates: + db.service_update(self._context, self.id, updates) + self.obj_reset_changes() + + @base.remotable + def destroy(self): + with self.obj_as_admin(): + db.service_destroy(self._context, self.id) + + +@base.CinderObjectRegistry.register +class ServiceList(base.ObjectListBase, base.CinderObject): + VERSION = '1.0' + + fields = { + 'objects': fields.ListOfObjectsField('Service'), + } + child_versions = { + '1.0': '1.0' + } + + @base.remotable_classmethod + def get_all(cls, context, filters=None): + services = db.service_get_all(context, filters) + return base.obj_make_list(context, cls(context), objects.Service, + services) + + @base.remotable_classmethod + def get_all_by_topic(cls, context, topic, disabled=None): + services = db.service_get_all_by_topic(context, topic, + disabled=disabled) + return base.obj_make_list(context, cls(context), objects.Service, + services) diff --git a/cinder/scheduler/host_manager.py b/cinder/scheduler/host_manager.py index 56056931b..45077e5e9 100644 --- a/cinder/scheduler/host_manager.py +++ b/cinder/scheduler/host_manager.py @@ -24,9 +24,9 @@ from oslo_log import log as logging from oslo_utils import timeutils from cinder import context as cinder_context -from cinder import db from cinder import exception from cinder.i18n import _LI, _LW +from cinder import objects from cinder.openstack.common.scheduler import filters from cinder.openstack.common.scheduler import weights from cinder import utils @@ -458,13 +458,13 @@ class HostManager(object): # Get resource usage across the available volume nodes: topic = CONF.volume_topic - volume_services = db.service_get_all_by_topic(context, - topic, - disabled=False) + volume_services = objects.ServiceList.get_all_by_topic(context, + topic, + disabled=False) active_hosts = set() no_capabilities_hosts = set() - for service in volume_services: - host = service['host'] + for service in volume_services.objects: + host = service.host if not utils.service_is_up(service): LOG.warning(_LW("volume service is down. (host: %s)"), host) continue diff --git a/cinder/service.py b/cinder/service.py index 9fce42cdf..36dc390ba 100644 --- a/cinder/service.py +++ b/cinder/service.py @@ -35,9 +35,9 @@ from osprofiler import profiler import osprofiler.web from cinder import context -from cinder import db from cinder import exception from cinder.i18n import _, _LE, _LI, _LW +from cinder import objects from cinder.objects import base as objects_base from cinder import rpc from cinder import version @@ -146,10 +146,9 @@ class Service(service.Service): self.manager.init_host() ctxt = context.get_admin_context() try: - service_ref = db.service_get_by_args(ctxt, - self.host, - self.binary) - self.service_id = service_ref['id'] + service_ref = objects.Service.get_by_args( + ctxt, self.host, self.binary) + self.service_id = service_ref.id except exception.NotFound: self._create_service_ref(ctxt) @@ -202,13 +201,14 @@ class Service(service.Service): def _create_service_ref(self, context): zone = CONF.storage_availability_zone - service_ref = db.service_create(context, - {'host': self.host, - 'binary': self.binary, - 'topic': self.topic, - 'report_count': 0, - 'availability_zone': zone}) - self.service_id = service_ref['id'] + kwargs = {'host': self.host, + 'binary': self.binary, + 'topic': self.topic, + 'report_count': 0, + 'availability_zone': zone} + service_ref = objects.Service(context=context, **kwargs) + service_ref.create() + self.service_id = service_ref.id def __getattr__(self, key): manager = self.__dict__.get('manager', None) @@ -256,7 +256,9 @@ class Service(service.Service): """Destroy the service object in the datastore.""" self.stop() try: - db.service_destroy(context.get_admin_context(), self.service_id) + service_ref = objects.Service.get_by_id( + context.get_admin_context(), self.service_id) + service_ref.destroy() except exception.NotFound: LOG.warning(_LW('Service killed that has no database entry')) @@ -303,22 +305,20 @@ class Service(service.Service): ctxt = context.get_admin_context() zone = CONF.storage_availability_zone - state_catalog = {} try: try: - service_ref = db.service_get(ctxt, self.service_id) + service_ref = objects.Service.get_by_id(ctxt, self.service_id) except exception.NotFound: LOG.debug('The service database object disappeared, ' 'recreating it.') self._create_service_ref(ctxt) - service_ref = db.service_get(ctxt, self.service_id) + service_ref = objects.Service.get_by_id(ctxt, self.service_id) - state_catalog['report_count'] = service_ref['report_count'] + 1 - if zone != service_ref['availability_zone']: - state_catalog['availability_zone'] = zone + service_ref.report_count += 1 + if zone != service_ref.availability_zone: + service_ref.availability_zone = zone - db.service_update(ctxt, - self.service_id, state_catalog) + service_ref.save() # TODO(termie): make this pattern be more elegant. if getattr(self, 'model_disconnected', False): diff --git a/cinder/tests/unit/api/contrib/test_hosts.py b/cinder/tests/unit/api/contrib/test_hosts.py index a3eaa1c72..524dff578 100644 --- a/cinder/tests/unit/api/contrib/test_hosts.py +++ b/cinder/tests/unit/api/contrib/test_hosts.py @@ -15,6 +15,7 @@ import datetime +from iso8601 import iso8601 from lxml import etree from oslo_utils import timeutils import webob.exc @@ -56,8 +57,9 @@ LIST_RESPONSE = [{'service-status': 'available', 'service': 'cinder-volume', 'host_name': 'test.host.1', 'last-update': curr_time}] -def stub_utcnow(): - return datetime.datetime(2013, 7, 3, 0, 0, 2) +def stub_utcnow(with_timezone=False): + tzinfo = iso8601.Utc() if with_timezone else None + return datetime.datetime(2013, 7, 3, 0, 0, 2, tzinfo=tzinfo) def stub_service_get_all(self, req): diff --git a/cinder/tests/unit/api/contrib/test_services.py b/cinder/tests/unit/api/contrib/test_services.py index fe9041365..df664ae06 100644 --- a/cinder/tests/unit/api/contrib/test_services.py +++ b/cinder/tests/unit/api/contrib/test_services.py @@ -16,6 +16,7 @@ import datetime +from iso8601 import iso8601 from oslo_utils import timeutils import webob.exc @@ -121,7 +122,7 @@ class FakeRequestWithHostBinary(object): GET = {"host": "host1", "binary": "cinder-volume"} -def fake_service_get_all(context): +def fake_service_get_all(context, filters=None): return fake_services_list @@ -152,8 +153,9 @@ def fake_policy_enforce(context, action, target): pass -def fake_utcnow(): - return datetime.datetime(2012, 10, 29, 13, 42, 11) +def fake_utcnow(with_timezone=False): + tzinfo = iso8601.Utc() if with_timezone else None + return datetime.datetime(2012, 10, 29, 13, 42, 11, tzinfo=tzinfo) class ServicesTest(test.TestCase): diff --git a/cinder/tests/unit/api/v1/stubs.py b/cinder/tests/unit/api/v1/stubs.py index 6d929628b..795486ad2 100644 --- a/cinder/tests/unit/api/v1/stubs.py +++ b/cinder/tests/unit/api/v1/stubs.py @@ -136,5 +136,5 @@ def stub_snapshot_update(self, context, *args, **param): pass -def stub_service_get_all_by_topic(context, topic): +def stub_service_get_all_by_topic(context, topic, disabled=None): return [{'availability_zone': "zone1:host1", "disabled": 0}] diff --git a/cinder/tests/unit/api/v2/stubs.py b/cinder/tests/unit/api/v2/stubs.py index cd56315e1..e42ff046d 100644 --- a/cinder/tests/unit/api/v2/stubs.py +++ b/cinder/tests/unit/api/v2/stubs.py @@ -194,7 +194,7 @@ def stub_snapshot_update(self, context, *args, **param): pass -def stub_service_get_all_by_topic(context, topic): +def stub_service_get_all_by_topic(context, topic, disabled=None): return [{'availability_zone': "zone1:host1", "disabled": 0}] diff --git a/cinder/tests/unit/fake_service.py b/cinder/tests/unit/fake_service.py new file mode 100644 index 000000000..676b39713 --- /dev/null +++ b/cinder/tests/unit/fake_service.py @@ -0,0 +1,56 @@ +# Copyright 2015 Intel Corp. +# +# Licensed 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 required 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. + +from oslo_utils import timeutils +from oslo_versionedobjects import fields + +from cinder import objects + + +def fake_db_service(**updates): + NOW = timeutils.utcnow().replace(microsecond=0) + db_service = { + 'created_at': NOW, + 'updated_at': None, + 'deleted_at': None, + 'deleted': False, + 'id': 123, + 'host': 'fake-host', + 'binary': 'fake-service', + 'topic': 'fake-service-topic', + 'report_count': 1, + 'disabled': False, + 'disabled_reason': None, + 'modified_at': NOW, + } + + for name, field in objects.Service.fields.items(): + if name in db_service: + continue + if field.nullable: + db_service[name] = None + elif field.default != fields.UnspecifiedDefault: + db_service[name] = field.default + else: + raise Exception('fake_db_service needs help with %s.' % name) + + if updates: + db_service.update(updates) + + return db_service + + +def fake_service_obj(context, **updates): + return objects.Service._from_db_object(context, objects.Service(), + fake_db_service(**updates)) diff --git a/cinder/tests/unit/objects/test_service.py b/cinder/tests/unit/objects/test_service.py new file mode 100644 index 000000000..b91ef3d02 --- /dev/null +++ b/cinder/tests/unit/objects/test_service.py @@ -0,0 +1,122 @@ +# Copyright 2015 Intel Corp. +# +# Licensed 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 required 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. + +import mock + +from oslo_utils import timeutils + +from cinder import context +from cinder import objects +from cinder.tests.unit import fake_service +from cinder.tests.unit import objects as test_objects + + +class TestService(test_objects.BaseObjectsTestCase): + def setUp(self): + super(TestService, self).setUp() + # NOTE (e0ne): base tests contains original RequestContext from + # oslo_context. We change it to our RequestContext implementation + # to have 'elevated' method + self.context = context.RequestContext(self.user_id, self.project_id, + is_admin=False) + + @staticmethod + def _compare(test, db, obj): + for field, value in db.items(): + if field in ('modified_at', 'created_at', + 'updated_at', 'deleted_at') and db[field]: + test.assertEqual(db[field], + timeutils.normalize_time(obj[field])) + else: + test.assertEqual(db[field], obj[field]) + + @mock.patch('cinder.db.service_get') + def test_get_by_id(self, service_get): + db_service = fake_service.fake_db_service() + service_get.return_value = db_service + service = objects.Service.get_by_id(self.context, 1) + self._compare(self, db_service, service) + service_get.assert_called_once_with(self.context, 1) + + @mock.patch('cinder.db.service_get_by_host_and_topic') + def test_get_by_host_and_topic(self, service_get_by_host_and_topic): + db_service = fake_service.fake_db_service() + service_get_by_host_and_topic.return_value = db_service + service = objects.Service.get_by_host_and_topic( + self.context, 'fake-host', 'fake-topic') + self._compare(self, db_service, service) + service_get_by_host_and_topic.assert_called_once_with( + self.context, 'fake-host', 'fake-topic') + + @mock.patch('cinder.db.service_get_by_args') + def test_get_by_args(self, service_get_by_args): + db_service = fake_service.fake_db_service() + service_get_by_args.return_value = db_service + service = objects.Service.get_by_args( + self.context, 'fake-host', 'fake-key') + self._compare(self, db_service, service) + service_get_by_args.assert_called_once_with( + self.context, 'fake-host', 'fake-key') + + @mock.patch('cinder.db.service_create') + def test_create(self, service_create): + db_service = fake_service.fake_db_service() + service_create.return_value = db_service + service = objects.Service(context=self.context) + service.create() + self.assertEqual(db_service['id'], service.id) + service_create.assert_called_once_with(self.context, {}) + + @mock.patch('cinder.db.service_update') + def test_save(self, service_update): + db_service = fake_service.fake_db_service() + service = objects.Service._from_db_object( + self.context, objects.Service(), db_service) + service.topic = 'foobar' + service.save() + service_update.assert_called_once_with(self.context, service.id, + {'topic': 'foobar'}) + + @mock.patch('cinder.db.service_destroy') + def test_destroy(self, service_destroy): + db_service = fake_service.fake_db_service() + service = objects.Service._from_db_object( + self.context, objects.Service(), db_service) + with mock.patch.object(service._context, 'elevated') as elevated_ctx: + service.destroy() + service_destroy.assert_called_once_with(elevated_ctx(), 123) + + +class TestServiceList(test_objects.BaseObjectsTestCase): + @mock.patch('cinder.db.service_get_all') + def test_get_all(self, service_get_all): + db_service = fake_service.fake_db_service() + service_get_all.return_value = [db_service] + + services = objects.ServiceList.get_all(self.context, 'foo') + service_get_all.assert_called_once_with(self.context, 'foo') + self.assertEqual(1, len(services)) + TestService._compare(self, db_service, services[0]) + + @mock.patch('cinder.db.service_get_all_by_topic') + def test_get_all_by_topic(self, service_get_all_by_topic): + db_service = fake_service.fake_db_service() + service_get_all_by_topic.return_value = [db_service] + + services = objects.ServiceList.get_all_by_topic( + self.context, 'foo', 'bar') + service_get_all_by_topic.assert_called_once_with( + self.context, 'foo', disabled='bar') + self.assertEqual(1, len(services)) + TestService._compare(self, db_service, services[0]) diff --git a/cinder/tests/unit/scheduler/test_host_manager.py b/cinder/tests/unit/scheduler/test_host_manager.py index 90cbe6ebb..2f27aced0 100644 --- a/cinder/tests/unit/scheduler/test_host_manager.py +++ b/cinder/tests/unit/scheduler/test_host_manager.py @@ -16,14 +16,18 @@ Tests For HostManager """ +from datetime import datetime + import mock from oslo_config import cfg from oslo_utils import timeutils from cinder import exception +from cinder import objects from cinder.openstack.common.scheduler import filters from cinder.scheduler import host_manager from cinder import test +from cinder.tests.unit.objects import test_service CONF = cfg.CONF @@ -162,7 +166,9 @@ class HostManagerTestCase(test.TestCase): current date/time. """ context = 'fake_context' - _mock_utcnow.side_effect = [400, 401, 402] + dates = [datetime.fromtimestamp(400), datetime.fromtimestamp(401), + datetime.fromtimestamp(402)] + _mock_utcnow.side_effect = dates services = [ # This is the first call to utcnow() @@ -191,14 +197,14 @@ class HostManagerTestCase(test.TestCase): host_volume_capabs) res = self.host_manager.get_pools(context) self.assertEqual(1, len(res)) - self.assertEqual(401, res[0]['capabilities']['timestamp']) + self.assertEqual(dates[1], res[0]['capabilities']['timestamp']) self.host_manager.update_service_capabilities(service_name, 'host1', host_volume_capabs) res = self.host_manager.get_pools(context) self.assertEqual(1, len(res)) - self.assertEqual(402, res[0]['capabilities']['timestamp']) + self.assertEqual(dates[2], res[0]['capabilities']['timestamp']) @mock.patch('cinder.db.service_get_all_by_topic') @mock.patch('cinder.utils.service_is_up') @@ -209,15 +215,30 @@ class HostManagerTestCase(test.TestCase): services = [ dict(id=1, host='host1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + binary=None, deleted=False, created_at=None, modified_at=None, + report_count=0, deleted_at=None, disabled_reason=None), dict(id=2, host='host2', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + binary=None, deleted=False, created_at=None, modified_at=None, + report_count=0, deleted_at=None, disabled_reason=None), dict(id=3, host='host3', topic='volume', disabled=False, - availability_zone='zone2', updated_at=timeutils.utcnow()), + availability_zone='zone2', updated_at=timeutils.utcnow(), + binary=None, deleted=False, created_at=None, modified_at=None, + report_count=0, deleted_at=None, disabled_reason=None), dict(id=4, host='host4', topic='volume', disabled=False, - availability_zone='zone3', updated_at=timeutils.utcnow()), + availability_zone='zone3', updated_at=timeutils.utcnow(), + binary=None, deleted=False, created_at=None, modified_at=None, + report_count=0, deleted_at=None, disabled_reason=None), ] + service_objs = [] + for db_service in services: + service_obj = objects.Service() + service_objs.append(objects.Service._from_db_object(context, + service_obj, + db_service)) + service_states = { 'host1': dict(volume_backend_name='AAA', total_capacity_gb=512, free_capacity_gb=200, @@ -247,17 +268,18 @@ class HostManagerTestCase(test.TestCase): topic, disabled=False) expected = [] - for service in services: + for service in service_objs: expected.append(mock.call(service)) self.assertEqual(expected, _mock_service_is_up.call_args_list) - # Get host_state_map and make sure we have the first 4 hosts + # Get host_state_map and make sure we have the first 3 hosts host_state_map = self.host_manager.host_state_map self.assertEqual(3, len(host_state_map)) for i in range(3): volume_node = services[i] host = volume_node['host'] - self.assertEqual(volume_node, host_state_map[host].service) + test_service.TestService._compare(self, volume_node, + host_state_map[host].service) # Second test: Now service_is_up returns False for host3 _mock_service_is_up.reset_mock() @@ -270,9 +292,7 @@ class HostManagerTestCase(test.TestCase): _mock_service_get_all_by_topic.assert_called_with(context, topic, disabled=False) - expected = [] - for service in services: - expected.append(mock.call(service)) + self.assertEqual(expected, _mock_service_is_up.call_args_list) self.assertTrue(_mock_warning.call_count > 0) @@ -283,8 +303,8 @@ class HostManagerTestCase(test.TestCase): for i in range(2): volume_node = services[i] host = volume_node['host'] - self.assertEqual(volume_node, - host_state_map[host].service) + test_service.TestService._compare(self, volume_node, + host_state_map[host].service) @mock.patch('cinder.db.service_get_all_by_topic') @mock.patch('cinder.utils.service_is_up') diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py index ce48cc098..e30ff0b2a 100644 --- a/cinder/tests/unit/test_cmd.py +++ b/cinder/tests/unit/test_cmd.py @@ -382,7 +382,7 @@ class TestCinderManageCmd(test.TestCase): host_cmds.list() get_admin_context.assert_called_once_with() - service_get_all.assert_called_once_with(mock.sentinel.ctxt) + service_get_all.assert_called_once_with(mock.sentinel.ctxt, None) self.assertEqual(expected_out, fake_out.getvalue()) @mock.patch('cinder.db.service_get_all') @@ -405,7 +405,7 @@ class TestCinderManageCmd(test.TestCase): host_cmds.list(zone='fake-az1') get_admin_context.assert_called_once_with() - service_get_all.assert_called_once_with(mock.sentinel.ctxt) + service_get_all.assert_called_once_with(mock.sentinel.ctxt, None) self.assertEqual(expected_out, fake_out.getvalue()) @mock.patch('cinder.objects.base.CinderObjectSerializer') @@ -653,8 +653,7 @@ class TestCinderManageCmd(test.TestCase): self.assertEqual(expected_out, fake_out.getvalue()) get_admin_context.assert_called_with() - service_get_all.assert_called_with(ctxt) - service_is_up.assert_called_with(service) + service_get_all.assert_called_with(ctxt, None) def test_get_arg_string(self): args1 = "foobar" diff --git a/cinder/tests/unit/test_service.py b/cinder/tests/unit/test_service.py index 2c95d65e5..5303f8938 100644 --- a/cinder/tests/unit/test_service.py +++ b/cinder/tests/unit/test_service.py @@ -28,6 +28,7 @@ from cinder import context from cinder import db from cinder import exception from cinder import manager +from cinder import objects from cinder import rpc from cinder import service from cinder import test @@ -134,7 +135,7 @@ class ServiceTestCase(test.TestCase): 'report_count': 0, 'availability_zone': 'nova', 'id': 1} - with mock.patch.object(service, 'db') as mock_db: + with mock.patch.object(objects.service, 'db') as mock_db: mock_db.service_get_by_args.side_effect = exception.NotFound() mock_db.service_create.return_value = service_ref mock_db.service_get.side_effect = db_exc.DBConnectionError() @@ -157,7 +158,7 @@ class ServiceTestCase(test.TestCase): 'report_count': 0, 'availability_zone': 'nova', 'id': 1} - with mock.patch.object(service, 'db') as mock_db: + with mock.patch.object(objects.service, 'db') as mock_db: mock_db.service_get_by_args.side_effect = exception.NotFound() mock_db.service_create.return_value = service_ref mock_db.service_get.side_effect = db_exc.DBError() @@ -180,7 +181,7 @@ class ServiceTestCase(test.TestCase): 'report_count': 0, 'availability_zone': 'nova', 'id': 1} - with mock.patch.object(service, 'db') as mock_db: + with mock.patch.object(objects.service, 'db') as mock_db: mock_db.service_get_by_args.side_effect = exception.NotFound() mock_db.service_create.return_value = service_ref mock_db.service_get.return_value = service_ref @@ -205,7 +206,7 @@ class ServiceTestCase(test.TestCase): 'report_count': 0, 'availability_zone': 'nova', 'id': 1} - with mock.patch.object(service, 'db') as mock_db: + with mock.patch('cinder.db') as mock_db: mock_db.service_get.return_value = service_ref serv = service.Service( @@ -230,7 +231,7 @@ class ServiceTestCase(test.TestCase): self.assertEqual(25, CONF.service_down_time) @mock.patch.object(rpc, 'get_server') - @mock.patch.object(service, 'db') + @mock.patch('cinder.db') def test_service_stop_waits_for_rpcserver(self, mock_db, mock_rpc): serv = service.Service( self.host, diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 537e15869..eaa586ebf 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -1358,7 +1358,7 @@ class VolumeTestCase(BaseVolumeTestCase): 'service_get_all_by_topic') as mock_get_service, \ mock.patch.object(volume_api, 'list_availability_zones') as mock_get_azs: - mock_get_service.return_value = ['foo'] + mock_get_service.return_value = [{'host': 'foo'}] mock_get_azs.return_value = {} volume_api.create(self.context, size=1, diff --git a/cinder/utils.py b/cinder/utils.py index a105963ff..a584daddb 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -508,7 +508,8 @@ def service_is_up(service): """Check whether a service is up based on last heartbeat.""" last_heartbeat = service['updated_at'] or service['created_at'] # Timestamps in DB are UTC. - elapsed = (timeutils.utcnow() - last_heartbeat).total_seconds() + elapsed = (timeutils.utcnow(with_timezone=True) - + last_heartbeat).total_seconds() return abs(elapsed) <= CONF.service_down_time diff --git a/cinder/volume/api.py b/cinder/volume/api.py index b4e593042..874743ef9 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -144,8 +144,8 @@ class API(base.Base): if refresh_cache or not enable_cache: topic = CONF.volume_topic ctxt = context.get_admin_context() - services = self.db.service_get_all_by_topic(ctxt, topic) - az_data = [(s['availability_zone'], s['disabled']) + services = objects.ServiceList.get_all_by_topic(ctxt, topic) + az_data = [(s.availability_zone, s.disabled) for s in services] disabled_map = {} for (az_name, disabled) in az_data: @@ -169,9 +169,10 @@ class API(base.Base): first_type_id, second_type_id, first_type=None, second_type=None): safe = False - if len(self.db.service_get_all_by_topic(context, - 'cinder-volume', - disabled=True)) == 1: + services = objects.ServiceList.get_all_by_topic(context, + 'cinder-volume', + disabled=True) + if len(services.objects) == 1: safe = True else: type_a = first_type or volume_types.get_volume_type( @@ -1314,13 +1315,12 @@ class API(base.Base): # Make sure the host is in the list of available hosts elevated = context.elevated() topic = CONF.volume_topic - services = self.db.service_get_all_by_topic(elevated, - topic, - disabled=False) + services = objects.ServiceList.get_all_by_topic( + elevated, topic, disabled=False) found = False for service in services: svc_host = volume_utils.extract_host(host, 'backend') - if utils.service_is_up(service) and service['host'] == svc_host: + if utils.service_is_up(service) and service.host == svc_host: found = True if not found: msg = _('No available service named %s') % host @@ -1515,7 +1515,7 @@ class API(base.Base): elevated = context.elevated() try: svc_host = volume_utils.extract_host(host, 'backend') - service = self.db.service_get_by_host_and_topic( + service = objects.Service.get_by_host_and_topic( elevated, svc_host, CONF.volume_topic) except exception.ServiceNotFound: with excutils.save_and_reraise_exception(): diff --git a/tools/lintstack.py b/tools/lintstack.py index 27c3c5a83..7c2999bb6 100755 --- a/tools/lintstack.py +++ b/tools/lintstack.py @@ -69,6 +69,8 @@ objects_ignore_messages = [ "Module 'cinder.objects' has no 'SnapshotList' member", "Module 'cinder.objects' has no 'Backup' member", "Module 'cinder.objects' has no 'BackupList' member", + "Module 'cinder.objects' has no 'Service' member", + "Module 'cinder.objects' has no 'ServiceList' member", ] objects_ignore_modules = ["cinder/objects/"] -- 2.45.2