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 <michal.dulko@intel.com>

Change-Id: I09f593f9f9aa8befa40d989b731159b78a429071
Partial-Implements: blueprint cinder-objects

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

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

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,

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.')

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

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

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
 

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)

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

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

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

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

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}]

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}]
 
 

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

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

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

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"

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,

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,

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
 
 

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():

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/"]