Fix resource deletion in clustering

The addition of the global Location setting for Resource
object broke clustering because it was relying on the
Location header to find the Action object associated
with the delete action.

To fix that, instead of exposing that location on a
location property, construct an Action object with the
id pulled from the location header in the delete method.
This way there is an object with a status property already.

fetch will need to be called on the Action returned to
fill in status information - but since wait_for_status
and wait_for_delete do that already, it should work with
those systems as expected.

Change-Id: Ifa44aacc4b4719b73e59d27ed0fcd35130358608

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

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)

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

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

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

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)

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)

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.