Skip to content

Commit

Permalink
source: Force OpenStack when it is only option (#2045)
Browse files Browse the repository at this point in the history 
Running on OpenStack Ironic was broken in 1efa8a0,
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
  • Loading branch information
@holmanb
holmanb committed Mar 2, 2023
1 parent 635b5a5 commit d1ffbea
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 313 deletions.
31 changes: 0 additions & 31 deletions cloudinit/distros/__init__.py
View file
Expand Up @@ -992,37 +992,6 @@ def do_as(self, command: list, user: str, cwd: str = "", **kwargs):
**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):
"""
Expand Down
39 changes: 1 addition & 38 deletions cloudinit/distros/freebsd.py
View file
Expand Up @@ -6,9 +6,7 @@

import os
import re
from functools import lru_cache
from io import StringIO
from typing import Optional

import cloudinit.distros.bsd
from cloudinit import log as logging
Expand Down Expand Up @@ -194,40 +192,5 @@ def update_package_sources(self):
freq=PER_INSTANCE,
)

@lru_cache()
def is_container(self) -> bool:
"""return whether we're running in a container.
Cached, because it's unlikely to change."""
jailed, _ = subp.subp(["sysctl", "-n", "security.jail.jailed"])
if jailed.strip() == "0":
return False
return True

@lru_cache()
def virtual(self) -> str:
"""return the kind of virtualisation system we're running under.
Cached, because it's unlikely to change."""
if self.is_container():
return "jail"
# map FreeBSD's kern.vm_guest to systemd-detect-virt, just like we do
# in ds-identify
VM_GUEST_TO_SYSTEMD = {
"hv": "microsoft",
"vbox": "oracle",
"generic": "vm-other",
}
vm, _ = subp.subp(["sysctl", "-n", "kern.vm_guest"])
vm = vm.strip()
if vm in VM_GUEST_TO_SYSTEMD:
return VM_GUEST_TO_SYSTEMD[vm]
return vm

@property
def is_virtual(self) -> Optional[bool]:
"""Detect if running on a virtual machine or bare metal.

This can fail on some platforms, so the signature is Optional[bool]
"""
if self.virtual() == "none":
return False
return True
# vi: ts=4 expandtab
41 changes: 6 additions & 35 deletions cloudinit/sources/DataSourceOpenStack.py
View file
Expand Up @@ -73,7 +73,7 @@ def __str__(self):
mstr = "%s [%s,ver=%s]" % (root, self.dsmode, self.version)
return mstr

def wait_for_metadata_service(self, max_wait=None, timeout=None):
def wait_for_metadata_service(self):
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):
Expand All @@ -90,23 +90,16 @@ def wait_for_metadata_service(self, max_wait=None, timeout=None):
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=max_wait,
timeout=timeout,
max_wait=url_params.max_wait_seconds,
timeout=url_params.timeout_seconds,
connect_synchronously=False,
)
if avail_url:
Expand Down Expand Up @@ -157,20 +150,11 @@ def _get_data(self):
False when unable to contact metadata service or when metadata
format is invalid or disabled.
"""
oracle_considered = "Oracle" in self.sys_cfg.get("datasource_list")

if self.perform_dhcp_setup: # Setup networking in init-local stage.
try:
with EphemeralDHCPv4(self.fallback_interface):
if not self.detect_openstack(
accept_oracle=not oracle_considered
):
LOG.debug(
"OpenStack datasource not running"
" on OpenStack (dhcp)"
)
return False

with EphemeralDHCPv4(self.fallback_interface):
results = util.log_time(
logfunc=LOG.debug,
msg="Crawl of metadata service",
Expand All @@ -180,13 +164,6 @@ def _get_data(self):
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:
Expand Down Expand Up @@ -265,8 +242,9 @@ def _crawl_metadata(self):
raise sources.InvalidMetaDataException(msg) from e
return result

def detect_openstack(self, accept_oracle=False):
def ds_detect(self):
"""Return True when a potential OpenStack platform is detected."""
accept_oracle = "Oracle" in self.sys_cfg.get("datasource_list")
if not util.is_x86():
# Non-Intel cpus don't properly report dmi product names
return True
Expand All @@ -280,13 +258,6 @@ def detect_openstack(self, accept_oracle=False):
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


Expand Down
29 changes: 28 additions & 1 deletion cloudinit/sources/__init__.py
View file
Expand Up @@ -307,6 +307,33 @@ def _unpickle(self, ci_pkl_version: int) -> None:
def __str__(self):
return type_utils.obj_name(self)

def ds_detect(self) -> bool:
"""Check if running on this datasource"""
return True

def override_ds_detect(self):
"""Override if either:
- only a single datasource defined (nothing to fall back to)
- TODO: commandline argument is used (ci.ds=OpenStack)
"""
return self.sys_cfg.get("datasource_list", []) in (
[self.dsname],
[self.dsname, "None"],
)

def _check_and_get_data(self):
"""Overrides runtime datasource detection"""
if self.override_ds_detect():
LOG.debug(
"Machine is configured to run on single datasource %s.", self
)
elif self.ds_detect():
LOG.debug("Machine is running on %s.", self)
else:
LOG.debug("Datasource type %s is not detected.", self)
return False
return self._get_data()

def _get_standardized_metadata(self, instance_data):
"""Return a dictionary of standardized metadata keys."""
local_hostname = self.get_hostname().hostname
Expand Down Expand Up @@ -370,7 +397,7 @@ def get_data(self) -> bool:
Minimally, the datasource should return a boolean True on success.
"""
self._dirty_cache = True
return_value = self._get_data()
return_value = self._check_and_get_data()
if not return_value:
return return_value
self.persist_instance_data()
Expand Down
5 changes: 0 additions & 5 deletions doc/examples/cloud-config-datasources.txt
View file
Expand Up @@ -16,11 +16,6 @@ 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
Expand Down
96 changes: 0 additions & 96 deletions tests/unittests/distros/test__init__.py
View file
Expand Up @@ -221,102 +221,6 @@ def test_expire_passwd_freebsd_uses_pw_command(self):
["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):
Expand Down

0 comments on commit d1ffbea

Please sign in to comment.