test: Remove side effects from tests (#5074)

These are the failures I could find when running tests in random order.
Changes that aren't self-explanatory should include a comment in the
test itself.

test_vultr.py includes a refactor to remove global state and remove
a test that didn't work and wasn't needed after the changes.

cloudinit/sources/helpers/openstack.py

@@ -763,7 +763,7 @@ def convert_net_json(network_json=None, known_macs=None):
                 cfg["type"] = "infiniband"
 
     for service in services:
-        cfg = service
+        cfg = copy.deepcopy(service)
         cfg.update({"type": "nameserver"})
         config.append(cfg)
 

tests/unittests/cmd/devel/test_net_convert.py

@@ -90,6 +90,18 @@
 """
 
 
+@pytest.fixture
+def mock_setup_logging():
+    """Mock setup_basic_logging to avoid changing log level.
+
+    net_convert.handle_args() can call setup_basic_logging() with a
+    WARNING level, which would be a side-effect for future tests.
+    It's behavior isn't checked in these tests, so mock it out.
+    """
+    with mock.patch(f"{M_PATH}log.setup_basic_logging"):
+        yield
+
+
 class TestNetConvert:
 
     missing_required_args = itertools.combinations(
@@ -155,7 +167,13 @@ def test_argparse_error_on_missing_args(self, cmdargs, capsys, tmpdir):
         ),
     )
     def test_convert_output_kind_artifacts(
-        self, output_kind, outfile_content, debug, capsys, tmpdir
+        self,
+        output_kind,
+        outfile_content,
+        debug,
+        capsys,
+        tmpdir,
+        mock_setup_logging,
     ):
         """Assert proper output-kind artifacts are written."""
         network_data = tmpdir.join("network_data")
@@ -186,7 +204,9 @@ def test_convert_output_kind_artifacts(
                 ] == chown.call_args_list
 
     @pytest.mark.parametrize("debug", (False, True))
-    def test_convert_netplan_passthrough(self, debug, tmpdir):
+    def test_convert_netplan_passthrough(
+        self, debug, tmpdir, mock_setup_logging
+    ):
         """Assert that if the network config's version is 2 and the renderer is
         Netplan, then the config is passed through as-is.
         """

tests/unittests/cmd/test_main.py

@@ -54,6 +54,18 @@ def setUp(self):
         self.patchUtils(self.new_root)
         self.stderr = StringIO()
         self.patchStdoutAndStderr(stderr=self.stderr)
+        # Every cc_ module calls get_meta_doc on import.
+        # This call will fail if filesystem redirection mocks are in place
+        # and the module hasn't already been imported which can depend
+        # on test ordering.
+        self.m_doc = mock.patch(
+            "cloudinit.config.schema.get_meta_doc", return_value={}
+        )
+        self.m_doc.start()
+
+    def tearDown(self):
+        self.m_doc.stop()
+        super().tearDown()
 
     def test_main_init_run_net_runs_modules(self):
         """Modules like write_files are run in 'net' mode."""

tests/unittests/config/test_cc_apt_configure.py

@@ -298,6 +298,7 @@ class TestAptConfigure:
         ),
     )
     @mock.patch(M_PATH + "get_apt_cfg")
+    @mock.patch.object(features, "APT_DEB822_SOURCE_LIST_FILE", True)
     def test_remove_source(
         self,
         m_get_apt_cfg,
@@ -312,7 +313,6 @@ def test_remove_source(
             "sourceparts": f"{tmpdir}/etc/apt/sources.list.d/",
         }
         cloud = get_cloud(distro_name)
-        features.APT_DEB822_SOURCE_LIST_FILE = True
         sources_file = tmpdir.join("/etc/apt/sources.list")
         deb822_sources_file = tmpdir.join(
             f"/etc/apt/sources.list.d/{distro_name}.sources"

tests/unittests/config/test_cc_ca_certs.py

@@ -380,6 +380,7 @@ def test_non_existent_cert_cfg(self):
                 cc_ca_certs.disable_default_ca_certs(distro_name, conf)
 
 
+@pytest.mark.usefixtures("clear_deprecation_log")
 class TestCACertsSchema:
     """Directly test schema rather than through handle."""
 

tests/unittests/config/test_cc_set_passwords.py

@@ -1,5 +1,6 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
+import copy
 import logging
 from unittest import mock
 
@@ -508,6 +509,7 @@ def test_chpasswd_parity(self, list_def, users_def):
 class TestExpire:
     @pytest.mark.parametrize("cfg", expire_cases)
     def test_expire(self, cfg, mocker, caplog):
+        cfg = copy.deepcopy(cfg)
         cloud = get_cloud()
         mocker.patch(f"{MODPATH}subp.subp")
         mocker.patch.object(cloud.distro, "chpasswd")
@@ -533,7 +535,9 @@ def test_expire(self, cfg, mocker, caplog):
     def test_expire_old_behavior(self, cfg, mocker, caplog):
         # Previously expire didn't apply to hashed passwords.
         # Ensure we can preserve that case on older releases
-        features.EXPIRE_APPLIES_TO_HASHED_USERS = False
+        mocker.patch.object(features, "EXPIRE_APPLIES_TO_HASHED_USERS", False)
+
+        cfg = copy.deepcopy(cfg)
         cloud = get_cloud()
         mocker.patch(f"{MODPATH}subp.subp")
         mocker.patch.object(cloud.distro, "chpasswd")

tests/unittests/config/test_cc_ssh.py

@@ -38,8 +38,10 @@ def publish_hostkey_test_setup(tmpdir):
         with open(filepath, "w") as f:
             f.write(" ".join(test_hostkeys[key_type]))
 
-    cc_ssh.KEY_FILE_TPL = os.path.join(hostkey_tmpdir, "ssh_host_%s_key")
-    yield test_hostkeys, test_hostkey_files
+    with mock.patch.object(
+        cc_ssh, "KEY_FILE_TPL", os.path.join(hostkey_tmpdir, "ssh_host_%s_key")
+    ):
+        yield test_hostkeys, test_hostkey_files
 
 
 def _replace_options(user: Optional[str] = None) -> str:
@@ -255,6 +257,7 @@ def test_handle_default_root(
     @mock.patch(MODPATH + "ug_util.normalize_users_groups")
     @mock.patch(MODPATH + "os.path.exists")
     @mock.patch(MODPATH + "util.fips_enabled", return_value=False)
+    @mock.patch.object(cc_ssh, "PUBLISH_HOST_KEYS", True)
     def test_handle_publish_hostkeys(
         self,
         m_fips,
@@ -268,7 +271,6 @@ def test_handle_publish_hostkeys(
     ):
         """Test handle with various configs for ssh_publish_hostkeys."""
         test_hostkeys, test_hostkey_files = publish_hostkey_test_setup
-        cc_ssh.PUBLISH_HOST_KEYS = True
         keys = ["key1"]
         user = "clouduser"
         # Return no matching keys for first glob, test keys for second.
@@ -282,7 +284,6 @@ def test_handle_publish_hostkeys(
         m_path_exists.return_value = True
         m_nug.return_value = ({user: {"default": user}}, {})
         cloud = get_cloud(distro="ubuntu", metadata={"public-keys": keys})
-        cloud.datasource.publish_host_keys = mock.Mock()
 
         expected_calls = []
         if expected_key_types is not None:
@@ -294,10 +295,15 @@ def test_handle_publish_hostkeys(
                     ]
                 )
             ]
-        cc_ssh.handle("name", cfg, cloud, None)
-        assert (
-            expected_calls == cloud.datasource.publish_host_keys.call_args_list
-        )
+
+        with mock.patch.object(
+            cloud.datasource, "publish_host_keys", mock.Mock()
+        ):
+            cc_ssh.handle("name", cfg, cloud, None)
+            assert (
+                expected_calls
+                == cloud.datasource.publish_host_keys.call_args_list
+            )
 
     @pytest.mark.parametrize(
         "ssh_keys_group_exists,sshd_version,expected_private_permissions",

tests/unittests/conftest.py

@@ -139,12 +139,21 @@ def dhclient_exists():
 log.configure_root_logger()
 
 
-@pytest.fixture(autouse=True)
-def disable_root_logger_setup(request):
-    with mock.patch("cloudinit.cmd.main.configure_root_logger", autospec=True):
+@pytest.fixture(autouse=True, scope="session")
+def disable_root_logger_setup():
+    with mock.patch("cloudinit.log.configure_root_logger", autospec=True):
         yield
 
 
+@pytest.fixture
+def clear_deprecation_log():
+    """Clear any deprecation warnings before and after running tests."""
+    # Since deprecations are de-duped, the existance (or non-existance) of
+    # a deprecation warning in a previous test can cause the next test to
+    # fail.
+    util.deprecate._log = set()
+
+
 PYTEST_VERSION_TUPLE = tuple(map(int, pytest.__version__.split(".")))
 
 if PYTEST_VERSION_TUPLE < (3, 9, 0):

tests/unittests/distros/test_netconfig.py

@@ -303,7 +303,7 @@ def setUp(self):
 
     def _get_distro(self, dname, renderers=None, activators=None):
         cls = distros.fetch(dname)
-        cfg = settings.CFG_BUILTIN
+        cfg = copy.deepcopy(settings.CFG_BUILTIN)
         cfg["system_info"]["distro"] = dname
         system_info_network_cfg = {}
         if renderers:

tests/unittests/helpers.py

@@ -162,6 +162,7 @@ def setUp(self):
             self.old_handlers = self.logger.handlers
             self.logger.handlers = [handler]
             self.old_level = logging.root.level
+            self.logger.level = logging.DEBUG
         if self.allowed_subp is True:
             subp.subp = _real_subp
         else:

tests/unittests/runs/test_simple_run.py

@@ -2,6 +2,7 @@
 
 import copy
 import os
+from unittest import mock
 
 from cloudinit import atomic_helper, safeyaml, stages, util
 from cloudinit.config.modules import Modules
@@ -45,6 +46,15 @@ def setUp(self):
         self.patchOS(self.new_root)
         self.patchUtils(self.new_root)
 
+        self.m_doc = mock.patch(
+            "cloudinit.config.schema.get_meta_doc", return_value={}
+        )
+        self.m_doc.start()
+
+    def tearDown(self):
+        self.m_doc.stop()
+        super().tearDown()
+
     def test_none_ds_populates_var_lib_cloud(self):
         """Init and run_section default behavior creates appropriate dirs."""
         # Now start verifying whats created

tests/unittests/sources/test_lxd.py

@@ -333,13 +333,13 @@ def test_network_config_when_unset(self, lxd_ds):
         assert NETWORK_V1 == lxd_ds.network_config
         assert LXD_V1_METADATA == lxd_ds._crawled_metadata
 
+    @mock.patch.object(lxd, "generate_network_config", return_value=NETWORK_V1)
     def test_network_config_crawled_metadata_no_network_config(
-        self, lxd_ds_no_network_config
+        self, m_generate, lxd_ds_no_network_config
     ):
         """network_config is correctly computed when _network_config is unset
         and _crawled_metadata does not contain network_config.
         """
-        lxd.generate_network_config = mock.Mock(return_value=NETWORK_V1)
         assert UNSET == lxd_ds_no_network_config._crawled_metadata
         assert UNSET == lxd_ds_no_network_config._network_config
         assert None is lxd_ds_no_network_config.userdata_raw
@@ -349,7 +349,7 @@ def test_network_config_crawled_metadata_no_network_config(
             LXD_V1_METADATA_NO_NETWORK_CONFIG
             == lxd_ds_no_network_config._crawled_metadata
         )
-        assert 1 == lxd.generate_network_config.call_count
+        assert 1 == m_generate.call_count
 
 
 class TestIsPlatformViable: