openstack/ironic-inspector
Code Issues Proposed changes

Use getaddrinfo instead of gethostbyname while resolving BMC address

Browse Source 
* According to python docs 'gethostbyname() does not support IPv6 name resolution'
  Changing it to socket.getaddrinfo.
* get_ipmi_address method returns tuple (<BMC Address>, ipv4, ipv6)

Change-Id: I7d57556ff00cf29aee6f1591dcb924086099c7d5
Story: #2004648
Task: #28601
This commit is contained in:
Nikolay Fedotov 2018-12-19 11:37:48 +03:00
parent 8d41543c1b
commit 771f2d396b
5 changed files with 73 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
ironic_inspector/common/ironic.py
View File

@ -58,18 +58,43 @@ def reset_ironic_session():
def get_ipmi_address(node):
"""Get the BMC address defined in node.driver_info dictionary
Possible names of BMC address value examined in order of list
['ipmi_address'] + CONF.ipmi_address_fields. The value could
be an IP address or a hostname. DNS lookup performed for the
first non empty value.
The first valid BMC address value returned along with
it's v4 and v6 IP addresses.
:param node: Node object with defined driver_info dictionary
:return: tuple (ipmi_address, ipv4_address, ipv6_address)
"""
none_address = None, None, None
ipmi_fields = ['ipmi_address'] + CONF.ipmi_address_fields
# NOTE(sambetts): IPMI Address is useless to us if bridging is enabled so
# just ignore it and return None
if node.driver_info.get("ipmi_bridging", "no") != "no":
return
return none_address
for name in ipmi_fields:
value = node.driver_info.get(name)
if not value:
continue
ipv4 = None
ipv6 = None
try:
ip = socket.gethostbyname(value)
addrinfo = socket.getaddrinfo(value, None, 0, 0, socket.SOL_TCP)
for family, socket_type, proto, canon_name, sockaddr in addrinfo:
ip = sockaddr[0]
if netaddr.IPAddress(ip).is_loopback():
LOG.warning('Ignoring loopback BMC address %s', ip,
node_info=node)
elif family == socket.AF_INET:
ipv4 = ip
elif family == socket.AF_INET6:
ipv6 = ip
except socket.gaierror:
msg = _('Failed to resolve the hostname (%(value)s)'
' for node %(uuid)s')
@ -77,12 +102,8 @@ def get_ipmi_address(node):
'uuid': node.uuid},
node_info=node)
if netaddr.IPAddress(ip).is_loopback():
LOG.warning('Ignoring loopback BMC address %s', ip,
node_info=node)
ip = None
return ip
return (value, ipv4, ipv6) if ipv4 or ipv6 else none_address
return none_address
def get_client(token=None,

2
ironic_inspector/introspect.py
View File

@ -53,7 +53,7 @@ def introspect(node_id, manage_boot=True, token=None):
raise utils.Error(msg % validation.power['reason'],
node_info=node)
bmc_address = ir_utils.get_ipmi_address(node)
bmc_address, bmc_ipv4, bmc_ipv6 = ir_utils.get_ipmi_address(node)
node_info = node_cache.start_introspection(node.uuid,
bmc_address=bmc_address,
manage_boot=manage_boot,

3
ironic_inspector/plugins/discovery.py
View File

@ -60,7 +60,8 @@ def _check_existing_nodes(introspection_data, node_driver_info, ironic):
# impact on performance on big clusters
nodes = ironic.node.list(fields=('uuid', 'driver_info'), limit=0)
for node in nodes:
if ipmi_address == ir_utils.get_ipmi_address(node):
bmc_address, bmc_ipv4, bmc_ipv6 = ir_utils.get_ipmi_address(node)
if ipmi_address in (bmc_ipv4, bmc_ipv6):
raise utils.Error(
_('Node %(uuid)s already has BMC address '
'%(ipmi_address)s, not enrolling') %

51
ironic_inspector/test/unit/test_common_ironic.py
View File

@ -87,22 +87,37 @@ class TestGetClientNoAuth(TestGetClientBase, base.BaseTest):
class TestGetIpmiAddress(base.BaseTest):
def setUp(self):
super(TestGetIpmiAddress, self).setUp()
self.ipmi_address = 'www.example.com'
self.ipmi_ipv4 = '192.168.1.1'
self.ipmi_ipv6 = 'fe80::1'
def test_ipv4_in_resolves(self):
node = mock.Mock(spec=['driver_info', 'uuid'],
driver_info={'ipmi_address': '192.168.1.1'})
ip = ir_utils.get_ipmi_address(node)
self.assertEqual('192.168.1.1', ip)
driver_info={'ipmi_address': self.ipmi_ipv4})
self.assertEqual((self.ipmi_ipv4, self.ipmi_ipv4, None),
ir_utils.get_ipmi_address(node))
@mock.patch('socket.gethostbyname')
def test_ipv6_in_resolves(self):
node = mock.Mock(spec=['driver_info', 'uuid'],
driver_info={'ipmi_address': self.ipmi_ipv6})
self.assertEqual((self.ipmi_ipv6, None, self.ipmi_ipv6),
ir_utils.get_ipmi_address(node))
@mock.patch('socket.getaddrinfo')
def test_good_hostname_resolves(self, mock_socket):
node = mock.Mock(spec=['driver_info', 'uuid'],
driver_info={'ipmi_address': 'www.example.com'})
mock_socket.return_value = '192.168.1.1'
ip = ir_utils.get_ipmi_address(node)
mock_socket.assert_called_once_with('www.example.com')
self.assertEqual('192.168.1.1', ip)
driver_info={'ipmi_address': self.ipmi_address})
mock_socket.return_value = [
(socket.AF_INET, None, None, None, (self.ipmi_ipv4,)),
(socket.AF_INET6, None, None, None, (self.ipmi_ipv6,))]
self.assertEqual((self.ipmi_address, self.ipmi_ipv4, self.ipmi_ipv6),
ir_utils.get_ipmi_address(node))
mock_socket.assert_called_once_with(self.ipmi_address, None, 0, 0,
socket.SOL_TCP)
@mock.patch('socket.gethostbyname')
@mock.patch('socket.getaddrinfo')
def test_bad_hostname_errors(self, mock_socket):
node = mock.Mock(spec=['driver_info', 'uuid'],
driver_info={'ipmi_address': 'meow'},
@ -112,24 +127,26 @@ class TestGetIpmiAddress(base.BaseTest):
def test_additional_fields(self):
node = mock.Mock(spec=['driver_info', 'uuid'],
driver_info={'foo': '192.168.1.1'})
self.assertIsNone(ir_utils.get_ipmi_address(node))
driver_info={'foo': self.ipmi_ipv4})
self.assertEqual((None, None, None),
ir_utils.get_ipmi_address(node))
self.cfg.config(ipmi_address_fields=['foo', 'bar', 'baz'])
ip = ir_utils.get_ipmi_address(node)
self.assertEqual('192.168.1.1', ip)
self.assertEqual((self.ipmi_ipv4, self.ipmi_ipv4, None),
ir_utils.get_ipmi_address(node))
def test_ipmi_bridging_enabled(self):
node = mock.Mock(spec=['driver_info', 'uuid'],
driver_info={'ipmi_address': 'www.example.com',
'ipmi_bridging': 'single'})
self.assertIsNone(ir_utils.get_ipmi_address(node))
self.assertEqual((None, None, None),
ir_utils.get_ipmi_address(node))
def test_loopback_address(self):
node = mock.Mock(spec=['driver_info', 'uuid'],
driver_info={'ipmi_address': '127.0.0.2'})
ip = ir_utils.get_ipmi_address(node)
self.assertIsNone(ip)
self.assertEqual((None, None, None),
ir_utils.get_ipmi_address(node))
class TestCapabilities(unittest.TestCase):

7
releasenotes/notes/ipv6-bmc-address-start-inspection-7a72794f25eb9f19.yaml Normal file
View File

@ -0,0 +1,7 @@
---
fixes:
- |
Fix starting inspection of node having IPv6 BMC address.
Inspection could not be initiated because v6 address
was being considered as a hostname. Thus resolving incorrect
hostname ended up with blocking error.