- Fix Python 3.12 unit test failures (#2099)

- net/nm: check for presence of ifcfg files when nm connection
 files are absent (#4645)
- tests/unittests: add a new unit test for network manager net
 activator (#4672)
This commit is contained in:
eabdullin 2024-01-25 13:31:59 +03:00
parent a04da1edff
commit 095ded93e0
4 changed files with 379 additions and 2 deletions

View File

@ -0,0 +1,120 @@
From e3f1ec3f57cc1f5eacff9506d285007966414481 Mon Sep 17 00:00:00 2001
From: James Falcon <james.falcon@canonical.com>
Date: Mon, 27 Mar 2023 15:02:41 -0500
Subject: [PATCH] Fix Python 3.12 unit test failures (#2099)
---
tests/unittests/net/test_dhcp.py | 2 +-
tests/unittests/test_util.py | 38 +++++++++++++++++---------------
2 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/tests/unittests/net/test_dhcp.py b/tests/unittests/net/test_dhcp.py
index 2b680e1fa4a..4c30dd64ec7 100644
--- a/tests/unittests/net/test_dhcp.py
+++ b/tests/unittests/net/test_dhcp.py
@@ -705,7 +705,7 @@ class TestEphemeralDhcpNoNetworkSetup(ResponsesTestCase):
) as lease:
self.assertEqual(fake_lease, lease)
# Ensure that dhcp discovery occurs
- m_dhcp.called_once_with()
+ m_dhcp.assert_called_once()
@pytest.mark.parametrize(
diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
index 865f202aa86..e5243ef3725 100644
--- a/tests/unittests/test_util.py
+++ b/tests/unittests/test_util.py
@@ -854,64 +854,66 @@ class TestBlkid(CiTestCase):
)
-@mock.patch("cloudinit.subp.which")
-@mock.patch("cloudinit.subp.subp")
+@mock.patch("cloudinit.util.subp.which")
+@mock.patch("cloudinit.util.subp.subp")
class TestUdevadmSettle(CiTestCase):
- def test_with_no_params(self, m_which, m_subp):
+ def test_with_no_params(self, m_subp, m_which):
"""called with no parameters."""
m_which.side_effect = lambda m: m in ("udevadm",)
util.udevadm_settle()
- m_subp.called_once_with(mock.call(["udevadm", "settle"]))
+ m_subp.assert_called_once_with(["udevadm", "settle"])
- def test_udevadm_not_present(self, m_which, m_subp):
+ def test_udevadm_not_present(self, m_subp, m_which):
"""where udevadm program does not exist should not invoke subp."""
m_which.side_effect = lambda m: m in ("",)
util.udevadm_settle()
- m_subp.called_once_with(["which", "udevadm"])
+ m_which.assert_called_once_with("udevadm")
+ m_subp.assert_not_called()
- def test_with_exists_and_not_exists(self, m_which, m_subp):
+ def test_with_exists_and_not_exists(self, m_subp, m_which):
"""with exists=file where file does not exist should invoke subp."""
m_which.side_effect = lambda m: m in ("udevadm",)
mydev = self.tmp_path("mydev")
util.udevadm_settle(exists=mydev)
- m_subp.called_once_with(
+ m_subp.assert_called_once_with(
["udevadm", "settle", "--exit-if-exists=%s" % mydev]
)
- def test_with_exists_and_file_exists(self, m_which, m_subp):
+ def test_with_exists_and_file_exists(self, m_subp, m_which):
"""with exists=file where file does exist should only invoke subp
once for 'which' call."""
m_which.side_effect = lambda m: m in ("udevadm",)
mydev = self.tmp_path("mydev")
util.write_file(mydev, "foo\n")
util.udevadm_settle(exists=mydev)
- m_subp.called_once_with(["which", "udevadm"])
+ m_which.assert_called_once_with("udevadm")
+ m_subp.assert_not_called()
- def test_with_timeout_int(self, m_which, m_subp):
+ def test_with_timeout_int(self, m_subp, m_which):
"""timeout can be an integer."""
m_which.side_effect = lambda m: m in ("udevadm",)
timeout = 9
util.udevadm_settle(timeout=timeout)
- m_subp.called_once_with(
+ m_subp.assert_called_once_with(
["udevadm", "settle", "--timeout=%s" % timeout]
)
- def test_with_timeout_string(self, m_which, m_subp):
+ def test_with_timeout_string(self, m_subp, m_which):
"""timeout can be a string."""
m_which.side_effect = lambda m: m in ("udevadm",)
timeout = "555"
util.udevadm_settle(timeout=timeout)
- m_subp.called_once_with(
+ m_subp.assert_called_once_with(
["udevadm", "settle", "--timeout=%s" % timeout]
)
- def test_with_exists_and_timeout(self, m_which, m_subp):
+ def test_with_exists_and_timeout(self, m_subp, m_which):
"""test call with both exists and timeout."""
m_which.side_effect = lambda m: m in ("udevadm",)
mydev = self.tmp_path("mydev")
timeout = "3"
- util.udevadm_settle(exists=mydev)
- m_subp.called_once_with(
+ util.udevadm_settle(exists=mydev, timeout=timeout)
+ m_subp.assert_called_once_with(
[
"udevadm",
"settle",
@@ -920,7 +922,7 @@ class TestUdevadmSettle(CiTestCase):
]
)
- def test_subp_exception_raises_to_caller(self, m_which, m_subp):
+ def test_subp_exception_raises_to_caller(self, m_subp, m_which):
m_which.side_effect = lambda m: m in ("udevadm",)
m_subp.side_effect = subp.ProcessExecutionError("BOOM")
self.assertRaises(subp.ProcessExecutionError, util.udevadm_settle)

View File

@ -0,0 +1,112 @@
From d1d5166895da471cff3606c70d4e8ab6eec1c006 Mon Sep 17 00:00:00 2001
From: Ani Sinha <anisinha@redhat.com>
Date: Thu, 7 Dec 2023 02:39:51 +0530
Subject: [PATCH] net/nm: check for presence of ifcfg files when nm connection
files are absent (#4645)
On systems that use network manager to manage connections and activate network
interfaces, they may also use ifcfg files for configuring
interfaces using ifcfg-rh network manager plugin. When network manager is used
as the activator, we need to also check for the presence of ifcfg interface
config file when the network manager connection file is absent and if ifcfg-rh
plugin is present.
Hence, with this change, network manager activator first tries to use network
manager connection files to bring up or bring down the interface. If the
connection files are not present and if ifcfg-rh plugin is present, it tries to
use ifcfg files for the interface. If the plugin or the ifcfg files are not
present, the activator fails to activate or deactivate the interface and it
bails out with warning log.
Fixes: GH-4640
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
cloudinit/net/activators.py | 7 +++++++
cloudinit/net/network_manager.py | 33 ++++++++++++++++++++++++++++++--
2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/cloudinit/net/activators.py b/cloudinit/net/activators.py
index e69da40d371..dd85886212c 100644
--- a/cloudinit/net/activators.py
+++ b/cloudinit/net/activators.py
@@ -135,6 +135,13 @@ class NetworkManagerActivator(NetworkActivator):
from cloudinit.net.network_manager import conn_filename
filename = conn_filename(device_name)
+ if filename is None:
+ LOG.warning(
+ "Unable to find an interface config file. "
+ "Unable to bring up interface."
+ )
+ return False
+
cmd = ["nmcli", "connection", "load", filename]
if _alter_interface(cmd, device_name):
cmd = ["nmcli", "connection", "up", "filename", filename]
diff --git a/cloudinit/net/network_manager.py b/cloudinit/net/network_manager.py
index 8a99eb3a1c5..76a0ac15eaa 100644
--- a/cloudinit/net/network_manager.py
+++ b/cloudinit/net/network_manager.py
@@ -17,10 +17,12 @@ from cloudinit import log as logging
from cloudinit import subp, util
from cloudinit.net import is_ipv6_address, renderer, subnet_is_ipv6
from cloudinit.net.network_state import NetworkState
+from cloudinit.net.sysconfig import available_nm_ifcfg_rh
NM_RUN_DIR = "/etc/NetworkManager"
NM_LIB_DIR = "/usr/lib/NetworkManager"
NM_CFG_FILE = "/etc/NetworkManager/NetworkManager.conf"
+IFCFG_CFG_FILE = "/etc/sysconfig/network-scripts"
NM_IPV6_ADDR_GEN_CONF = """# This is generated by cloud-init. Do not edit.
#
[.config]
@@ -374,7 +376,7 @@ class Renderer(renderer.Renderer):
for con_id, conn in self.connections.items():
if not conn.valid():
continue
- name = conn_filename(con_id, target)
+ name = nm_conn_filename(con_id, target)
util.write_file(name, conn.dump(), 0o600)
# Select EUI64 to be used by default by NM for creating the address
@@ -384,12 +386,39 @@ class Renderer(renderer.Renderer):
)
-def conn_filename(con_id, target=None):
+def nm_conn_filename(con_id, target=None):
target_con_dir = subp.target_path(target, NM_RUN_DIR)
con_file = f"cloud-init-{con_id}.nmconnection"
return f"{target_con_dir}/system-connections/{con_file}"
+def sysconfig_conn_filename(devname, target=None):
+ target_con_dir = subp.target_path(target, IFCFG_CFG_FILE)
+ con_file = f"ifcfg-{devname}"
+ return f"{target_con_dir}/{con_file}"
+
+
+def conn_filename(devname):
+ """
+ This function returns the name of the interface config file.
+ It first checks for presence of network manager connection file.
+ If absent and ifcfg-rh plugin for network manager is available,
+ it returns the name of the ifcfg file if it is present. If the
+ plugin is not present or the plugin is present but ifcfg file is
+ not, it returns None.
+ This function is called from NetworkManagerActivator class in
+ activators.py.
+ """
+ conn_file = nm_conn_filename(devname)
+ # If the network manager connection file is absent, also check for
+ # presence of ifcfg files for the same interface (if nm-ifcfg-rh plugin is
+ # present, network manager can handle ifcfg files). If both network manager
+ # connection file and ifcfg files are absent, return None.
+ if not os.path.isfile(conn_file) and available_nm_ifcfg_rh():
+ conn_file = sysconfig_conn_filename(devname)
+ return conn_file if os.path.isfile(conn_file) else None
+
+
def cloud_init_nm_conf_filename(target=None):
target_con_dir = subp.target_path(target, NM_RUN_DIR)
conf_file = "30-cloud-init-ip6-addr-gen-mode.conf"

View File

@ -0,0 +1,130 @@
From bb474df78bfe45ea5f05907eb710e8d5de764fc8 Mon Sep 17 00:00:00 2001
From: Ani Sinha <anisinha@redhat.com>
Date: Thu, 7 Dec 2023 21:03:13 +0530
Subject: [PATCH] tests/unittests: add a new unit test for network manager net
activator (#4672)
Some changes in behavior in network manager net activator was brought in with
the commit
d1d5166895da ("net/nm: check for presence of ifcfg files when nm connection files are absent")
This change adds some unit tests that exercizes network manager activator's
bring_up_interface() method that tests failure scenarios as well as cases
where an ifcfg file is used to bring the interface up.
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
tests/unittests/test_net_activators.py | 103 +++++++++++++++++++++++++
1 file changed, 103 insertions(+)
diff --git a/tests/unittests/test_net_activators.py b/tests/unittests/test_net_activators.py
index 2a363ec415b..d53701efafb 100644
--- a/tests/unittests/test_net_activators.py
+++ b/tests/unittests/test_net_activators.py
@@ -347,3 +347,106 @@ class TestActivatorsBringDown:
activator.bring_down_all_interfaces(network_state)
for call in m_subp.call_args_list:
assert call in expected_call_list
+
+
+class TestNetworkManagerActivatorBringUp:
+ @patch("cloudinit.subp.subp", return_value=("", ""))
+ @patch(
+ "cloudinit.net.network_manager.available_nm_ifcfg_rh",
+ return_value=True,
+ )
+ @patch("os.path.isfile")
+ @patch("os.path.exists", return_value=True)
+ def test_bring_up_interface_no_nm_conn(
+ self, m_exists, m_isfile, m_plugin, m_subp
+ ):
+ """
+ There is no network manager connection file but ifcfg-rh plugin is
+ present and ifcfg interface config files are also present. In this
+ case, we should use ifcfg files.
+ """
+
+ def fake_isfile_no_nmconn(filename):
+ return False if filename.endswith(".nmconnection") else True
+
+ m_isfile.side_effect = fake_isfile_no_nmconn
+
+ expected_call_list = [
+ (
+ (
+ [
+ "nmcli",
+ "connection",
+ "load",
+ "".join(
+ [
+ "/etc/sysconfig/network-scripts/ifcfg-eth0",
+ ]
+ ),
+ ],
+ ),
+ {},
+ ),
+ (
+ (
+ [
+ "nmcli",
+ "connection",
+ "up",
+ "filename",
+ "".join(
+ [
+ "/etc/sysconfig/network-scripts/ifcfg-eth0",
+ ]
+ ),
+ ],
+ ),
+ {},
+ ),
+ ]
+
+ index = 0
+ assert NetworkManagerActivator.bring_up_interface("eth0")
+ for call in m_subp.call_args_list:
+ assert call == expected_call_list[index]
+ index += 1
+
+ @patch("cloudinit.subp.subp", return_value=("", ""))
+ @patch(
+ "cloudinit.net.network_manager.available_nm_ifcfg_rh",
+ return_value=False,
+ )
+ @patch("os.path.isfile")
+ @patch("os.path.exists", return_value=True)
+ def test_bring_up_interface_no_plugin_no_nm_conn(
+ self, m_exists, m_isfile, m_plugin, m_subp
+ ):
+ """
+ The ifcfg-rh plugin is absent and nmconnection file is also
+ not present. In this case, we can't use ifcfg file and the
+ interface bring up should fail.
+ """
+
+ def fake_isfile_no_nmconn(filename):
+ return False if filename.endswith(".nmconnection") else True
+
+ m_isfile.side_effect = fake_isfile_no_nmconn
+ assert not NetworkManagerActivator.bring_up_interface("eth0")
+
+ @patch("cloudinit.subp.subp", return_value=("", ""))
+ @patch(
+ "cloudinit.net.network_manager.available_nm_ifcfg_rh",
+ return_value=True,
+ )
+ @patch("os.path.isfile", return_value=False)
+ @patch("os.path.exists", return_value=True)
+ def test_bring_up_interface_no_conn_file(
+ self, m_exists, m_isfile, m_plugin, m_subp
+ ):
+ """
+ Neither network manager connection files are present nor
+ ifcfg files are present. Even if ifcfg-rh plugin is present,
+ we can not bring up the interface. So bring_up_interface()
+ should fail.
+ """
+ assert not NetworkManagerActivator.bring_up_interface("eth0")

View File

@ -1,6 +1,6 @@
Name: cloud-init Name: cloud-init
Version: 23.1.1 Version: 23.1.1
Release: 11%{?dist}.alma.1 Release: 12%{?dist}.alma.1
Summary: Cloud instance init scripts Summary: Cloud instance init scripts
License: ASL 2.0 or GPLv3 License: ASL 2.0 or GPLv3
URL: http://launchpad.net/cloud-init URL: http://launchpad.net/cloud-init
@ -39,8 +39,16 @@ Patch28: 0028-logging-keep-current-file-mode-of-log-file-if-its-st.patch
Patch29: 0029-DS-VMware-modify-a-few-log-level-4284.patch Patch29: 0029-DS-VMware-modify-a-few-log-level-4284.patch
Patch30: 0030-NM-renderer-set-default-IPv6-addr-gen-mode-for-all-i.patch Patch30: 0030-NM-renderer-set-default-IPv6-addr-gen-mode-for-all-i.patch
# Patches were taken from:
# https://github.com/canonical/cloud-init/commit/e3f1ec3f57cc1f5eacff9506d285007966414481
Patch31: 0031-Fix-Python-3.12-unit-test-failures-2009.patch
# https://github.com/canonical/cloud-init/commit/d1d5166895da471cff3606c70d4e8ab6eec1c006
Patch32: 0032-net-nm-check-for-presence-of-ifcfg-files.patch
# https://github.com/canonical/cloud-init/commit/bb474df78bfe45ea5f05907eb710e8d5de764fc8
Patch33: 0033-tests-unittests-add-a-new-unit-test-for-network.patch
# AlmaLinux OS patches # AlmaLinux OS patches
Patch31: 0031-Improvements-for-AlmaLinux-OS-and-CloudLinux-OS.patch Patch1001: 0031-Improvements-for-AlmaLinux-OS-and-CloudLinux-OS.patch
BuildArch: noarch BuildArch: noarch
@ -261,6 +269,13 @@ fi
%config(noreplace) %{_sysconfdir}/rsyslog.d/21-cloudinit.conf %config(noreplace) %{_sysconfdir}/rsyslog.d/21-cloudinit.conf
%changelog %changelog
* Thu Jan 25 2024 Eduard Abdullin <eabdullin@almalinux.org> - 23.1.1-12.alma.1
- Fix Python 3.12 unit test failures (#2099)
- net/nm: check for presence of ifcfg files when nm connection
files are absent (#4645)
- tests/unittests: add a new unit test for network manager net
activator (#4672)
* Fri Oct 7 2023 Elkhan Mammadli <elkhan@almalinux.org> - 23.1.1-11.alma.1 * Fri Oct 7 2023 Elkhan Mammadli <elkhan@almalinux.org> - 23.1.1-11.alma.1
- 0031-Improvements-for-AlmaLinux-OS-and-CloudLinux-OS.patch - 0031-Improvements-for-AlmaLinux-OS-and-CloudLinux-OS.patch