From 771f2d396b23510364cfb05582175d44cf3b44e6 Mon Sep 17 00:00:00 2001 From: Nikolay Fedotov Date: Wed, 19 Dec 2018 11:37:48 +0300 Subject: [PATCH] Use getaddrinfo instead of gethostbyname while resolving BMC address * According to python docs 'gethostbyname() does not support IPv6 name resolution' Changing it to socket.getaddrinfo. * get_ipmi_address method returns tuple (, ipv4, ipv6) Change-Id: I7d57556ff00cf29aee6f1591dcb924086099c7d5 Story: #2004648 Task: #28601 --- ironic_inspector/common/ironic.py | 37 +++++++++++--- ironic_inspector/introspect.py | 2 +- ironic_inspector/plugins/discovery.py | 3 +- .../test/unit/test_common_ironic.py | 51 ++++++++++++------- ...ess-start-inspection-7a72794f25eb9f19.yaml | 7 +++ 5 files changed, 73 insertions(+), 27 deletions(-) create mode 100644 releasenotes/notes/ipv6-bmc-address-start-inspection-7a72794f25eb9f19.yaml diff --git a/ironic_inspector/common/ironic.py b/ironic_inspector/common/ironic.py index 26ce965b8..587fea34e 100644 --- a/ironic_inspector/common/ironic.py +++ b/ironic_inspector/common/ironic.py @@ -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, diff --git a/ironic_inspector/introspect.py b/ironic_inspector/introspect.py index 8190d072d..cb48d67f2 100644 --- a/ironic_inspector/introspect.py +++ b/ironic_inspector/introspect.py @@ -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, diff --git a/ironic_inspector/plugins/discovery.py b/ironic_inspector/plugins/discovery.py index 9b65251a3..71a9d4efe 100644 --- a/ironic_inspector/plugins/discovery.py +++ b/ironic_inspector/plugins/discovery.py @@ -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') % diff --git a/ironic_inspector/test/unit/test_common_ironic.py b/ironic_inspector/test/unit/test_common_ironic.py index 6afb908da..66407d0bd 100644 --- a/ironic_inspector/test/unit/test_common_ironic.py +++ b/ironic_inspector/test/unit/test_common_ironic.py @@ -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): diff --git a/releasenotes/notes/ipv6-bmc-address-start-inspection-7a72794f25eb9f19.yaml b/releasenotes/notes/ipv6-bmc-address-start-inspection-7a72794f25eb9f19.yaml new file mode 100644 index 000000000..26bbba43c --- /dev/null +++ b/releasenotes/notes/ipv6-bmc-address-start-inspection-7a72794f25eb9f19.yaml @@ -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.