source: Force OpenStack when it is only option (#2045)

Running on OpenStack Ironic was broken in 1efa8a0a0,
which prevented a system configured to run on only
Openstack from actually running this ds. This change
also prevents the kernel commandline definition from
working. This change was required to prevent
unnecessarily probing OpenStack on Ec2, and is
therefore still required. 

This commit reverts an earlier attempt[1][2] to
automatically detect OpenStack, due to regression
it caused. Additionally, this change allows a
system that defines a datasource list containing
only [OpenStack] or [OpenStack, None] to attempt
running on OpenStack, overriding ds_detect(). A
datasource list that defines [OpenStack, None]
still falls back to DataSourceNone if OpenStack
fails to reach the IMDS.

This change also lays groundwork for the following
future work:

1. Add support for other datasources
2. Also override datasource checking when the kernel
   command line defines a datasource. This work needs
   to be done manually to support non-systemd systems.

Besides forcing OpenStack to run when it is the only
datasource in the datasource list, this commit also:

[1] 0220295 (it breaks some use cases)
[2] 29faf66 (no longer used)

LP: #2008727

4 files changed, 44 insertions, 196 deletions

diff --git a/tests/unittests/distros/test__init__.py b/tests/unittests/distros/test__init__.py
index ea017d58..7c5187fd 100644
--- a/tests/unittests/distros/test__init__.py
+++ b/tests/unittests/distros/test__init__.py
@@ -221,102 +221,6 @@ class TestGenericDistro(helpers.FilesystemMockingTestCase):
             ["pw", "usermod", "myuser", "-p", "01-Jan-1970"]
         )
 
-    @mock.patch("cloudinit.distros.uses_systemd")
-    @mock.patch(
-        "cloudinit.distros.subp.which",
-    )
-    @mock.patch(
-        "cloudinit.distros.subp.subp",
-    )
-    def test_virtualization_detected(self, m_subp, m_which, m_uses_systemd):
-        m_uses_systemd.return_value = True
-        m_which.return_value = "/usr/bin/systemd-detect-virt"
-        m_subp.return_value = ("kvm", None)
-
-        cls = distros.fetch("ubuntu")
-        d = cls("ubuntu", {}, None)
-        self.assertTrue(d.is_virtual)
-
-    @mock.patch("cloudinit.distros.uses_systemd")
-    @mock.patch(
-        "cloudinit.distros.subp.subp",
-    )
-    def test_virtualization_not_detected(self, m_subp, m_uses_systemd):
-        m_uses_systemd.return_value = True
-        m_subp.return_value = ("none", None)
-
-        cls = distros.fetch("ubuntu")
-        d = cls("ubuntu", {}, None)
-        self.assertFalse(d.is_virtual)
-
-    @mock.patch("cloudinit.distros.uses_systemd")
-    def test_virtualization_unknown(self, m_uses_systemd):
-        m_uses_systemd.return_value = True
-
-        from cloudinit.subp import ProcessExecutionError
-
-        cls = distros.fetch("ubuntu")
-        d = cls("ubuntu", {}, None)
-        with mock.patch(
-            "cloudinit.distros.subp.which",
-            return_value=None,
-        ):
-            self.assertIsNone(
-                d.is_virtual,
-                "Reflect unknown state when detection"
-                " binary cannot be found",
-            )
-
-        with mock.patch(
-            "cloudinit.distros.subp.subp",
-            side_effect=ProcessExecutionError(),
-        ):
-            self.assertIsNone(
-                d.is_virtual, "Reflect unknown state on ProcessExecutionError"
-            )
-
-    def test_virtualization_on_freebsd(self):
-        # This test function is a bit unusual:
-        # We need to first mock away the `ifconfig -a` subp call
-        # Then, we can use side-effects to get the results of two subp calls
-        # needed for is_container()/virtual() which is_virtual depends on.
-        # We also have to clear cache between each of those assertions.
-
-        cls = distros.fetch("freebsd")
-        with mock.patch(
-            "cloudinit.distros.subp.subp", return_value=("", None)
-        ):
-            d = cls("freebsd", {}, None)
-        # This mock is called by `sysctl -n security.jail.jailed`
-        with mock.patch(
-            "cloudinit.distros.subp.subp",
-            side_effect=[("0\n", None), ("literaly any truthy value", None)],
-        ):
-            self.assertFalse(d.is_container())
-            d.is_container.cache_clear()
-            self.assertTrue(d.is_container())
-            d.is_container.cache_clear()
-
-        # This mock is called by `sysctl -n kern.vm_guest`
-        with mock.patch(
-            "cloudinit.distros.subp.subp",
-            # fmt: off
-            side_effect=[
-                ("0\n", None), ("hv\n", None),  # virtual
-                ("0\n", None), ("none\n", None),  # physical
-                ("0\n", None), ("hv\n", None)  # virtual
-            ],
-            # fmt: on
-        ):
-            self.assertEqual(d.virtual(), "microsoft")
-            d.is_container.cache_clear()
-            d.virtual.cache_clear()
-            self.assertEqual(d.virtual(), "none")
-            d.is_container.cache_clear()
-            d.virtual.cache_clear()
-
-            self.assertTrue(d.is_virtual)
-
 
 class TestGetPackageMirrors:
     def return_first(self, mlist):
diff --git a/tests/unittests/sources/test_openstack.py b/tests/unittests/sources/test_openstack.py
index 0d807749..1a2f5924 100644
--- a/tests/unittests/sources/test_openstack.py
+++ b/tests/unittests/sources/test_openstack.py
@@ -301,12 +301,12 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
             responses_mock=self.responses,
         )
         distro = mock.MagicMock(spec=Distro)
-        distro.is_virtual = False
         ds_os = ds.DataSourceOpenStack(
             settings.CFG_BUILTIN, distro, helpers.Paths({"run_dir": self.tmp})
         )
         self.assertIsNone(ds_os.version)
-        self.assertTrue(ds_os.get_data())
+        with mock.patch.object(ds_os, "ds_detect", return_value=True):
+            self.assertTrue(ds_os.get_data())
         self.assertEqual(2, ds_os.version)
         md = dict(ds_os.metadata)
         md.pop("instance-id", None)
@@ -351,7 +351,7 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
 
         self.assertIsNone(ds_os_local.version)
         with test_helpers.mock.patch.object(
-            ds_os_local, "detect_openstack"
+            ds_os_local, "ds_detect"
         ) as m_detect_os:
             m_detect_os.return_value = True
             found = ds_os_local.get_data()
@@ -383,9 +383,7 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
             settings.CFG_BUILTIN, distro, helpers.Paths({"run_dir": self.tmp})
         )
         self.assertIsNone(ds_os.version)
-        with test_helpers.mock.patch.object(
-            ds_os, "detect_openstack"
-        ) as m_detect_os:
+        with test_helpers.mock.patch.object(ds_os, "ds_detect") as m_detect_os:
             m_detect_os.return_value = True
             found = ds_os.get_data()
         self.assertFalse(found)
@@ -414,7 +412,8 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
             "timeout": 0,
         }
         self.assertIsNone(ds_os.version)
-        self.assertFalse(ds_os.get_data())
+        with mock.patch.object(ds_os, "ds_detect", return_value=True):
+            self.assertFalse(ds_os.get_data())
         self.assertIsNone(ds_os.version)
 
     def test_network_config_disabled_by_datasource_config(self):
@@ -489,9 +488,7 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
             "timeout": 0,
         }
         self.assertIsNone(ds_os.version)
-        with test_helpers.mock.patch.object(
-            ds_os, "detect_openstack"
-        ) as m_detect_os:
+        with test_helpers.mock.patch.object(ds_os, "ds_detect") as m_detect_os:
             m_detect_os.return_value = True
             found = ds_os.get_data()
         self.assertFalse(found)
@@ -589,53 +586,17 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
             settings.CFG_BUILTIN, distro, helpers.Paths({"run_dir": self.tmp})
         )
 
-    def test_detect_openstack_non_intel_x86(self, m_is_x86):
+    def test_ds_detect_non_intel_x86(self, m_is_x86):
         """Return True on non-intel platforms because dmi isn't conclusive."""
         m_is_x86.return_value = False
         self.assertTrue(
-            self._fake_ds().detect_openstack(),
-            "Expected detect_openstack == True",
+            self._fake_ds().ds_detect(),
+            "Expected ds_detect == True",
         )
 
-    def test_detect_openstack_bare_metal(self, m_is_x86):
-        """Return True if the distro is non-virtual."""
-        m_is_x86.return_value = True
-
-        distro = mock.MagicMock(spec=Distro)
-        distro.is_virtual = False
-
-        fake_ds = self._fake_ds()
-        fake_ds.distro = distro
-
-        self.assertFalse(
-            fake_ds.distro.is_virtual,
-            "Expected distro.is_virtual == False",
-        )
-
-        with test_helpers.mock.patch.object(
-            fake_ds, "wait_for_metadata_service"
-        ) as m_wait_for_metadata_service:
-            m_wait_for_metadata_service.return_value = True
-
-            self.assertTrue(
-                fake_ds.wait_for_metadata_service(),
-                "Expected wait_for_metadata_service == True",
-            )
-
-            self.assertTrue(
-                fake_ds.detect_openstack(), "Expected detect_openstack == True"
-            )
-
-            self.assertTrue(
-                m_wait_for_metadata_service.called,
-                "Expected wait_for_metadata_service to be called",
-            )
-
     @test_helpers.mock.patch(MOCK_PATH + "util.get_proc_env")
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
-    def test_not_detect_openstack_intel_x86_ec2(
-        self, m_dmi, m_proc_env, m_is_x86
-    ):
+    def test_not_ds_detect_intel_x86_ec2(self, m_dmi, m_proc_env, m_is_x86):
         """Return False on EC2 platforms."""
         m_is_x86.return_value = True
         # No product_name in proc/1/environ
@@ -650,15 +611,13 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertFalse(
-            self._fake_ds().detect_openstack(),
-            "Expected detect_openstack == False on EC2",
+            self._fake_ds().ds_detect(),
+            "Expected ds_detect == False on EC2",
         )
         m_proc_env.assert_called_with(1)
 
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
-    def test_detect_openstack_intel_product_name_compute(
-        self, m_dmi, m_is_x86
-    ):
+    def test_ds_detect_intel_product_name_compute(self, m_dmi, m_is_x86):
         """Return True on OpenStack compute and nova instances."""
         m_is_x86.return_value = True
         openstack_product_names = ["OpenStack Nova", "OpenStack Compute"]
@@ -666,12 +625,12 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
         for product_name in openstack_product_names:
             m_dmi.return_value = product_name
             self.assertTrue(
-                self._fake_ds().detect_openstack(),
-                "Failed to detect_openstack",
+                self._fake_ds().ds_detect(),
+                "Failed to ds_detect",
             )
 
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
-    def test_detect_openstack_opentelekomcloud_chassis_asset_tag(
+    def test_ds_detect_opentelekomcloud_chassis_asset_tag(
         self, m_dmi, m_is_x86
     ):
         """Return True on OpenStack reporting OpenTelekomCloud asset-tag."""
@@ -686,14 +645,12 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertTrue(
-            self._fake_ds().detect_openstack(),
-            "Expected detect_openstack == True on OpenTelekomCloud",
+            self._fake_ds().ds_detect(),
+            "Expected ds_detect == True on OpenTelekomCloud",
         )
 
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
-    def test_detect_openstack_sapccloud_chassis_asset_tag(
-        self, m_dmi, m_is_x86
-    ):
+    def test_ds_detect_sapccloud_chassis_asset_tag(self, m_dmi, m_is_x86):
         """Return True on OpenStack reporting SAP CCloud VM asset-tag."""
         m_is_x86.return_value = True
 
@@ -706,14 +663,12 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertTrue(
-            self._fake_ds().detect_openstack(),
-            "Expected detect_openstack == True on SAP CCloud VM",
+            self._fake_ds().ds_detect(),
+            "Expected ds_detect == True on SAP CCloud VM",
         )
 
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
-    def test_detect_openstack_huaweicloud_chassis_asset_tag(
-        self, m_dmi, m_is_x86
-    ):
+    def test_ds_detect_huaweicloud_chassis_asset_tag(self, m_dmi, m_is_x86):
         """Return True on OpenStack reporting Huawei Cloud VM asset-tag."""
         m_is_x86.return_value = True
 
@@ -726,14 +681,12 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_asset_tag_dmi_read
         self.assertTrue(
-            self._fake_ds().detect_openstack(),
-            "Expected detect_openstack == True on Huawei Cloud VM",
+            self._fake_ds().ds_detect(),
+            "Expected ds_detect == True on Huawei Cloud VM",
         )
 
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
-    def test_detect_openstack_oraclecloud_chassis_asset_tag(
-        self, m_dmi, m_is_x86
-    ):
+    def test_ds_detect_oraclecloud_chassis_asset_tag(self, m_dmi, m_is_x86):
         """Return True on OpenStack reporting Oracle cloud asset-tag."""
         m_is_x86.return_value = True
 
@@ -745,16 +698,19 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
             assert False, "Unexpected dmi read of %s" % dmi_key
 
         m_dmi.side_effect = fake_dmi_read
+        ds = self._fake_ds()
+        ds.sys_cfg = {"datasource_list": ["Oracle"]}
         self.assertTrue(
-            self._fake_ds().detect_openstack(accept_oracle=True),
-            "Expected detect_openstack == True on OracleCloud.com",
+            ds.ds_detect(),
+            "Expected ds_detect == True on OracleCloud.com",
         )
+        ds.sys_cfg = {"datasource_list": []}
         self.assertFalse(
-            self._fake_ds().detect_openstack(accept_oracle=False),
-            "Expected detect_openstack == False.",
+            ds.ds_detect(),
+            "Expected ds_detect == False.",
         )
 
-    def _test_detect_openstack_nova_compute_chassis_asset_tag(
+    def _test_ds_detect_nova_compute_chassis_asset_tag(
         self, m_dmi, m_is_x86, chassis_tag
     ):
         """Return True on OpenStack reporting generic asset-tag."""
@@ -769,27 +725,25 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertTrue(
-            self._fake_ds().detect_openstack(),
-            "Expected detect_openstack == True on Generic OpenStack Platform",
+            self._fake_ds().ds_detect(),
+            "Expected ds_detect == True on Generic OpenStack Platform",
         )
 
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
-    def test_detect_openstack_nova_chassis_asset_tag(self, m_dmi, m_is_x86):
-        self._test_detect_openstack_nova_compute_chassis_asset_tag(
+    def test_ds_detect_nova_chassis_asset_tag(self, m_dmi, m_is_x86):
+        self._test_ds_detect_nova_compute_chassis_asset_tag(
             m_dmi, m_is_x86, "OpenStack Nova"
         )
 
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
-    def test_detect_openstack_compute_chassis_asset_tag(self, m_dmi, m_is_x86):
-        self._test_detect_openstack_nova_compute_chassis_asset_tag(
+    def test_ds_detect_compute_chassis_asset_tag(self, m_dmi, m_is_x86):
+        self._test_ds_detect_nova_compute_chassis_asset_tag(
             m_dmi, m_is_x86, "OpenStack Compute"
         )
 
     @test_helpers.mock.patch(MOCK_PATH + "util.get_proc_env")
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
-    def test_detect_openstack_by_proc_1_environ(
-        self, m_dmi, m_proc_env, m_is_x86
-    ):
+    def test_ds_detect_by_proc_1_environ(self, m_dmi, m_proc_env, m_is_x86):
         """Return True when nova product_name specified in /proc/1/environ."""
         m_is_x86.return_value = True
         # Nova product_name in proc/1/environ
@@ -807,8 +761,8 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertTrue(
-            self._fake_ds().detect_openstack(),
-            "Expected detect_openstack == True on OpenTelekomCloud",
+            self._fake_ds().ds_detect(),
+            "Expected ds_detect == True on OpenTelekomCloud",
         )
         m_proc_env.assert_called_with(1)
 
diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
index 03be0c92..cc75209e 100644
--- a/tests/unittests/test_ds_identify.py
+++ b/tests/unittests/test_ds_identify.py
@@ -950,7 +950,7 @@ class TestOracle(DsIdentifyBase):
         """Simple negative test of Oracle."""
         mycfg = copy.deepcopy(VALID_CFG["Oracle"])
         mycfg["files"][P_CHASSIS_ASSET_TAG] = "Not Oracle"
-        self._check_via_dict(mycfg, ds=["openstack", "none"], rc=RC_FOUND)
+        self._check_via_dict(mycfg, rc=RC_NOT_FOUND)
 
 
 def blkid_out(disks=None):
@@ -1056,7 +1056,6 @@ VALID_CFG = {
     "Ec2-brightbox-negative": {
         "ds": "Ec2",
         "files": {P_PRODUCT_SERIAL: "tricky-host.bobrightbox.com\n"},
-        "mocks": [MOCK_VIRT_IS_KVM],
     },
     "GCE": {
         "ds": "GCE",
@@ -1598,7 +1597,6 @@ VALID_CFG = {
     "Ec2-E24Cloud-negative": {
         "ds": "Ec2",
         "files": {P_SYS_VENDOR: "e24cloudyday\n"},
-        "mocks": [MOCK_VIRT_IS_KVM],
     },
     "VMware-NoValidTransports": {
         "ds": "VMware",
@@ -1757,7 +1755,6 @@ VALID_CFG = {
     "VMware-GuestInfo-NoVirtID": {
         "ds": "VMware",
         "mocks": [
-            MOCK_VIRT_IS_KVM,
             {
                 "name": "vmware_has_rpctool",
                 "ret": 0,
@@ -1863,7 +1860,6 @@ VALID_CFG = {
             P_PRODUCT_NAME: "3DS Outscale VM\n",
             P_SYS_VENDOR: "Not 3DS Outscale\n",
         },
-        "mocks": [MOCK_VIRT_IS_KVM],
     },
     "Ec2-Outscale-negative-productname": {
         "ds": "Ec2",
@@ -1871,7 +1867,6 @@ VALID_CFG = {
             P_PRODUCT_NAME: "Not 3DS Outscale VM\n",
             P_SYS_VENDOR: "3DS Outscale\n",
         },
-        "mocks": [MOCK_VIRT_IS_KVM],
     },
 }
 
diff --git a/tests/unittests/util.py b/tests/unittests/util.py
index da04c6b2..e7094ec5 100644
--- a/tests/unittests/util.py
+++ b/tests/unittests/util.py
@@ -1,5 +1,4 @@
 # This file is part of cloud-init. See LICENSE file for license information.
-from typing import Optional
 from unittest import mock
 
 from cloudinit import cloud, distros, helpers
@@ -146,10 +145,6 @@ class MockDistro(distros.Distro):
     def package_command(self, command, args=None, pkgs=None):
         pass
 
-    @property
-    def is_virtual(self) -> Optional[bool]:
-        return True
-
     def update_package_sources(self):
         return (True, "yay")