From c760678ccc33fbf26b52c6c3763778b60ed02176 Mon Sep 17 00:00:00 2001 From: Paul Peereboom Date: Tue, 24 Dec 2019 14:16:32 +0100 Subject: [PATCH] Fix the interface filenames for Red Hat amphora images Code was not using the correct filenames for the 'route', 'route6', 'rule' and 'rule6' files on Red Hat images. Changed to use config option 'agent_server_network_file' if it's specified, else the file of the correct name, and added unit tests for each. Conflicts: octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py Change-Id: I335287da66524d026f0c42086d885b478c568bbd Task: 37881 Story: 2007051 (cherry picked from commit 93cd9fc075b3d77490090b89e449f18938710b26) --- .../backends/agent/api_server/osutils.py | 52 ++++-- .../backends/agent/api_server/test_osutils.py | 175 +++++++++++++++++- ...ages-interface-files-5ba1be40c65940d9.yaml | 8 + 3 files changed, 220 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/fix-redhat-amphora-images-interface-files-5ba1be40c65940d9.yaml diff --git a/octavia/amphorae/backends/agent/api_server/osutils.py b/octavia/amphorae/backends/agent/api_server/osutils.py index 4f5431c938..cf5109eb39 100644 --- a/octavia/amphorae/backends/agent/api_server/osutils.py +++ b/octavia/amphorae/backends/agent/api_server/osutils.py @@ -357,24 +357,30 @@ class RH(BaseOS): name = self._map_package_name(package_name) return "rpm -q --queryformat %{{VERSION}} {name}".format(name=name) - def get_network_interface_file(self, interface): + @staticmethod + def _get_network_interface_file(prefix, interface): if CONF.amphora_agent.agent_server_network_file: return CONF.amphora_agent.agent_server_network_file if CONF.amphora_agent.agent_server_network_dir: - return os.path.join(CONF.amphora_agent.agent_server_network_dir, - 'ifcfg-' + interface) - network_dir = consts.RH_AMP_NET_DIR_TEMPLATE.format( - netns=consts.AMPHORA_NAMESPACE) - return os.path.join(network_dir, 'ifcfg-' + interface) + network_dir = CONF.amphora_agent.agent_server_network_dir + else: + network_dir = consts.RH_AMP_NET_DIR_TEMPLATE.format( + netns=consts.AMPHORA_NAMESPACE) + return os.path.join(network_dir, prefix + interface) + + def get_network_interface_file(self, interface): + return self._get_network_interface_file('ifcfg-', interface) def get_alias_network_interface_file(self, interface): return self.get_network_interface_file(interface + ':0') - def get_static_routes_interface_file(self, interface): - return self.get_network_interface_file('route-' + interface) + def get_static_routes_interface_file(self, interface, version): + route = 'route6-' if version == 6 else 'route-' + return self._get_network_interface_file(route, interface) - def get_route_rules_interface_file(self, interface): - return self.get_network_interface_file('rule-' + interface) + def get_route_rules_interface_file(self, interface, version): + rule = 'rule6-' if version == 6 else 'rule-' + return self._get_network_interface_file(rule, interface) def get_network_path(self): return '/etc/sysconfig/network-scripts' @@ -432,7 +438,8 @@ class RH(BaseOS): render_host_routes, template_vip_alias) routes_interface_file_path = ( - self.get_static_routes_interface_file(primary_interface)) + self.get_static_routes_interface_file(primary_interface, + ip.version)) template_routes = j2_env.get_template(self.ROUTE_ETH_X_CONF) self.write_static_routes_interface_file( @@ -443,7 +450,8 @@ class RH(BaseOS): if (CONF.controller_worker.loadbalancer_topology == consts.TOPOLOGY_SINGLE): route_rules_interface_file_path = ( - self.get_route_rules_interface_file(primary_interface)) + self.get_route_rules_interface_file(primary_interface, + ip.version)) template_rules = j2_env.get_template(self.RULE_ETH_X_CONF) self.write_static_routes_interface_file( @@ -493,16 +501,32 @@ class RH(BaseOS): if fixed_ips: host_routes = [] + host_routes_ipv6 = [] for fixed_ip in fixed_ips: - host_routes.extend(self.get_host_routes(fixed_ip)) + ip_addr = fixed_ip['ip_address'] + ip = ipaddress.ip_address(ip_addr if isinstance( + ip_addr, six.text_type) else six.u(ip_addr)) + if ip.version == 6: + host_routes_ipv6.extend(self.get_host_routes(fixed_ip)) + else: + host_routes.extend(self.get_host_routes(fixed_ip)) routes_interface_file_path = ( - self.get_static_routes_interface_file(netns_interface)) + self.get_static_routes_interface_file(netns_interface, 4)) template_routes = j2_env.get_template(self.ROUTE_ETH_X_CONF) self.write_static_routes_interface_file( routes_interface_file_path, netns_interface, host_routes, template_routes, None, None, None) + + routes_interface_file_path_ipv6 = ( + self.get_static_routes_interface_file(netns_interface, 6)) + template_routes = j2_env.get_template(self.ROUTE_ETH_X_CONF) + + self.write_static_routes_interface_file( + routes_interface_file_path_ipv6, netns_interface, + host_routes_ipv6, template_routes, None, None, None) + self._write_ifup_ifdown_local_scripts_if_possible() @classmethod diff --git a/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py b/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py index 1a8409b33d..2127d2dfb1 100644 --- a/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py +++ b/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py @@ -160,6 +160,114 @@ class TestOSUtils(base.TestCase): get_network_interface_file(consts.NETNS_PRIMARY_INTERFACE)) self.assertEqual(ubuntu_real_nic_path, ubuntu_interface_file) + def _test_RH_get_static_routes_interface_file(self, version): + conf = self.useFixture(oslo_fixture.Config(config.cfg.CONF)) + + fake_agent_server_network_dir = "/path/to/interface" + fake_agent_server_network_file = "/path/to/interfaces_file" + + route = 'route6' if version == 6 else 'route' + rh_route_name = '{route}-{nic}'.format( + route=route, nic=consts.NETNS_PRIMARY_INTERFACE) + rh_fake_route_path = os.path.join(fake_agent_server_network_dir, + rh_route_name) + rh_real_route_path = os.path.join( + consts.RH_AMP_NET_DIR_TEMPLATE.format( + netns=consts.AMPHORA_NAMESPACE), + rh_route_name) + + # Check that agent_server_network_file is returned, when provided + conf.config(group="amphora_agent", + agent_server_network_file=fake_agent_server_network_file) + + rh_route_file = ( + self.rh_os_util. + get_static_routes_interface_file(consts.NETNS_PRIMARY_INTERFACE, + version)) + self.assertEqual(fake_agent_server_network_file, rh_route_file) + + # Check that agent_server_network_dir is used, when provided + conf.config(group="amphora_agent", agent_server_network_file=None) + conf.config(group="amphora_agent", + agent_server_network_dir=fake_agent_server_network_dir) + + rh_route_file = ( + self.rh_os_util. + get_static_routes_interface_file(consts.NETNS_PRIMARY_INTERFACE, + version)) + self.assertEqual(rh_fake_route_path, rh_route_file) + + # Check When neither agent_server_network_dir or + # agent_server_network_file where provided. + conf.config(group="amphora_agent", agent_server_network_file=None) + conf.config(group="amphora_agent", agent_server_network_dir=None) + + rh_route_file = ( + self.rh_os_util. + get_static_routes_interface_file(consts.NETNS_PRIMARY_INTERFACE, + version)) + self.assertEqual(rh_real_route_path, rh_route_file) + + def test_RH_get_static_routes_interface_file(self): + self._test_RH_get_static_routes_interface_file(4) + + def test_RH_get_static_routes_interface_file_ipv6(self): + self._test_RH_get_static_routes_interface_file(6) + + def _test_RH_get_route_rules_interface_file(self, version): + conf = self.useFixture(oslo_fixture.Config(config.cfg.CONF)) + + fake_agent_server_network_dir = "/path/to/interface" + fake_agent_server_network_file = "/path/to/interfaces_file" + + rule = 'rule6' if version == 6 else 'rule' + rh_route_rules_name = '{rule}-{nic}'.format( + rule=rule, nic=consts.NETNS_PRIMARY_INTERFACE) + rh_fake_route_rules_path = os.path.join(fake_agent_server_network_dir, + rh_route_rules_name) + rh_real_route_rules_path = os.path.join( + consts.RH_AMP_NET_DIR_TEMPLATE.format( + netns=consts.AMPHORA_NAMESPACE), + rh_route_rules_name) + + # Check that agent_server_network_file is returned, when provided + conf.config(group="amphora_agent", + agent_server_network_file=fake_agent_server_network_file) + + rh_route_rules_file = ( + self.rh_os_util. + get_route_rules_interface_file(consts.NETNS_PRIMARY_INTERFACE, + version)) + self.assertEqual(fake_agent_server_network_file, rh_route_rules_file) + + # Check that agent_server_network_dir is used, when provided + conf.config(group="amphora_agent", agent_server_network_file=None) + conf.config(group="amphora_agent", + agent_server_network_dir=fake_agent_server_network_dir) + + rh_route_rules_file = ( + self.rh_os_util. + get_route_rules_interface_file(consts.NETNS_PRIMARY_INTERFACE, + version)) + self.assertEqual(rh_fake_route_rules_path, rh_route_rules_file) + + # Check When neither agent_server_network_dir or + # agent_server_network_file where provided. + conf.config(group="amphora_agent", agent_server_network_file=None) + conf.config(group="amphora_agent", agent_server_network_dir=None) + + rh_route_rules_file = ( + self.rh_os_util. + get_route_rules_interface_file(consts.NETNS_PRIMARY_INTERFACE, + version)) + self.assertEqual(rh_real_route_rules_path, rh_route_rules_file) + + def test_RH_get_route_rules_interface_file(self): + self._test_RH_get_route_rules_interface_file(4) + + def test_RH_get_route_rules_interface_file_ipv6(self): + self._test_RH_get_route_rules_interface_file(6) + def test_cmd_get_version_of_installed_package(self): package_name = 'foo' ubuntu_cmd = "dpkg-query -W -f=${{Version}} {name}".format( @@ -294,13 +402,33 @@ class TestOSUtils(base.TestCase): ) def test_write_port_interface_file(self): + FIXED_IP = u'192.0.2.2' + NEXTHOP = u'192.0.2.1' + DEST = u'198.51.100.0/24' + host_routes = [ + {'nexthop': NEXTHOP, + 'destination': str(ipaddress.ip_network(DEST))} + ] + FIXED_IP_IPV6 = u'2001:db8::2' + NEXTHOP_IPV6 = u'2001:db8::1' + DEST_IPV6 = u'2001:db8:51:100::/64' + host_routes_ipv6 = [ + {'nexthop': NEXTHOP_IPV6, + 'destination': str(ipaddress.ip_network(DEST_IPV6))} + ] + ip_addr = {'ip_address': FIXED_IP, 'host_routes': host_routes} + ipv6_addr = {'ip_address': FIXED_IP_IPV6, + 'host_routes': host_routes_ipv6} + netns_interface = 'eth1234' MTU = 1450 - fixed_ips = [] + fixed_ips = [ip_addr, ipv6_addr] path = 'mypath' mock_template = mock.MagicMock() mock_open = self.useFixture(test_utils.OpenFixture(path)).mock_open mock_gen_text = mock.MagicMock() + mock_local_scripts = mock.MagicMock() + mock_wr_fi = mock.MagicMock() with mock.patch('os.open'), mock.patch.object( os, 'fdopen', mock_open), mock.patch.object( @@ -315,6 +443,51 @@ class TestOSUtils(base.TestCase): mock_gen_text.assert_called_once_with( netns_interface, fixed_ips, MTU, mock_template) + mock_gen_text.reset_mock() + + with mock.patch('os.open'), mock.patch.object( + os, 'fdopen', mock_open), mock.patch.object( + osutils.BaseOS, '_generate_network_file_text', + mock_gen_text), mock.patch.object( + osutils.RH, '_write_ifup_ifdown_local_scripts_if_possible', + mock_local_scripts), mock.patch.object( + osutils.RH, 'write_static_routes_interface_file', mock_wr_fi): + self.rh_os_util.write_port_interface_file( + netns_interface=netns_interface, + fixed_ips=fixed_ips, + mtu=MTU, + interface_file_path=path, + template_port=mock_template) + + rh_route_name = 'route-{nic}'.format(nic=netns_interface) + rh_real_route_path = os.path.join( + consts.RH_AMP_NET_DIR_TEMPLATE.format( + netns=consts.AMPHORA_NAMESPACE), + rh_route_name) + rh_route_name_ipv6 = 'route6-{nic}'.format(nic=netns_interface) + rh_real_route_path_ipv6 = os.path.join( + consts.RH_AMP_NET_DIR_TEMPLATE.format( + netns=consts.AMPHORA_NAMESPACE), + rh_route_name_ipv6) + + exp_routes = [ + {'network': ipaddress.ip_network(DEST), 'gw': NEXTHOP} + ] + exp_routes_ipv6 = [ + {'network': ipaddress.ip_network(DEST_IPV6), 'gw': NEXTHOP_IPV6} + ] + expected_calls = [ + mock.call(rh_real_route_path, netns_interface, + exp_routes, mock.ANY, None, None, None), + mock.call(rh_real_route_path_ipv6, netns_interface, + exp_routes_ipv6, mock.ANY, None, None, None)] + + mock_gen_text.assert_called_once_with( + netns_interface, fixed_ips, MTU, mock_template) + self.assertEqual(2, mock_wr_fi.call_count) + mock_wr_fi.assert_has_calls(expected_calls) + mock_local_scripts.assert_called_once() + @mock.patch('shutil.copy2') @mock.patch('os.makedirs') @mock.patch('shutil.copytree') diff --git a/releasenotes/notes/fix-redhat-amphora-images-interface-files-5ba1be40c65940d9.yaml b/releasenotes/notes/fix-redhat-amphora-images-interface-files-5ba1be40c65940d9.yaml new file mode 100644 index 0000000000..f1eacc91af --- /dev/null +++ b/releasenotes/notes/fix-redhat-amphora-images-interface-files-5ba1be40c65940d9.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixed code that configured the CentOS/Red Hat amphora images to use the + correct names for the network 'ifcfg' files for static routes and + routing rules. It was using the wrong name for the routes file, + and did not support IPv6 in either file. For more information, see + https://storyboard.openstack.org/#!/story/2007051