From 442849b7a7ed6dfc3d06a14a0516e79fc78db8d9 Mon Sep 17 00:00:00 2001 From: Nisha Agarwal Date: Fri, 10 Feb 2017 08:16:49 -0800 Subject: [PATCH] Follow up patch for SNMPv3 support Fixes the minor issues with SNMPv3 inspection Closes-Bug: 1609622 Change-Id: Ic5c397e57060f4d6d01fa9f16f371408af7aae3e --- doc/source/drivers/ilo.rst | 4 ++-- ironic/drivers/modules/ilo/common.py | 16 +++++++++++----- .../unit/drivers/modules/ilo/test_common.py | 18 ++++++------------ ...mp-inspection-support-e68fd6d57cb33846.yaml | 14 +++++++++----- 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/doc/source/drivers/ilo.rst b/doc/source/drivers/ilo.rst index 8a32c060ab..fb50dc2293 100644 --- a/doc/source/drivers/ilo.rst +++ b/doc/source/drivers/ilo.rst @@ -927,10 +927,10 @@ The following iLO drivers support hardware inspection: 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". + are "MD5" and "SHA". The iLO default value is "MD5". * ``snmp_auth_priv_protocol`` : The Privacy protocol. The valid - values are "AES" and "DES". The default value is "DES". + values are "AES" and "DES". The iLO default value is "DES". The inspection process will discover the following essential properties (properties required for scheduling deployment): diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index 7bae670978..9b2c9183d1 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -287,16 +287,20 @@ def _parse_snmp_driver_info(info): "node's driver_info: %s") % missing_info) for param in SNMP_OPTIONAL_PROPERTIES: + value = None try: value = six.text_type(info[param]).upper() + except KeyError: + pass + if value: 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 + else: + snmp_info = None return snmp_info @@ -324,9 +328,11 @@ def get_ilo_object(node): 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'] + info['auth_protocol'] = str(snmp_info['snmp_auth_protocol']) + if snmp_info.get('snmp_auth_priv_protocol'): + info['priv_protocol'] = str(snmp_info['snmp_auth_priv_protocol']) + else: + info = None ilo_object = ilo_client.IloClient(driver_info['ilo_address'], driver_info['ilo_username'], driver_info['ilo_password'], diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index c2685d308b..1b412c78d0 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -93,8 +93,7 @@ class IloValidateParametersTestCase(db_base.DbTestCase): 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'} + 'snmp_auth_priv_password': '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']) @@ -223,8 +222,7 @@ 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, - snmp_credentials=None): + def _test_get_ilo_object(self, ilo_client_mock, isFile_mock, ca_file=None): self.info['client_timeout'] = 600 self.info['client_port'] = 4433 self.info['ca_file'] = ca_file @@ -238,7 +236,7 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): self.info['client_timeout'], self.info['client_port'], cacert=self.info['ca_file'], - snmp_credentials=snmp_credentials) + snmp_credentials=None) self.assertEqual('ilo_object', returned_ilo_object) @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) @@ -247,9 +245,10 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): 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_priv_pp': '4321', 'auth_protocol': 'SHA', - 'priv_protocol': 'AES'} + 'priv_protocol': 'AES', + 'snmp_inspection': True} d_info = {'client_timeout': 600, 'client_port': 4433, 'ca_file': 'ca_file', @@ -262,11 +261,6 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): 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'], diff --git a/releasenotes/notes/add-snmp-inspection-support-e68fd6d57cb33846.yaml b/releasenotes/notes/add-snmp-inspection-support-e68fd6d57cb33846.yaml index f309923adc..c12a45289e 100644 --- a/releasenotes/notes/add-snmp-inspection-support-e68fd6d57cb33846.yaml +++ b/releasenotes/notes/add-snmp-inspection-support-e68fd6d57cb33846.yaml @@ -1,14 +1,18 @@ --- -feature: +fixes: - 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``. +features: + - To enable SNMPv3 inspection in iLO drivers, 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``) + * ``snmp_auth_protocol`` (optional, defaults to iLO default + value ``MD5``) + * ``snmp_auth_priv_protocol`` (optional, defaults to iLO default + value ``DES``)