From 5450c4525313581b2ebc0e1ab6ac6252964c595f Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Fri, 4 Dec 2020 18:36:14 +0100 Subject: [PATCH] Complete compute.service operations Complete the operations on the compute.service resource. Change-Id: I0b7d41b407c436dd5583158bc6b5815847cffa31 --- doc/source/user/proxies/compute.rst | 3 +- openstack/compute/v2/_proxy.py | 126 ++++++++++++--- openstack/compute/v2/service.py | 107 ++++++++++--- .../functional/compute/v2/test_service.py | 47 ++++++ openstack/tests/unit/compute/v2/test_proxy.py | 115 +++++++++++--- .../tests/unit/compute/v2/test_service.py | 148 ++++++++++++++++-- ...e-service-force-down-6f462d62959a5315.yaml | 9 ++ 7 files changed, 481 insertions(+), 74 deletions(-) create mode 100644 openstack/tests/functional/compute/v2/test_service.py create mode 100644 releasenotes/notes/rename-service-force-down-6f462d62959a5315.yaml diff --git a/doc/source/user/proxies/compute.rst b/doc/source/user/proxies/compute.rst index eaa904271..65d7c4916 100644 --- a/doc/source/user/proxies/compute.rst +++ b/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 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/openstack/compute/v2/_proxy.py b/openstack/compute/v2/_proxy.py index 33a22d9ca..bf7295df8 100644 --- a/openstack/compute/v2/_proxy.py +++ b/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): - """Force a service down + # ========== Services ========== + + 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) - service.force_down(self, host, binary) + if utils.supports_microversion(self, '2.53'): + 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) - service.disable(self, - host, binary, - disabled_reason) + if utils.supports_microversion(self, '2.53'): + attrs = { + 'status': 'disabled' + } + 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: None + :returns: Updated service instance + :rtype: class: `~openstack.compute.v2.service.Service` """ - service = self._get_resource(_service.Service, service) - service.enable(self, host, binary) + if utils.supports_microversion(self, '2.53'): + 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 diff --git a/openstack/compute/v2/service.py b/openstack/compute/v2/service.py index d1361ffd8..4694928ed 100644 --- a/openstack/compute/v2/service.py +++ b/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) - - def force_down(self, session, host, binary): - """Force a service down.""" + response = session.put(url, json=body, microversion=microversion) + self._translate_response(response) + return self + 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.""" diff --git a/openstack/tests/functional/compute/v2/test_service.py b/openstack/tests/functional/compute/v2/test_service.py new file mode 100644 index 000000000..31323cc88 --- /dev/null +++ b/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) diff --git a/openstack/tests/unit/compute/v2/test_proxy.py b/openstack/tests/unit/compute/v2/test_proxy.py index 498d864ce..1fc21da07 100644 --- a/openstack/tests/unit/compute/v2/test_proxy.py +++ b/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, diff --git a/openstack/tests/unit/compute/v2/test_service.py b/openstack/tests/unit/compute/v2/test_service.py index 9f6c20ac7..0db702852 100644 --- a/openstack/tests/unit/compute/v2/test_service.py +++ b/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') - self.assertIsNone(res.body) + res = sot.set_forced_down(self.sess, 'host1', 'nova-compute', True) + 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) diff --git a/releasenotes/notes/rename-service-force-down-6f462d62959a5315.yaml b/releasenotes/notes/rename-service-force-down-6f462d62959a5315.yaml new file mode 100644 index 000000000..bad5c790b --- /dev/null +++ b/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