From 25274d01dec312e43c9d49ef51ab219031069a34 Mon Sep 17 00:00:00 2001 From: eabdullin Date: Thu, 25 Jan 2024 13:31:59 +0300 Subject: [PATCH] - 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) --- ...-Python-3.12-unit-test-failures-2009.patch | 120 ++++++++++++++++ ...nm-check-for-presence-of-ifcfg-files.patch | 112 +++++++++++++++ ...ests-add-a-new-unit-test-for-network.patch | 130 ++++++++++++++++++ SPECS/cloud-init.spec | 19 ++- 4 files changed, 379 insertions(+), 2 deletions(-) create mode 100644 SOURCES/0031-Fix-Python-3.12-unit-test-failures-2009.patch create mode 100644 SOURCES/0032-net-nm-check-for-presence-of-ifcfg-files.patch create mode 100644 SOURCES/0033-tests-unittests-add-a-new-unit-test-for-network.patch diff --git a/SOURCES/0031-Fix-Python-3.12-unit-test-failures-2009.patch b/SOURCES/0031-Fix-Python-3.12-unit-test-failures-2009.patch new file mode 100644 index 0000000..7e5d55f --- /dev/null +++ b/SOURCES/0031-Fix-Python-3.12-unit-test-failures-2009.patch @@ -0,0 +1,120 @@ +From e3f1ec3f57cc1f5eacff9506d285007966414481 Mon Sep 17 00:00:00 2001 +From: James Falcon +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) diff --git a/SOURCES/0032-net-nm-check-for-presence-of-ifcfg-files.patch b/SOURCES/0032-net-nm-check-for-presence-of-ifcfg-files.patch new file mode 100644 index 0000000..1f3cbbc --- /dev/null +++ b/SOURCES/0032-net-nm-check-for-presence-of-ifcfg-files.patch @@ -0,0 +1,112 @@ +From d1d5166895da471cff3606c70d4e8ab6eec1c006 Mon Sep 17 00:00:00 2001 +From: Ani Sinha +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 +--- + 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" diff --git a/SOURCES/0033-tests-unittests-add-a-new-unit-test-for-network.patch b/SOURCES/0033-tests-unittests-add-a-new-unit-test-for-network.patch new file mode 100644 index 0000000..548d1a6 --- /dev/null +++ b/SOURCES/0033-tests-unittests-add-a-new-unit-test-for-network.patch @@ -0,0 +1,130 @@ +From bb474df78bfe45ea5f05907eb710e8d5de764fc8 Mon Sep 17 00:00:00 2001 +From: Ani Sinha +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 +--- + 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") diff --git a/SPECS/cloud-init.spec b/SPECS/cloud-init.spec index 6cdd6b2..41f89e4 100644 --- a/SPECS/cloud-init.spec +++ b/SPECS/cloud-init.spec @@ -1,6 +1,6 @@ Name: cloud-init Version: 23.1.1 -Release: 11%{?dist}.alma.1 +Release: 12%{?dist}.alma.1 Summary: Cloud instance init scripts License: ASL 2.0 or GPLv3 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 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 -Patch31: 0031-Improvements-for-AlmaLinux-OS-and-CloudLinux-OS.patch +Patch1001: 0031-Improvements-for-AlmaLinux-OS-and-CloudLinux-OS.patch BuildArch: noarch @@ -261,6 +269,13 @@ fi %config(noreplace) %{_sysconfdir}/rsyslog.d/21-cloudinit.conf %changelog +* Thu Jan 25 2024 Eduard Abdullin - 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 - 23.1.1-11.alma.1 - 0031-Improvements-for-AlmaLinux-OS-and-CloudLinux-OS.patch