openstack/ironic-python-agent
Code Issues Proposed changes

Merge "Bind to interface routable to the ironic host, not a random one"

Browse Source
This commit is contained in:
Jenkins
2016-03-21 19:17:48 +00:00
committed by Gerrit Code Review
parent 218f629183 6829d34c15
commit c5531af3b3
6 changed files with 176 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
imagebuild/tinyipa/build_files/finalreqs.lst
View File

@@ -3,6 +3,7 @@ coreutils.tcz
dmidecode.tcz dmidecode.tcz
gdisk.tcz gdisk.tcz
hdparm.tcz hdparm.tcz
iproute2.tcz
parted.tcz parted.tcz
python.tcz python.tcz
raid-dm-3.16.6-tinycore64.tcz raid-dm-3.16.6-tinycore64.tcz

77
ironic_python_agent/agent.py
View File

@@ -15,11 +15,14 @@
import os import os
import random import random
import select import select
import socket
import threading import threading
import time import time
from oslo_concurrency import processutils
from oslo_log import log from oslo_log import log
import pkg_resources import pkg_resources
from six.moves.urllib import parse as urlparse
from stevedore import extension from stevedore import extension
from wsgiref import simple_server from wsgiref import simple_server
@@ -30,6 +33,7 @@ from ironic_python_agent.extensions import base
from ironic_python_agent import hardware from ironic_python_agent import hardware
from ironic_python_agent import inspector from ironic_python_agent import inspector
from ironic_python_agent import ironic_api_client from ironic_python_agent import ironic_api_client
from ironic_python_agent import utils
LOG = log.getLogger(__name__) LOG = log.getLogger(__name__)
@@ -183,6 +187,21 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
version=self.version version=self.version
) )
def _get_route_source(self, dest):
"""Get the IP address to send packages to destination."""
try:
out, _err = utils.execute('ip', 'route', 'get', dest)
except (EnvironmentError, processutils.ProcessExecutionError) as e:
LOG.warning('Cannot get route to host %(dest)s: %(err)s',
{'dest': dest, 'err': e})
return
try:
return out.strip().split('\n')[0].split('src')[1].strip()
except IndexError:
LOG.warning('No route to host %(dest)s, route record: %(rec)s',
{'dest': dest, 'rec': out})
def set_agent_advertise_addr(self): def set_agent_advertise_addr(self):
"""Set advertised IP address for the agent, if not already set. """Set advertised IP address for the agent, if not already set.
@@ -190,52 +209,38 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
find a better one. If the agent's network interface is None, replace find a better one. If the agent's network interface is None, replace
that as well. that as well.
:raises: LookupAgentInterfaceError if a valid network interface cannot
be found.
:raises: LookupAgentIPError if an IP address could not be found :raises: LookupAgentIPError if an IP address could not be found
""" """
if self.advertise_address[0] is not None: if self.advertise_address[0] is not None:
return return
if self.network_interface is None: found_ip = None
ifaces = self.get_agent_network_interfaces() if self.network_interface is not None:
# TODO(dtantsur): deprecate this
found_ip = hardware.dispatch_to_managers('get_ipv4_addr',
self.network_interface)
else: else:
ifaces = [self.network_interface] url = urlparse.urlparse(self.api_url)
ironic_host = url.hostname
# Try resolving it in case it's not an IP address
try:
ironic_host = socket.gethostbyname(ironic_host)
except socket.gaierror:
LOG.debug('Count not resolve %s, maybe no DNS', ironic_host)
attempts = 0 for attempt in range(self.ip_lookup_attempts):
while (attempts < self.ip_lookup_attempts): found_ip = self._get_route_source(ironic_host)
for iface in ifaces: if found_ip:
found_ip = hardware.dispatch_to_managers('get_ipv4_addr', break
iface)
if found_ip is not None:
self.advertise_address = (found_ip,
self.advertise_address[1])
self.network_interface = iface
return
attempts += 1
time.sleep(self.ip_lookup_sleep)
raise errors.LookupAgentIPError('Agent could not find a valid IP ' time.sleep(self.ip_lookup_sleep)
'address.')
def get_agent_network_interfaces(self): if found_ip:
"""Get a list of all network interfaces available. self.advertise_address = (found_ip,
self.advertise_address[1])
Excludes loopback connections.
:returns: list of network interfaces available.
:raises: LookupAgentInterfaceError if a valid interface could not
be found.
"""
iface_list = [iface.serialize()['name'] for iface in
hardware.dispatch_to_managers('list_network_interfaces')]
iface_list = [name for name in iface_list if 'lo' not in name]
if len(iface_list) == 0:
raise errors.LookupAgentInterfaceError('Agent could not find a '
'valid network interface.')
else: else:
return iface_list raise errors.LookupAgentIPError('Agent could not find a valid IP '
'address.')
def get_node_uuid(self): def get_node_uuid(self):
"""Get UUID for Ironic node. """Get UUID for Ironic node.

9
ironic_python_agent/errors.py
View File

@@ -136,15 +136,6 @@ class LookupAgentIPError(IronicAPIError):
super(LookupAgentIPError, self).__init__(details) super(LookupAgentIPError, self).__init__(details)
class LookupAgentInterfaceError(IronicAPIError):
"""Error raised when agent interface lookup fails."""
message = 'Error finding network interface for Ironic Agent'
def __init__(self, details):
super(LookupAgentInterfaceError, self).__init__(details)
class ImageDownloadError(RESTError): class ImageDownloadError(RESTError):
"""Error raised when an image cannot be downloaded.""" """Error raised when an image cannot be downloaded."""

179
ironic_python_agent/tests/unit/test_agent.py
View File

@@ -13,9 +13,11 @@
# limitations under the License. # limitations under the License.
import json import json
import socket
import time import time
import mock import mock
from oslo_concurrency import processutils
from oslo_config import cfg from oslo_config import cfg
from oslotest import base as test_base from oslotest import base as test_base
import pkg_resources import pkg_resources
@@ -28,6 +30,7 @@ from ironic_python_agent import errors
from ironic_python_agent.extensions import base from ironic_python_agent.extensions import base
from ironic_python_agent import hardware from ironic_python_agent import hardware
from ironic_python_agent import inspector from ironic_python_agent import inspector
from ironic_python_agent import utils
EXPECTED_ERROR = RuntimeError('command execution failed') EXPECTED_ERROR = RuntimeError('command execution failed')
@@ -229,56 +232,6 @@ class TestBaseAgent(test_base.BaseTestCase):
self.agent.heartbeater.start.assert_called_once_with() self.agent.heartbeater.start.assert_called_once_with()
@mock.patch('os.read')
@mock.patch('select.poll')
@mock.patch('time.sleep', return_value=None)
@mock.patch.object(hardware.GenericHardwareManager,
'list_network_interfaces')
@mock.patch.object(hardware.GenericHardwareManager, 'get_ipv4_addr')
def test_ipv4_lookup(self,
mock_get_ipv4,
mock_list_net,
mock_time_sleep,
mock_poll,
mock_read):
homeless_agent = agent.IronicPythonAgent('https://fake_api.example.'
'org:8081/',
(None, 9990),
('192.0.2.1', 9999),
3,
10,
None,
300,
1,
'agent_ipmitool',
False)
mock_poll.return_value.poll.return_value = True
mock_read.return_value = 'a'
# Can't find network interfaces, and therefore can't find IP
mock_list_net.return_value = []
mock_get_ipv4.return_value = None
self.assertRaises(errors.LookupAgentInterfaceError,
homeless_agent.set_agent_advertise_addr)
# Can look up network interfaces, but not IP. Network interface not
# set, because no interface yields an IP.
mock_ifaces = [hardware.NetworkInterface('eth0', '00:00:00:00:00:00'),
hardware.NetworkInterface('eth1', '00:00:00:00:00:01')]
mock_list_net.return_value = mock_ifaces
self.assertRaises(errors.LookupAgentIPError,
homeless_agent.set_agent_advertise_addr)
self.assertEqual(6, mock_get_ipv4.call_count)
self.assertIsNone(homeless_agent.network_interface)
# First interface eth0 has no IP, second interface eth1 has an IP
mock_get_ipv4.side_effect = [None, '1.1.1.1']
homeless_agent.heartbeater.run()
self.assertEqual(('1.1.1.1', 9990), homeless_agent.advertise_address)
self.assertEqual('eth1', homeless_agent.network_interface)
def test_async_command_success(self): def test_async_command_success(self):
result = base.AsyncCommandResult('foo_command', {'fail': False}, result = base.AsyncCommandResult('foo_command', {'fail': False},
foo_execute) foo_execute)
@@ -383,3 +336,129 @@ class TestAgentStandalone(test_base.BaseTestCase):
self.assertFalse(self.agent.heartbeater.called) self.assertFalse(self.agent.heartbeater.called)
self.assertFalse(self.agent.api_client.lookup_node.called) self.assertFalse(self.agent.api_client.lookup_node.called)
@mock.patch.object(socket, 'gethostbyname', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
class TestAdvertiseAddress(test_base.BaseTestCase):
def setUp(self):
super(TestAdvertiseAddress, self).setUp()
self.agent = agent.IronicPythonAgent(
api_url='https://fake_api.example.org:8081/',
advertise_address=(None, 9990),
listen_address=('0.0.0.0', 9999),
ip_lookup_attempts=5,
ip_lookup_sleep=10,
network_interface=None,
lookup_timeout=300,
lookup_interval=1,
driver_name='agent_ipmitool',
standalone=False)
def test_advertise_address_provided(self, mock_exec, mock_gethostbyname):
self.agent.advertise_address = ('1.2.3.4', 9990)
self.agent.set_agent_advertise_addr()
self.assertEqual(('1.2.3.4', 9990), self.agent.advertise_address)
self.assertFalse(mock_exec.called)
self.assertFalse(mock_gethostbyname.called)
@mock.patch.object(hardware.GenericHardwareManager, 'get_ipv4_addr',
autospec=True)
def test_with_network_interface(self, mock_get_ipv4, mock_exec,
mock_gethostbyname):
self.agent.network_interface = 'em1'
mock_get_ipv4.return_value = '1.2.3.4'
self.agent.set_agent_advertise_addr()
self.assertEqual(('1.2.3.4', 9990), self.agent.advertise_address)
mock_get_ipv4.assert_called_once_with(mock.ANY, 'em1')
self.assertFalse(mock_exec.called)
self.assertFalse(mock_gethostbyname.called)
@mock.patch.object(hardware.GenericHardwareManager, 'get_ipv4_addr',
autospec=True)
def test_with_network_interface_failed(self, mock_get_ipv4, mock_exec,
mock_gethostbyname):
self.agent.network_interface = 'em1'
mock_get_ipv4.return_value = None
self.assertRaises(errors.LookupAgentIPError,
self.agent.set_agent_advertise_addr)
mock_get_ipv4.assert_called_once_with(mock.ANY, 'em1')
self.assertFalse(mock_exec.called)
self.assertFalse(mock_gethostbyname.called)
def test_route_with_ip(self, mock_exec, mock_gethostbyname):
self.agent.api_url = 'http://1.2.1.2:8081/v1'
mock_gethostbyname.side_effect = socket.gaierror()
mock_exec.return_value = (
"""1.2.1.2 via 192.168.122.1 dev eth0 src 192.168.122.56
cache """,
""
)
self.agent.set_agent_advertise_addr()
self.assertEqual(('192.168.122.56', 9990),
self.agent.advertise_address)
mock_exec.assert_called_once_with('ip', 'route', 'get', '1.2.1.2')
mock_gethostbyname.assert_called_once_with('1.2.1.2')
def test_route_with_host(self, mock_exec, mock_gethostbyname):
mock_gethostbyname.return_value = '1.2.1.2'
mock_exec.return_value = (
"""1.2.1.2 via 192.168.122.1 dev eth0 src 192.168.122.56
cache """,
""
)
self.agent.set_agent_advertise_addr()
self.assertEqual(('192.168.122.56', 9990),
self.agent.advertise_address)
mock_exec.assert_called_once_with('ip', 'route', 'get', '1.2.1.2')
mock_gethostbyname.assert_called_once_with('fake_api.example.org')
@mock.patch.object(time, 'sleep', autospec=True)
def test_route_retry(self, mock_sleep, mock_exec, mock_gethostbyname):
mock_gethostbyname.return_value = '1.2.1.2'
mock_exec.side_effect = [
processutils.ProcessExecutionError('boom'),
(
"Error: some error text",
""
),
(
"""1.2.1.2 via 192.168.122.1 dev eth0 src 192.168.122.56
cache """,
""
)
]
self.agent.set_agent_advertise_addr()
self.assertEqual(('192.168.122.56', 9990),
self.agent.advertise_address)
mock_exec.assert_called_with('ip', 'route', 'get', '1.2.1.2')
mock_gethostbyname.assert_called_once_with('fake_api.example.org')
mock_sleep.assert_called_with(10)
self.assertEqual(3, mock_exec.call_count)
self.assertEqual(2, mock_sleep.call_count)
@mock.patch.object(time, 'sleep', autospec=True)
def test_route_failed(self, mock_sleep, mock_exec, mock_gethostbyname):
mock_gethostbyname.return_value = '1.2.1.2'
mock_exec.side_effect = processutils.ProcessExecutionError('boom')
self.assertRaises(errors.LookupAgentIPError,
self.agent.set_agent_advertise_addr)
mock_exec.assert_called_with('ip', 'route', 'get', '1.2.1.2')
mock_gethostbyname.assert_called_once_with('fake_api.example.org')
self.assertEqual(5, mock_exec.call_count)
self.assertEqual(5, mock_sleep.call_count)

1
ironic_python_agent/tests/unit/test_errors.py
View File

@@ -103,7 +103,6 @@ class TestErrors(test_base.BaseTestCase):
(errors.HeartbeatError(DETAILS), SAME_DETAILS), (errors.HeartbeatError(DETAILS), SAME_DETAILS),
(errors.LookupNodeError(DETAILS), SAME_DETAILS), (errors.LookupNodeError(DETAILS), SAME_DETAILS),
(errors.LookupAgentIPError(DETAILS), SAME_DETAILS), (errors.LookupAgentIPError(DETAILS), SAME_DETAILS),
(errors.LookupAgentInterfaceError(DETAILS), SAME_DETAILS),
(errors.ImageDownloadError('image_id', DETAILS), (errors.ImageDownloadError('image_id', DETAILS),
DIFF_CL_DETAILS), DIFF_CL_DETAILS),
(errors.ImageChecksumError( (errors.ImageChecksumError(

5
releasenotes/notes/advertise-address-c3b152fe475fb539.yaml Normal file
View File

@@ -0,0 +1,5 @@
---
fixes:
- IPA will now advertise IP address via which the Ironic API is routed
instead of using the first available.
See https://bugs.launchpad.net/ironic-python-agent/+bug/1558956.