Follow up patch for SNMPv3 support

Fixes the minor issues with SNMPv3 inspection

Closes-Bug: 1609622

Change-Id: Ic5c397e57060f4d6d01fa9f16f371408af7aae3e

4 files changed, 28 insertions, 24 deletions

diff --git a/doc/source/drivers/ilo.rst b/doc/source/drivers/ilo.rst
index 8a32c060a..fb50dc229 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 7bae67097..9b2c9183d 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 c2685d308..1b412c78d 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 f309923ad..c12a45289 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``)