BMC can be configured using different lan channel

mkowalski · mkowalski · commit 7c28332e9a59 · 2017-08-09T15:35:57.000+02:00
It may happen that BMC is configured to use non-zero channel.
In this case we should iterate across all of them as long as we
get a correct IP address (in this case different than "0.0.0.0"
which is a placeholder for not configured console).

Change-Id: I3c351af1882b24c8f56e4363249b19b5c3a4a446
Closes-Bug: #1702514
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
@@ -927,21 +927,38 @@ def _ata_erase(self, block_device):
         return True
 
     def get_bmc_address(self):
+        """Attempt to detect BMC IP address
+
+        :return: IP address of lan channel or 0.0.0.0 in case none of them is
+                 configured properly
+        """
         # These modules are rarely loaded automatically
         utils.try_execute('modprobe', 'ipmi_msghandler')
         utils.try_execute('modprobe', 'ipmi_devintf')
         utils.try_execute('modprobe', 'ipmi_si')
 
         try:
-            out, _e = utils.execute(
-                "ipmitool lan print | grep -e 'IP Address [^S]' "
-                "| awk '{ print $4 }'", shell=True)
+            # From all the channels 0-15, only 1-7 can be assigned to different
+            # types of communication media and protocols and effectively used
+            for channel in range(1, 8):
+                out, e = utils.execute(
+                    "ipmitool lan print {} | awk '/IP Address[[:space:]]*:/"
+                    " {{print $4}}'".format(channel), shell=True)
+                # Invalid channel cannot be followed by a valid one, so we can
+                # safely break here
+                if e.startswith("Invalid channel"):
+                    break
+                # In case we get empty IP or 0.0.0.0 on a valid channel,
+                # we need to keep querying
+                if out.strip() not in ('', '0.0.0.0'):
+                    return out.strip()
+
         except (processutils.ProcessExecutionError, OSError) as e:
             # Not error, because it's normal in virtual environment
             LOG.warning("Cannot get BMC address: %s", e)
             return
 
-        return out.strip()
+        return '0.0.0.0'
 
     def get_clean_steps(self, node, ports):
         return [
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
@@ -1541,6 +1541,40 @@ def test_get_bmc_address_virt(self, mocked_execute):
         mocked_execute.side_effect = processutils.ProcessExecutionError()
         self.assertIsNone(self.hardware.get_bmc_address())
 
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_get_bmc_address_zeroed(self, mocked_execute):
+        mocked_execute.return_value = '0.0.0.0\n', ''
+        self.assertEqual('0.0.0.0', self.hardware.get_bmc_address())
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_get_bmc_address_invalid(self, mocked_execute):
+        # In case of invalid lan channel, stdout is empty and the error
+        # on stderr is "Invalid channel"
+        mocked_execute.return_value = '\n', 'Invalid channel: 55'
+        self.assertEqual('0.0.0.0', self.hardware.get_bmc_address())
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_get_bmc_address_random_error(self, mocked_execute):
+        mocked_execute.return_value = '192.1.2.3\n', 'Random error message'
+        self.assertEqual('192.1.2.3', self.hardware.get_bmc_address())
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_get_bmc_address_iterate_channels(self, mocked_execute):
+        # For channel 1 we simulate unconfigured IP
+        # and for any other we return a correct IP address
+        def side_effect(*args, **kwargs):
+            if args[0].startswith("ipmitool lan print 1"):
+                return '0.0.0.0\n', ''
+            else:
+                return '192.1.2.3\n', ''
+        mocked_execute.side_effect = side_effect
+        self.assertEqual('192.1.2.3', self.hardware.get_bmc_address())
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_get_bmc_address_not_available(self, mocked_execute):
+        mocked_execute.return_value = '', ''
+        self.assertEqual('0.0.0.0', self.hardware.get_bmc_address())
+
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_system_vendor_info(self, mocked_execute):
         mocked_execute.return_value = (
diff --git a/releasenotes/notes/multiple-lan-channels-ee32d80150f990bf.yaml b/releasenotes/notes/multiple-lan-channels-ee32d80150f990bf.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    IPMI interface can be configured to use lan channel different than
+    a default one. In that case querying it will return 0.0.0.0 as an IP
+    address representing an interface lacking configuration. In order to get
+    the real IP address, we need to iterate through all the possible channels.
