From eb0381beab9e7c8fd7e33f1add565f63981ea0f5 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 1 Sep 2023 13:02:02 -0500 Subject: [PATCH 1/5] integration tests: Fix cgroup parsing (#4402) It's possible to still have cgroup output for cloud-config.service even if there are no spawned processes. Instead, use systemd-cgls to list the cgroup and ensure there are isn't more than 1 line of output. --- tests/integration_tests/bugs/test_lp1813396.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/bugs/test_lp1813396.py b/tests/integration_tests/bugs/test_lp1813396.py index 47fe1e94abe..06546003a0e 100644 --- a/tests/integration_tests/bugs/test_lp1813396.py +++ b/tests/integration_tests/bugs/test_lp1813396.py @@ -29,5 +29,9 @@ def test_gpg_no_tty(client: IntegrationInstance): "Imported key 'E4D304DF' from keyserver 'keyserver.ubuntu.com'", ] verify_ordered_items_in_text(to_verify, log) - result = client.execute("systemctl status cloud-config.service") - assert "CGroup" not in result.stdout + processes_in_cgroup = int( + client.execute( + "systemd-cgls -u cloud-config.service 2>/dev/null | wc -l" + ).stdout + ) + assert processes_in_cgroup < 2 From 37b81560ac0ebc58bc0c617a1d4710588608e88b Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 5 Sep 2023 16:01:01 -0600 Subject: [PATCH 2/5] apt: kill dirmngr/gpg-agent without gpgconf dependency Upstream commit 842d0452 introduced a dependency on gpgconf utility without a strict Depends: clause in debian/control. While gnupg is in Recommends for cloud-init, some images like Ubuntu minimal will not install the package Recommends to keep images smaller. To avoid adding a hard Requires dependency on gpgconf, use os.kill directly to stop any dirmngr and gpg-agent when spawned by cloud-config's root ppid due to apt-related user-data. NOOP If no gpg-agent or dirmngr processes are spawned by PPID 1. LP: #2034273 --- cloudinit/config/cc_apt_configure.py | 12 ++++- .../integration_tests/bugs/test_lp1813396.py | 6 ++- .../test_apt_configure_sources_list_v1.py | 45 ++++++++++++++----- .../test_apt_configure_sources_list_v3.py | 13 ++++-- tests/unittests/config/test_apt_source_v1.py | 44 +++++++++++++++--- 5 files changed, 98 insertions(+), 22 deletions(-) diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index 976631458d8..f20e66b4b4e 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -12,6 +12,7 @@ import os import pathlib import re +import signal from textwrap import dedent from cloudinit import gpg @@ -244,11 +245,18 @@ def apply_apt(cfg, cloud, target): ) # GH: 4344 - stop gpg-agent/dirmgr daemons spawned by gpg key imports. # Daemons spawned by cloud-config.service on systemd v253 report (running) - subp.subp( - ["gpgconf", "--kill", "all"], + gpg_process_out, _err = subp.subp( + ["ps", "-o", "ppid,pid", "-C", "dirmngr", "-C", "gpg-agent"], target=target, capture=True, + rcs=[0, 1], ) + gpg_pids = re.findall(r"(?P\d+)\s+(?P\d+)", gpg_process_out) + root_gpg_pids = [int(pid[1]) for pid in gpg_pids if pid[0] == "1"] + if root_gpg_pids: + LOG.debug("Killing gpg-agent and dirmngr pids: %s", root_gpg_pids) + for gpg_pid in root_gpg_pids: + os.kill(gpg_pid, signal.SIGKILL) def debconf_set_selections(selections, target=None): diff --git a/tests/integration_tests/bugs/test_lp1813396.py b/tests/integration_tests/bugs/test_lp1813396.py index 06546003a0e..d726e518b4c 100644 --- a/tests/integration_tests/bugs/test_lp1813396.py +++ b/tests/integration_tests/bugs/test_lp1813396.py @@ -6,7 +6,10 @@ import pytest from tests.integration_tests.instances import IntegrationInstance -from tests.integration_tests.util import verify_ordered_items_in_text +from tests.integration_tests.util import ( + verify_clean_log, + verify_ordered_items_in_text, +) USER_DATA = """\ #cloud-config @@ -29,6 +32,7 @@ def test_gpg_no_tty(client: IntegrationInstance): "Imported key 'E4D304DF' from keyserver 'keyserver.ubuntu.com'", ] verify_ordered_items_in_text(to_verify, log) + verify_clean_log(log) processes_in_cgroup = int( client.execute( "systemd-cgls -u cloud-config.service 2>/dev/null | wc -l" diff --git a/tests/unittests/config/test_apt_configure_sources_list_v1.py b/tests/unittests/config/test_apt_configure_sources_list_v1.py index 1e73717d137..52542411012 100644 --- a/tests/unittests/config/test_apt_configure_sources_list_v1.py +++ b/tests/unittests/config/test_apt_configure_sources_list_v1.py @@ -121,20 +121,30 @@ def apt_source_list(self, distro, mirror, mirrorcheck=None): def test_apt_v1_source_list_debian(self): """Test rendering of a source.list from template for debian""" - with mock.patch.object(subp, "subp") as mocksubp: + with mock.patch.object( + subp, "subp", return_value=("PPID PID", "") + ) as mocksubp: self.apt_source_list( "debian", "http://httpredir.debian.org/debian" ) mocksubp.assert_called_once_with( - ["gpgconf", "--kill", "all"], capture=True, target=None + ["ps", "-o", "ppid,pid", "-C", "dirmngr", "-C", "gpg-agent"], + capture=True, + target=None, + rcs=[0, 1], ) def test_apt_v1_source_list_ubuntu(self): """Test rendering of a source.list from template for ubuntu""" - with mock.patch.object(subp, "subp") as mocksubp: + with mock.patch.object( + subp, "subp", return_value=("PPID PID", "") + ) as mocksubp: self.apt_source_list("ubuntu", "http://archive.ubuntu.com/ubuntu/") mocksubp.assert_called_once_with( - ["gpgconf", "--kill", "all"], capture=True, target=None + ["ps", "-o", "ppid,pid", "-C", "dirmngr", "-C", "gpg-agent"], + capture=True, + target=None, + rcs=[0, 1], ) @staticmethod @@ -152,7 +162,9 @@ def test_apt_v1_srcl_debian_mirrorfail(self): with mock.patch.object( util, "is_resolvable", side_effect=self.myresolve ) as mockresolve: - with mock.patch.object(subp, "subp") as mocksubp: + with mock.patch.object( + subp, "subp", return_value=("PPID PID", "") + ) as mocksubp: self.apt_source_list( "debian", [ @@ -164,7 +176,10 @@ def test_apt_v1_srcl_debian_mirrorfail(self): mockresolve.assert_any_call("http://does.not.exist") mockresolve.assert_any_call("http://httpredir.debian.org/debian") mocksubp.assert_called_once_with( - ["gpgconf", "--kill", "all"], capture=True, target=None + ["ps", "-o", "ppid,pid", "-C", "dirmngr", "-C", "gpg-agent"], + capture=True, + target=None, + rcs=[0, 1], ) def test_apt_v1_srcl_ubuntu_mirrorfail(self): @@ -172,7 +187,9 @@ def test_apt_v1_srcl_ubuntu_mirrorfail(self): with mock.patch.object( util, "is_resolvable", side_effect=self.myresolve ) as mockresolve: - with mock.patch.object(subp, "subp") as mocksubp: + with mock.patch.object( + subp, "subp", return_value=("PPID PID", "") + ) as mocksubp: self.apt_source_list( "ubuntu", [ @@ -184,7 +201,10 @@ def test_apt_v1_srcl_ubuntu_mirrorfail(self): mockresolve.assert_any_call("http://does.not.exist") mockresolve.assert_any_call("http://archive.ubuntu.com/ubuntu/") mocksubp.assert_called_once_with( - ["gpgconf", "--kill", "all"], capture=True, target=None + ["ps", "-o", "ppid,pid", "-C", "dirmngr", "-C", "gpg-agent"], + capture=True, + target=None, + rcs=[0, 1], ) def test_apt_v1_srcl_custom(self): @@ -194,7 +214,9 @@ def test_apt_v1_srcl_custom(self): # the second mock restores the original subp with mock.patch.object(util, "write_file") as mockwrite: - with mock.patch.object(subp, "subp") as mocksubp: + with mock.patch.object( + subp, "subp", return_value=("PPID PID", "") + ) as mocksubp: with mock.patch.object( Distro, "get_primary_arch", return_value="amd64" ): @@ -204,7 +226,10 @@ def test_apt_v1_srcl_custom(self): "/etc/apt/sources.list", EXPECTED_CONVERTED_CONTENT, mode=420 ) mocksubp.assert_called_once_with( - ["gpgconf", "--kill", "all"], capture=True, target=None + ["ps", "-o", "ppid,pid", "-C", "dirmngr", "-C", "gpg-agent"], + capture=True, + target=None, + rcs=[0, 1], ) diff --git a/tests/unittests/config/test_apt_configure_sources_list_v3.py b/tests/unittests/config/test_apt_configure_sources_list_v3.py index c98efa28b7e..47732582318 100644 --- a/tests/unittests/config/test_apt_configure_sources_list_v3.py +++ b/tests/unittests/config/test_apt_configure_sources_list_v3.py @@ -125,7 +125,9 @@ def _apt_source_list(self, distro, cfg, cfg_on_empty=False): mock_shouldcfg = stack.enter_context( mock.patch(cfg_func, return_value=(cfg_on_empty, "test")) ) - mock_subp = stack.enter_context(mock.patch.object(subp, "subp")) + mock_subp = stack.enter_context( + mock.patch.object(subp, "subp", return_value=("PPID PID", "")) + ) cc_apt_configure.handle("test", cfg, mycloud, None) return ( @@ -237,7 +239,9 @@ def test_apt_v3_srcl_custom(self): mycloud = get_cloud() with mock.patch.object(util, "write_file") as mockwrite: - with mock.patch.object(subp, "subp") as mocksubp: + with mock.patch.object( + subp, "subp", return_value=("PPID PID", "") + ) as mocksubp: with mock.patch.object( Distro, "get_primary_arch", return_value="amd64" ): @@ -250,7 +254,10 @@ def test_apt_v3_srcl_custom(self): ] mockwrite.assert_has_calls(calls) mocksubp.assert_called_once_with( - ["gpgconf", "--kill", "all"], capture=True, target=None + ["ps", "-o", "ppid,pid", "-C", "dirmngr", "-C", "gpg-agent"], + capture=True, + target=None, + rcs=[0, 1], ) diff --git a/tests/unittests/config/test_apt_source_v1.py b/tests/unittests/config/test_apt_source_v1.py index 5775082d2a5..c1908581e01 100644 --- a/tests/unittests/config/test_apt_source_v1.py +++ b/tests/unittests/config/test_apt_source_v1.py @@ -9,6 +9,7 @@ import pathlib import re import shutil +import signal import tempfile from unittest import mock from unittest.mock import call @@ -74,7 +75,9 @@ def setUp(self): get_arch = apatcher.start() get_arch.return_value = "amd64" self.addCleanup(apatcher.stop) - subp_patcher = mock.patch.object(subp, "subp") + subp_patcher = mock.patch.object( + subp, "subp", return_value=("PPID PID", "") + ) self.m_subp = subp_patcher.start() self.addCleanup(subp_patcher.stop) @@ -566,12 +569,24 @@ def test_apt_src_keyidonly(self): """Test specification of a keyid without source""" cfg = {"keyid": "03683F77", "filename": self.aptlistfile} cfg = self.wrapv1conf([cfg]) - + SAMPLE_GPG_AGENT_DIRMNGR_PIDS = """\ + PPID PID + 1 1057 + 1 1095 + 1511 2493 + 1511 2509 +""" with mock.patch.object( - subp, "subp", return_value=("fakekey 1212", "") + subp, + "subp", + side_effect=[ + ("fakekey 1212", ""), + (SAMPLE_GPG_AGENT_DIRMNGR_PIDS, ""), + ], ): with mock.patch.object(cc_apt_configure, "apt_key") as mockobj: - cc_apt_configure.handle("test", cfg, self.cloud, None) + with mock.patch.object(cc_apt_configure.os, "kill") as m_kill: + cc_apt_configure.handle("test", cfg, self.cloud, None) calls = ( call( @@ -582,6 +597,10 @@ def test_apt_src_keyidonly(self): ), ) mockobj.assert_has_calls(calls, any_order=True) + self.assertEqual( + ([call(1057, signal.SIGKILL), call(1095, signal.SIGKILL)]), + m_kill.call_args_list, + ) # filename should be ignored on key only self.assertFalse(os.path.isfile(self.aptlistfile)) @@ -658,7 +677,18 @@ def test_apt_src_ppa(self): target=None, ), mock.call( - ["gpgconf", "--kill", "all"], capture=True, target=None + [ + "ps", + "-o", + "ppid,pid", + "-C", + "dirmngr", + "-C", + "gpg-agent", + ], + capture=True, + target=None, + rcs=[0, 1], ), ], ) @@ -681,7 +711,9 @@ def test_apt_src_ppa_tri(self): } cfg = self.wrapv1conf([cfg1, cfg2, cfg3]) - with mock.patch.object(subp, "subp") as mockobj: + with mock.patch.object( + subp, "subp", return_value=("PPID PID", "") + ) as mockobj: cc_apt_configure.handle("test", cfg, self.cloud, None) calls = [ call( From 3582dbebc7bc1a933ae378f8ca7bbd1480a188c8 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 5 Sep 2023 16:31:40 -0600 Subject: [PATCH 3/5] Release 23.3.1 Bump the version in cloudinit/version.py to 23.3.1 and update ChangeLog. --- ChangeLog | 4 ++++ cloudinit/version.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 399f4f791a7..fb2ba3ab7fc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +23.3.1 + - apt: kill dirmngr/gpg-agent without gpgconf dependency (LP: #2034273) + - integration tests: Fix cgroup parsing (#4402) + 23.3 - Bump pycloudlib to 1!5.1.0 for ec2 mantic daily image support (#4390) - Fix cc_keyboard in mantic (LP: #2030788) diff --git a/cloudinit/version.py b/cloudinit/version.py index e6cd3d5bc1b..8e77fa98c59 100644 --- a/cloudinit/version.py +++ b/cloudinit/version.py @@ -4,7 +4,7 @@ # # This file is part of cloud-init. See LICENSE file for license information. -__VERSION__ = "23.3" +__VERSION__ = "23.3.1" _PACKAGED_VERSION = "@@PACKAGED_VERSION@@" FEATURES = [ From 0fff03ee74510a3880fe2655d404e9ef4cb1f69e Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 5 Sep 2023 17:03:54 -0600 Subject: [PATCH 4/5] update changelog (new upstream snapshot) --- debian/changelog | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/debian/changelog b/debian/changelog index 9418f8117a9..6697ba00155 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +cloud-init (23.3.1-0ubuntu1) UNRELEASED; urgency=medium + + * New upstream bug fix release based on 23.3.1. + List of changes from upstream can be found at + https://raw.githubusercontent.com/canonical/cloud-init/23.3.1/ChangeLog + - Bugs fixed in this snapshot: (LP: #2034273) + + -- Chad Smith Tue, 05 Sep 2023 17:03:54 -0600 + cloud-init (23.3-0ubuntu1) mantic; urgency=medium * d/po/templates.pot: refresh with debconf-updatepo From 9a6076dcb3d8763d59d6fe468ea5cec6fd7d1f47 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 5 Sep 2023 17:04:00 -0600 Subject: [PATCH 5/5] releasing cloud-init version 23.3.1-0ubuntu1 --- debian/changelog | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/debian/changelog b/debian/changelog index 6697ba00155..315d87a531c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,11 +1,11 @@ -cloud-init (23.3.1-0ubuntu1) UNRELEASED; urgency=medium +cloud-init (23.3.1-0ubuntu1) mantic; urgency=medium * New upstream bug fix release based on 23.3.1. List of changes from upstream can be found at https://raw.githubusercontent.com/canonical/cloud-init/23.3.1/ChangeLog - Bugs fixed in this snapshot: (LP: #2034273) - -- Chad Smith Tue, 05 Sep 2023 17:03:54 -0600 + -- Chad Smith Tue, 05 Sep 2023 17:03:59 -0600 cloud-init (23.3-0ubuntu1) mantic; urgency=medium