From 109b43852745b530a48aa097884354761c27d323 Mon Sep 17 00:00:00 2001 From: Ruby Loo Date: Thu, 29 Sep 2016 10:49:49 -0400 Subject: [PATCH] If no resource, don't call Resource.to_dict() This adds a new method base.Manager._get_as_dict() which checks whether there is a resource first, before calling Resource.to_dict(). Code was modified to call this new method, instead of calling Resource.to_dict() directly. This fixes the AttributeError that occurs if, for example, one tries to get the list of driver passthru methods of the 'fake' driver (via 'ironic driver-get-vendor-passthru-methods fake'). There are no methods, and without this change, an AttributeError exception is raised. With this fix, an empty list is returned. Change-Id: Ib6b691cd39ede9c5902b4df29023fd974b367a7d Closes-Bug: #1626806 --- ironicclient/common/base.py | 14 +++++++++ ironicclient/tests/unit/common/test_base.py | 30 +++++++++++++++++++ ironicclient/v1/driver.py | 5 ++-- ironicclient/v1/node.py | 11 +++---- ...ource-attributeerror-d0cb327abab7dcc0.yaml | 8 +++++ 5 files changed, 58 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/no-resource-attributeerror-d0cb327abab7dcc0.yaml diff --git a/ironicclient/common/base.py b/ironicclient/common/base.py index 61050c7ea..08727525a 100644 --- a/ironicclient/common/base.py +++ b/ironicclient/common/base.py @@ -83,6 +83,20 @@ class Manager(object): except IndexError: return None + def _get_as_dict(self, resource_id, fields=None): + """Retrieve a resource as a dictionary + + :param resource_id: Identifier of the resource. + :param fields: List of specific fields to be returned. + :returns: a dictionary representing the resource; may be empty + """ + + resource = self._get(resource_id, fields=fields) + if resource: + return resource.to_dict() + else: + return {} + def _format_body_data(self, body, response_key): if response_key: try: diff --git a/ironicclient/tests/unit/common/test_base.py b/ironicclient/tests/unit/common/test_base.py index 927f71216..ed25d5345 100644 --- a/ironicclient/tests/unit/common/test_base.py +++ b/ironicclient/tests/unit/common/test_base.py @@ -15,6 +15,7 @@ import copy +import mock import testtools from ironicclient.common import base @@ -121,6 +122,35 @@ class ManagerTestCase(testtools.TestCase): self.manager.create, **INVALID_ATTRIBUTE_TESTABLE_RESOURCE) + def test__get(self): + resource_id = TESTABLE_RESOURCE['uuid'] + resource = self.manager._get(resource_id) + expect = [ + ('GET', '/v1/testableresources/%s' % resource_id, + {}, None), + ] + self.assertEqual(expect, self.api.calls) + self.assertEqual(resource_id, resource.uuid) + self.assertEqual(TESTABLE_RESOURCE['attribute1'], resource.attribute1) + + def test__get_as_dict(self): + resource_id = TESTABLE_RESOURCE['uuid'] + resource = self.manager._get_as_dict(resource_id) + expect = [ + ('GET', '/v1/testableresources/%s' % resource_id, + {}, None), + ] + self.assertEqual(expect, self.api.calls) + self.assertEqual(TESTABLE_RESOURCE, resource) + + @mock.patch.object(base.Manager, '_get', autospec=True) + def test__get_as_dict_empty(self, mock_get): + mock_get.return_value = None + resource_id = TESTABLE_RESOURCE['uuid'] + resource = self.manager._get_as_dict(resource_id) + mock_get.assert_called_once_with(mock.ANY, resource_id, fields=None) + self.assertEqual({}, resource) + def test_get(self): resource = self.manager.get(TESTABLE_RESOURCE['uuid']) expect = [ diff --git a/ironicclient/v1/driver.py b/ironicclient/v1/driver.py index 5cb94b36c..0e93495a6 100644 --- a/ironicclient/v1/driver.py +++ b/ironicclient/v1/driver.py @@ -41,7 +41,7 @@ class DriverManager(base.Manager): return self._delete(resource_id=driver_name) def properties(self, driver_name): - return self._get(resource_id='%s/properties' % driver_name).to_dict() + return self._get_as_dict('%s/properties' % driver_name) def raid_logical_disk_properties(self, driver_name): """Returns the RAID logical disk properties for the driver. @@ -92,5 +92,4 @@ class DriverManager(base.Manager): _('Unknown HTTP method: %s') % http_method) def get_vendor_passthru_methods(self, driver_name): - path = "%s/vendor_passthru/methods" % driver_name - return self.get(path).to_dict() + return self._get_as_dict("%s/vendor_passthru/methods" % driver_name) diff --git a/ironicclient/v1/node.py b/ironicclient/v1/node.py index 9327308a5..10354694e 100644 --- a/ironicclient/v1/node.py +++ b/ironicclient/v1/node.py @@ -334,10 +334,7 @@ class NodeManager(base.CreateManager): def get_console(self, node_uuid): path = "%s/states/console" % node_uuid - info = self.get(path) - if not info: - return {} - return info.to_dict() + return self._get_as_dict(path) def set_console_mode(self, node_uuid, enabled): """Set the console mode for the node. @@ -358,15 +355,15 @@ class NodeManager(base.CreateManager): def get_boot_device(self, node_uuid): path = "%s/management/boot_device" % node_uuid - return self.get(path).to_dict() + return self._get_as_dict(path) def get_supported_boot_devices(self, node_uuid): path = "%s/management/boot_device/supported" % node_uuid - return self.get(path).to_dict() + return self._get_as_dict(path) def get_vendor_passthru_methods(self, node_ident): path = "%s/vendor_passthru/methods" % node_ident - return self.get(path).to_dict() + return self._get_as_dict(path) def wait_for_provision_state(self, node_ident, expected_state, timeout=0, diff --git a/releasenotes/notes/no-resource-attributeerror-d0cb327abab7dcc0.yaml b/releasenotes/notes/no-resource-attributeerror-d0cb327abab7dcc0.yaml new file mode 100644 index 000000000..9e4e5905e --- /dev/null +++ b/releasenotes/notes/no-resource-attributeerror-d0cb327abab7dcc0.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - For node resources that had no boot devices, no supported boot devices, or + no passthru methods (and driver resources with no properties or no passthru + methods), issuing a request to get that information (for example, + 'ironic driver-get-vendor-passthru-methods fake') would result in the + error "'NoneType' has no attribute 'to_dict'". This is fixed; an empty + list is now returned.