From 5fbd785e91ea6424713560f7dbdced8238cd1fb1 Mon Sep 17 00:00:00 2001 From: Nisha Agarwal Date: Thu, 28 Feb 2019 01:37:15 -0800 Subject: [PATCH] Revert "Add NIC inspection for Gen9" This reverts commit 7286d7019c53ff55431690e07df02029acd171b2. The fix is reverted because iLO4 behaviour is not consistent in reporting the NIC data even for MCTP compliant NIC cards. And iLO4 does not has the intelligence to detect the NIC health status out-of-band for non-MCTP compliant NIC cards. It reports the correct status only when AMS is running on the server which requires OS to be present onto the system. Change-Id: I02c27d820b27bd532f23ab7ef276b39f868f5205 --- proliantutils/ilo/client.py | 8 - proliantutils/ilo/ribcl.py | 6 +- proliantutils/ilo/ris.py | 67 ------- proliantutils/tests/ilo/ris_sample_outputs.py | 177 ------------------ proliantutils/tests/ilo/test_client.py | 60 +----- proliantutils/tests/ilo/test_ribcl.py | 24 +-- proliantutils/tests/ilo/test_ris.py | 91 --------- 7 files changed, 7 insertions(+), 426 deletions(-) diff --git a/proliantutils/ilo/client.py b/proliantutils/ilo/client.py index 23995631..6adaf87f 100644 --- a/proliantutils/ilo/client.py +++ b/proliantutils/ilo/client.py @@ -618,14 +618,6 @@ class IloClient(operations.IloOperations): "RIBCL/Redfish failed to get the disk size. " "Returning local_gb as 0.") LOG.debug(msg) - # TODO(nisha): The get_essential_properties() is not implemented - # in ris.py so far. Since the active MACs cannot be retreived using - # RIBCL, the logic is implemented in ris.py for Gen9s.Eventually - # the whole get_essential_properties() need to be implemented - # in ris.py. - if ('Gen9' in self.model): - macs = self.ris.get_active_macs() - data['macs'] = macs return data def get_server_capabilities(self): diff --git a/proliantutils/ilo/ribcl.py b/proliantutils/ilo/ribcl.py index ab650e6e..6e675458 100644 --- a/proliantutils/ilo/ribcl.py +++ b/proliantutils/ilo/ribcl.py @@ -802,11 +802,7 @@ class RIBCLOperations(operations.IloOperations): properties['cpus'] = cpus properties['cpu_arch'] = cpu_arch properties['local_gb'] = self._parse_storage_embedded_health(data) - # TODO(nisha): Remove this check once get_essential_properties() is - # completely implemented in ris.py for Gen9's. - macs = {} - if 'Gen8' in self.get_product_name(): - macs = self._parse_nics_embedded_health(data) + macs = self._parse_nics_embedded_health(data) return_value = {'properties': properties, 'macs': macs} return return_value diff --git a/proliantutils/ilo/ris.py b/proliantutils/ilo/ris.py index e8b595a6..31f5d582 100755 --- a/proliantutils/ilo/ris.py +++ b/proliantutils/ilo/ris.py @@ -2043,70 +2043,3 @@ class RISOperations(rest.RestConnectorBase, operations.IloOperations): settings_result = bios_settings.get("SettingsResult").get("Messages") status = "failed" if len(settings_result) > 1 else "success" return {"status": status, "results": settings_result} - - def _get_network_adapters(self): - """Gets the network adapters list. - - :raises: IloError, on an error from iLO. - :raises: IloCommandNotSupportedError, if the command is - not supported on the server. - :returns: the list of Network Adapters attached to the system. - """ - system = self._get_host_details() - if ('links' in system['Oem']['Hp'] and - 'NetworkAdapters' in system['Oem']['Hp']['links']): - # Get the NetworkAdapters URI and list - uri = system['Oem']['Hp']['links']['NetworkAdapters']['href'] - status, headers, net_details = self._rest_get(uri) - - if status >= 300: - msg = self._get_extended_error(net_details) - raise exception.IloError(msg) - return net_details - - else: - msg = ('"links/NetworkAdapters" section in ComputerSystem/Oem/Hp' - ' does not exist') - raise exception.IloCommandNotSupportedError(msg) - - def get_active_macs(self): - """Gets the MACs which are physically connected. - - :raises: IloError, on an error from iLO. - :raises: IloCommandNotSupportedError, if the command is - not supported on the server. - :returns: the dictionary of active MAC addresses like - {'NIC.LOM.1.1': '50:65:F3:6C:47:F8'} - """ - net_details = self._get_network_adapters() - active_macs = {} - if ('links' in net_details and - 'Member' in net_details['links']): - for member in net_details['links']['Member']: - net_uri = member['href'] - status, headers, mac_details = self._rest_get(net_uri) - if status >= 300: - msg = self._get_extended_error(mac_details) - raise exception.IloError(msg) - - # In RIBCL and Redfish we are able to get the port - # numbers from the data received but it is not available - # as part of RIS output. Hence choosing the nearest - # and unique key as "StructuredName" from RIS output - # which returns value as 'NIC.LOM.1.1', 'NIC.LOM.1.2', etc. - # The last digit is actually the port number. - for macs in mac_details['PhysicalPorts']: - if (macs['Status']['Health'] == 'OK' and - macs['Status']['State'] == 'Enabled'): - mac_dict = {macs['Oem']['Hp']['StructuredName']: - macs['MacAddress']} - active_macs.update(mac_dict) - else: - msg = ('"links" or "links/Member" section in NetworkAdapters' - ' does not exist') - raise exception.IloCommandNotSupportedError(msg) - - if (len(active_macs) == 0): - LOG.warning(self._("Node doesn't have any NIC physically " - "connected.")) - return active_macs diff --git a/proliantutils/tests/ilo/ris_sample_outputs.py b/proliantutils/tests/ilo/ris_sample_outputs.py index 4efbadbc..539bbb0b 100755 --- a/proliantutils/tests/ilo/ris_sample_outputs.py +++ b/proliantutils/tests/ilo/ris_sample_outputs.py @@ -169,9 +169,6 @@ RESPONSE_BODY_FOR_REST_OP = """ "MEMORY": { "href": "/rest/v1/Systems/1/Memory" }, - "NetworkAdapters": { - "href": "/rest/v1/Systems/1/NetworkAdapters" - }, "PCIDevices": { "href": "/rest/v1/Systems/1/PCIDevices" }, @@ -4846,177 +4843,3 @@ ArrayControllers" } } """ - -NO_MACS_CONNECTED = """ -{ - "@odata.context": "/redfish/v1/$metadata#Systems/Members/1/\ -NetworkAdapters/Members/$entity", - "@odata.id": "/redfish/v1/Systems/1/NetworkAdapters/1/", - "@odata.type": "#BaseNetworkAdapter.1.1.0.BaseNetworkAdapter", - "Firmware": { - "Current": { - "VersionString": null - } - }, - "Id": "1", - "Name": "HP Ethernet 1Gb 2-port 361i Adapter - NIC", - "PartNumber": null, - "PhysicalPorts": [{ - "FullDuplex": false, - "IPv4Addresses": [{ - "Address": null - }], - "IPv6Addresses": [{ - "Address": null - }], - "MacAddress": "50:65:F3:6C:47:F8", - "Name": null, - "Oem": { - "Hp": { - "@odata.type": "#HpBaseNetworkAdapterExt.1.0.0.\ -HpBaseNetworkAdapterExt", - "BadReceives": null, - "BadTransmits": null, - "GoodReceives": null, - "GoodTransmits": null, - "StructuredName": "NIC.LOM.1.1", - "Team": null, - "Type": "HpBaseNetworkAdapterExt.1.0.0" - } - }, - "SpeedMbps": 0, - "Status": { - "Health": "Warning", - "State": "Disabled" - }, - "UEFIDevicePath": "PciRoot(0x0)/Pci(0x2,0x3)/Pci(0x0,0x0)" - }], - "SerialNumber": null, - "Status": { - "Health": "Warning", - "State": "Disabled" - }, - "StructuredName": "NIC.LOM.1.1", - "Type": "BaseNetworkAdapter.1.1.0", - "UEFIDevicePath": "PciRoot(0x0)/Pci(0x2,0x3)/Pci(0x0,0x0)", - "links": { - "self": { - "href": "/rest/v1/Systems/1/NetworkAdapters/1" - } - } -} -""" - -ONE_MAC_CONNECTED = """ -{ - "@odata.context": "/redfish/v1/$metadata#Systems/Members/1/\ -NetworkAdapters/Members/$entity", - "@odata.id": "/redfish/v1/Systems/1/NetworkAdapters/1/", - "@odata.type": "#BaseNetworkAdapter.1.1.0.BaseNetworkAdapter", - "Firmware": { - "Current": { - "VersionString": null - } - }, - "Id": "1", - "Name": "HP Ethernet 1Gb 2-port 361i Adapter - NIC", - "PartNumber": null, - "PhysicalPorts": [{ - "FullDuplex": false, - "IPv4Addresses": [{ - "Address": null - }], - "IPv6Addresses": [{ - "Address": null - }], - "MacAddress": "50:65:F3:6C:47:F8", - "Name": null, - "Oem": { - "Hp": { - "@odata.type": "#HpBaseNetworkAdapterExt.1.0.0.\ -HpBaseNetworkAdapterExt", - "BadReceives": null, - "BadTransmits": null, - "GoodReceives": null, - "GoodTransmits": null, - "StructuredName": "NIC.LOM.1.1", - "Team": null, - "Type": "HpBaseNetworkAdapterExt.1.0.0" - } - }, - "SpeedMbps": 0, - "Status": { - "Health": "OK", - "State": "Enabled" - }, - "UEFIDevicePath": "PciRoot(0x0)/Pci(0x2,0x3)/Pci(0x0,0x0)" - }, { - "FullDuplex": false, - "IPv4Addresses": [{ - "Address": null - }], - "IPv6Addresses": [{ - "Address": null - }], - "MacAddress": "50:65:F3:6C:47:F9", - "Name": null, - "Oem": { - "Hp": { - "@odata.type": "#HpBaseNetworkAdapterExt.1.0.0.\ -HpBaseNetworkAdapterExt", - "BadReceives": null, - "BadTransmits": null, - "GoodReceives": null, - "GoodTransmits": null, - "StructuredName": "NIC.LOM.1.2", - "Team": null, - "Type": "HpBaseNetworkAdapterExt.1.0.0" - } - }, - "SpeedMbps": 0, - "Status": { - "Health": "Warning", - "State": "Disabled" - }, - "UEFIDevicePath": "PciRoot(0x0)/Pci(0x2,0x3)/Pci(0x0,0x1)" - }], - "SerialNumber": null, - "Status": { - "Health": "OK", - "State": "Enabled" - }, - "StructuredName": "NIC.LOM.1.1", - "Type": "BaseNetworkAdapter.1.1.0", - "UEFIDevicePath": "PciRoot(0x0)/Pci(0x2,0x3)/Pci(0x0,0x0)", - "links": { - "self": { - "href": "/rest/v1/Systems/1/NetworkAdapters/1" - } - } -} -""" - -NETWORK_ADAPTER_LIST = """ -{ - "@odata.context": "/redfish/v1/$metadata#Systems/Members/1/NetworkAdapters", - "@odata.id": "/redfish/v1/Systems/1/NetworkAdapters/", - "@odata.type": "#BaseNetworkAdapterCollection.BaseNetworkAdapterCollection", - "Description": "NetworkAdapters view", - "MemberType": "BaseNetworkAdapter.1", - "Members": [{ - "@odata.id": "/redfish/v1/Systems/1/NetworkAdapters/1/" - }], - "Members@odata.count": 1, - "Name": "NetworkAdapters Collection", - "Total": 1, - "Type": "Collection.1.0.0", - "links": { - "Member": [{ - "href": "/rest/v1/Systems/1/NetworkAdapters/1" - }], - "self": { - "href": "/rest/v1/Systems/1/NetworkAdapters" - } - } -} -""" diff --git a/proliantutils/tests/ilo/test_client.py b/proliantutils/tests/ilo/test_client.py index 0150cf48..ab5de7d2 100644 --- a/proliantutils/tests/ilo/test_client.py +++ b/proliantutils/tests/ilo/test_client.py @@ -1086,12 +1086,11 @@ class IloClientTestCase(testtools.TestCase): self.client.set_bios_settings(d, apply_filter) bios_settings_mock.assert_called_once_with(d, False) - @mock.patch.object(ris.RISOperations, 'get_active_macs') @mock.patch.object(client.IloClient, '_call_method') @mock.patch.object(snmp_cpqdisk_sizes, 'get_local_gb') def test_get_essential_prop_no_snmp_ris(self, snmp_mock, - call_mock, mac_mock): + call_mock): self.client.model = 'Gen9' properties = {'local_gb': 250} data = {'properties': properties} @@ -1099,15 +1098,12 @@ class IloClientTestCase(testtools.TestCase): self.client.get_essential_properties() call_mock.assert_called_once_with('get_essential_properties') self.assertFalse(snmp_mock.called) - self.assertTrue(mac_mock.called) - @mock.patch.object(ris.RISOperations, 'get_active_macs') @mock.patch.object(client.IloClient, '_call_method') @mock.patch.object(snmp_cpqdisk_sizes, 'get_local_gb') def test_get_essential_prop_no_snmp_local_gb_0(self, snmp_mock, - call_mock, - mac_mock): + call_mock): self.client.model = 'Gen9' properties = {'local_gb': 0} data = {'properties': properties} @@ -1115,14 +1111,12 @@ class IloClientTestCase(testtools.TestCase): self.client.get_essential_properties() call_mock.assert_called_once_with('get_essential_properties') self.assertFalse(snmp_mock.called) - self.assertTrue(mac_mock.called) - @mock.patch.object(ris.RISOperations, 'get_active_macs') @mock.patch.object(client.IloClient, '_call_method') @mock.patch.object(snmp_cpqdisk_sizes, 'get_local_gb') def test_get_essential_prop_snmp_true(self, snmp_mock, - call_mock, mac_mock): + call_mock): self.client.model = 'Gen9' snmp_credentials = {'auth_user': 'user', 'auth_prot_pp': '1234', @@ -1139,15 +1133,12 @@ class IloClientTestCase(testtools.TestCase): call_mock.assert_called_once_with('get_essential_properties') snmp_mock.assert_called_once_with( self.client.ipmi_host_info['address'], snmp_credentials) - self.assertTrue(mac_mock.called) - @mock.patch.object(ris.RISOperations, 'get_active_macs') @mock.patch.object(client.IloClient, '_call_method') @mock.patch.object(snmp_cpqdisk_sizes, 'get_local_gb') def test_get_essential_prop_snmp_true_local_gb_0(self, snmp_mock, - call_mock, - mac_mock): + call_mock): self.client.model = 'Gen9' snmp_credentials = {'auth_user': 'user', 'auth_prot_pp': '1234', @@ -1164,14 +1155,11 @@ class IloClientTestCase(testtools.TestCase): call_mock.assert_called_once_with('get_essential_properties') snmp_mock.assert_called_once_with( self.client.ipmi_host_info['address'], snmp_credentials) - self.assertTrue(mac_mock.called) - @mock.patch.object(ris.RISOperations, 'get_active_macs') @mock.patch.object(snmp_cpqdisk_sizes, 'get_local_gb') @mock.patch.object(client.IloClient, '_call_method') def test_get_essential_prop_snmp_false_local_gb_0(self, call_mock, - snmp_mock, - mac_mock): + snmp_mock): self.client.model = 'Gen9' snmp_credentials = {'auth_user': 'user', @@ -1188,44 +1176,6 @@ class IloClientTestCase(testtools.TestCase): self.client.get_essential_properties() call_mock.assert_called_once_with('get_essential_properties') self.assertFalse(snmp_mock.called) - self.assertTrue(mac_mock.called) - - @mock.patch.object(ris.RISOperations, 'get_active_macs') - @mock.patch.object(client.IloClient, '_call_method') - def test_get_essential_prop_macs_gen9(self, call_mock, - mac_mock): - - self.client.model = 'Gen9' - properties = {'local_gb': 123456789} - mac_mock.return_value = {'NIC.LOM.1.1': '50:65:F3:6C:47:F8'} - mac_dict = {'Port 1': '50:65:F3:6C:47:F8', - 'Port 2': '50:65:F3:6C:47:F9'} - data = {'properties': properties, 'macs': mac_dict} - expected_data = {'properties': properties, - 'macs': {'NIC.LOM.1.1': '50:65:F3:6C:47:F8'}} - call_mock.return_value = data - actual_data = self.client.get_essential_properties() - self.assertEqual(actual_data, expected_data) - call_mock.assert_called_once_with('get_essential_properties') - self.assertTrue(mac_mock.called) - - @mock.patch.object(ris.RISOperations, 'get_active_macs') - @mock.patch.object(client.IloClient, '_call_method') - def test_get_essential_prop_macs_gen8(self, call_mock, - mac_mock): - - self.client.model = 'Gen8' - properties = {'local_gb': 123456789} - mac_dict = {'Port 1': '50:65:F3:6C:47:F8', - 'Port 2': '50:65:F3:6C:47:F9'} - data = {'properties': properties, 'macs': mac_dict} - expected_data = {'properties': properties, - 'macs': mac_dict} - call_mock.return_value = data - actual_data = self.client.get_essential_properties() - self.assertEqual(actual_data, expected_data) - call_mock.assert_called_once_with('get_essential_properties') - self.assertFalse(mac_mock.called) @mock.patch.object(client.IloClient, '_call_method') def test_inject_nmi(self, call_mock): diff --git a/proliantutils/tests/ilo/test_ribcl.py b/proliantutils/tests/ilo/test_ribcl.py index 873b1046..403e7774 100644 --- a/proliantutils/tests/ilo/test_ribcl.py +++ b/proliantutils/tests/ilo/test_ribcl.py @@ -712,13 +712,11 @@ class IloRibclTestCase(unittest.TestCase): self.assertIsInstance(gpu_cnt, dict) self.assertIn('pci_gpu_devices', gpu_cnt) - @mock.patch.object(ribcl.RIBCLOperations, 'get_product_name') @mock.patch.object(ribcl.RIBCLOperations, 'get_host_health_data') - def test_get_essential_properties(self, health_data_mock, prod_mock): + def test_get_essential_properties(self, health_data_mock): data = constants.GET_EMBEDDED_HEALTH_OUTPUT json_data = json.loads(data) health_data_mock.return_value = json_data - prod_mock.return_value = "Gen8" expected_properties = {'macs': { u'Port 4': u'40:a8:f0:1e:86:77', u'Port 3': u'40:a8:f0:1e:86:76', @@ -737,26 +735,6 @@ class IloRibclTestCase(unittest.TestCase): self.assertIn('properties', properties) self.assertEqual(expected_properties, properties) - @mock.patch.object(ribcl.RIBCLOperations, 'get_product_name') - @mock.patch.object(ribcl.RIBCLOperations, 'get_host_health_data') - def test_get_essential_properties_Gen9(self, health_data_mock, prod_mock): - data = constants.GET_EMBEDDED_HEALTH_OUTPUT - json_data = json.loads(data) - health_data_mock.return_value = json_data - prod_mock.return_value = "Gen9" - expected_properties = {'macs': {}, - 'properties': { - 'memory_mb': 32768, - 'cpu_arch': 'x86_64', - 'local_gb': 98, - 'cpus': 32} - } - properties = self.ilo.get_essential_properties() - self.assertIsInstance(properties, dict) - self.assertIn('macs', properties) - self.assertIn('properties', properties) - self.assertEqual(expected_properties, properties) - @mock.patch.object(ribcl.RIBCLOperations, 'get_product_name') @mock.patch.object(ribcl.RIBCLOperations, 'get_host_health_data') @mock.patch.object(ribcl.RIBCLOperations, 'get_supported_boot_mode') diff --git a/proliantutils/tests/ilo/test_ris.py b/proliantutils/tests/ilo/test_ris.py index b29d78d1..8516bcec 100755 --- a/proliantutils/tests/ilo/test_ris.py +++ b/proliantutils/tests/ilo/test_ris.py @@ -2641,94 +2641,3 @@ class TestRISOperationsPrivateMethods(testtools.TestCase): 'ProLiant BL460c Gen9', self.client.create_raid_configuration, raid_config) - - @mock.patch.object(ris.RISOperations, '_rest_get') - @mock.patch.object(ris.RISOperations, '_get_host_details') - def test__get_network_adapters(self, get_host_details_mock, get_mock): - system_data = json.loads(ris_outputs.RESPONSE_BODY_FOR_REST_OP) - get_host_details_mock.return_value = system_data - net_uri = '/rest/v1/Systems/1/NetworkAdapters' - net_list = json.loads(ris_outputs.NETWORK_ADAPTER_LIST) - - get_mock.return_value = (200, ris_outputs.GET_HEADERS, - net_list) - self.client._get_network_adapters() - get_mock.assert_called_once_with(net_uri) - - @mock.patch.object(ris.RISOperations, '_rest_get') - @mock.patch.object(ris.RISOperations, '_get_host_details') - def test__get_network_adapters_fail(self, get_host_details_mock, - get_mock): - system_data = json.loads(ris_outputs.RESPONSE_BODY_FOR_REST_OP) - get_host_details_mock.return_value = system_data - net_uri = '/rest/v1/Systems/1/NetworkAdapters' - net_list = json.loads(ris_outputs.NETWORK_ADAPTER_LIST) - get_mock.return_value = (301, ris_outputs.GET_HEADERS, - net_list) - self.assertRaises(exception.IloError, - self.client._get_network_adapters) - get_mock.assert_called_once_with(net_uri) - - @mock.patch.object(ris.RISOperations, '_get_host_details') - def test__get_network_adapters_not_supported(self, get_details_mock): - host_response = json.loads(ris_outputs.RESPONSE_BODY_FOR_REST_OP) - del host_response['Oem']['Hp']['links']['NetworkAdapters'] - get_details_mock.return_value = host_response - self.assertRaises(exception.IloCommandNotSupportedError, - self.client._get_network_adapters) - get_details_mock.assert_called_once_with() - - @mock.patch.object(ris.RISOperations, '_rest_get') - @mock.patch.object(ris.RISOperations, '_get_network_adapters') - def test_get_active_macs_one_mac(self, network_mock, rest_mock): - net_details = json.loads(ris_outputs.NETWORK_ADAPTER_LIST) - mac_details = json.loads(ris_outputs.ONE_MAC_CONNECTED) - network_mock.return_value = net_details - uri = '/rest/v1/Systems/1/NetworkAdapters/1' - rest_mock.return_value = (200, ris_outputs.GET_HEADERS, - mac_details) - expected_macs = {'NIC.LOM.1.1': '50:65:F3:6C:47:F8'} - actual_macs = self.client.get_active_macs() - self.assertEqual(expected_macs, actual_macs) - network_mock.assert_called_once_with() - rest_mock.assert_called_once_with(uri) - - @mock.patch.object(ris.RISOperations, '_rest_get') - @mock.patch.object(ris.RISOperations, '_get_network_adapters') - def test_get_active_macs_no_connected_mac(self, network_mock, rest_mock): - net_details = json.loads(ris_outputs.NETWORK_ADAPTER_LIST) - mac_details = json.loads(ris_outputs.NO_MACS_CONNECTED) - network_mock.return_value = net_details - rest_mock.return_value = (200, ris_outputs.GET_HEADERS, - mac_details) - expected_macs = {} - actual_macs = self.client.get_active_macs() - self.assertEqual(expected_macs, actual_macs) - network_mock.assert_called_once_with() - self.assertTrue(rest_mock.called) - - @mock.patch.object(ris.RISOperations, '_rest_get') - @mock.patch.object(ris.RISOperations, '_get_network_adapters') - def test_get_active_macs_error(self, network_mock, rest_mock): - net_details = json.loads(ris_outputs.NETWORK_ADAPTER_LIST) - network_mock.return_value = net_details - uri = "/rest/v1/Systems/1/NetworkAdapters/1" - rest_mock.return_value = (301, ris_outputs.GET_HEADERS, - ris_outputs.REST_FAILURE_OUTPUT) - exc = self.assertRaises(exception.IloError, - self.client.get_active_macs) - network_mock.assert_called_once_with() - rest_mock.assert_called_once_with(uri) - self.assertIn('FakeFailureMessage', str(exc)) - - @mock.patch.object(ris.RISOperations, '_get_network_adapters') - def test_get_active_macs_not_supported(self, network_mock): - net_details = json.loads(ris_outputs.NETWORK_ADAPTER_LIST) - network_mock.return_value = net_details - del net_details['links']['Member'] - exc = self.assertRaises(exception.IloCommandNotSupportedError, - self.client.get_active_macs) - msg = ('"links" or "links/Member" section in NetworkAdapters' - ' does not exist') - network_mock.assert_called_once_with() - self.assertIn(msg, str(exc))