refactor: don't import subp function directly (canonical#5065)

When subp function is imported directly, it cannot be easily mocked in
unit tests. Update imports and tests accordingly.

cloudinit/config/cc_ansible.py

@@ -9,12 +9,12 @@
 from textwrap import dedent
 from typing import Optional
 
+from cloudinit import subp
 from cloudinit.cloud import Cloud
 from cloudinit.config import Config
 from cloudinit.config.schema import MetaSchema, get_meta_doc
 from cloudinit.distros import ALL_DISTROS, Distro
 from cloudinit.settings import PER_INSTANCE
-from cloudinit.subp import subp, which
 from cloudinit.util import Version, get_cfg_by_path
 
 meta: MetaSchema = {
@@ -100,7 +100,7 @@ def do_as(self, command: list, **kwargs):
         return self.distro.do_as(command, self.run_user, **kwargs)
 
     def subp(self, command, **kwargs):
-        return subp(command, update_env=self.env, **kwargs)
+        return subp.subp(command, update_env=self.env, **kwargs)
 
     @abc.abstractmethod
     def is_installed(self):
@@ -165,7 +165,7 @@ def install(self, pkg_name: str):
             self.distro.install_packages([pkg_name])
 
     def is_installed(self) -> bool:
-        return bool(which("ansible"))
+        return bool(subp.which("ansible"))
 
 
 def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:

cloudinit/config/cc_ubuntu_autoinstall.py

@@ -6,7 +6,7 @@
 import re
 from textwrap import dedent
 
-from cloudinit import util
+from cloudinit import subp, util
 from cloudinit.cloud import Cloud
 from cloudinit.config import Config
 from cloudinit.config.schema import (
@@ -16,7 +16,6 @@
     get_meta_doc,
 )
 from cloudinit.settings import PER_ONCE
-from cloudinit.subp import subp
 
 LOG = logging.getLogger(__name__)
 
@@ -85,7 +84,7 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:
         return
 
     util.wait_for_snap_seeded(cloud)
-    snap_list, _ = subp(["snap", "list"])
+    snap_list, _ = subp.subp(["snap", "list"])
     installer_present = None
     for snap_name in LIVE_INSTALLER_SNAPS:
         if re.search(snap_name, snap_list):

tests/unittests/config/test_cc_ansible.py

@@ -287,8 +287,8 @@ def test_filter_args(self):
         ),
     )
     def test_required_keys(self, cfg, exception, mocker):
-        mocker.patch(M_PATH + "subp", return_value=("", ""))
-        mocker.patch(M_PATH + "which", return_value=True)
+        mocker.patch(M_PATH + "subp.subp", return_value=("", ""))
+        mocker.patch(M_PATH + "subp.which", return_value=True)
         mocker.patch(M_PATH + "AnsiblePull.check_deps")
         mocker.patch(
             M_PATH + "AnsiblePull.get_version",
@@ -319,28 +319,30 @@ def test_required_keys(self, cfg, exception, mocker):
                         ["python3-pip"]
                     )
 
-    @mock.patch(M_PATH + "which", return_value=False)
+    @mock.patch(M_PATH + "subp.which", return_value=False)
     def test_deps_not_installed(self, m_which):
         """assert exception raised if package not installed"""
         with raises(ValueError):
             cc_ansible.AnsiblePullDistro(get_cloud().distro).check_deps()
 
-    @mock.patch(M_PATH + "which", return_value=True)
+    @mock.patch(M_PATH + "subp.which", return_value=True)
     def test_deps(self, m_which):
         """assert exception not raised if package installed"""
         cc_ansible.AnsiblePullDistro(get_cloud().distro).check_deps()
 
-    @mock.patch(M_PATH + "subp", return_value=("stdout", "stderr"))
-    @mock.patch(M_PATH + "which", return_value=False)
+    @mock.patch(M_PATH + "subp.subp", return_value=("stdout", "stderr"))
+    @mock.patch(M_PATH + "subp.which", return_value=False)
     def test_pip_bootstrap(self, m_which, m_subp):
         distro = get_cloud(mocked_distro=True).distro
         with mock.patch("builtins.__import__", side_effect=ImportError):
             cc_ansible.AnsiblePullPip(distro, "ansible").install("")
         distro.install_packages.assert_called_once()
 
-    @mock.patch(M_PATH + "which", return_value=True)
-    @mock.patch(M_PATH + "subp", return_value=("stdout", "stderr"))
-    @mock.patch("cloudinit.distros.subp", return_value=("stdout", "stderr"))
+    @mock.patch(M_PATH + "subp.which", return_value=True)
+    @mock.patch(M_PATH + "subp.subp", return_value=("stdout", "stderr"))
+    @mock.patch(
+        "cloudinit.distros.subp.subp", return_value=("stdout", "stderr")
+    )
     @mark.parametrize(
         ("cfg", "expected"),
         (
@@ -406,7 +408,8 @@ def test_do_not_run(self, m_validate):
         assert not m_validate.called
 
     @mock.patch(
-        "cloudinit.config.cc_ansible.subp", side_effect=[(distro_version, "")]
+        "cloudinit.config.cc_ansible.subp.subp",
+        side_effect=[(distro_version, "")],
     )
     def test_parse_version_distro(self, m_subp):
         """Verify that the expected version is returned"""
@@ -424,8 +427,8 @@ def test_parse_version_pip(self, m_subp):
         expected = util.Version(2, 13, 2)
         assert received == expected
 
-    @mock.patch(M_PATH + "subp", return_value=("stdout", "stderr"))
-    @mock.patch(M_PATH + "which", return_value=True)
+    @mock.patch(M_PATH + "subp.subp", return_value=("stdout", "stderr"))
+    @mock.patch(M_PATH + "subp.which", return_value=True)
     def test_ansible_env_var(self, m_which, m_subp):
         cc_ansible.handle("", CFG_FULL_PULL, get_cloud(), [])
 

tests/unittests/config/test_cc_ubuntu_autoinstall.py

@@ -66,7 +66,7 @@ def test_runtime_validation_errors(self, src_cfg, error_msg):
 
 
 @mock.patch(MODPATH + "util.wait_for_snap_seeded")
-@mock.patch(MODPATH + "subp")
+@mock.patch(MODPATH + "subp.subp")
 class TestHandleAutoinstall:
     """Test cc_ubuntu_autoinstall handling of config."""