Stop leaking server objects

We should not be returning raw client objects when creating or
rebuilding a server.

The usage document is updated to indicate that access to resource
values via attribute is deprecated, and the examples in the README
now reflect dict-style access.

Change-Id: Iac38d4c0b29f867cc3cefaccf48c1c3fcd17a3d9

README.rst

@@ -37,9 +37,9 @@ Sometimes an example is nice.
   # But you can also access the underlying python-*client objects
   cinder = cloud.cinder_client
   volumes = cinder.volumes.list()
-  volume_id = [v for v in volumes if v.status == 'available'][0].id
+  volume_id = [v for v in volumes if v['status'] == 'available'][0]['id']
   nova = cloud.nova_client
-  print nova.volumes.create_server_volume(s.id, volume_id, None)
+  print nova.volumes.create_server_volume(s['id'], volume_id, None)
   attachments = []
   print volume_id
   while not attachments:

doc/source/usage.rst

@@ -1,10 +1,17 @@
-========
+=====
 Usage
-========
+=====
 
 To use shade in a project::
 
 	import shade
 
+.. warning::
+  Several of the API methods return a ``dict`` that describe a resource.
+  It is possible to access keys of the dict as an attribute (e.g.,
+  ``server.id`` instead of ``server['id']``) to maintain some backward
+  compatibility, but attribute access is deprecated. New code should
+  assume a normal dictionary and access values via key.
+
 .. automodule:: shade
    :members:

shade/__init__.py

@@ -1677,7 +1677,11 @@ class OpenStackCloud(object):
     def create_server(self, auto_ip=True, ips=None, ip_pool=None,
                       root_volume=None, terminate_volume=False,
                       wait=False, timeout=180, **bootkwargs):
+        """Create a virtual server instance.
 
+        :returns: A dict representing the created server.
+        :raises: OpenStackCloudException on operation error.
+        """
         if root_volume:
             if terminate_volume:
                 suffix = ':::1'
@@ -1703,21 +1707,22 @@ class OpenStackCloud(object):
                     timeout,
                     "Timeout waiting for the server to come up."):
                 try:
-                    server = self.manager.submitTask(
+                    server = meta.obj_to_dict(
+                        self.manager.submitTask(
                             _tasks.ServerGet(server=server))
+                    )
                 except Exception:
                     continue
 
-                if server.status == 'ACTIVE':
+                if server['status'] == 'ACTIVE':
                     return self.add_ips_to_server(
                         server, auto_ip, ips, ip_pool)
 
-                if server.status == 'ERROR':
+                if server['status'] == 'ERROR':
                     raise OpenStackCloudException(
                         "Error in creating the server",
-                        extra_data=dict(
+                        extra_data=dict(server=server))
-                            server=meta.obj_to_dict(server)))
+        return meta.obj_to_dict(server)
-        return server
 
     def rebuild_server(self, server_id, image_id, wait=False, timeout=180):
         try:
@@ -1733,20 +1738,21 @@ class OpenStackCloud(object):
                     "Timeout waiting for server {0} to "
                     "rebuild.".format(server_id)):
                 try:
-                    server = self.manager.submitTask(
+                    server = meta.obj_to_dict(
+                        self.manager.submitTask(
                             _tasks.ServerGet(server=server))
+                    )
                 except Exception:
                     continue
 
-                if server.status == 'ACTIVE':
+                if server['status'] == 'ACTIVE':
-                    break
+                    return server
 
-                if server.status == 'ERROR':
+                if server['status'] == 'ERROR':
                     raise OpenStackCloudException(
                         "Error in rebuilding the server",
-                        extra_data=dict(
+                        extra_data=dict(server=server))
-                            server=meta.obj_to_dict(server)))
+        return meta.obj_to_dict(server)
-        return server
 
     def delete_server(self, name, wait=False, timeout=180):
         server = self.get_server(name)

shade/tests/functional/test_compute.py

@@ -19,7 +19,6 @@ test_compute
 Functional tests for `shade` compute methods.
 """
 
-from novaclient.v2.servers import Server
 from shade import openstack_cloud
 from shade.tests import base
 from shade.tests.functional.util import pick_flavor, pick_image
@@ -47,10 +46,9 @@ class TestCompute(base.TestCase):
         self.addCleanup(self._cleanup_servers)
         server = self.cloud.create_server(name='test_create_server',
                                           image=self.image, flavor=self.flavor)
-        self.assertIsInstance(server, Server)
+        self.assertEquals(server['name'], 'test_create_server')
-        self.assertEquals(server.name, 'test_create_server')
+        self.assertEquals(server['image']['id'], self.image.id)
-        self.assertEquals(server.image['id'], self.image.id)
+        self.assertEquals(server['flavor']['id'], self.flavor.id)
-        self.assertEquals(server.flavor['id'], self.flavor.id)
 
     def test_delete_server(self):
         self.cloud.create_server(name='test_delete_server',

shade/tests/unit/test_create_server.py

@@ -20,6 +20,7 @@ Tests for the `create_server` command.
 """
 
 from mock import patch, Mock
+from shade import meta
 from shade import OpenStackCloud
 from shade.exc import (OpenStackCloudException, OpenStackCloudTimeout)
 from shade.tests import base, fakes
@@ -115,8 +116,8 @@ class TestCreateServer(base.TestCase):
                 "servers.get.return_value": fake_server
             }
             OpenStackCloud.nova_client = Mock(**config)
-            self.assertEqual(
+            self.assertEqual(meta.obj_to_dict(fake_server),
-                self.client.create_server(), fake_server)
+                             self.client.create_server())
 
     def test_create_server_wait(self):
         """

shade/tests/unit/test_rebuild_server.py

@@ -20,6 +20,7 @@ Tests for the `rebuild_server` command.
 """
 
 from mock import patch, Mock
+from shade import meta
 from shade import OpenStackCloud
 from shade.exc import (OpenStackCloudException, OpenStackCloudTimeout)
 from shade.tests.unit import base
@@ -85,8 +86,8 @@ class TestRebuildServer(base.TestCase):
                 "servers.rebuild.return_value": mock_server
             }
             OpenStackCloud.nova_client = Mock(**config)
-            self.assertEqual(
+            self.assertEqual(meta.obj_to_dict(mock_server),
-                self.client.rebuild_server("a", "b"), mock_server)
+                             self.client.rebuild_server("a", "b"))
 
     def test_rebuild_server_wait(self):
         """
@@ -100,6 +101,5 @@ class TestRebuildServer(base.TestCase):
                 "servers.get.return_value": mock_server
             }
             OpenStackCloud.nova_client = Mock(**config)
-            self.assertEqual(
+            self.assertEqual(meta.obj_to_dict(mock_server),
-                self.client.rebuild_server("a", "b", wait=True),
+                             self.client.rebuild_server("a", "b", wait=True))
-                mock_server)