Compute location properly in server

The shade layer pulls the az and project_id info out of the server
record and feeds it into the location field. This is because the
location field should be where the resource is, and the existing token
should only be a fallback.

This isn't happening in the resource layer, so start doing that.

As a result of digging, make sure we always pass in the connection
object to the Resources when we make them. And stop creating a bare
resource in order to use the 'new' class method. It's a class method, we
don't need to instantiate.

Change-Id: Ic8850cd2863e2c806464b92cde2c749f6cc01a91

openstack/cloud/_normalize.py

@@ -483,10 +483,11 @@ class Normalizer(object):
             server, 'OS-EXT-AZ:availability_zone', None, self.strict_mode)
         # the server resource has this already, but it's missing az info
         # from the resource.
-        # TODO(mordred) Fix server resource to set az in the location
+        # TODO(mordred) create_server is still normalizing servers that aren't
-        server.pop('location', None)
+        # from the resource layer.
-        ret['location'] = self._get_current_location(
+        ret['location'] = server.pop(
-            project_id=project_id, zone=az)
+            'location', self._get_current_location(
+                project_id=project_id, zone=az))
 
         # Ensure volumes is always in the server dict, even if empty
         ret['volumes'] = _pop_or_get(

openstack/compute/v2/server.py

@@ -138,6 +138,14 @@ class Server(resource.Resource, metadata.MetadataMixin, resource.TagMixin):
     #: only.
     instance_name = resource.Body('OS-EXT-SRV-ATTR:instance_name')
 
+    def __init__(self, *args, **kwargs):
+        super(Server, self).__init__(*args, **kwargs)
+
+        if self._connection:
+            self.location = self._connection._get_current_location(
+                project_id=self.project_id,
+                zone=self.availability_zone)
+
     def _prepare_request(self, requires_id=True, prepend_key=True,
                          base_path=None):
         request = super(Server, self)._prepare_request(requires_id=requires_id,

openstack/proxy.py

@@ -56,6 +56,17 @@ class Proxy(_adapter.OpenStackSDKAdapter):
                           self.retriable_status_codes)
         super(Proxy, self).__init__(*args, **kwargs)
 
+    def _get_connection(self):
+        """Get the Connection object associated with this Proxy.
+
+        When the Session is created, a reference to the Connection is attached
+        to the ``_sdk_connection`` attribute. We also add a reference to it
+        directly on ourselves. Use one of them.
+        """
+        return getattr(
+            self, '_connection', getattr(
+                self.session, '_sdk_connection', None))
+
     def _get_resource(self, resource_type, value, **attrs):
         """Get a resource object to work on
 
@@ -69,16 +80,19 @@ class Proxy(_adapter.OpenStackSDKAdapter):
         :param path_args: A dict containing arguments for forming the request
                           URL, if needed.
         """
+        conn = self._get_connection()
         if value is None:
             # Create a bare resource
-            res = resource_type.new(**attrs)
+            res = resource_type.new(connection=conn, **attrs)
         elif (isinstance(value, dict)
               and not isinstance(value, resource.Resource)):
-            res = resource_type._from_munch(value)
+            res = resource_type._from_munch(
+                value, connection=conn)
             res._update(**attrs)
         elif not isinstance(value, resource_type):
             # Create from an ID
-            res = resource_type.new(id=value, **attrs)
+            res = resource_type.new(
+                id=value, connection=conn, **attrs)
         else:
             # An existing resource instance
             res = value
@@ -205,7 +219,8 @@ class Proxy(_adapter.OpenStackSDKAdapter):
         :returns: The result of the ``create``
         :rtype: :class:`~openstack.resource.Resource`
         """
-        res = resource_type.new(**attrs)
+        conn = self._get_connection()
+        res = resource_type.new(connection=conn, **attrs)
         return res.create(self, base_path=base_path)
 
     @_check_resource(strict=False)
@@ -265,9 +280,10 @@ class Proxy(_adapter.OpenStackSDKAdapter):
                  :class:`~openstack.resource.Resource` that doesn't match
                  the ``resource_type``.
         """
-        res = self._get_resource(resource_type, value, **attrs)
+        return resource_type.list(
-        return res.list(self, paginated=paginated,
+            self, paginated=paginated,
-                        base_path=base_path, **attrs)
+            base_path=base_path,
+            **attrs)
 
     def _head(self, resource_type, value=None, base_path=None, **attrs):
         """Retrieve a resource's header

openstack/resource.py

@@ -765,11 +765,7 @@ class Resource(dict):
         :param dict kwargs: Each of the named arguments will be set as
                             attributes on the resulting Resource object.
         """
-        res = cls(_synchronized=True, **kwargs)
+        return cls(_synchronized=True, connection=connection, **kwargs)
-        # TODO(shade) Done as a second call rather than a constructor param
-        # because otherwise the mocking in the tests goes nuts.
-        res._connection = connection
-        return res
 
     @classmethod
     def _from_munch(cls, obj, synchronized=True, connection=None):
@@ -1338,11 +1334,10 @@ class Resource(dict):
                 # argument and is practically a reserved word.
                 raw_resource.pop("self", None)
 
-                value = cls.existing(microversion=microversion, **raw_resource)
+                value = cls.existing(
-                # TODO(shade) Done as a second call rather than a constructor
+                    microversion=microversion,
-                # param because otherwise the mocking in the tests goes nuts.
+                    connection=session._get_connection(),
-                if hasattr(session, '_sdk_connection'):
+                    **raw_resource)
-                    value._connection = session._sdk_connection
                 marker = value.id
                 yield value
                 total_yielded += 1
@@ -1447,13 +1442,13 @@ class Resource(dict):
         :raises: :class:`openstack.exceptions.ResourceNotFound` if nothing
                  is found and ignore_missing is ``False``.
         """
+        session = cls._get_session(session)
         # Try to short-circuit by looking directly for a matching ID.
         try:
-            match = cls.existing(id=name_or_id, **params)
+            match = cls.existing(
-            # TODO(shade) Done as a second call rather than a constructor
+                id=name_or_id,
-            # param because otherwise the mocking in the tests goes nuts.
+                connection=session._get_connection(),
-            if hasattr(session, '_sdk_connection'):
+                **params)
-                match._connection = session._sdk_connection
             return match.fetch(session)
         except exceptions.NotFoundException:
             pass

openstack/tests/unit/cloud/test_fwaas.py

@@ -48,9 +48,10 @@ class TestFirewallRule(FirewallTestCase):
     mock_firewall_rule = None
 
     def setUp(self, cloud_config_fixture='clouds.yaml'):
-        self.mock_firewall_rule = FirewallRule(
-            **self._mock_firewall_rule_attrs).to_dict()
         super(TestFirewallRule, self).setUp()
+        self.mock_firewall_rule = FirewallRule(
+            connection=self.cloud,
+            **self._mock_firewall_rule_attrs).to_dict()
 
     def test_create_firewall_rule(self):
         # attributes that are passed to the tested function
@@ -260,9 +261,10 @@ class TestFirewallPolicy(FirewallTestCase):
     mock_firewall_policy = None
 
     def setUp(self, cloud_config_fixture='clouds.yaml'):
-        self.mock_firewall_policy = FirewallPolicy(
-            **self._mock_firewall_policy_attrs).to_dict()
         super(TestFirewallPolicy, self).setUp()
+        self.mock_firewall_policy = FirewallPolicy(
+            connection=self.cloud,
+            **self._mock_firewall_policy_attrs).to_dict()
 
     def test_create_firewall_policy(self):
         # attributes that are passed to the tested method
@@ -272,7 +274,7 @@ class TestFirewallPolicy(FirewallTestCase):
         # policy that is returned by the POST request
         created_attrs = deepcopy(self._mock_firewall_policy_attrs)
         created_attrs['firewall_rules'][0] = TestFirewallRule.firewall_rule_id
-        created_policy = FirewallPolicy(**created_attrs)
+        created_policy = FirewallPolicy(connection=self.cloud, **created_attrs)
 
         # attributes used to validate the request inside register_uris()
         validate_attrs = deepcopy(created_attrs)
@@ -421,7 +423,9 @@ class TestFirewallPolicy(FirewallTestCase):
                      self.mock_firewall_policy.copy(),
                      self.mock_firewall_policy.copy()]})
         ])
-        policy = FirewallPolicy(**self.mock_firewall_policy)
+        policy = FirewallPolicy(
+            connection=self.cloud,
+            **self.mock_firewall_policy)
         self.assertListEqual(self.cloud.list_firewall_policies(),
                              [policy, policy])
         self.assert_calls()
@@ -434,12 +438,16 @@ class TestFirewallPolicy(FirewallTestCase):
                  json={'firewall_policies': [
                      self.mock_firewall_policy]})
         ])
-        self.assertListEqual(self.cloud.list_firewall_policies(filters),
+        self.assertListEqual(
-                             [FirewallPolicy(**self.mock_firewall_policy)])
+            self.cloud.list_firewall_policies(filters), [
+                FirewallPolicy(
+                    connection=self.cloud,
+                    **self.mock_firewall_policy)])
         self.assert_calls()
 
     def test_update_firewall_policy(self):
         lookup_rule = FirewallRule(
+            connection=self.cloud,
             **TestFirewallRule._mock_firewall_rule_attrs).to_dict()
         params = {'firewall_rules': [lookup_rule['id']],
                   'description': 'updated!'}
@@ -520,7 +528,9 @@ class TestFirewallPolicy(FirewallTestCase):
         self.cloud.network.find_firewall_policy = _find
 
     def test_insert_rule_into_policy(self):
-        rule0 = FirewallRule(**TestFirewallRule._mock_firewall_rule_attrs)
+        rule0 = FirewallRule(
+            connection=self.cloud,
+            **TestFirewallRule._mock_firewall_rule_attrs)
 
         _rule1_attrs = deepcopy(
             TestFirewallRule._mock_firewall_rule_attrs)
@@ -834,15 +844,19 @@ class TestFirewallGroup(FirewallTestCase):
     mock_returned_firewall_rule = None
 
     def setUp(self, cloud_config_fixture='clouds.yaml'):
+        super(TestFirewallGroup, self).setUp()
         self.mock_egress_policy = FirewallPolicy(
+            connection=self.cloud,
             **self._mock_egress_policy_attrs).to_dict()
         self.mock_ingress_policy = FirewallPolicy(
+            connection=self.cloud,
             **self._mock_ingress_policy_attrs).to_dict()
         self.mock_firewall_group = FirewallGroup(
+            connection=self.cloud,
             **self._mock_firewall_group_attrs).to_dict()
         self.mock_returned_firewall_group = FirewallGroup(
+            connection=self.cloud,
             **self._mock_returned_firewall_group_attrs).to_dict()
-        super(TestFirewallGroup, self).setUp()
 
     def test_create_firewall_group(self):
         create_group_attrs = self._mock_firewall_group_attrs.copy()
@@ -901,7 +915,10 @@ class TestFirewallGroup(FirewallTestCase):
         ])
         r = self.cloud.create_firewall_group(**firewall_group)
         self.assertDictEqual(
-            FirewallGroup(**created_firewall).to_dict(), r.to_dict())
+            FirewallGroup(
+                connection=self.cloud,
+                **created_firewall).to_dict(),
+            r.to_dict())
         self.assert_calls()
 
     def test_delete_firewall_group(self):
@@ -1010,7 +1027,7 @@ class TestFirewallGroup(FirewallTestCase):
                  uri=self._make_mock_url('firewall_groups'),
                  json={'firewall_groups': [returned_attrs, returned_attrs]})
         ])
-        group = FirewallGroup(**returned_attrs)
+        group = FirewallGroup(connection=self.cloud, **returned_attrs)
         self.assertListEqual([group, group], self.cloud.list_firewall_groups())
         self.assert_calls()
 

openstack/tests/unit/network/v2/test_floating_ip.py

@@ -10,11 +10,11 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-from keystoneauth1 import adapter
 import mock
-from openstack.tests.unit import base
 
+from openstack import proxy
 from openstack.network.v2 import floating_ip
+from openstack.tests.unit import base
 
 IDENTIFIER = 'IDENTIFIER'
 EXAMPLE = {
@@ -73,9 +73,10 @@ class TestFloatingIP(base.TestCase):
         self.assertEqual(EXAMPLE['tags'], sot.tags)
 
     def test_find_available(self):
-        mock_session = mock.Mock(spec=adapter.Adapter)
+        mock_session = mock.Mock(spec=proxy.Proxy)
         mock_session.get_filter = mock.Mock(return_value={})
         mock_session.default_microversion = None
+        mock_session.session = self.cloud.session
         data = {'id': 'one', 'floating_ip_address': '10.0.0.1'}
         fake_response = mock.Mock()
         body = {floating_ip.FloatingIP.resources_key: [data]}
@@ -93,7 +94,7 @@ class TestFloatingIP(base.TestCase):
             microversion=None)
 
     def test_find_available_nada(self):
-        mock_session = mock.Mock(spec=adapter.Adapter)
+        mock_session = mock.Mock(spec=proxy.Proxy)
         mock_session.default_microversion = None
         fake_response = mock.Mock()
         body = {floating_ip.FloatingIP.resources_key: []}

openstack/tests/unit/test_proxy.py

@@ -54,7 +54,10 @@ class TestProxyPrivate(base.TestCase):
         self.sot = mock.Mock()
         self.sot.method = method
 
-        self.fake_proxy = proxy.Proxy("session")
+        self.session = mock.Mock()
+        self.session._sdk_connection = self.cloud
+        self.fake_proxy = proxy.Proxy(self.session)
+        self.fake_proxy._connection = self.cloud
 
     def _test_correct(self, value):
         decorated = proxy._check_resource(strict=False)(self.sot.method)
@@ -119,7 +122,7 @@ class TestProxyPrivate(base.TestCase):
 
         result = self.fake_proxy._get_resource(fake_type, None, **attrs)
 
-        fake_type.new.assert_called_with(**attrs)
+        fake_type.new.assert_called_with(connection=self.cloud, **attrs)
         self.assertEqual(value, result)
 
     def test__get_resource_from_id(self):
@@ -143,7 +146,8 @@ class TestProxyPrivate(base.TestCase):
 
         result = self.fake_proxy._get_resource(Fake, id, **attrs)
 
-        self.assertDictEqual(dict(id=id, **attrs), Fake.call)
+        self.assertDictEqual(
+            dict(id=id, connection=mock.ANY, **attrs), Fake.call)
         self.assertEqual(value, result)
 
     def test__get_resource_from_resource(self):
@@ -169,7 +173,7 @@ class TestProxyPrivate(base.TestCase):
 
         result = self.fake_proxy._get_resource(cls, m, **attrs)
 
-        cls._from_munch.assert_called_once_with(m)
+        cls._from_munch.assert_called_once_with(m, connection=self.cloud)
         res._update.assert_called_once_with(**attrs)
         self.assertEqual(result, res)
 
@@ -180,6 +184,7 @@ class TestProxyDelete(base.TestCase):
         super(TestProxyDelete, self).setUp()
 
         self.session = mock.Mock()
+        self.session._sdk_connection = self.cloud
 
         self.fake_id = 1
         self.res = mock.Mock(spec=DeleteableResource)
@@ -187,6 +192,7 @@ class TestProxyDelete(base.TestCase):
         self.res.delete = mock.Mock()
 
         self.sot = proxy.Proxy(self.session)
+        self.sot._connection = self.cloud
         DeleteableResource.new = mock.Mock(return_value=self.res)
 
     def test_delete(self):
@@ -194,7 +200,8 @@ class TestProxyDelete(base.TestCase):
         self.res.delete.assert_called_with(self.sot)
 
         self.sot._delete(DeleteableResource, self.fake_id)
-        DeleteableResource.new.assert_called_with(id=self.fake_id)
+        DeleteableResource.new.assert_called_with(
+            connection=self.cloud, id=self.fake_id)
         self.res.delete.assert_called_with(self.sot)
 
         # Delete generally doesn't return anything, so we will normally
@@ -244,6 +251,7 @@ class TestProxyUpdate(base.TestCase):
         self.res.commit = mock.Mock(return_value=self.fake_result)
 
         self.sot = proxy.Proxy(self.session)
+        self.sot._connection = self.cloud
 
         self.attrs = {"x": 1, "y": 2, "z": 3}
 
@@ -278,12 +286,14 @@ class TestProxyCreate(base.TestCase):
         super(TestProxyCreate, self).setUp()
 
         self.session = mock.Mock()
+        self.session._sdk_connection = self.cloud
 
         self.fake_result = "fake_result"
         self.res = mock.Mock(spec=CreateableResource)
         self.res.create = mock.Mock(return_value=self.fake_result)
 
         self.sot = proxy.Proxy(self.session)
+        self.sot._connection = self.cloud
 
     def test_create_attributes(self):
         CreateableResource.new = mock.Mock(return_value=self.res)
@@ -292,7 +302,8 @@ class TestProxyCreate(base.TestCase):
         rv = self.sot._create(CreateableResource, **attrs)
 
         self.assertEqual(rv, self.fake_result)
-        CreateableResource.new.assert_called_once_with(**attrs)
+        CreateableResource.new.assert_called_once_with(
+            connection=self.cloud, **attrs)
         self.res.create.assert_called_once_with(self.sot, base_path=None)
 
     def test_create_attributes_override_base_path(self):
@@ -303,7 +314,8 @@ class TestProxyCreate(base.TestCase):
         rv = self.sot._create(CreateableResource, base_path=base_path, **attrs)
 
         self.assertEqual(rv, self.fake_result)
-        CreateableResource.new.assert_called_once_with(**attrs)
+        CreateableResource.new.assert_called_once_with(
+            connection=self.cloud, **attrs)
         self.res.create.assert_called_once_with(self.sot, base_path=base_path)
 
 
@@ -313,6 +325,7 @@ class TestProxyGet(base.TestCase):
         super(TestProxyGet, self).setUp()
 
         self.session = mock.Mock()
+        self.session._sdk_connection = self.cloud
 
         self.fake_id = 1
         self.fake_name = "fake_name"
@@ -322,6 +335,7 @@ class TestProxyGet(base.TestCase):
         self.res.fetch = mock.Mock(return_value=self.fake_result)
 
         self.sot = proxy.Proxy(self.session)
+        self.sot._connection = self.cloud
         RetrieveableResource.new = mock.Mock(return_value=self.res)
 
     def test_get_resource(self):
@@ -346,7 +360,8 @@ class TestProxyGet(base.TestCase):
     def test_get_id(self):
         rv = self.sot._get(RetrieveableResource, self.fake_id)
 
-        RetrieveableResource.new.assert_called_with(id=self.fake_id)
+        RetrieveableResource.new.assert_called_with(
+            connection=self.cloud, id=self.fake_id)
         self.res.fetch.assert_called_with(
             self.sot, requires_id=True, base_path=None,
             error_message=mock.ANY)
@@ -357,7 +372,8 @@ class TestProxyGet(base.TestCase):
         rv = self.sot._get(RetrieveableResource, self.fake_id,
                            base_path=base_path)
 
-        RetrieveableResource.new.assert_called_with(id=self.fake_id)
+        RetrieveableResource.new.assert_called_with(
+            connection=self.cloud, id=self.fake_id)
         self.res.fetch.assert_called_with(
             self.sot, requires_id=True, base_path=base_path,
             error_message=mock.ANY)
@@ -383,6 +399,7 @@ class TestProxyList(base.TestCase):
         self.fake_response = [resource.Resource()]
 
         self.sot = proxy.Proxy(self.session)
+        self.sot._connection = self.cloud
         ListableResource.list = mock.Mock()
         ListableResource.list.return_value = self.fake_response
 
@@ -410,6 +427,7 @@ class TestProxyHead(base.TestCase):
         super(TestProxyHead, self).setUp()
 
         self.session = mock.Mock()
+        self.session._sdk_connection = self.cloud
 
         self.fake_id = 1
         self.fake_name = "fake_name"
@@ -419,6 +437,7 @@ class TestProxyHead(base.TestCase):
         self.res.head = mock.Mock(return_value=self.fake_result)
 
         self.sot = proxy.Proxy(self.session)
+        self.sot._connection = self.cloud
         HeadableResource.new = mock.Mock(return_value=self.res)
 
     def test_head_resource(self):
@@ -437,6 +456,7 @@ class TestProxyHead(base.TestCase):
     def test_head_id(self):
         rv = self.sot._head(HeadableResource, self.fake_id)
 
-        HeadableResource.new.assert_called_with(id=self.fake_id)
+        HeadableResource.new.assert_called_with(
+            connection=self.cloud, id=self.fake_id)
         self.res.head.assert_called_with(self.sot, base_path=None)
         self.assertEqual(rv, self.fake_result)

openstack/tests/unit/test_resource.py

@@ -1068,6 +1068,8 @@ class TestResourceActions(base.TestCase):
         self.session.post = mock.Mock(return_value=self.response)
         self.session.delete = mock.Mock(return_value=self.response)
         self.session.head = mock.Mock(return_value=self.response)
+        self.session.session = self.session
+        self.session._get_connection = mock.Mock(return_value=self.cloud)
         self.session.default_microversion = None
         self.session.retriable_status_codes = None
 
@@ -2048,20 +2050,23 @@ class TestResourceFind(base.TestCase):
                 mock_match.fetch.return_value = value
                 return mock_match
 
-        result = Test.find("session", "name")
+        result = Test.find(self.cloud.compute, "name")
 
         self.assertEqual(result, value)
 
     def test_no_match_raise(self):
         self.assertRaises(exceptions.ResourceNotFound, self.no_results.find,
-                          "session", "name", ignore_missing=False)
+                          self.cloud.compute, "name", ignore_missing=False)
 
     def test_no_match_return(self):
         self.assertIsNone(
-            self.no_results.find("session", "name", ignore_missing=True))
+            self.no_results.find(
+                self.cloud.compute, "name", ignore_missing=True))
 
     def test_find_result(self):
-        self.assertEqual(self.result, self.one_result.find("session", "name"))
+        self.assertEqual(
+            self.result,
+            self.one_result.find(self.cloud.compute, "name"))
 
     def test_match_empty_results(self):
         self.assertIsNone(resource.Resource._get_one_match("name", []))
@@ -2126,7 +2131,7 @@ class TestWaitForStatus(base.TestCase):
         res.status = status
 
         result = resource.wait_for_status(
-            "session", res, status, "failures", "interval", "wait")
+            self.cloud.compute, res, status, "failures", "interval", "wait")
 
         self.assertTrue(result, res)
 
@@ -2136,7 +2141,7 @@ class TestWaitForStatus(base.TestCase):
         res.status = status
 
         result = resource.wait_for_status(
-            "session", res, 'lOling', "failures", "interval", "wait")
+            self.cloud.compute, res, 'lOling', "failures", "interval", "wait")
 
         self.assertTrue(result, res)
 
@@ -2146,7 +2151,7 @@ class TestWaitForStatus(base.TestCase):
         res.mood = status
 
         result = resource.wait_for_status(
-            "session", res, status, "failures", "interval", "wait",
+            self.cloud.compute, res, status, "failures", "interval", "wait",
             attribute='mood')
 
         self.assertTrue(result, res)
@@ -2236,7 +2241,7 @@ class TestWaitForStatus(base.TestCase):
 
         self.assertRaises(exceptions.ResourceTimeout,
                           resource.wait_for_status,
-                          "session", res, status, None, 0.01, 0.1)
+                          self.cloud.compute, res, status, None, 0.01, 0.1)
 
     def test_no_sleep(self):
         res = mock.Mock()
@@ -2245,7 +2250,7 @@ class TestWaitForStatus(base.TestCase):
 
         self.assertRaises(exceptions.ResourceTimeout,
                           resource.wait_for_status,
-                          "session", res, "status", None, 0, -1)
+                          self.cloud.compute, res, "status", None, 0, -1)
 
 
 class TestWaitForDelete(base.TestCase):
@@ -2259,7 +2264,7 @@ class TestWaitForDelete(base.TestCase):
             None, None,
             exceptions.ResourceNotFound('Not Found', response)]
 
-        result = resource.wait_for_delete("session", res, 1, 3)
+        result = resource.wait_for_delete(self.cloud.compute, res, 1, 3)
 
         self.assertEqual(result, res)
 
@@ -2271,7 +2276,7 @@ class TestWaitForDelete(base.TestCase):
         self.assertRaises(
             exceptions.ResourceTimeout,
             resource.wait_for_delete,
-            "session", res, 0.1, 0.3)
+            self.cloud.compute, res, 0.1, 0.3)
 
 
 @mock.patch.object(resource.Resource, '_get_microversion_for', autospec=True)

releasenotes/notes/location-server-resource-af77fdab5d35d421.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Corrected the location property on the ``Server`` resource to
+    use the ``project_id`` from the remote resource rather than the
+    information from the token of the user.