diff --git a/openstack/clustering/v1/_async_resource.py b/openstack/clustering/v1/_async_resource.py new file mode 100644 index 000000000..cd01ac3fe --- /dev/null +++ b/openstack/clustering/v1/_async_resource.py @@ -0,0 +1,45 @@ +# 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 import exceptions +from openstack import resource + +from openstack.clustering.v1 import action as _action + + +class AsyncResource(resource.Resource): + + def delete(self, session, error_message=None): + """Delete the remote resource based on this instance. + + :param session: The session to use for making this request. + :type session: :class:`~keystoneauth1.adapter.Adapter` + + :return: An :class:`~openstack.clustering.v1.action.Action` + instance. The ``fetch`` method will need to be used + to populate the `Action` with status information. + :raises: :exc:`~openstack.exceptions.MethodNotSupported` if + :data:`Resource.allow_commit` is not set to ``True``. + :raises: :exc:`~openstack.exceptions.ResourceNotFound` if + the resource was not found. + """ + response = self._raw_delete(session) + return self._delete_response(response, error_message) + + def _delete_response(self, response, error_message=None): + exceptions.raise_from_response(response, error_message=error_message) + location = response.headers['Location'] + action_id = location.split('/')[-1] + action = _action.Action.existing( + id=action_id, + connection=self._connection) + return action diff --git a/openstack/clustering/v1/cluster.py b/openstack/clustering/v1/cluster.py index 26e22cd2f..e545080c4 100644 --- a/openstack/clustering/v1/cluster.py +++ b/openstack/clustering/v1/cluster.py @@ -13,8 +13,10 @@ from openstack import resource from openstack import utils +from openstack.clustering.v1 import _async_resource -class Cluster(resource.Resource): + +class Cluster(_async_resource.AsyncResource): resource_key = 'cluster' resources_key = 'clusters' base_path = '/clusters' @@ -184,6 +186,5 @@ class Cluster(resource.Resource): """Force delete a cluster.""" body = {'force': True} url = utils.urljoin(self.base_path, self.id) - resp = session.delete(url, json=body) - self._translate_response(resp, has_body=False) - return self + response = session.delete(url, json=body) + return self._delete_response(response) diff --git a/openstack/clustering/v1/node.py b/openstack/clustering/v1/node.py index cf9fe05e2..909ff6e95 100644 --- a/openstack/clustering/v1/node.py +++ b/openstack/clustering/v1/node.py @@ -13,8 +13,10 @@ from openstack import resource from openstack import utils +from openstack.clustering.v1 import _async_resource -class Node(resource.Resource): + +class Node(_async_resource.AsyncResource): resource_key = 'node' resources_key = 'nodes' base_path = '/nodes' @@ -158,9 +160,8 @@ class Node(resource.Resource): """Force delete a node.""" body = {'force': True} url = utils.urljoin(self.base_path, self.id) - resp = session.delete(url, json=body) - self._translate_response(resp, has_body=False) - return self + response = session.delete(url, json=body) + return self._delete_response(response) class NodeDetail(Node): diff --git a/openstack/resource.py b/openstack/resource.py index 8ecc39e55..97d7f3a46 100644 --- a/openstack/resource.py +++ b/openstack/resource.py @@ -415,6 +415,7 @@ class Resource(dict): _uri = None _computed = None _original_body = None + _delete_response_class = None def __init__(self, _synchronized=False, connection=None, **attrs): """The base resource @@ -1240,6 +1241,16 @@ class Resource(dict): :raises: :exc:`~openstack.exceptions.ResourceNotFound` if the resource was not found. """ + + response = self._raw_delete(session) + kwargs = {} + if error_message: + kwargs['error_message'] = error_message + + self._translate_response(response, has_body=False, **kwargs) + return self + + def _raw_delete(self, session): if not self.allow_delete: raise exceptions.MethodNotSupported(self, "delete") @@ -1247,15 +1258,10 @@ class Resource(dict): session = self._get_session(session) microversion = self._get_microversion_for(session, 'delete') - response = session.delete(request.url, - headers={"Accept": ""}, - microversion=microversion) - kwargs = {} - if error_message: - kwargs['error_message'] = error_message - - self._translate_response(response, has_body=False, **kwargs) - return self + return session.delete( + request.url, + headers={"Accept": ""}, + microversion=microversion) @classmethod def list(cls, session, paginated=False, base_path=None, **params): diff --git a/openstack/tests/functional/clustering/test_cluster.py b/openstack/tests/functional/clustering/test_cluster.py index e118a7873..e964d37d4 100644 --- a/openstack/tests/functional/clustering/test_cluster.py +++ b/openstack/tests/functional/clustering/test_cluster.py @@ -70,9 +70,10 @@ class TestCluster(base.BaseFunctionalTest): assert isinstance(self.cluster, cluster.Cluster) def tearDown(self): - self.conn.clustering.delete_cluster(self.cluster.id) - self.conn.clustering.wait_for_delete(self.cluster, - wait=self._wait_for_timeout) + if self.cluster: + self.conn.clustering.delete_cluster(self.cluster.id) + self.conn.clustering.wait_for_delete( + self.cluster, wait=self._wait_for_timeout) test_network.delete_network(self.conn, self.network, self.subnet) @@ -100,3 +101,31 @@ class TestCluster(base.BaseFunctionalTest): time.sleep(2) sot = self.conn.clustering.get_cluster(self.cluster) self.assertEqual(new_cluster_name, sot.name) + + def test_delete(self): + cluster_delete_action = self.conn.clustering.delete_cluster( + self.cluster.id) + + self.conn.clustering.wait_for_delete(self.cluster, + wait=self._wait_for_timeout) + + action = self.conn.clustering.get_action(cluster_delete_action.id) + self.assertEqual(action.target_id, self.cluster.id) + self.assertEqual(action.action, 'CLUSTER_DELETE') + self.assertEqual(action.status, 'SUCCEEDED') + + self.cluster = None + + def test_force_delete(self): + cluster_delete_action = self.conn.clustering.delete_cluster( + self.cluster.id, False, True) + + self.conn.clustering.wait_for_delete(self.cluster, + wait=self._wait_for_timeout) + + action = self.conn.clustering.get_action(cluster_delete_action.id) + self.assertEqual(action.target_id, self.cluster.id) + self.assertEqual(action.action, 'CLUSTER_DELETE') + self.assertEqual(action.status, 'SUCCEEDED') + + self.cluster = None diff --git a/openstack/tests/unit/clustering/v1/test_cluster.py b/openstack/tests/unit/clustering/v1/test_cluster.py index 2d0a22ada..ac6eab76f 100644 --- a/openstack/tests/unit/clustering/v1/test_cluster.py +++ b/openstack/tests/unit/clustering/v1/test_cluster.py @@ -311,14 +311,15 @@ class TestCluster(base.TestCase): sot = cluster.Cluster(**FAKE) resp = mock.Mock() - resp.headers = {} + fake_action_id = 'f1de9847-2382-4272-8e73-cab0bc194663' + resp.headers = {'Location': fake_action_id} resp.json = mock.Mock(return_value={"foo": "bar"}) resp.status_code = 200 sess = mock.Mock() sess.delete = mock.Mock(return_value=resp) res = sot.force_delete(sess) - self.assertEqual(sot, res) + self.assertEqual(fake_action_id, res.id) url = 'clusters/%s' % sot.id body = {'force': True} sess.delete.assert_called_once_with(url, json=body) diff --git a/openstack/tests/unit/clustering/v1/test_node.py b/openstack/tests/unit/clustering/v1/test_node.py index c2838a093..328065788 100644 --- a/openstack/tests/unit/clustering/v1/test_node.py +++ b/openstack/tests/unit/clustering/v1/test_node.py @@ -142,14 +142,15 @@ class TestNode(base.TestCase): sot = node.Node(**FAKE) resp = mock.Mock() - resp.headers = {} + fake_action_id = 'f1de9847-2382-4272-8e73-cab0bc194663' + resp.headers = {'Location': fake_action_id} resp.json = mock.Mock(return_value={"foo": "bar"}) resp.status_code = 200 sess = mock.Mock() sess.delete = mock.Mock(return_value=resp) res = sot.force_delete(sess) - self.assertEqual(sot, res) + self.assertEqual(fake_action_id, res.id) url = 'nodes/%s' % sot.id body = {'force': True} sess.delete.assert_called_once_with(url, json=body) diff --git a/releasenotes/notes/clustering-resource-deletion-bed869ba47c2aac1.yaml b/releasenotes/notes/clustering-resource-deletion-bed869ba47c2aac1.yaml new file mode 100644 index 000000000..877a57171 --- /dev/null +++ b/releasenotes/notes/clustering-resource-deletion-bed869ba47c2aac1.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + Fixed a regression in deleting Node and Cluster resources + in clustering caused by the addition of the ``location`` + property to all resource objects. Previously the delete + calls had directly returned the ``location`` field + returned in the headers from the clustering service pointing + to an Action resource that could be fetched to get status + on the delete operation. The delete calls now return an + Action resource directly that is correctly constructed + so that ``wait_for_status`` and ``wait_for_deleted`` + work as expected.