diff --git a/doc/source/drivers/ilo.rst b/doc/source/drivers/ilo.rst index 77e8e8e047..8a32c060ab 100644 --- a/doc/source/drivers/ilo.rst +++ b/doc/source/drivers/ilo.rst @@ -903,8 +903,34 @@ The following iLO drivers support hardware inspection: .. note:: - * The RAID needs to be pre-configured prior to inspection otherwise - proliantutils returns 0 for disk size. + * The disk size is returned by RIBCL/RIS only when RAID is preconfigured + on the storage. If the storage is Direct Attached Storage, then + RIBCL/RIS fails to get the disk size. + * The SNMPv3 inspection gets disk size for all types of storages. + If RIBCL/RIS is unable to get disk size and SNMPv3 inspection is + requested, the proliantutils does SNMPv3 inspection to get the + disk size. If proliantutils is unable to get the disk size, it raises + an error. This feature is available in proliantutils release + version >= 2.2.0. + * The iLO must be updated with SNMPv3 authentication details. + Refer to the section `SNMPv3 Authentication` in `HPE iLO4 User Guide`_ + for setting up authentication details on iLO. + The following parameters are mandatory to be given in driver_info + for SNMPv3 inspection: + + * ``snmp_auth_user`` : The SNMPv3 user. + + * ``snmp_auth_prot_password`` : The auth protocol pass phrase. + + * ``snmp_auth_priv_password`` : The privacy protocol pass phrase. + + The following parameters are optional for SNMPv3 inspection: + + * ``snmp_auth_protocol`` : The Auth Protocol. The valid values + are "MD5" and "SHA". The default value is "MD5". + + * ``snmp_auth_priv_protocol`` : The Privacy protocol. The valid + values are "AES" and "DES". The default value is "DES". The inspection process will discover the following essential properties (properties required for scheduling deployment): @@ -1565,3 +1591,4 @@ use the ``proliant-tools`` element in DIB:: .. _`Enabling HTTPS in Swift`: http://docs.openstack.org/project-install-guide/baremetal/draft/enabling-https.html#enabling-https-in-swift .. _`Enabling HTTPS in Image service`: http://docs.openstack.org/project-install-guide/baremetal/draft/enabling-https.html#enabling-https-in-image-service +.. _`HPE iLO4 User Guide`: http://h20566.www2.hpe.com/hpsc/doc/public/display?docId=c03334051 diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index 4f57ca85eb..7bae670978 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -56,8 +56,28 @@ REQUIRED_PROPERTIES = { OPTIONAL_PROPERTIES = { 'client_port': _("port to be used for iLO operations. Optional."), 'client_timeout': _("timeout (in seconds) for iLO operations. Optional."), - 'ca_file': _("CA certificate file to validate iLO. optional"), + 'ca_file': _("CA certificate file to validate iLO. Optional") } + +SNMP_PROPERTIES = { + 'snmp_auth_user': _("User for SNMPv3. " + "Required for SNMP inspection"), + 'snmp_auth_prot_password': _("Authentication Protocol Passphrase. " + "Required for SNMP inspection"), + 'snmp_auth_priv_password': _("Authentication Privacy Passphrase. " + "Required for SNMP inspection"), +} + +SNMP_OPTIONAL_PROPERTIES = { + 'snmp_auth_protocol': _("Authentication Protocol. Optional, used " + "for SNMP inspection. If not specified, the " + "default value as 'MD5' is used."), + 'snmp_auth_priv_protocol': _("Privacy Protocol. Optional, " + "used for SNMP inspection. " + "If not specified, the default value " + "as 'DES' is used.") +} + CONSOLE_PROPERTIES = { 'console_port': _("node's UDP port to connect to. Only required for " "console access.") @@ -221,6 +241,10 @@ def parse_driver_info(node): except ValueError: not_integers.append(param) + snmp_info = _parse_snmp_driver_info(info) + if snmp_info: + d_info.update(snmp_info) + for param in CONSOLE_PROPERTIES: value = info.get(param) if value: @@ -237,6 +261,45 @@ def parse_driver_info(node): return d_info +def _parse_snmp_driver_info(info): + """Parses the SNMP related driver_info parameters. + + :param info: driver_info dictionary. + :returns: a dictionary containing SNMP information. + :raises exception.MissingParameterValue if any of the mandatory + parameter values are not provided. + :raises exception.InvalidParameterValue if the value provided + for SNMP_OPTIONAL_PROPERTIES has an invalid value. + """ + snmp_info = {} + missing_info = [] + valid_values = {'snmp_auth_protocol': ['MD5', 'SHA'], + 'snmp_auth_priv_protocol': ['AES', 'DES']} + if info.get('snmp_auth_user'): + for param in SNMP_PROPERTIES: + try: + snmp_info[param] = info[param] + except KeyError: + missing_info.append(param) + if missing_info: + raise exception.MissingParameterValue(_( + "The following required SNMP parameters are missing from the " + "node's driver_info: %s") % missing_info) + + for param in SNMP_OPTIONAL_PROPERTIES: + try: + value = six.text_type(info[param]).upper() + if value not in valid_values[param]: + raise exception.InvalidParameterValue(_( + "Invalid value %(value)s given for driver_info " + "parameter %(param)s") % {'param': param, + 'value': info[param]}) + snmp_info[param] = value + except KeyError: + pass + return snmp_info + + def get_ilo_object(node): """Gets an IloClient object from proliantutils library. @@ -250,12 +313,27 @@ def get_ilo_object(node): is missing on the node """ driver_info = parse_driver_info(node) + snmp_info = _parse_snmp_driver_info(driver_info) + info = {} + # This mapping is done as per what proliantutils expect the input + # to be. This will be removed once proliantutils is fixed for this + # in its next release. + if snmp_info: + info['snmp_inspection'] = True + info['auth_user'] = snmp_info['snmp_auth_user'] + info['auth_prot_pp'] = snmp_info['snmp_auth_prot_password'] + info['auth_priv_pp'] = snmp_info['snmp_auth_priv_password'] + if snmp_info.get('snmp_auth_protocol'): + info['auth_protocol'] = snmp_info['snmp_auth_protocol'] + if snmp_info.get('snmp_priv_protocol'): + info['priv_protocol'] = snmp_info['snmp_auth_priv_protocol'] ilo_object = ilo_client.IloClient(driver_info['ilo_address'], driver_info['ilo_username'], driver_info['ilo_password'], driver_info['client_timeout'], driver_info['client_port'], - cacert=driver_info.get('ca_file')) + cacert=driver_info.get('ca_file'), + snmp_credentials=info) return ilo_object diff --git a/ironic/drivers/modules/ilo/inspect.py b/ironic/drivers/modules/ilo/inspect.py index 81c11a1daf..80fcb940ca 100644 --- a/ironic/drivers/modules/ilo/inspect.py +++ b/ironic/drivers/modules/ilo/inspect.py @@ -163,7 +163,10 @@ def _get_capabilities(node, ilo_object): class IloInspect(base.InspectInterface): def get_properties(self): - return ilo_common.REQUIRED_PROPERTIES + props = ilo_common.REQUIRED_PROPERTIES.copy() + props.update(ilo_common.SNMP_PROPERTIES) + props.update(ilo_common.SNMP_OPTIONAL_PROPERTIES) + return props @METRICS.timer('IloInspect.validate') def validate(self, task): diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 1d2321c4e2..49d3de9414 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -4817,21 +4817,27 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, def test_driver_properties_fake_ilo(self): expected = ['ilo_address', 'ilo_username', 'ilo_password', 'client_port', 'client_timeout', 'ilo_change_password', - 'ca_file'] + 'ca_file', 'snmp_auth_user', 'snmp_auth_prot_password', + 'snmp_auth_priv_password', 'snmp_auth_protocol', + 'snmp_auth_priv_protocol'] self._check_driver_properties("fake_ilo", expected) def test_driver_properties_ilo_iscsi(self): expected = ['ilo_address', 'ilo_username', 'ilo_password', 'client_port', 'client_timeout', 'ilo_deploy_iso', 'console_port', 'ilo_change_password', - 'deploy_forces_oob_reboot', 'ca_file'] + 'deploy_forces_oob_reboot', 'ca_file', 'snmp_auth_user', + 'snmp_auth_prot_password', 'snmp_auth_priv_password', + 'snmp_auth_protocol', 'snmp_auth_priv_protocol'] self._check_driver_properties("iscsi_ilo", expected) def test_driver_properties_agent_ilo(self): expected = ['ilo_address', 'ilo_username', 'ilo_password', 'client_port', 'client_timeout', 'ilo_deploy_iso', 'console_port', 'ilo_change_password', - 'ca_file'] + 'ca_file', 'snmp_auth_user', 'snmp_auth_prot_password', + 'snmp_auth_priv_password', 'snmp_auth_protocol', + 'snmp_auth_priv_protocol'] self._check_driver_properties("agent_ilo", expected) def test_driver_properties_fail(self): diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index f1699c94b2..c2685d308b 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -72,14 +72,88 @@ class IloValidateParametersTestCase(db_base.DbTestCase): self.assertEqual(60, info['client_timeout']) self.assertEqual(443, info['client_port']) self.assertEqual('/home/user/cafile.pem', info['ca_file']) + self.assertEqual('user', info['snmp_auth_user']) + self.assertEqual('1234', info['snmp_auth_prot_password']) + self.assertEqual('4321', info['snmp_auth_priv_password']) + self.assertEqual('SHA', info['snmp_auth_protocol']) + self.assertEqual('AES', info['snmp_auth_priv_protocol']) - def test_parse_driver_info_ca_file_in_driver_info(self): - self.node.driver_info['ca_file'] = '/home/user/cafile.pem' + @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) + def test_parse_driver_info_snmp_inspection_false(self, isFile_mock): + info = ilo_common.parse_driver_info(self.node) + self.assertEqual(INFO_DICT['ilo_address'], info['ilo_address']) + self.assertEqual(INFO_DICT['ilo_username'], info['ilo_username']) + self.assertEqual(INFO_DICT['ilo_password'], info['ilo_password']) + self.assertEqual(60, info['client_timeout']) + self.assertEqual(443, info['client_port']) + + @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) + def test_parse_driver_info_snmp_true_no_auth_priv_protocols(self, + isFile_mock): + d_info = {'ca_file': '/home/user/cafile.pem', + 'snmp_auth_prot_password': '1234', + 'snmp_auth_user': 'user', + 'snmp_auth_priv_password': '4321', + 'auth_priv_pp': '4321'} + self.node.driver_info.update(d_info) + info = ilo_common.parse_driver_info(self.node) + self.assertEqual(INFO_DICT['ilo_address'], info['ilo_address']) + self.assertEqual(INFO_DICT['ilo_username'], info['ilo_username']) + self.assertEqual(INFO_DICT['ilo_password'], info['ilo_password']) + self.assertEqual(60, info['client_timeout']) + self.assertEqual(443, info['client_port']) + self.assertEqual('/home/user/cafile.pem', info['ca_file']) + self.assertEqual('user', info['snmp_auth_user']) + self.assertEqual('1234', info['snmp_auth_prot_password']) + self.assertEqual('4321', info['snmp_auth_priv_password']) + + def test_parse_driver_info_ca_file_and_snmp_inspection_true(self): + d_info = {'ca_file': '/home/user/cafile.pem', + 'snmp_auth_prot_password': '1234', + 'snmp_auth_user': 'user', + 'snmp_auth_priv_password': '4321', + 'snmp_auth_protocol': 'SHA', + 'snmp_auth_priv_protocol': 'AES'} + self.node.driver_info.update(d_info) self._test_parse_driver_info() - def test_parse_driver_info_ca_file_in_conf_file(self): - self.config(ca_file='/home/user/cafile.pem', group='ilo') - self._test_parse_driver_info() + def test_parse_driver_info_snmp_true_invalid_auth_protocol(self): + d_info = {'ca_file': '/home/user/cafile.pem', + 'snmp_auth_prot_password': '1234', + 'snmp_auth_user': 'user', + 'snmp_auth_priv_password': '4321', + 'snmp_auth_protocol': 'abc', + 'snmp_auth_priv_protocol': 'AES'} + self.node.driver_info.update(d_info) + self.assertRaises(exception.InvalidParameterValue, + ilo_common.parse_driver_info, self.node) + + def test_parse_driver_info_snmp_true_invalid_priv_protocol(self): + d_info = {'ca_file': '/home/user/cafile.pem', + 'snmp_auth_prot_password': '1234', + 'snmp_auth_user': 'user', + 'snmp_auth_priv_password': '4321', + 'snmp_auth_protocol': 'SHA', + 'snmp_auth_priv_protocol': 'xyz'} + self.node.driver_info.update(d_info) + self.assertRaises(exception.InvalidParameterValue, + ilo_common.parse_driver_info, self.node) + + def test_parse_driver_info_snmp_true_integer_auth_protocol(self): + d_info = {'ca_file': '/home/user/cafile.pem', + 'snmp_auth_prot_password': '1234', + 'snmp_auth_user': 'user', + 'snmp_auth_priv_password': '4321', + 'snmp_auth_protocol': 12, + 'snmp_auth_priv_protocol': 'AES'} + self.node.driver_info.update(d_info) + self.assertRaises(exception.InvalidParameterValue, + ilo_common.parse_driver_info, self.node) + + def test_parse_driver_info_snmp_inspection_true_raises(self): + self.node.driver_info['snmp_auth_user'] = 'abc' + self.assertRaises(exception.MissingParameterValue, + ilo_common.parse_driver_info, self.node) def test_parse_driver_info_missing_address(self): del self.node.driver_info['ilo_address'] @@ -149,7 +223,8 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) @mock.patch.object(ilo_client, 'IloClient', spec_set=True, autospec=True) - def _test_get_ilo_object(self, ilo_client_mock, isFile_mock, ca_file=None): + def _test_get_ilo_object(self, ilo_client_mock, isFile_mock, ca_file=None, + snmp_credentials=None): self.info['client_timeout'] = 600 self.info['client_port'] = 4433 self.info['ca_file'] = ca_file @@ -162,7 +237,44 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): self.info['ilo_password'], self.info['client_timeout'], self.info['client_port'], - cacert=self.info['ca_file']) + cacert=self.info['ca_file'], + snmp_credentials=snmp_credentials) + self.assertEqual('ilo_object', returned_ilo_object) + + @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) + @mock.patch.object(ilo_client, 'IloClient', spec_set=True, + autospec=True) + def test_get_ilo_object_snmp(self, ilo_client_mock, isFile_mock): + info = {'auth_user': 'user', + 'auth_prot_pp': '1234', + 'priv_prot_pp': '4321', + 'auth_protocol': 'SHA', + 'priv_protocol': 'AES'} + d_info = {'client_timeout': 600, + 'client_port': 4433, + 'ca_file': 'ca_file', + 'snmp_auth_user': 'user', + 'snmp_auth_prot_password': '1234', + 'snmp_auth_priv_password': '4321', + 'snmp_auth_protocol': 'SHA', + 'snmp_auth_priv_protocol': 'AES'} + self.info.update(d_info) + self.node.driver_info = self.info + ilo_client_mock.return_value = 'ilo_object' + returned_ilo_object = ilo_common.get_ilo_object(self.node) + info = {'auth_user': 'user', + 'auth_prot_pp': '1234', + 'priv_prot_pp': '4321', + 'auth_protocol': 'SHA', + 'priv_protocol': 'AES'} + ilo_client_mock.assert_called_with( + self.info['ilo_address'], + self.info['ilo_username'], + self.info['ilo_password'], + self.info['client_timeout'], + self.info['client_port'], + cacert=self.info['ca_file'], + snmp_credentials=info) self.assertEqual('ilo_object', returned_ilo_object) def test_get_ilo_object_cafile(self): diff --git a/ironic/tests/unit/drivers/modules/ilo/test_inspect.py b/ironic/tests/unit/drivers/modules/ilo/test_inspect.py index 53cdd58496..2bf33b681a 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_inspect.py @@ -48,6 +48,8 @@ class IloInspectTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: properties = ilo_common.REQUIRED_PROPERTIES.copy() + properties.update(ilo_common.SNMP_PROPERTIES) + properties.update(ilo_common.SNMP_OPTIONAL_PROPERTIES) self.assertEqual(properties, task.driver.inspect.get_properties()) diff --git a/releasenotes/notes/add-snmp-inspection-support-e68fd6d57cb33846.yaml b/releasenotes/notes/add-snmp-inspection-support-e68fd6d57cb33846.yaml new file mode 100644 index 0000000000..f309923adc --- /dev/null +++ b/releasenotes/notes/add-snmp-inspection-support-e68fd6d57cb33846.yaml @@ -0,0 +1,14 @@ +--- +feature: + - Fixes disk size detection for out-of-band + inspection in iLO drivers, by optionally using SNMPv3 to + get the disk size for certain types of storage. + + To enable this, the the following parameters must be + set in the node's ``driver_info``. + + * ``snmp_auth_user`` + * ``snmp_auth_prot_password`` + * ``snmp_auth_priv_password`` + * ``snmp_auth_protocol`` (optional, defaults to ``MD5``) + * ``snmp_auth_priv_protocol`` (optional, defaults to ``DES``)