From 88294f20cf19f5538c8ccf9d909109ed7b599888 Mon Sep 17 00:00:00 2001 From: Nisha Agarwal Date: Thu, 27 Jul 2017 22:39:08 +0000 Subject: [PATCH] Redfish: Add local_gb attribute to get_essential_properties This commit adds the local_gb attribute to the get_essential_properties() method. It also adds the utility function max_safe() which doesn't raise ValueError exception if max() fail with that exception. Change-Id: Ie1543e9f5653a025b484726ce278500ee8fdb261 --- proliantutils/ilo/client.py | 1 + proliantutils/redfish/redfish.py | 7 +- .../system/storage/array_controller.py | 8 +- .../resources/system/storage/common.py | 112 ++++++++++++++++++ .../resources/system/storage/logical_drive.py | 7 +- .../system/storage/physical_drive.py | 5 +- .../system/storage/simple_storage.py | 14 +-- .../resources/system/storage/smart_storage.py | 10 +- .../resources/system/storage/storage.py | 12 +- .../resources/system/storage/volume.py | 5 +- .../redfish/resources/system/system.py | 11 +- proliantutils/redfish/utils.py | 14 +++ .../resources/system/storage/test_common.py | 96 +++++++++++++++ proliantutils/tests/redfish/test_redfish.py | 14 ++- proliantutils/tests/redfish/test_utils.py | 7 ++ 15 files changed, 284 insertions(+), 39 deletions(-) create mode 100644 proliantutils/redfish/resources/system/storage/common.py create mode 100644 proliantutils/tests/redfish/resources/system/storage/test_common.py diff --git a/proliantutils/ilo/client.py b/proliantutils/ilo/client.py index 53c247ac..03254564 100644 --- a/proliantutils/ilo/client.py +++ b/proliantutils/ilo/client.py @@ -84,6 +84,7 @@ SUPPORTED_REDFISH_METHODS = [ 'clear_secure_boot_keys', 'get_server_capabilities', 'get_supported_boot_mode', + 'get_essential_properties', ] LOG = log.get_logger(__name__) diff --git a/proliantutils/redfish/redfish.py b/proliantutils/redfish/redfish.py index ce2f80b5..bb8bd49c 100644 --- a/proliantutils/redfish/redfish.py +++ b/proliantutils/redfish/redfish.py @@ -18,6 +18,7 @@ import json from six.moves.urllib import parse import sushy +from sushy.resources.system import mappings as sushy_map from sushy import utils from proliantutils import exception @@ -28,6 +29,8 @@ from proliantutils import log from proliantutils.redfish import main from proliantutils.redfish.resources.manager import constants as mgr_cons from proliantutils.redfish.resources.system import constants as sys_cons +from proliantutils.redfish.resources.system.storage \ + import common as common_storage from proliantutils.redfish import utils as rf_utils """ @@ -828,7 +831,9 @@ class RedfishOperations(operations.IloOperations): # local_gb = sushy_system.storage_summary prop = {'memory_mb': (sushy_system.memory_summary.size_gib * 1024), 'cpus': sushy_system.processors.summary.count, - 'cpu_arch': sushy_system.processors.summary.architecture} + 'cpu_arch': sushy_map.PROCESSOR_ARCH_VALUE_MAP_REV.get( + sushy_system.processors.summary.architecture), + 'local_gb': common_storage.get_local_gb(sushy_system)} return {'properties': prop, 'macs': sushy_system.ethernet_interfaces.summary} except sushy.exceptions.SushyError as e: diff --git a/proliantutils/redfish/resources/system/storage/array_controller.py b/proliantutils/redfish/resources/system/storage/array_controller.py index 1f5f2d17..454e07ab 100644 --- a/proliantutils/redfish/resources/system/storage/array_controller.py +++ b/proliantutils/redfish/resources/system/storage/array_controller.py @@ -79,8 +79,8 @@ class HPEArrayControllerCollection(base.ResourceCollectionBase): """ if self._logical_drives_maximum_size_mib is None: self._logical_drives_maximum_size_mib = ( - max([member.logical_drives.maximum_size_mib - for member in self.get_members()])) + utils.max_safe([member.logical_drives.maximum_size_mib + for member in self.get_members()])) return self._logical_drives_maximum_size_mib @property @@ -91,8 +91,8 @@ class HPEArrayControllerCollection(base.ResourceCollectionBase): """ if self._physical_drives_maximum_size_mib is None: self._physical_drives_maximum_size_mib = ( - max([member.physical_drives.maximum_size_mib - for member in self.get_members()])) + utils.max_safe([member.physical_drives.maximum_size_mib + for member in self.get_members()])) return self._physical_drives_maximum_size_mib def refresh(self): diff --git a/proliantutils/redfish/resources/system/storage/common.py b/proliantutils/redfish/resources/system/storage/common.py new file mode 100644 index 00000000..26aa5f91 --- /dev/null +++ b/proliantutils/redfish/resources/system/storage/common.py @@ -0,0 +1,112 @@ +# Copyright 2017 Hewlett Packard Enterprise Development LP +# +# 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. + +__author__ = 'HPE' + +import sushy + +from proliantutils import exception +from proliantutils import log +from proliantutils.redfish import utils + +LOG = log.get_logger(__name__) + + +def _get_attribute_value_of(resource, attribute_name, default=None): + """Gets the value of attribute_name from the resource + + It catches the exception, if any, while retrieving the + value of attribute_name from resource and returns default. + + :param resource: The resource object + :attribute_name: Property of the resource + :returns the property value if no error encountered + else return 0. + """ + try: + return getattr(resource, attribute_name) + except (sushy.exceptions.SushyError, + exception.MissingAttributeError) as e: + msg = (('The Redfish controller failed to get the ' + 'attribute %(attribute)s from resource %(resource)s. ' + 'Error %(error)s') % {'error': str(e), + 'attribute': attribute_name, + 'resource': + resource.__class__.__name__}) + LOG.debug(msg) + return default + + +def get_local_gb(system_obj): + """Gets the largest volume or the largest disk + + :param system_obj: The HPESystem object. + :returns the size in GB + """ + local_max_bytes = 0 + logical_max_mib = 0 + volume_max_bytes = 0 + physical_max_mib = 0 + drives_max_bytes = 0 + simple_max_bytes = 0 + + # Gets the resources and properties + # its quite possible for a system to lack the resource, hence its + # URI may also be lacking. + + # Check if smart_storage resource exist at the system + smart_resource = _get_attribute_value_of(system_obj, 'smart_storage') + # Check if storage resource exist at the system + storage_resource = _get_attribute_value_of(system_obj, 'storages') + + if smart_resource is not None: + logical_max_mib = _get_attribute_value_of( + smart_resource, 'logical_drives_maximum_size_mib', default=0) + if storage_resource is not None: + volume_max_bytes = _get_attribute_value_of( + storage_resource, 'volumes_maximum_size_bytes', default=0) + + # Get the largest volume from the system. + local_max_bytes = utils.max_safe([(logical_max_mib * 1024 * 1024), + volume_max_bytes]) + # if volume is not found, then traverse through the possible disk drives + # and get the biggest disk. + if local_max_bytes == 0: + if smart_resource is not None: + physical_max_mib = _get_attribute_value_of( + smart_resource, 'physical_drives_maximum_size_mib', default=0) + + if storage_resource is not None: + drives_max_bytes = _get_attribute_value_of( + storage_resource, 'drives_maximum_size_bytes', default=0) + + # Check if the SimpleStorage resource exist at the system. + simple_resource = _get_attribute_value_of(system_obj, + 'simple_storages') + if simple_resource is not None: + simple_max_bytes = _get_attribute_value_of( + simple_resource, 'maximum_size_bytes', default=0) + + local_max_bytes = utils.max_safe([(physical_max_mib * 1024 * 1024), + drives_max_bytes, simple_max_bytes]) + # Convert the received size to GB and reduce the value by 1 Gb as + # ironic requires the local_gb to be returned 1 less than actual size. + local_gb = 0 + if local_max_bytes > 0: + local_gb = int(local_max_bytes / (1024 * 1024 * 1024)) - 1 + else: + msg = ('The maximum size for the hard disk or logical ' + 'volume could not be determined.') + LOG.debug(msg) + return local_gb diff --git a/proliantutils/redfish/resources/system/storage/logical_drive.py b/proliantutils/redfish/resources/system/storage/logical_drive.py index d2b6069e..279c74c1 100644 --- a/proliantutils/redfish/resources/system/storage/logical_drive.py +++ b/proliantutils/redfish/resources/system/storage/logical_drive.py @@ -12,11 +12,9 @@ # License for the specific language governing permissions and limitations # under the License. -import logging - from sushy.resources import base -LOG = logging.getLogger(__name__) +from proliantutils.redfish import utils class HPELogicalDrive(base.ResourceBase): @@ -48,7 +46,8 @@ class HPELogicalDriveCollection(base.ResourceCollectionBase): """ if self._maximum_size_mib is None: self._maximum_size_mib = ( - max([member.capacity_mib for member in self.get_members()])) + utils.max_safe([member.capacity_mib + for member in self.get_members()])) return self._maximum_size_mib def refresh(self): diff --git a/proliantutils/redfish/resources/system/storage/physical_drive.py b/proliantutils/redfish/resources/system/storage/physical_drive.py index 7b743c6d..8b382fbf 100644 --- a/proliantutils/redfish/resources/system/storage/physical_drive.py +++ b/proliantutils/redfish/resources/system/storage/physical_drive.py @@ -16,6 +16,8 @@ import logging from sushy.resources import base +from proliantutils.redfish import utils + LOG = logging.getLogger(__name__) @@ -50,7 +52,8 @@ class HPEPhysicalDriveCollection(base.ResourceCollectionBase): """ if self._maximum_size_mib is None: self._maximum_size_mib = ( - max([member.capacity_mib for member in self.get_members()])) + utils.max_safe([member.capacity_mib + for member in self.get_members()])) return self._maximum_size_mib def refresh(self): diff --git a/proliantutils/redfish/resources/system/storage/simple_storage.py b/proliantutils/redfish/resources/system/storage/simple_storage.py index 5e27c5db..a9db991d 100644 --- a/proliantutils/redfish/resources/system/storage/simple_storage.py +++ b/proliantutils/redfish/resources/system/storage/simple_storage.py @@ -12,12 +12,10 @@ # License for the specific language governing permissions and limitations # under the License. -import logging from sushy.resources import base - -LOG = logging.getLogger(__name__) +from proliantutils.redfish import utils class SimpleStorage(base.ResourceBase): @@ -45,9 +43,9 @@ class SimpleStorage(base.ResourceBase): """ if self._maximum_size_bytes is None: self._maximum_size_bytes = ( - max([device.get('CapacityBytes') - for device in self.devices - if device.get('CapacityBytes') is not None])) + utils.max_safe([device.get('CapacityBytes') + for device in self.devices + if device.get('CapacityBytes') is not None])) return self._maximum_size_bytes def refresh(self): @@ -71,8 +69,8 @@ class SimpleStorageCollection(base.ResourceCollectionBase): """ if self._maximum_size_bytes is None: self._maximum_size_bytes = ( - max([member.maximum_size_bytes - for member in self.get_members()])) + utils.max_safe([member.maximum_size_bytes + for member in self.get_members()])) return self._maximum_size_bytes def refresh(self): diff --git a/proliantutils/redfish/resources/system/storage/smart_storage.py b/proliantutils/redfish/resources/system/storage/smart_storage.py index b21112f5..5818e1c1 100644 --- a/proliantutils/redfish/resources/system/storage/smart_storage.py +++ b/proliantutils/redfish/resources/system/storage/smart_storage.py @@ -56,8 +56,9 @@ class HPESmartStorage(base.ResourceBase): """ if self._logical_drives_maximum_size_mib is None: self._logical_drives_maximum_size_mib = ( - max([member.logical_drives.maximum_size_mib - for member in self.array_controllers.get_members()])) + utils.max_safe( + [member.logical_drives.maximum_size_mib + for member in self.array_controllers.get_members()])) return self._logical_drives_maximum_size_mib @property @@ -68,8 +69,9 @@ class HPESmartStorage(base.ResourceBase): """ if self._physical_drives_maximum_size_mib is None: self._physical_drives_maximum_size_mib = ( - max([member.physical_drives.maximum_size_mib - for member in self.array_controllers.get_members()])) + utils.max_safe( + [member.physical_drives.maximum_size_mib + for member in self.array_controllers.get_members()])) return self._physical_drives_maximum_size_mib def refresh(self): diff --git a/proliantutils/redfish/resources/system/storage/storage.py b/proliantutils/redfish/resources/system/storage/storage.py index 38721d5f..300cbeeb 100644 --- a/proliantutils/redfish/resources/system/storage/storage.py +++ b/proliantutils/redfish/resources/system/storage/storage.py @@ -74,8 +74,8 @@ class Storage(base.ResourceBase): """ if self._drives_maximum_size_bytes is None: self._drives_maximum_size_bytes = ( - max([member.capacity_bytes - for member in self._drives_list()])) + utils.max_safe([member.capacity_bytes + for member in self._drives_list()])) return self._drives_maximum_size_bytes def refresh(self): @@ -101,8 +101,8 @@ class StorageCollection(base.ResourceCollectionBase): """ if self._volumes_maximum_size_bytes is None: self._volumes_maximum_size_bytes = ( - max([member.volumes.maximum_size_bytes - for member in self.get_members()])) + utils.max_safe([member.volumes.maximum_size_bytes + for member in self.get_members()])) return self._volumes_maximum_size_bytes @property @@ -113,8 +113,8 @@ class StorageCollection(base.ResourceCollectionBase): """ if self._drives_maximum_size_bytes is None: self._drives_maximum_size_bytes = ( - max([member.drives_maximum_size_bytes - for member in self.get_members()])) + utils.max_safe([member.drives_maximum_size_bytes + for member in self.get_members()])) return self._drives_maximum_size_bytes def refresh(self): diff --git a/proliantutils/redfish/resources/system/storage/volume.py b/proliantutils/redfish/resources/system/storage/volume.py index 43869660..e33f3f11 100644 --- a/proliantutils/redfish/resources/system/storage/volume.py +++ b/proliantutils/redfish/resources/system/storage/volume.py @@ -15,6 +15,8 @@ from sushy.resources import base +from proliantutils.redfish import utils + class Volume(base.ResourceBase): @@ -44,7 +46,8 @@ class VolumeCollection(base.ResourceCollectionBase): """ if self._maximum_size_bytes is None: self._maximum_size_bytes = ( - max([member.capacity_bytes for member in self.get_members()])) + utils.max_safe([member.capacity_bytes + for member in self.get_members()])) return self._maximum_size_bytes def refresh(self): diff --git a/proliantutils/redfish/resources/system/system.py b/proliantutils/redfish/resources/system/system.py index 8c3ed8e8..e453e731 100644 --- a/proliantutils/redfish/resources/system/system.py +++ b/proliantutils/redfish/resources/system/system.py @@ -79,13 +79,14 @@ class HPESystem(system.System): _bios_settings = None # ref to BIOSSettings instance _secure_boot = None # ref to SecureBoot instance - _smart_storage = None - _storages = None - _pci_devices = None + _smart_storage = None # SmartStorage instance + _simple_storages = None # SimpleStorage instance + _storages = None # Storage instance + _pci_devices = None # PCIDevice instance - _ethernet_interfaces = None + _ethernet_interfaces = None # EthernetInterface instance - _memory = None + _memory = None # Memory instance def _get_hpe_push_power_button_action_element(self): push_action = self._hpe_actions.computer_system_ext_powerbutton diff --git a/proliantutils/redfish/utils.py b/proliantutils/redfish/utils.py index 9f2fbdb5..76205c86 100644 --- a/proliantutils/redfish/utils.py +++ b/proliantutils/redfish/utils.py @@ -108,3 +108,17 @@ def is_operation_allowed(method, resource, subresouce_path): :returns: True if the operation is allowed else False """ return method in get_allowed_operations(resource, subresouce_path) + + +def max_safe(iterable): + """Creates a wrapper over python max() function. + + This function is just a wrapper over pthon max(). + It catches the exceptions and let max() return without any error. + """ + + try: + return max(iterable) + except ValueError: + # The TypeError is not caught here as that should be thrown. + return 0 diff --git a/proliantutils/tests/redfish/resources/system/storage/test_common.py b/proliantutils/tests/redfish/resources/system/storage/test_common.py new file mode 100644 index 00000000..2de67af4 --- /dev/null +++ b/proliantutils/tests/redfish/resources/system/storage/test_common.py @@ -0,0 +1,96 @@ +# Copyright 2017 Hewlett Packard Enterprise Development LP +# +# 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. +__author__ = 'HPE' + +import ddt +import mock +import sushy +import testtools + +from proliantutils import exception +from proliantutils.redfish.resources.system.storage import common + + +@ddt.ddt +class CommonMethodsTestCase(testtools.TestCase): + + def setUp(self): + super(CommonMethodsTestCase, self).setUp() + self.system_obj = mock.MagicMock() + + @ddt.data((953837, 60000, 60000, 60000, 60000, 930), + (sushy.exceptions.SushyError, 1000169537536, 60000, 60000, + 60000, 930), + (953837, sushy.exceptions.SushyError, 60000, 60000, 60000, + 930), + (sushy.exceptions.SushyError, sushy.exceptions.SushyError, + 953837, 60000, 40000, 930), + (sushy.exceptions.SushyError, sushy.exceptions.SushyError, + sushy.exceptions.SushyError, 1000169537536, 40000, 930), + (sushy.exceptions.SushyError, sushy.exceptions.SushyError, + sushy.exceptions.SushyError, sushy.exceptions.SushyError, + 1000169537536, 930), + (sushy.exceptions.SushyError, sushy.exceptions.SushyError, + sushy.exceptions.SushyError, sushy.exceptions.SushyError, + sushy.exceptions.SushyError, 0), + ) + @ddt.unpack + def test_get_local_gb(self, logical_max, volume_max, physical_max, + drive_max, simple_max, expected): + + def _mock_property(value): + if value is sushy.exceptions.SushyError: + mock_value = mock.PropertyMock(side_effect=value) + else: + mock_value = mock.PropertyMock(return_value=value) + return mock_value + system_obj = self.system_obj + type(system_obj.smart_storage).logical_drives_maximum_size_mib = ( + _mock_property(logical_max)) + type(system_obj.storages).volumes_maximum_size_bytes = ( + _mock_property(volume_max)) + type(system_obj.smart_storage).physical_drives_maximum_size_mib = ( + _mock_property(physical_max)) + type(system_obj.storages).drives_maximum_size_bytes = ( + _mock_property(drive_max)) + type(system_obj.simple_storages).maximum_size_bytes = ( + _mock_property(simple_max)) + actual = common.get_local_gb(system_obj) + self.assertEqual(expected, actual) + + def test__get_attribute_value_of(self): + system_obj = self.system_obj + si_mock = mock.PropertyMock(return_value=1000169537536) + type(system_obj.simple_storages).maximum_size_bytes = si_mock + actual = common._get_attribute_value_of(system_obj.simple_storages, + 'maximum_size_bytes') + self.assertEqual(1000169537536, actual) + + def test__get_attribute_value_of_sushy_error(self): + system_obj = self.system_obj + si_mock = mock.PropertyMock(side_effect=sushy.exceptions.SushyError) + type(system_obj.simple_storages).maximum_size_bytes = si_mock + actual = common._get_attribute_value_of(system_obj.simple_storages, + 'maximum_size_bytes', + default=0) + self.assertEqual(0, actual) + + def test__get_attribute_value_of_fail_missing_attribute(self): + system_obj = self.system_obj + si_mock = mock.PropertyMock( + side_effect=exception.MissingAttributeError) + type(system_obj.simple_storages).maximum_size_bytes = si_mock + actual = common._get_attribute_value_of(system_obj.simple_storages, + 'maximum_size_bytes') + self.assertIsNone(actual) diff --git a/proliantutils/tests/redfish/test_redfish.py b/proliantutils/tests/redfish/test_redfish.py index 0dea4aff..4b6990b6 100644 --- a/proliantutils/tests/redfish/test_redfish.py +++ b/proliantutils/tests/redfish/test_redfish.py @@ -35,6 +35,8 @@ from proliantutils.redfish.resources.system import iscsi from proliantutils.redfish.resources.system import memory from proliantutils.redfish.resources.system import pci_device from proliantutils.redfish.resources.system.storage import array_controller +from proliantutils.redfish.resources.system.storage \ + import common as common_storage from proliantutils.redfish.resources.system import system as pro_sys from sushy.resources.system import system @@ -979,8 +981,9 @@ class RedfishOperationsTestCase(testtools.TestCase): 'on the server.', self.rf_client.clear_secure_boot_keys) + @mock.patch.object(common_storage, 'get_local_gb') @mock.patch.object(redfish.RedfishOperations, '_get_sushy_system') - def test_get_essential_properties(self, get_system_mock): + def test_get_essential_properties(self, get_system_mock, local_gb_mock): memory_mock = mock.PropertyMock(return_value=20) type(get_system_mock.return_value.memory_summary).size_gib = ( memory_mock) @@ -992,12 +995,13 @@ class RedfishOperationsTestCase(testtools.TestCase): arch_mock) type(get_system_mock.return_value.ethernet_interfaces).summary = ( {'1': '12:44:6A:3B:04:11'}) - # TODO(nisha): To add after local_gb changes merge. - # type(get_system_mock.return_value).storage_summary = 600 + + local_gb_mock.return_value = 600 actual = self.rf_client.get_essential_properties() expected = {'properties': {'cpus': 40, - 'cpu_arch': 'x86 or x86-64', - 'memory_mb': 20480}, + 'cpu_arch': 'x86', + 'memory_mb': 20480, + 'local_gb': 600}, 'macs': {'1': '12:44:6A:3B:04:11'}} self.assertEqual(expected, actual) diff --git a/proliantutils/tests/redfish/test_utils.py b/proliantutils/tests/redfish/test_utils.py index 4da77928..1b2582cb 100644 --- a/proliantutils/tests/redfish/test_utils.py +++ b/proliantutils/tests/redfish/test_utils.py @@ -104,3 +104,10 @@ class UtilsTestCase(testtools.TestCase): ret_val = utils.is_operation_allowed(method, self.sys_inst, subresource_path) self.assertEqual(ret_val, expected) + + @ddt.data(([2, 4, 6], 6), + ([], 0)) + @ddt.unpack + def test_max_safe(self, iterable, expected): + actual = utils.max_safe(iterable) + self.assertEqual(expected, actual)