Fix OpenStack datasource detection on bare metal (#1923)

LP: #1815990

8 files changed, 239 insertions, 52 deletions

diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index e61320c1..e6d360a4 100644
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -983,6 +983,37 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta):
             **kwargs,
         )
 
+    @property
+    def is_virtual(self) -> Optional[bool]:
+        """Detect if running on a virtual machine or bare metal.
+
+        If the detection fails, it returns None.
+        """
+        if not uses_systemd():
+            # For non systemd systems the method should be
+            # implemented in the distro class.
+            LOG.warning("is_virtual should be implemented on distro class")
+            return None
+
+        try:
+            detect_virt_path = subp.which("systemd-detect-virt")
+            if detect_virt_path:
+                out, _ = subp.subp(
+                    [detect_virt_path], capture=True, rcs=[0, 1]
+                )
+
+                return not out.strip() == "none"
+            else:
+                err_msg = "detection binary not found"
+        except subp.ProcessExecutionError as e:
+            err_msg = str(e)
+
+        LOG.warning(
+            "Failed to detect virtualization with systemd-detect-virt: %s",
+            err_msg,
+        )
+        return None
+
 
 def _apply_hostname_transformations_to_url(url: str, transformations: list):
     """
diff --git a/cloudinit/sources/DataSourceOpenStack.py b/cloudinit/sources/DataSourceOpenStack.py
index a07e355c..86ed3dd5 100644
--- a/cloudinit/sources/DataSourceOpenStack.py
+++ b/cloudinit/sources/DataSourceOpenStack.py
@@ -73,7 +73,7 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
         mstr = "%s [%s,ver=%s]" % (root, self.dsmode, self.version)
         return mstr
 
-    def wait_for_metadata_service(self):
+    def wait_for_metadata_service(self, max_wait=None, timeout=None):
         urls = self.ds_cfg.get("metadata_urls", DEF_MD_URLS)
         filtered = [x for x in urls if util.is_resolvable_url(x)]
         if set(filtered) != set(urls):
@@ -90,16 +90,23 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
         md_urls = []
         url2base = {}
         for url in urls:
+            # Wait for a specific openstack metadata url
             md_url = url_helper.combine_url(url, "openstack")
             md_urls.append(md_url)
             url2base[md_url] = url
 
         url_params = self.get_url_params()
+        if max_wait is None:
+            max_wait = url_params.max_wait_seconds
+
+        if timeout is None:
+            timeout = url_params.timeout_seconds
+
         start_time = time.time()
         avail_url, _response = url_helper.wait_for_url(
             urls=md_urls,
-            max_wait=url_params.max_wait_seconds,
-            timeout=url_params.timeout_seconds,
+            max_wait=max_wait,
+            timeout=timeout,
             connect_synchronously=False,
         )
         if avail_url:
@@ -151,8 +158,6 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
             format is invalid or disabled.
         """
         oracle_considered = "Oracle" in self.sys_cfg.get("datasource_list")
-        if not detect_openstack(accept_oracle=not oracle_considered):
-            return False
 
         if self.perform_dhcp_setup:  # Setup networking in init-local stage.
             try:
@@ -160,6 +165,15 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
                     self.fallback_interface,
                     tmp_dir=self.distro.get_tmp_exec_path(),
                 ):
+                    if not self.detect_openstack(
+                        accept_oracle=not oracle_considered
+                    ):
+                        LOG.debug(
+                            "OpenStack datasource not running"
+                            " on OpenStack (dhcp)"
+                        )
+                        return False
+
                     results = util.log_time(
                         logfunc=LOG.debug,
                         msg="Crawl of metadata service",
@@ -169,6 +183,13 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
                 util.logexc(LOG, str(e))
                 return False
         else:
+            if not self.detect_openstack(accept_oracle=not oracle_considered):
+                LOG.debug(
+                    "OpenStack datasource not running"
+                    " on OpenStack (non-dhcp)"
+                )
+                return False
+
             try:
                 results = self._crawl_metadata()
             except sources.InvalidMetaDataException as e:
@@ -247,6 +268,30 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
             raise sources.InvalidMetaDataException(msg) from e
         return result
 
+    def detect_openstack(self, accept_oracle=False):
+        """Return True when a potential OpenStack platform is detected."""
+        if not util.is_x86():
+            # Non-Intel cpus don't properly report dmi product names
+            return True
+
+        product_name = dmi.read_dmi_data("system-product-name")
+        if product_name in VALID_DMI_PRODUCT_NAMES:
+            return True
+        elif dmi.read_dmi_data("chassis-asset-tag") in VALID_DMI_ASSET_TAGS:
+            return True
+        elif accept_oracle and oracle._is_platform_viable():
+            return True
+        elif util.get_proc_env(1).get("product_name") == DMI_PRODUCT_NOVA:
+            return True
+        # On bare metal hardware, the product name is not set like
+        # in a virtual OpenStack vm. We check if the system is virtual
+        # and if the openstack specific metadata service has been found.
+        elif not self.distro.is_virtual and self.wait_for_metadata_service(
+            max_wait=15, timeout=5
+        ):
+            return True
+        return False
+
 
 class DataSourceOpenStackLocal(DataSourceOpenStack):
     """Run in init-local using a dhcp discovery prior to metadata crawl.
@@ -267,22 +312,6 @@ def read_metadata_service(base_url, ssl_details=None, timeout=5, retries=5):
     return reader.read_v2()
 
 
-def detect_openstack(accept_oracle=False):
-    """Return True when a potential OpenStack platform is detected."""
-    if not util.is_x86():
-        return True  # Non-Intel cpus don't properly report dmi product names
-    product_name = dmi.read_dmi_data("system-product-name")
-    if product_name in VALID_DMI_PRODUCT_NAMES:
-        return True
-    elif dmi.read_dmi_data("chassis-asset-tag") in VALID_DMI_ASSET_TAGS:
-        return True
-    elif accept_oracle and oracle._is_platform_viable():
-        return True
-    elif util.get_proc_env(1).get("product_name") == DMI_PRODUCT_NOVA:
-        return True
-    return False
-
-
 # Used to match classes to dependencies
 datasources = [
     (DataSourceOpenStackLocal, (sources.DEP_FILESYSTEM,)),
diff --git a/doc/examples/cloud-config-datasources.txt b/doc/examples/cloud-config-datasources.txt
index 43b34418..9b5df6b0 100644
--- a/doc/examples/cloud-config-datasources.txt
+++ b/doc/examples/cloud-config-datasources.txt
@@ -16,6 +16,11 @@ datasource:
      - http://169.254.169.254:80
      - http://instance-data:8773
 
+  OpenStack:
+    # The default list of metadata services to check for OpenStack.
+    metadata_urls:
+     - http://169.254.169.254
+
   MAAS:
     timeout : 50
     max_wait : 120
diff --git a/tests/unittests/distros/test__init__.py b/tests/unittests/distros/test__init__.py
index 7c5187fd..4201c687 100644
--- a/tests/unittests/distros/test__init__.py
+++ b/tests/unittests/distros/test__init__.py
@@ -221,6 +221,60 @@ 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"
+            )
+
 
 class TestGetPackageMirrors:
     def return_first(self, mlist):
diff --git a/tests/unittests/sources/test_openstack.py b/tests/unittests/sources/test_openstack.py
index 75a0dda1..02516772 100644
--- a/tests/unittests/sources/test_openstack.py
+++ b/tests/unittests/sources/test_openstack.py
@@ -14,6 +14,7 @@ import pytest
 import responses
 
 from cloudinit import helpers, settings, util
+from cloudinit.distros import Distro
 from cloudinit.sources import UNSET, BrokenMetadata
 from cloudinit.sources import DataSourceOpenStack as ds
 from cloudinit.sources import convert_vendordata
@@ -299,15 +300,13 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
             OS_FILES,
             responses_mock=self.responses,
         )
+        distro = mock.MagicMock(spec=Distro)
+        distro.is_virtual = False
         ds_os = ds.DataSourceOpenStack(
-            settings.CFG_BUILTIN, None, helpers.Paths({"run_dir": self.tmp})
+            settings.CFG_BUILTIN, distro, helpers.Paths({"run_dir": self.tmp})
         )
         self.assertIsNone(ds_os.version)
-        mock_path = MOCK_PATH + "detect_openstack"
-        with test_helpers.mock.patch(mock_path) as m_detect_os:
-            m_detect_os.return_value = True
-            found = ds_os.get_data()
-        self.assertTrue(found)
+        self.assertTrue(ds_os.get_data())
         self.assertEqual(2, ds_os.version)
         md = dict(ds_os.metadata)
         md.pop("instance-id", None)
@@ -351,8 +350,9 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
         ]
 
         self.assertIsNone(ds_os_local.version)
-        mock_path = MOCK_PATH + "detect_openstack"
-        with test_helpers.mock.patch(mock_path) as m_detect_os:
+        with test_helpers.mock.patch.object(
+            ds_os_local, "detect_openstack"
+        ) as m_detect_os:
             m_detect_os.return_value = True
             found = ds_os_local.get_data()
         self.assertTrue(found)
@@ -377,12 +377,15 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
         _register_uris(
             self.VERSION, {}, {}, os_files, responses_mock=self.responses
         )
+        distro = mock.MagicMock(spec=Distro)
+        distro.is_virtual = True
         ds_os = ds.DataSourceOpenStack(
-            settings.CFG_BUILTIN, None, helpers.Paths({"run_dir": self.tmp})
+            settings.CFG_BUILTIN, distro, helpers.Paths({"run_dir": self.tmp})
         )
         self.assertIsNone(ds_os.version)
-        mock_path = MOCK_PATH + "detect_openstack"
-        with test_helpers.mock.patch(mock_path) as m_detect_os:
+        with test_helpers.mock.patch.object(
+            ds_os, "detect_openstack"
+        ) as m_detect_os:
             m_detect_os.return_value = True
             found = ds_os.get_data()
         self.assertFalse(found)
@@ -401,19 +404,17 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
         _register_uris(
             self.VERSION, {}, {}, os_files, responses_mock=self.responses
         )
+        distro = mock.MagicMock(spec=Distro)
+        distro.is_virtual = True
         ds_os = ds.DataSourceOpenStack(
-            settings.CFG_BUILTIN, None, helpers.Paths({"run_dir": self.tmp})
+            settings.CFG_BUILTIN, distro, helpers.Paths({"run_dir": self.tmp})
         )
         ds_os.ds_cfg = {
             "max_wait": 0,
             "timeout": 0,
         }
         self.assertIsNone(ds_os.version)
-        mock_path = MOCK_PATH + "detect_openstack"
-        with test_helpers.mock.patch(mock_path) as m_detect_os:
-            m_detect_os.return_value = True
-            found = ds_os.get_data()
-        self.assertFalse(found)
+        self.assertFalse(ds_os.get_data())
         self.assertIsNone(ds_os.version)
 
     def test_network_config_disabled_by_datasource_config(self):
@@ -478,16 +479,19 @@ class TestOpenStackDataSource(test_helpers.ResponsesTestCase):
         _register_uris(
             self.VERSION, {}, {}, os_files, responses_mock=self.responses
         )
+        distro = mock.MagicMock(spec=Distro)
+        distro.is_virtual = True
         ds_os = ds.DataSourceOpenStack(
-            settings.CFG_BUILTIN, None, helpers.Paths({"run_dir": self.tmp})
+            settings.CFG_BUILTIN, distro, helpers.Paths({"run_dir": self.tmp})
         )
         ds_os.ds_cfg = {
             "max_wait": 0,
             "timeout": 0,
         }
         self.assertIsNone(ds_os.version)
-        mock_path = MOCK_PATH + "detect_openstack"
-        with test_helpers.mock.patch(mock_path) as m_detect_os:
+        with test_helpers.mock.patch.object(
+            ds_os, "detect_openstack"
+        ) as m_detect_os:
             m_detect_os.return_value = True
             found = ds_os.get_data()
         self.assertFalse(found)
@@ -575,13 +579,58 @@ class TestVendorDataLoading(test_helpers.TestCase):
 
 @test_helpers.mock.patch(MOCK_PATH + "util.is_x86")
 class TestDetectOpenStack(test_helpers.CiTestCase):
+    def setUp(self):
+        self.tmp = self.tmp_dir()
+
+    def _fake_ds(self) -> ds.DataSourceOpenStack:
+        distro = mock.MagicMock(spec=Distro)
+        distro.is_virtual = True
+        return ds.DataSourceOpenStack(
+            settings.CFG_BUILTIN, distro, helpers.Paths({"run_dir": self.tmp})
+        )
+
     def test_detect_openstack_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(
-            ds.detect_openstack(), "Expected detect_openstack == True"
+            self._fake_ds().detect_openstack(),
+            "Expected detect_openstack == 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(
@@ -601,7 +650,8 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertFalse(
-            ds.detect_openstack(), "Expected detect_openstack == False on EC2"
+            self._fake_ds().detect_openstack(),
+            "Expected detect_openstack == False on EC2",
         )
         m_proc_env.assert_called_with(1)
 
@@ -616,7 +666,8 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
         for product_name in openstack_product_names:
             m_dmi.return_value = product_name
             self.assertTrue(
-                ds.detect_openstack(), "Failed to detect_openstack"
+                self._fake_ds().detect_openstack(),
+                "Failed to detect_openstack",
             )
 
     @test_helpers.mock.patch(MOCK_PATH + "dmi.read_dmi_data")
@@ -635,7 +686,7 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertTrue(
-            ds.detect_openstack(),
+            self._fake_ds().detect_openstack(),
             "Expected detect_openstack == True on OpenTelekomCloud",
         )
 
@@ -655,7 +706,7 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertTrue(
-            ds.detect_openstack(),
+            self._fake_ds().detect_openstack(),
             "Expected detect_openstack == True on SAP CCloud VM",
         )
 
@@ -675,7 +726,7 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_asset_tag_dmi_read
         self.assertTrue(
-            ds.detect_openstack(),
+            self._fake_ds().detect_openstack(),
             "Expected detect_openstack == True on Huawei Cloud VM",
         )
 
@@ -695,11 +746,11 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertTrue(
-            ds.detect_openstack(accept_oracle=True),
+            self._fake_ds().detect_openstack(accept_oracle=True),
             "Expected detect_openstack == True on OracleCloud.com",
         )
         self.assertFalse(
-            ds.detect_openstack(accept_oracle=False),
+            self._fake_ds().detect_openstack(accept_oracle=False),
             "Expected detect_openstack == False.",
         )
 
@@ -718,7 +769,7 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertTrue(
-            ds.detect_openstack(),
+            self._fake_ds().detect_openstack(),
             "Expected detect_openstack == True on Generic OpenStack Platform",
         )
 
@@ -756,7 +807,7 @@ class TestDetectOpenStack(test_helpers.CiTestCase):
 
         m_dmi.side_effect = fake_dmi_read
         self.assertTrue(
-            ds.detect_openstack(),
+            self._fake_ds().detect_openstack(),
             "Expected detect_openstack == 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 cc75209e..03be0c92 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, rc=RC_NOT_FOUND)
+        self._check_via_dict(mycfg, ds=["openstack", "none"], rc=RC_FOUND)
 
 
 def blkid_out(disks=None):
@@ -1056,6 +1056,7 @@ VALID_CFG = {
     "Ec2-brightbox-negative": {
         "ds": "Ec2",
         "files": {P_PRODUCT_SERIAL: "tricky-host.bobrightbox.com\n"},
+        "mocks": [MOCK_VIRT_IS_KVM],
     },
     "GCE": {
         "ds": "GCE",
@@ -1597,6 +1598,7 @@ VALID_CFG = {
     "Ec2-E24Cloud-negative": {
         "ds": "Ec2",
         "files": {P_SYS_VENDOR: "e24cloudyday\n"},
+        "mocks": [MOCK_VIRT_IS_KVM],
     },
     "VMware-NoValidTransports": {
         "ds": "VMware",
@@ -1755,6 +1757,7 @@ VALID_CFG = {
     "VMware-GuestInfo-NoVirtID": {
         "ds": "VMware",
         "mocks": [
+            MOCK_VIRT_IS_KVM,
             {
                 "name": "vmware_has_rpctool",
                 "ret": 0,
@@ -1860,6 +1863,7 @@ 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",
@@ -1867,6 +1871,7 @@ 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 e7094ec5..da04c6b2 100644
--- a/tests/unittests/util.py
+++ b/tests/unittests/util.py
@@ -1,4 +1,5 @@
 # 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
@@ -145,6 +146,10 @@ 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")
 
diff --git a/tools/ds-identify b/tools/ds-identify
index cd07565d..da23e836 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -1262,6 +1262,13 @@ dscheck_OpenStack() {
         *) return ${DS_MAYBE};;
     esac
 
+    # If we are on bare metal, then we maybe are on a
+    # bare metal Ironic environment.
+    detect_virt
+    if [ "${_RET}" = "none" ]; then
+      return ${DS_MAYBE}
+    fi
+
     return ${DS_NOT_FOUND}
 }