From d6ff5116f4a5586238d44a196ec89d844b151f9d Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <divius.inside@gmail.com>
Date: Mon, 11 Sep 2017 16:08:33 +0200
Subject: [PATCH] Remove assumption that a valid IPMI channel cannot follow an
invalid one
It seems to be incorrect at least for some iLO machines.
Also harden the code against invalid output from ipmitool.
Change-Id: I733785e9c7d86eadca963f0776910504bf91bcfe
Closes-Bug: #1714944
---
ironic_python_agent/hardware.py | 21 ++++++++++++-------
.../tests/unit/test_hardware.py | 4 ++++
...ipmi-address-channel-b6b8010c41d05c1b.yaml | 6 ++++++
3 files changed, 24 insertions(+), 7 deletions(-)
create mode 100644 releasenotes/notes/ipmi-address-channel-b6b8010c41d05c1b.yaml
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 568d5bf49..30549dbc0 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -21,6 +21,7 @@ import time
from ironic_lib import disk_utils
from ironic_lib import utils as il_utils
+import netaddr
from oslo_concurrency import processutils
from oslo_config import cfg
from oslo_log import log
@@ -944,14 +945,20 @@ class GenericHardwareManager(HardwareManager):
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()
+ continue
+ out = out.strip()
+
+ try:
+ netaddr.IPAddress(out)
+ except netaddr.AddrFormatError:
+ LOG.warning('Invalid IP address: %s', out)
+ continue
+
+ # In case we get 0.0.0.0 on a valid channel, we need to keep
+ # querying
+ if out != '0.0.0.0':
+ return out
except (processutils.ProcessExecutionError, OSError) as e:
# Not error, because it's normal in virtual environment
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index 216f9776e..901bae2d0 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -1564,7 +1564,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
# and for any other we return a correct IP address
def side_effect(*args, **kwargs):
if args[0].startswith("ipmitool lan print 1"):
+ return '', 'Invalid channel 1\n'
+ elif args[0].startswith("ipmitool lan print 2"):
return '0.0.0.0\n', ''
+ elif args[0].startswith("ipmitool lan print 3"):
+ return 'meow', ''
else:
return '192.1.2.3\n', ''
mocked_execute.side_effect = side_effect
diff --git a/releasenotes/notes/ipmi-address-channel-b6b8010c41d05c1b.yaml b/releasenotes/notes/ipmi-address-channel-b6b8010c41d05c1b.yaml
new file mode 100644
index 000000000..64e34d0f2
--- /dev/null
+++ b/releasenotes/notes/ipmi-address-channel-b6b8010c41d05c1b.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+ - |
+ Fixes incorrect assumption that a valid channel cannot follow an invalid
+ one in IPMI (`bug 1714944
+ <https://bugs.launchpad.net/ironic-python-agent/+bug/1714944>`_).