commit ed53860f36e5b16cce9b307186a155095812a091 Author: Bob Fournier Date: Tue Sep 29 11:38:30 2020 -0400 Log only fields set in redfish response, not entire json Currently the entire json response received via redfish is being logged. This fills up the logs and can result in log messages being truncated, hiding useful data. Instead only log the attributes that have been set from the response. Change-Id: I88a426acf5b000ace0041ce43e91b376bdb7b1c7 Story: 2008177 Task: 40933 diff --git a/releasenotes/notes/redfish-response-log-294f3f10b770e356.yaml b/releasenotes/notes/redfish-response-log-294f3f10b770e356.yaml new file mode 100644 index 0000000..12604f0 --- /dev/null +++ b/releasenotes/notes/redfish-response-log-294f3f10b770e356.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Reduce the logging from sushy by logging only attributes and values + set in the redfish response, not the entire json. diff --git a/sushy/resources/base.py b/sushy/resources/base.py index 574ec52..8628b56 100644 --- a/sushy/resources/base.py +++ b/sushy/resources/base.py @@ -214,6 +214,9 @@ class ListField(Field): return instances + def __getitem__(self, key): + return getattr(self, key) + class DictionaryField(Field): """Base class for fields consisting of dictionary of several sub-fields.""" @@ -247,6 +250,9 @@ class DictionaryField(Field): return instances + def __getitem__(self, key): + return getattr(self, key) + class MappedField(Field): """Field taking real value from a mapping.""" @@ -514,17 +520,49 @@ class ResourceBase(object, metaclass=abc.ABCMeta): self.refresh(json_doc=json_doc) + def _get_value(self, val): + """Iterate through the input to get values for all attributes + + :param val: Either a value or a resource + :returns: Attribute value, which may be a dictionary + """ + if isinstance(val, dict): + subfields = {} + for key, s_val in val.items(): + subfields[key] = self._get_value(s_val) + return subfields + + elif isinstance(val, list): + return [self._get_value(val[i]) for i in range(len(val))] + + elif (isinstance(val, DictionaryField) + or isinstance(val, CompositeField) + or isinstance(val, ListField)): + subfields = {} + for attr, field in val._subfields.items(): + subfields[attr] = self._get_value(val.__getitem__(attr)) + return subfields + + return val + def _parse_attributes(self, json_doc): """Parse the attributes of a resource. Parsed JSON fields are set to `self` as declared in the class. :param json_doc: parsed JSON document in form of Python types + :returns: dictionary of attribute/values after parsing """ + settings = {} for attr, field in _collect_fields(self): # Hide the Field object behind the real value setattr(self, attr, field._load(json_doc, self)) + # Get the attribute/value pairs that have been parsed + settings[attr] = self._get_value(getattr(self, attr)) + + return settings + def refresh(self, force=True, json_doc=None): """Refresh the resource @@ -553,10 +591,10 @@ class ResourceBase(object, metaclass=abc.ABCMeta): else: self._json = self._reader.get_data().json_doc + attributes = self._parse_attributes(self._json) LOG.debug('Received representation of %(type)s %(path)s: %(json)s', {'type': self.__class__.__name__, - 'path': self._path, 'json': self._json}) - self._parse_attributes(self._json) + 'path': self._path, 'json': attributes}) self._do_refresh(force) # Mark it fresh diff --git a/sushy/tests/unit/resources/chassis/test_chassis.py b/sushy/tests/unit/resources/chassis/test_chassis.py index b4436f5..7bdbc13 100644 --- a/sushy/tests/unit/resources/chassis/test_chassis.py +++ b/sushy/tests/unit/resources/chassis/test_chassis.py @@ -74,6 +74,22 @@ class ChassisTestCase(base.TestCase): self.chassis.physical_security.intrusion_sensor_re_arm ) + def test__parse_attributes_return(self): + attributes = self.chassis._parse_attributes(self.json_doc) + + # Test that various types are returned correctly + self.assertEqual('Blade', attributes.get('name')) + self.assertEqual(sushy.INDICATOR_LED_OFF, + attributes.get('indicator_led')) + self.assertEqual(sushy.POWER_STATE_ON, attributes.get('power_state')) + self.assertEqual({'intrusion_sensor': + sushy.CHASSIS_INTRUSION_SENSOR_NORMAL, + 'intrusion_sensor_number': + 123, + 'intrusion_sensor_re_arm': + 'manual re arm chassis intrusion sensor'}, + attributes.get('physical_security')) + def test_get_allowed_reset_chasis_values(self): # | GIVEN | expected = {sushy.RESET_TYPE_POWER_CYCLE, diff --git a/sushy/tests/unit/resources/chassis/test_power.py b/sushy/tests/unit/resources/chassis/test_power.py index 9438606..ffe5727 100644 --- a/sushy/tests/unit/resources/chassis/test_power.py +++ b/sushy/tests/unit/resources/chassis/test_power.py @@ -135,3 +135,56 @@ class PowerTestCase(base.TestCase): self.power.power_supplies[1].part_number) self.assertEqual('425-591-654', self.power.power_supplies[1].spare_part_number) + + def test__parse_attributes_return(self): + attributes = self.power._parse_attributes(self.json_doc) + + # Test that various types are returned correctly + self.assertEqual('Quad Blade Chassis Power', attributes.get('name')) + self.assertEqual([{'firmware_version': '2.20', + 'identity': '0', + 'indicator_led': None, + 'input_ranges': + [{'input_type': 'ac', + 'maximum_frequency_hz': 63, + 'maximum_voltage': 250, + 'minimum_frequency_hz': 47, + 'minimum_voltage': 185, + 'output_wattage': 1450}], + 'last_power_output_watts': 650, + 'line_input_voltage': 220, + 'line_input_voltage_type': 'ac240v', + 'manufacturer': 'Cyberdyne', + 'model': '325457-A06', + 'name': 'Power Supply 0', + 'part_number': '425-591-654', + 'power_capacity_watts': 1450, + 'power_supply_type': 'ac', + 'serial_number': '1S0000523', + 'spare_part_number': '425-591-654', + 'status': {'health': 'ok', 'health_rollup': None, + 'state': 'enabled'}}, + {'firmware_version': '2.20', + 'identity': '1', + 'indicator_led': None, + 'input_ranges': + [{'input_type': 'ac', + 'maximum_frequency_hz': 63, + 'maximum_voltage': 250, + 'minimum_frequency_hz': 47, + 'minimum_voltage': 185, + 'output_wattage': 1450}], + 'last_power_output_watts': 635, + 'line_input_voltage': 222, + 'line_input_voltage_type': 'ac240v', + 'manufacturer': 'Cyberdyne', + 'model': '325457-A06', + 'name': 'Power Supply 1', + 'part_number': '425-591-654', + 'power_capacity_watts': 1450, + 'power_supply_type': 'ac', + 'serial_number': '1S0000524', + 'spare_part_number': '425-591-654', + 'status': {'health': 'ok', 'health_rollup': None, + 'state': 'enabled'}}], + attributes.get('power_supplies')) diff --git a/sushy/tests/unit/resources/chassis/test_thermal.py b/sushy/tests/unit/resources/chassis/test_thermal.py index dc4cdc0..830e6cc 100644 --- a/sushy/tests/unit/resources/chassis/test_thermal.py +++ b/sushy/tests/unit/resources/chassis/test_thermal.py @@ -73,3 +73,48 @@ class ThermalTestCase(base.TestCase): self.assertEqual(120, self.thermal.temperatures[0].max_reading_range_temp) self.assertEqual('CPU', self.thermal.temperatures[0].physical_context) + + def test__parse_attributes_return(self): + attributes = self.thermal._parse_attributes(self.json_doc) + + # Test that various types are returned correctly + self.assertEqual([{'identity': '0', + 'indicator_led': None, + 'lower_threshold_critical': None, + 'lower_threshold_fatal': 2000, + 'lower_threshold_non_critical': None, + 'manufacturer': None, + 'max_reading_range': 10000, + 'min_reading_range': 0, + 'model': None, + 'name': 'CPU Fan', + 'part_number': None, + 'physical_context': 'CPU', + 'reading': 6000, + 'reading_units': 'RPM', + 'serial_number': None, + 'status': + {'health': 'ok', 'health_rollup': None, + 'state': 'enabled'}, + 'upper_threshold_critical': None, + 'upper_threshold_fatal': None, + 'upper_threshold_non_critical': None}], + attributes.get('fans')) + self.assertEqual([{'identity': '0', + 'lower_threshold_critical': None, + 'lower_threshold_fatal': None, + 'lower_threshold_non_critical': None, + 'max_allowable_operating_value': None, + 'max_reading_range_temp': 120, + 'min_allowable_operating_value': None, + 'min_reading_range_temp': 0, + 'name': 'CPU Temp', + 'physical_context': 'CPU', + 'reading_celsius': 62, + 'sensor_number': None, + 'status': {'health': 'ok', 'health_rollup': None, + 'state': 'enabled'}, + 'upper_threshold_critical': 90, + 'upper_threshold_fatal': 95, + 'upper_threshold_non_critical': 75}], + attributes.get('temperatures')) diff --git a/sushy/tests/unit/resources/manager/test_virtual_media.py b/sushy/tests/unit/resources/manager/test_virtual_media.py index 284a0e2..6bbb8d2 100644 --- a/sushy/tests/unit/resources/manager/test_virtual_media.py +++ b/sushy/tests/unit/resources/manager/test_virtual_media.py @@ -38,7 +38,7 @@ class VirtualMediaTestCase(base.TestCase): self.conn, '/redfish/v1/Managers/BMC/VirtualMedia/Floppy1', redfish_version='1.0.2') - def test__parse_atrtributes(self): + def test__parse_attributes(self): self.sys_virtual_media._parse_attributes(self.json_doc) self.assertEqual('Virtual Removable Media', self.sys_virtual_media.name) @@ -55,6 +55,18 @@ class VirtualMediaTestCase(base.TestCase): self.assertEqual(True, self.sys_virtual_media.inserted) self.assertEqual(False, self.sys_virtual_media.write_protected) + def test__parse_attributes_return(self): + attributes = self.sys_virtual_media._parse_attributes(self.json_doc) + + # Test that various types are returned correctly + self.assertEqual('https://www.dmtf.org/freeImages/Sardine.img', + attributes.get('image')) + self.assertEqual(sushy.CONNECTED_VIA_URI, + attributes.get('connected_via')) + self.assertEqual([sushy.VIRTUAL_MEDIA_FLOPPY, + sushy.VIRTUAL_MEDIA_USBSTICK], + attributes.get('media_types')) + def test_insert_media_none(self): self.sys_virtual_media._actions.insert_media = None self.assertRaisesRegex( diff --git a/sushy/tests/unit/resources/registry/test_message_registry.py b/sushy/tests/unit/resources/registry/test_message_registry.py index 5b4360a..fea7e7c 100644 --- a/sushy/tests/unit/resources/registry/test_message_registry.py +++ b/sushy/tests/unit/resources/registry/test_message_registry.py @@ -74,6 +74,39 @@ class MessageRegistryTestCase(base.TestCase): self.assertEqual( 'Try Later', self.registry.messages['MissingThings'].resolution) + def test__parse_attributes_return(self): + attributes = self.registry._parse_attributes(self.json_doc) + + self.assertEqual({'Failed': + {'description': 'Nothing is OK', + 'message': 'The property %1 broke everything.', + 'number_of_args': 1, + 'param_types': ['string'], + 'resolution': 'Panic', + 'severity': 'critical'}, + 'MissingThings': + {'description': '', + 'message': + "Property's %1 value cannot be less than %2.", + 'number_of_args': 2, + 'param_types': ['string', 'number'], + 'resolution': 'Try Later', + 'severity': 'warning'}, + 'Success': + {'description': 'Everything OK', + 'message': 'Everything done successfully.', + 'number_of_args': 0, 'param_types': None, + 'resolution': 'None', 'severity': 'ok'}, + 'TooBig': + {'description': 'Value too big', + 'message': + "Property's %1 value cannot be greater than %2.", + 'number_of_args': 2, + 'param_types': ['string', 'number'], + 'resolution': 'Try again', + 'severity': 'warning'}}, + attributes.get('messages')) + def test__parse_attributes_missing_msg_desc(self): self.json_doc['Messages']['Success'].pop('Description') self.registry._parse_attributes(self.json_doc) diff --git a/sushy/tests/unit/resources/registry/test_message_registry_file.py b/sushy/tests/unit/resources/registry/test_message_registry_file.py index 1afa407..2dc858f 100644 --- a/sushy/tests/unit/resources/registry/test_message_registry_file.py +++ b/sushy/tests/unit/resources/registry/test_message_registry_file.py @@ -54,6 +54,15 @@ class MessageRegistryFileTestCase(base.TestCase): self.assertEqual('Test.1.0.json', self.reg_file.location[0].archive_file) + def test__parse_attributes_return(self): + attributes = self.reg_file._parse_attributes(self.json_doc) + + # Test that various types are returned correctly + self.assertEqual('Test Message Registry File', attributes.get('name')) + self.assertEqual('Test', attributes.get('identity')) + self.assertEqual(['en'], attributes.get('languages')) + self.assertEqual('Test.1.0', attributes.get('registry')) + @mock.patch('sushy.resources.registry.message_registry.MessageRegistry', autospec=True) @mock.patch('sushy.resources.base.JsonDataReader', autospec=True) diff --git a/sushy/tests/unit/resources/system/test_bios.py b/sushy/tests/unit/resources/system/test_bios.py index da26f73..eb8e18d 100644 --- a/sushy/tests/unit/resources/system/test_bios.py +++ b/sushy/tests/unit/resources/system/test_bios.py @@ -83,6 +83,28 @@ class BiosTestCase(base.TestCase): self.assertEqual(settings.UPDATE_FAILURE, self.sys_bios.update_status.status) + def test__parse_attributes_return(self): + attributes = self.sys_bios._parse_attributes(self.bios_json) + + # Test that various types are returned correctly + self.assertEqual('BIOS Configuration Current Settings', + attributes.get('name')) + self.assertEqual({'AdminPhone': '', + 'BootMode': 'Uefi', + 'EmbeddedSata': 'Raid', + 'NicBoot1': 'NetworkBoot', + 'NicBoot2': 'Disabled', + 'PowerProfile': 'MaxPerf', + 'ProcCoreDisable': 0, + 'ProcHyperthreading': 'Enabled', + 'ProcTurboMode': 'Enabled', + 'UsbControl': 'UsbEnabled'}, + attributes.get('attributes')) + self.assertEqual({'maintenance_window_duration_in_seconds': 600, + 'maintenance_window_start_time': + parser.parse('2020-09-01T04:30:00-06:00')}, + attributes.get('maintenance_window')) + def test_set_attribute(self): self.sys_bios.set_attribute('ProcTurboMode', 'Disabled') self.sys_bios._conn.patch.assert_called_once_with( diff --git a/sushy/tests/unit/resources/system/test_system.py b/sushy/tests/unit/resources/system/test_system.py index fbaf70e..acb21e8 100644 --- a/sushy/tests/unit/resources/system/test_system.py +++ b/sushy/tests/unit/resources/system/test_system.py @@ -82,6 +82,39 @@ class SystemTestCase(base.TestCase): for oem_vendor in self.sys_inst.oem_vendors: self.assertIn(oem_vendor, ('Contoso', 'Chipwise')) + def test__parse_attributes_return(self): + attributes = self.sys_inst._parse_attributes(self.json_doc) + + # Test that various types are returned correctly + self.assertEqual('Chicago-45Z-2381', attributes.get('asset_tag')) + self.assertEqual(sushy.INDICATOR_LED_OFF, + attributes.get('indicator_led')) + self.assertEqual({'health': res_cons.HEALTH_OK, + 'health_rollup': res_cons.HEALTH_OK, + 'state': res_cons.STATE_ENABLED}, + attributes.get('status')) + self.assertEqual({'maintenance_window_duration_in_seconds': 1, + 'maintenance_window_start_time': + parser.parse('2016-03-07T14:44:30-05:05')}, + attributes.get('maintenance_window')) + self.assertEqual({'reset': {'allowed_values': + ['On', 'ForceOff', 'GracefulShutdown', + 'GracefulRestart', 'ForceRestart', 'Nmi', + 'ForceOn', 'PushPowerButton'], + 'operation_apply_time_support': + {'_maintenance_window_resource': + {'resource_uri': + '/redfish/v1/Systems/437XR1138R2'}, + 'maintenance_window_duration_in_seconds': 600, + 'maintenance_window_start_time': + parser.parse('2017-05-03T23:12:37-05:00'), + 'supported_values': + ['Immediate', 'AtMaintenanceWindowStart']}, + 'target_uri': + '/redfish/v1/Systems/437XR1138R2/Actions/' + 'ComputerSystem.Reset'}}, + attributes.get('_actions')) + def test__parse_attributes_missing_actions(self): self.sys_inst.json.pop('Actions') self.assertRaisesRegex( diff --git a/sushy/tests/unit/resources/updateservice/test_softwareinventory.py b/sushy/tests/unit/resources/updateservice/test_softwareinventory.py index 1601579..b365fb4 100644 --- a/sushy/tests/unit/resources/updateservice/test_softwareinventory.py +++ b/sushy/tests/unit/resources/updateservice/test_softwareinventory.py @@ -55,6 +55,17 @@ class SoftwareInventoryTestCase(base.TestCase): self.assertTrue(self.soft_inv.updateable) self.assertEqual('1.45.455b66-rev4', self.soft_inv.version) + def test__parse_attributes_return(self): + attributes = self.soft_inv._parse_attributes(self.json_doc) + + # Test that various types are returned correctly + self.assertEqual('BMC', attributes.get('identity')) + self.assertEqual({'health': res_cons.HEALTH_OK, + 'health_rollup': None, + 'state': res_cons.STATE_ENABLED}, + attributes.get('status')) + self.assertEqual(True, attributes.get('updateable')) + def test__parse_attributes_missing_identity(self): self.soft_inv.json.pop('Id') self.assertRaisesRegex(