Complete compute.service operations

Complete the operations on the compute.service resource.

Change-Id: I0b7d41b407c436dd5583158bc6b5815847cffa31

doc/source/user/proxies/compute.rst

@@ -78,7 +78,8 @@ Service Operations
 
 .. autoclass:: openstack.compute.v2._proxy.Proxy
   :noindex:
-  :members: services, enable_service, disable_service, force_service_down
+  :members: services, enable_service, disable_service, update_service_forced_down,
+            delete_service, update_service, find_service
 
 Volume Attachment Operations
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

openstack/compute/v2/_proxy.py

@@ -29,6 +29,7 @@ from openstack.compute.v2 import server_ip
 from openstack.compute.v2 import service as _service
 from openstack.compute.v2 import volume_attachment as _volume_attachment
 from openstack.network.v2 import security_group as _sg
+from openstack import exceptions
 from openstack import proxy
 from openstack import resource
 from openstack import utils
@@ -1402,57 +1403,140 @@ class Proxy(proxy.Proxy):
         """
         return self._get(_hypervisor.Hypervisor, hypervisor)
 
-    def force_service_down(self, service, host, binary):
+    # ========== Services ==========
-        """Force a service down
+
+    def update_service_forced_down(
+        self, service, host=None, binary=None, forced=True
+    ):
+        """Update service forced_down information
 
         :param service: Either the ID of a service or a
-                       :class:`~openstack.compute.v2.server.Service` instance.
+            :class:`~openstack.compute.v2.service.Service` instance.
         :param str host: The host where service runs.
         :param str binary: The name of service.
+        :param bool forced: Whether or not this service was forced down
+            manually by an administrator after the service was fenced.
 
-        :returns: None
+        :returns: Updated service instance
+        :rtype: class: `~openstack.compute.v2.service.Service`
         """
-        service = self._get_resource(_service.Service, service)
+        if utils.supports_microversion(self, '2.53'):
-        service.force_down(self, host, binary)
+            return self.update_service(
+                service, forced_down=forced)
 
-    def disable_service(self, service, host, binary, disabled_reason=None):
+        service = self._get_resource(_service.Service, service)
+        if (
+            (not host or not binary)
+            and (not service.host or not service.binary)
+        ):
+            raise ValueError(
+                'Either service instance should have host and binary '
+                'or they should be passed')
+        service.set_forced_down(self, host, binary, forced)
+
+    force_service_down = update_service_forced_down
+
+    def disable_service(
+        self, service, host=None, binary=None, disabled_reason=None
+    ):
         """Disable a service
 
         :param service: Either the ID of a service or a
-                       :class:`~openstack.compute.v2.server.Service` instance.
+            :class:`~openstack.compute.v2.service.Service` instance.
         :param str host: The host where service runs.
         :param str binary: The name of service.
         :param str disabled_reason: The reason of force down a service.
 
-        :returns: None
+        :returns: Updated service instance
+        :rtype: class: `~openstack.compute.v2.service.Service`
         """
-        service = self._get_resource(_service.Service, service)
+        if utils.supports_microversion(self, '2.53'):
-        service.disable(self,
+            attrs = {
-                        host, binary,
+                'status': 'disabled'
-                        disabled_reason)
+            }
+            if disabled_reason:
+                attrs['disabled_reason'] = disabled_reason
+            return self.update_service(
+                service, **attrs)
 
-    def enable_service(self, service, host, binary):
+        service = self._get_resource(_service.Service, service)
+        return service.disable(
+            self, host, binary, disabled_reason)
+
+    def enable_service(self, service, host=None, binary=None):
         """Enable a service
 
         :param service: Either the ID of a service or a
-                       :class:`~openstack.compute.v2.server.Service` instance.
+            :class:`~openstack.compute.v2.service.Service` instance.
         :param str host: The host where service runs.
         :param str binary: The name of service.
 
-
+        :returns: Updated service instance
-        :returns: None
+        :rtype: class: `~openstack.compute.v2.service.Service`
         """
-        service = self._get_resource(_service.Service, service)
+        if utils.supports_microversion(self, '2.53'):
-        service.enable(self, host, binary)
+            return self.update_service(
+                service, status='enabled')
 
-    def services(self):
+        service = self._get_resource(_service.Service, service)
+        return service.enable(self, host, binary)
+
+    def services(self, **query):
         """Return a generator of service
 
+        :params dict query: Query parameters
         :returns: A generator of service
         :rtype: class: `~openstack.compute.v2.service.Service`
         """
+        return self._list(_service.Service, **query)
 
-        return self._list(_service.Service)
+    def find_service(self, name_or_id, ignore_missing=True, **attrs):
+        """Find a service from name or id to get the corresponding info
+
+        :param name_or_id: The name or id of a service
+        :param dict attrs: Additional attributes like 'host'
+
+        :returns:
+            One: class:`~openstack.compute.v2.hypervisor.Hypervisor` object
+            or None
+        """
+        return self._find(_service.Service, name_or_id,
+                          ignore_missing=ignore_missing, **attrs)
+
+    def delete_service(self, service, ignore_missing=True):
+        """Delete a service
+
+        :param service:
+            The value can be either the ID of a service or a
+            :class:`~openstack.compute.v2.service.Service` instance.
+        :param bool ignore_missing: When set to ``False``
+            :class:`~openstack.exceptions.ResourceNotFound` will be raised when
+            the volume attachment does not exist.  When set to ``True``, no
+            exception will be set when attempting to delete a nonexistent
+            volume attachment.
+
+        :returns: ``None``
+        """
+        self._delete(
+            _service.Service, service, ignore_missing=ignore_missing)
+
+    def update_service(self, service, **attrs):
+        """Update a service
+
+        :param server: Either the ID of a service or a
+            :class:`~openstack.compute.v2.service.Service` instance.
+        :attrs kwargs: The attributes to update on the service represented
+            by ``service``.
+
+        :returns: The updated service
+        :rtype: :class:`~openstack.compute.v2.service.Service`
+        """
+        if utils.supports_microversion(self, '2.53'):
+            return self._update(_service.Service, service, **attrs)
+
+        raise exceptions.SDKException(
+            'Method require at least microversion 2.53'
+        )
 
     def create_volume_attachment(self, server, **attrs):
         """Create a new volume attachment from attributes

openstack/compute/v2/service.py

@@ -10,6 +10,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from openstack import exceptions
 from openstack import resource
 from openstack import utils
 
@@ -22,37 +23,105 @@ class Service(resource.Resource):
     # capabilities
     allow_list = True
     allow_commit = True
+    allow_delete = True
+
+    _query_mapping = resource.QueryParameters(
+        'name', 'binary', 'host',
+        name='binary',
+    )
 
     # Properties
-    #: Status of service
-    status = resource.Body('status')
-    #: State of service
-    state = resource.Body('state')
-    #: Name of service
-    binary = resource.Body('binary')
-    #: Id of service
-    id = resource.Body('id')
-    #: Disabled reason of service
-    disables_reason = resource.Body('disabled_reason')
-    #: Host where service runs
-    host = resource.Body('host')
     #: The availability zone of service
     availability_zone = resource.Body("zone")
+    #: Binary name of service
+    binary = resource.Body('binary')
+    #: Disabled reason of service
+    disabled_reason = resource.Body('disabled_reason')
+    #: Whether or not this service was forced down manually by an administrator
+    #: after the service was fenced
+    is_forced_down = resource.Body('forced_down', type=bool)
+    #: The name of the host where service runs
+    host = resource.Body('host')
+    #: Service name
+    name = resource.Body('name', alias='binary')
+    #: State of service
+    state = resource.Body('state')
+    #: Status of service
+    status = resource.Body('status')
+    #: The date and time when the resource was updated
+    updated_at = resource.Body('updated_at')
 
-    def _action(self, session, action, body):
+    _max_microversion = '2.69'
+
+    @classmethod
+    def find(cls, session, name_or_id, ignore_missing=True, **params):
+        # No direct request possible, thus go directly to list
+        data = cls.list(session, **params)
+
+        result = None
+        for maybe_result in data:
+            # Since ID might be both int and str force cast
+            id_value = str(cls._get_id(maybe_result))
+            name_value = maybe_result.name
+
+            if str(name_or_id) in (id_value, name_value):
+                if 'host' in params and maybe_result['host'] != params['host']:
+                    continue
+                # Only allow one resource to be found. If we already
+                # found a match, raise an exception to show it.
+                if result is None:
+                    result = maybe_result
+                else:
+                    msg = "More than one %s exists with the name '%s'."
+                    msg = (msg % (cls.__name__, name_or_id))
+                    raise exceptions.DuplicateResource(msg)
+
+        if result is not None:
+            return result
+
+        if ignore_missing:
+            return None
+        raise exceptions.ResourceNotFound(
+            "No %s found for %s" % (cls.__name__, name_or_id))
+
+    def commit(self, session, prepend_key=False, **kwargs):
+        # we need to set prepend_key to false
+        return super(Service, self).commit(
+            session, prepend_key=prepend_key, **kwargs)
+
+    def _action(self, session, action, body, microversion=None):
+        if not microversion:
+            microversion = session.default_microversion
         url = utils.urljoin(Service.base_path, action)
-        return session.put(url, json=body)
+        response = session.put(url, json=body, microversion=microversion)
-
+        self._translate_response(response)
-    def force_down(self, session, host, binary):
+        return self
-        """Force a service down."""
 
+    def set_forced_down(
+        self, session, host=None, binary=None, forced=False
+    ):
+        """Update forced_down information of a service."""
+        microversion = session.default_microversion
+        body = {}
+        if not host:
+            host = self.host
+        if not binary:
+            binary = self.binary
         body = {
             'host': host,
             'binary': binary,
-            'forced_down': True,
         }
+        if utils.supports_microversion(session, '2.11'):
+            body['forced_down'] = forced
+            # Using forced_down works only 2.11-2.52, therefore pin it
+            microversion = '2.11'
 
-        return self._action(session, 'force-down', body)
+        # This will not work with newest microversions
+        return self._action(
+            session, 'force-down', body,
+            microversion=microversion)
+
+    force_down = set_forced_down
 
     def enable(self, session, host, binary):
         """Enable service."""

openstack/tests/functional/compute/v2/test_service.py

@@ -0,0 +1,47 @@
+# 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 openstack.tests.functional import base
+
+
+class TestService(base.BaseFunctionalTest):
+
+    def setUp(self):
+        super(TestService, self).setUp()
+        self._set_operator_cloud(interface='admin')
+
+    def test_list(self):
+        sot = list(self.conn.compute.services())
+        self.assertIsNotNone(sot)
+
+    def test_disable_enable(self):
+        for srv in self.conn.compute.services():
+            # only nova-compute can be updated
+            if srv.name == 'nova-compute':
+                self.conn.compute.disable_service(srv)
+                self.conn.compute.enable_service(srv)
+
+    def test_update(self):
+        for srv in self.conn.compute.services():
+            if srv.name == 'nova-compute':
+                self.conn.compute.update_service_forced_down(
+                    srv, None, None, True)
+                self.conn.compute.update_service_forced_down(
+                    srv, srv.host, srv.binary, False)
+                self.conn.compute.update_service(srv, status='enabled')
+
+    def test_find(self):
+        for srv in self.conn.compute.services():
+            if srv.name != 'nova-conductor':
+                # In devstack there are 2 nova-conductor instances on same host
+                self.conn.compute.find_service(
+                    srv.name, host=srv.host, ignore_missing=False)

openstack/tests/unit/compute/v2/test_proxy.py

@@ -352,6 +352,99 @@ class TestAggregate(TestComputeProxy):
             expected_args=[[{'id': '1'}, {'id': '2'}]])
 
 
+class TestService(TestComputeProxy):
+    def test_services(self):
+        self.verify_list_no_kwargs(
+            self.proxy.services, service.Service)
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=False)
+    def test_enable_service_252(self, mv_mock):
+        self._verify2(
+            'openstack.compute.v2.service.Service.enable',
+            self.proxy.enable_service,
+            method_args=["value", "host1", "nova-compute"],
+            expected_args=[self.proxy, "host1", "nova-compute"]
+        )
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=True)
+    def test_enable_service_253(self, mv_mock):
+        self._verify2(
+            'openstack.proxy.Proxy._update',
+            self.proxy.enable_service,
+            method_args=["value"],
+            method_kwargs={},
+            expected_args=[service.Service, "value"],
+            expected_kwargs={'status': 'enabled'}
+        )
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=False)
+    def test_disable_service_252(self, mv_mock):
+        self._verify2(
+            'openstack.compute.v2.service.Service.disable',
+            self.proxy.disable_service,
+            method_args=["value", "host1", "nova-compute"],
+            expected_args=[self.proxy, "host1", "nova-compute", None])
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=True)
+    def test_disable_service_253(self, mv_mock):
+        self._verify2(
+            'openstack.proxy.Proxy._update',
+            self.proxy.disable_service,
+            method_args=["value"],
+            method_kwargs={'disabled_reason': 'some_reason'},
+            expected_args=[service.Service, "value"],
+            expected_kwargs={
+                'status': 'disabled',
+                'disabled_reason': 'some_reason'
+            }
+        )
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=False)
+    def test_force_service_down_252(self, mv_mock):
+        self._verify2(
+            'openstack.compute.v2.service.Service.set_forced_down',
+            self.proxy.update_service_forced_down,
+            method_args=["value", "host1", "nova-compute"],
+            expected_args=[self.proxy, "host1", "nova-compute", True])
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=False)
+    def test_force_service_down_252_empty_vals(self, mv_mock):
+        self.assertRaises(
+            ValueError,
+            self.proxy.update_service_forced_down,
+            "value", None, None
+        )
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=False)
+    def test_force_service_down_252_empty_vals_svc(self, mv_mock):
+        self._verify2(
+            'openstack.compute.v2.service.Service.set_forced_down',
+            self.proxy.update_service_forced_down,
+            method_args=[{'host': 'a', 'binary': 'b'}, None, None],
+            expected_args=[self.proxy, None, None, True])
+
+    def test_find_service(self):
+        self.verify_find(
+            self.proxy.find_service,
+            service.Service,
+        )
+
+    def test_find_service_args(self):
+        self.verify_find(
+            self.proxy.find_service,
+            service.Service,
+            method_kwargs={'host': 'h1'},
+            expected_kwargs={'host': 'h1'}
+        )
+
+
 class TestCompute(TestComputeProxy):
     def test_extension_find(self):
         self.verify_find(self.proxy.find_extension, extension.Extension)
@@ -793,28 +886,6 @@ class TestCompute(TestComputeProxy):
         self.verify_get(self.proxy.get_hypervisor,
                         hypervisor.Hypervisor)
 
-    def test_services(self):
-        self.verify_list_no_kwargs(self.proxy.services,
-                                   service.Service)
-
-    def test_enable_service(self):
-        self._verify('openstack.compute.v2.service.Service.enable',
-                     self.proxy.enable_service,
-                     method_args=["value", "host1", "nova-compute"],
-                     expected_args=["host1", "nova-compute"])
-
-    def test_disable_service(self):
-        self._verify('openstack.compute.v2.service.Service.disable',
-                     self.proxy.disable_service,
-                     method_args=["value", "host1", "nova-compute"],
-                     expected_args=["host1", "nova-compute", None])
-
-    def test_force_service_down(self):
-        self._verify('openstack.compute.v2.service.Service.force_down',
-                     self.proxy.force_service_down,
-                     method_args=["value", "host1", "nova-compute"],
-                     expected_args=["host1", "nova-compute"])
-
     def test_live_migrate_server(self):
         self._verify('openstack.compute.v2.server.Server.live_migrate',
                      self.proxy.live_migrate_server,

openstack/tests/unit/compute/v2/test_service.py

@@ -12,6 +12,7 @@
 
 from unittest import mock
 
+from openstack import exceptions
 from openstack.compute.v2 import service
 from openstack.tests.unit import base
 
@@ -31,10 +32,13 @@ class TestService(base.TestCase):
     def setUp(self):
         super(TestService, self).setUp()
         self.resp = mock.Mock()
-        self.resp.body = None
+        self.resp.body = {'service': {}}
         self.resp.json = mock.Mock(return_value=self.resp.body)
+        self.resp.status_code = 200
+        self.resp.headers = {}
         self.sess = mock.Mock()
         self.sess.put = mock.Mock(return_value=self.resp)
+        self.sess.default_microversion = '2.1'
 
     def test_basic(self):
         sot = service.Service()
@@ -45,20 +49,125 @@ class TestService(base.TestCase):
         self.assertTrue(sot.allow_list)
         self.assertFalse(sot.allow_fetch)
 
+        self.assertDictEqual({
+            'binary': 'binary',
+            'host': 'host',
+            'limit': 'limit',
+            'marker': 'marker',
+            'name': 'binary',
+        },
+            sot._query_mapping._mapping)
+
     def test_make_it(self):
         sot = service.Service(**EXAMPLE)
         self.assertEqual(EXAMPLE['host'], sot.host)
         self.assertEqual(EXAMPLE['binary'], sot.binary)
+        self.assertEqual(EXAMPLE['binary'], sot.name)
         self.assertEqual(EXAMPLE['status'], sot.status)
         self.assertEqual(EXAMPLE['state'], sot.state)
         self.assertEqual(EXAMPLE['zone'], sot.availability_zone)
         self.assertEqual(EXAMPLE['id'], sot.id)
 
-    def test_force_down(self):
+    def test_find_single_match(self):
+        data = [
+            service.Service(name='bin1', host='host', id=1),
+            service.Service(name='bin2', host='host', id=2),
+        ]
+        with mock.patch.object(service.Service, 'list') as list_mock:
+            list_mock.return_value = data
+
+            sot = service.Service.find(
+                self.sess, 'bin1', ignore_missing=True, host='host'
+            )
+
+            self.assertEqual(data[0], sot)
+
+    def test_find_with_id_single_match(self):
+        data = [
+            service.Service(name='bin1', host='host', id=1),
+            service.Service(name='bin2', host='host', id='2'),
+        ]
+        with mock.patch.object(service.Service, 'list') as list_mock:
+            list_mock.return_value = data
+
+            sot = service.Service.find(
+                self.sess, '2', ignore_missing=True,
+                binary='bin1', host='host'
+            )
+
+            self.assertEqual(data[1], sot)
+
+            # Verify find when ID is int
+            sot = service.Service.find(
+                self.sess, 1, ignore_missing=True,
+                binary='bin1', host='host'
+            )
+
+            self.assertEqual(data[0], sot)
+
+    def test_find_no_match(self):
+        data = [
+            service.Service(name='bin1', host='host', id=1),
+            service.Service(name='bin2', host='host', id=2),
+        ]
+        with mock.patch.object(service.Service, 'list') as list_mock:
+            list_mock.return_value = data
+
+            self.assertIsNone(service.Service.find(
+                self.sess, 'fake', ignore_missing=True, host='host'
+            ))
+
+    def test_find_no_match_exception(self):
+        data = [
+            service.Service(name='bin1', host='host', id=1),
+            service.Service(name='bin2', host='host', id=2),
+        ]
+        with mock.patch.object(service.Service, 'list') as list_mock:
+            list_mock.return_value = data
+
+            self.assertRaises(
+                exceptions.ResourceNotFound,
+                service.Service.find,
+                self.sess, 'fake', ignore_missing=False, host='host'
+            )
+
+    def test_find_multiple_match(self):
+        data = [
+            service.Service(name='bin1', host='host', id=1),
+            service.Service(name='bin1', host='host', id=2),
+        ]
+        with mock.patch.object(service.Service, 'list') as list_mock:
+            list_mock.return_value = data
+
+            self.assertRaises(
+                exceptions.DuplicateResource,
+                service.Service.find,
+                self.sess, 'bin1', ignore_missing=False, host='host'
+            )
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=False)
+    def test_set_forced_down_before_211(self, mv_mock):
         sot = service.Service(**EXAMPLE)
 
-        res = sot.force_down(self.sess, 'host1', 'nova-compute')
+        res = sot.set_forced_down(self.sess, 'host1', 'nova-compute', True)
-        self.assertIsNone(res.body)
+        self.assertIsNotNone(res)
+
+        url = 'os-services/force-down'
+        body = {
+            'binary': 'nova-compute',
+            'host': 'host1',
+        }
+        self.sess.put.assert_called_with(
+            url, json=body, microversion=self.sess.default_microversion)
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=True)
+    def test_set_forced_down_after_211(self, mv_mock):
+        sot = service.Service(**EXAMPLE)
+
+        res = sot.set_forced_down(self.sess, 'host1', 'nova-compute', True)
+        self.assertIsNotNone(res)
 
         url = 'os-services/force-down'
         body = {
@@ -67,13 +176,30 @@ class TestService(base.TestCase):
             'forced_down': True,
         }
         self.sess.put.assert_called_with(
-            url, json=body)
+            url, json=body, microversion='2.11')
+
+    @mock.patch('openstack.utils.supports_microversion', autospec=True,
+                return_value=True)
+    def test_set_forced_down_after_253(self, mv_mock):
+        sot = service.Service(**EXAMPLE)
+
+        res = sot.set_forced_down(self.sess, None, None, True)
+        self.assertIsNotNone(res)
+
+        url = 'os-services/force-down'
+        body = {
+            'binary': sot.binary,
+            'host': sot.host,
+            'forced_down': True,
+        }
+        self.sess.put.assert_called_with(
+            url, json=body, microversion='2.11')
 
     def test_enable(self):
         sot = service.Service(**EXAMPLE)
 
         res = sot.enable(self.sess, 'host1', 'nova-compute')
-        self.assertIsNone(res.body)
+        self.assertIsNotNone(res)
 
         url = 'os-services/enable'
         body = {
@@ -81,13 +207,13 @@ class TestService(base.TestCase):
             'host': 'host1',
         }
         self.sess.put.assert_called_with(
-            url, json=body)
+            url, json=body, microversion=self.sess.default_microversion)
 
     def test_disable(self):
         sot = service.Service(**EXAMPLE)
 
         res = sot.disable(self.sess, 'host1', 'nova-compute')
-        self.assertIsNone(res.body)
+        self.assertIsNotNone(res)
 
         url = 'os-services/disable'
         body = {
@@ -95,7 +221,7 @@ class TestService(base.TestCase):
             'host': 'host1',
         }
         self.sess.put.assert_called_with(
-            url, json=body)
+            url, json=body, microversion=self.sess.default_microversion)
 
     def test_disable_with_reason(self):
         sot = service.Service(**EXAMPLE)
@@ -103,7 +229,7 @@ class TestService(base.TestCase):
 
         res = sot.disable(self.sess, 'host1', 'nova-compute', reason=reason)
 
-        self.assertIsNone(res.body)
+        self.assertIsNotNone(res)
 
         url = 'os-services/disable-log-reason'
         body = {
@@ -112,4 +238,4 @@ class TestService(base.TestCase):
             'disabled_reason': reason
         }
         self.sess.put.assert_called_with(
-            url, json=body)
+            url, json=body, microversion=self.sess.default_microversion)

releasenotes/notes/rename-service-force-down-6f462d62959a5315.yaml

@@ -0,0 +1,9 @@
+---
+upgrade:
+  - |
+    compute.force_service_down function is renamed to
+    update_service_forced_down to better fit the operation meaning.
+  - |
+    compute.v2.service.force_down is renamed to set_forced_down to fit the operation meaning.
+  - |
+    return of compute.service modification operations is changed to be the service itself