392 lines
14 KiB
Diff
392 lines
14 KiB
Diff
|
From 65207b6778fa97ff450a9200c28e4770c9128854 Mon Sep 17 00:00:00 2001
|
||
|
From: James Falcon <james.falcon@canonical.com>
|
||
|
Date: Tue, 2 Jan 2024 11:29:17 -0600
|
||
|
Subject: [PATCH 1/3] fix: Correct v2 NetworkManager route rendering (#4637)
|
||
|
|
||
|
RH-Author: Cathy Avery <cavery@redhat.com>
|
||
|
RH-MergeRequest: 128: Fixes for cloud-init fails to configure DNS/search domains for network-config v1
|
||
|
RH-Jira: RHEL-27134
|
||
|
RH-Acked-by: Ani Sinha <anisinha@redhat.com>
|
||
|
RH-Acked-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
|
||
|
RH-Commit: [1/2] abfebdde6b8b914d5a7de8853beca1fe206a5b23
|
||
|
|
||
|
fix: Correct v2 NetworkManager route rendering
|
||
|
|
||
|
Because network v2 route defintions can have mixed v4 and v6 routes, we
|
||
|
need to determine the IP family per route rather than per subnet.
|
||
|
|
||
|
Similar, ensure dns-search is rendered correctly.
|
||
|
|
||
|
Fixes GH-4518
|
||
|
|
||
|
(cherry picked from commit c2c100e8c9fd8709539b3ab2b0ee34c66ba3f2f7)
|
||
|
Signed-off-by: Cathy Avery <cavery@redhat.com>
|
||
|
---
|
||
|
cloudinit/net/__init__.py | 2 +
|
||
|
cloudinit/net/network_manager.py | 87 +++++++++-------
|
||
|
tests/unittests/test_net.py | 165 ++++++++++++++++++++++++++++++-
|
||
|
3 files changed, 219 insertions(+), 35 deletions(-)
|
||
|
|
||
|
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
|
||
|
index c0888f52..65e7ff33 100644
|
||
|
--- a/cloudinit/net/__init__.py
|
||
|
+++ b/cloudinit/net/__init__.py
|
||
|
@@ -1287,6 +1287,8 @@ def subnet_is_ipv6(subnet) -> bool:
|
||
|
"""Common helper for checking network_state subnets for ipv6."""
|
||
|
# 'static6', 'dhcp6', 'ipv6_dhcpv6-stateful', 'ipv6_dhcpv6-stateless' or
|
||
|
# 'ipv6_slaac'
|
||
|
+ # This function is inappropriate for v2-based routes as routes defined
|
||
|
+ # under v2 subnets can contain ipv4 and ipv6 simultaneously
|
||
|
if subnet["type"].endswith("6") or subnet["type"] in IPV6_DYNAMIC_TYPES:
|
||
|
# This is a request either static6 type or DHCPv6.
|
||
|
return True
|
||
|
diff --git a/cloudinit/net/network_manager.py b/cloudinit/net/network_manager.py
|
||
|
index 76a0ac15..bd6e6d75 100644
|
||
|
--- a/cloudinit/net/network_manager.py
|
||
|
+++ b/cloudinit/net/network_manager.py
|
||
|
@@ -12,10 +12,15 @@ import itertools
|
||
|
import logging
|
||
|
import os
|
||
|
import uuid
|
||
|
-from typing import Optional
|
||
|
+from typing import List, Optional
|
||
|
|
||
|
from cloudinit import subp, util
|
||
|
-from cloudinit.net import is_ipv6_address, renderer, subnet_is_ipv6
|
||
|
+from cloudinit.net import (
|
||
|
+ is_ipv6_address,
|
||
|
+ is_ipv6_network,
|
||
|
+ renderer,
|
||
|
+ subnet_is_ipv6,
|
||
|
+)
|
||
|
from cloudinit.net.network_state import NetworkState
|
||
|
from cloudinit.net.sysconfig import available_nm_ifcfg_rh
|
||
|
|
||
|
@@ -158,11 +163,11 @@ class NMConnection:
|
||
|
if self.config[family]["method"] == "auto" and method == "manual":
|
||
|
return
|
||
|
|
||
|
- if (
|
||
|
- subnet_type == "ipv6_dhcpv6-stateful"
|
||
|
- or subnet_type == "ipv6_dhcpv6-stateless"
|
||
|
- or subnet_type == "ipv6_slaac"
|
||
|
- ):
|
||
|
+ if subnet_type in [
|
||
|
+ "ipv6_dhcpv6-stateful",
|
||
|
+ "ipv6_dhcpv6-stateless",
|
||
|
+ "ipv6_slaac",
|
||
|
+ ]:
|
||
|
# set ipv4 method to 'disabled' to align with sysconfig renderer.
|
||
|
self._set_default("ipv4", "method", "disabled")
|
||
|
|
||
|
@@ -174,7 +179,8 @@ class NMConnection:
|
||
|
Adds a numbered property, such as address<n> or route<n>, ensuring
|
||
|
the appropriate value gets used for <n>.
|
||
|
"""
|
||
|
-
|
||
|
+ if not self.config.has_section(section):
|
||
|
+ self.config[section] = {}
|
||
|
for index in itertools.count(1):
|
||
|
key = f"{key_prefix}{index}"
|
||
|
if not self.config.has_option(section, key):
|
||
|
@@ -189,40 +195,37 @@ class NMConnection:
|
||
|
value = subnet["address"] + "/" + str(subnet["prefix"])
|
||
|
self._add_numbered(family, "address", value)
|
||
|
|
||
|
- def _add_route(self, family, route):
|
||
|
- """
|
||
|
- Adds a ipv[46].route<n> property.
|
||
|
- """
|
||
|
-
|
||
|
+ def _add_route(self, route):
|
||
|
+ """Adds a ipv[46].route<n> property."""
|
||
|
+ # Because network v2 route definitions can have mixed v4 and v6
|
||
|
+ # routes, determine the family per route based on the gateway
|
||
|
+ family = "ipv6" if is_ipv6_network(route["gateway"]) else "ipv4"
|
||
|
value = route["network"] + "/" + str(route["prefix"])
|
||
|
if "gateway" in route:
|
||
|
value = value + "," + route["gateway"]
|
||
|
self._add_numbered(family, "route", value)
|
||
|
|
||
|
- def _add_nameserver(self, dns):
|
||
|
+ def _add_nameserver(self, dns: str) -> None:
|
||
|
"""
|
||
|
Extends the ipv[46].dns property with a name server.
|
||
|
"""
|
||
|
-
|
||
|
- # FIXME: the subnet contains IPv4 and IPv6 name server mixed
|
||
|
- # together. We might be getting an IPv6 name server while
|
||
|
- # we're dealing with an IPv4 subnet. Sort this out by figuring
|
||
|
- # out the correct family and making sure a valid section exist.
|
||
|
family = "ipv6" if is_ipv6_address(dns) else "ipv4"
|
||
|
- self._set_default(family, "method", "disabled")
|
||
|
-
|
||
|
- self._set_default(family, "dns", "")
|
||
|
- self.config[family]["dns"] = self.config[family]["dns"] + dns + ";"
|
||
|
+ if self.config.has_section(family):
|
||
|
+ self._set_default(family, "dns", "")
|
||
|
+ self.config[family]["dns"] = self.config[family]["dns"] + dns + ";"
|
||
|
|
||
|
- def _add_dns_search(self, family, dns_search):
|
||
|
+ def _add_dns_search(self, dns_search: List[str]) -> None:
|
||
|
"""
|
||
|
Extends the ipv[46].dns-search property with a name server.
|
||
|
"""
|
||
|
-
|
||
|
- self._set_default(family, "dns-search", "")
|
||
|
- self.config[family]["dns-search"] = (
|
||
|
- self.config[family]["dns-search"] + ";".join(dns_search) + ";"
|
||
|
- )
|
||
|
+ for family in ["ipv4", "ipv6"]:
|
||
|
+ if self.config.has_section(family):
|
||
|
+ self._set_default(family, "dns-search", "")
|
||
|
+ self.config[family]["dns-search"] = (
|
||
|
+ self.config[family]["dns-search"]
|
||
|
+ + ";".join(dns_search)
|
||
|
+ + ";"
|
||
|
+ )
|
||
|
|
||
|
def con_uuid(self):
|
||
|
"""
|
||
|
@@ -304,8 +307,11 @@ class NMConnection:
|
||
|
|
||
|
device_mtu = iface["mtu"]
|
||
|
ipv4_mtu = None
|
||
|
+ found_nameservers = []
|
||
|
+ found_dns_search = []
|
||
|
|
||
|
# Deal with Layer 3 configuration
|
||
|
+ use_top_level_dns = "dns" in iface
|
||
|
for subnet in iface["subnets"]:
|
||
|
family = "ipv6" if subnet_is_ipv6(subnet) else "ipv4"
|
||
|
|
||
|
@@ -315,15 +321,28 @@ class NMConnection:
|
||
|
if "gateway" in subnet:
|
||
|
self.config[family]["gateway"] = subnet["gateway"]
|
||
|
for route in subnet["routes"]:
|
||
|
- self._add_route(family, route)
|
||
|
- if "dns_nameservers" in subnet:
|
||
|
+ self._add_route(route)
|
||
|
+ if not use_top_level_dns and "dns_nameservers" in subnet:
|
||
|
for nameserver in subnet["dns_nameservers"]:
|
||
|
- self._add_nameserver(nameserver)
|
||
|
- if "dns_search" in subnet:
|
||
|
- self._add_dns_search(family, subnet["dns_search"])
|
||
|
+ found_nameservers.append(nameserver)
|
||
|
+ if not use_top_level_dns and "dns_search" in subnet:
|
||
|
+ found_dns_search.append(subnet["dns_search"])
|
||
|
if family == "ipv4" and "mtu" in subnet:
|
||
|
ipv4_mtu = subnet["mtu"]
|
||
|
|
||
|
+ # Now add our DNS search domains. We add them later because we
|
||
|
+ # only want them if an IP family has already been defined
|
||
|
+ if use_top_level_dns:
|
||
|
+ for nameserver in iface["dns"]["nameservers"]:
|
||
|
+ self._add_nameserver(nameserver)
|
||
|
+ if iface["dns"]["search"]:
|
||
|
+ self._add_dns_search(iface["dns"]["search"])
|
||
|
+ else:
|
||
|
+ for nameserver in found_nameservers:
|
||
|
+ self._add_nameserver(nameserver)
|
||
|
+ for dns_search in found_dns_search:
|
||
|
+ self._add_dns_search(dns_search)
|
||
|
+
|
||
|
# we do not want to set may-fail to false for both ipv4 and ipv6 dhcp
|
||
|
# at the at the same time. This will make the network configuration
|
||
|
# work only when both ipv4 and ipv6 dhcp succeeds. This may not be
|
||
|
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
|
||
|
index e52c2497..678ec39b 100644
|
||
|
--- a/tests/unittests/test_net.py
|
||
|
+++ b/tests/unittests/test_net.py
|
||
|
@@ -2934,9 +2934,9 @@ pre-down route del -net 10.0.0.0/8 gw 11.0.0.1 metric 3 || true
|
||
|
may-fail=false
|
||
|
address1=192.168.0.2/24
|
||
|
gateway=192.168.0.1
|
||
|
+ address2=192.168.2.10/24
|
||
|
dns=192.168.0.10;10.23.23.134;
|
||
|
dns-search=barley.maas;sacchromyces.maas;brettanomyces.maas;
|
||
|
- address2=192.168.2.10/24
|
||
|
|
||
|
"""
|
||
|
),
|
||
|
@@ -4114,6 +4114,148 @@ iface bond0 inet6 static
|
||
|
"""
|
||
|
),
|
||
|
},
|
||
|
+ "v2-mixed-routes": {
|
||
|
+ "expected_network_manager": {
|
||
|
+ "cloud-init-eth0.nmconnection": textwrap.dedent(
|
||
|
+ """\
|
||
|
+ # Generated by cloud-init. Changes will be lost.
|
||
|
+
|
||
|
+ [connection]
|
||
|
+ id=cloud-init eth0
|
||
|
+ uuid=1dd9a779-d327-56e1-8454-c65e2556c12c
|
||
|
+ autoconnect-priority=120
|
||
|
+ type=ethernet
|
||
|
+ interface-name=eth0
|
||
|
+
|
||
|
+ [user]
|
||
|
+ org.freedesktop.NetworkManager.origin=cloud-init
|
||
|
+
|
||
|
+ [ethernet]
|
||
|
+
|
||
|
+ [ipv4]
|
||
|
+ method=auto
|
||
|
+ may-fail=true
|
||
|
+ route1=169.254.42.42/32,62.210.0.1
|
||
|
+ route2=169.254.42.43/32,62.210.0.2
|
||
|
+ address1=192.168.1.20/16
|
||
|
+ dns=8.8.8.8;
|
||
|
+ dns-search=lab;home;
|
||
|
+
|
||
|
+ [ipv6]
|
||
|
+ route1=::/0,fe80::dc00:ff:fe20:186
|
||
|
+ route2=fe80::dc00:ff:fe20:188/64,fe80::dc00:ff:fe20:187
|
||
|
+ method=auto
|
||
|
+ may-fail=true
|
||
|
+ address1=2001:bc8:1210:232:dc00:ff:fe20:185/64
|
||
|
+ dns=FEDC::1;
|
||
|
+ dns-search=lab;home;
|
||
|
+
|
||
|
+ """
|
||
|
+ )
|
||
|
+ },
|
||
|
+ "yaml": textwrap.dedent(
|
||
|
+ """\
|
||
|
+ version: 2
|
||
|
+ ethernets:
|
||
|
+ eth0:
|
||
|
+ dhcp4: true
|
||
|
+ dhcp6: true
|
||
|
+ nameservers:
|
||
|
+ search: [lab, home]
|
||
|
+ addresses: [8.8.8.8, "FEDC::1"]
|
||
|
+ routes:
|
||
|
+ - to: 169.254.42.42/32
|
||
|
+ via: 62.210.0.1
|
||
|
+ - via: fe80::dc00:ff:fe20:186
|
||
|
+ to: ::/0
|
||
|
+ - to: 169.254.42.43/32
|
||
|
+ via: 62.210.0.2
|
||
|
+ - via: fe80::dc00:ff:fe20:187
|
||
|
+ to: fe80::dc00:ff:fe20:188
|
||
|
+ addresses:
|
||
|
+ - 192.168.1.20/16
|
||
|
+ - 2001:bc8:1210:232:dc00:ff:fe20:185/64
|
||
|
+ """
|
||
|
+ ),
|
||
|
+ },
|
||
|
+ "v2-dns-no-if-ips": {
|
||
|
+ "expected_network_manager": {
|
||
|
+ "cloud-init-eth0.nmconnection": textwrap.dedent(
|
||
|
+ """\
|
||
|
+ # Generated by cloud-init. Changes will be lost.
|
||
|
+
|
||
|
+ [connection]
|
||
|
+ id=cloud-init eth0
|
||
|
+ uuid=1dd9a779-d327-56e1-8454-c65e2556c12c
|
||
|
+ autoconnect-priority=120
|
||
|
+ type=ethernet
|
||
|
+ interface-name=eth0
|
||
|
+
|
||
|
+ [user]
|
||
|
+ org.freedesktop.NetworkManager.origin=cloud-init
|
||
|
+
|
||
|
+ [ethernet]
|
||
|
+
|
||
|
+ [ipv4]
|
||
|
+ method=auto
|
||
|
+ may-fail=true
|
||
|
+ dns=8.8.8.8;
|
||
|
+ dns-search=lab;home;
|
||
|
+
|
||
|
+ [ipv6]
|
||
|
+ method=auto
|
||
|
+ may-fail=true
|
||
|
+ dns=FEDC::1;
|
||
|
+ dns-search=lab;home;
|
||
|
+
|
||
|
+ """
|
||
|
+ )
|
||
|
+ },
|
||
|
+ "yaml": textwrap.dedent(
|
||
|
+ """\
|
||
|
+ version: 2
|
||
|
+ ethernets:
|
||
|
+ eth0:
|
||
|
+ dhcp4: true
|
||
|
+ dhcp6: true
|
||
|
+ nameservers:
|
||
|
+ search: [lab, home]
|
||
|
+ addresses: [8.8.8.8, "FEDC::1"]
|
||
|
+ """
|
||
|
+ ),
|
||
|
+ },
|
||
|
+ "v2-dns-no-dhcp": {
|
||
|
+ "expected_network_manager": {
|
||
|
+ "cloud-init-eth0.nmconnection": textwrap.dedent(
|
||
|
+ """\
|
||
|
+ # Generated by cloud-init. Changes will be lost.
|
||
|
+
|
||
|
+ [connection]
|
||
|
+ id=cloud-init eth0
|
||
|
+ uuid=1dd9a779-d327-56e1-8454-c65e2556c12c
|
||
|
+ autoconnect-priority=120
|
||
|
+ type=ethernet
|
||
|
+ interface-name=eth0
|
||
|
+
|
||
|
+ [user]
|
||
|
+ org.freedesktop.NetworkManager.origin=cloud-init
|
||
|
+
|
||
|
+ [ethernet]
|
||
|
+
|
||
|
+ """
|
||
|
+ )
|
||
|
+ },
|
||
|
+ "yaml": textwrap.dedent(
|
||
|
+ """\
|
||
|
+ version: 2
|
||
|
+ ethernets:
|
||
|
+ eth0:
|
||
|
+ nameservers:
|
||
|
+ search: [lab, home]
|
||
|
+ addresses: [8.8.8.8, "FEDC::1"]
|
||
|
+ """
|
||
|
+ ),
|
||
|
+ },
|
||
|
}
|
||
|
|
||
|
|
||
|
@@ -6214,6 +6356,27 @@ class TestNetworkManagerRendering(CiTestCase):
|
||
|
entry[self.expected_name], self.expected_conf_d, found
|
||
|
)
|
||
|
|
||
|
+ def test_v2_mixed_routes(self):
|
||
|
+ entry = NETWORK_CONFIGS["v2-mixed-routes"]
|
||
|
+ found = self._render_and_read(network_config=yaml.load(entry["yaml"]))
|
||
|
+ self._compare_files_to_expected(
|
||
|
+ entry[self.expected_name], self.expected_conf_d, found
|
||
|
+ )
|
||
|
+
|
||
|
+ def test_v2_dns_no_ips(self):
|
||
|
+ entry = NETWORK_CONFIGS["v2-dns-no-if-ips"]
|
||
|
+ found = self._render_and_read(network_config=yaml.load(entry["yaml"]))
|
||
|
+ self._compare_files_to_expected(
|
||
|
+ entry[self.expected_name], self.expected_conf_d, found
|
||
|
+ )
|
||
|
+
|
||
|
+ def test_v2_dns_no_dhcp(self):
|
||
|
+ entry = NETWORK_CONFIGS["v2-dns-no-dhcp"]
|
||
|
+ found = self._render_and_read(network_config=yaml.load(entry["yaml"]))
|
||
|
+ self._compare_files_to_expected(
|
||
|
+ entry[self.expected_name], self.expected_conf_d, found
|
||
|
+ )
|
||
|
+
|
||
|
|
||
|
@mock.patch(
|
||
|
"cloudinit.net.is_openvswitch_internal_interface",
|
||
|
--
|
||
|
2.41.0
|
||
|
|