Skip to content

Commit

Permalink
FreeBSD: fix for get_linux_distro() and lru_cache (#59)
Browse files Browse the repository at this point in the history
Since `is_FreeBSD()` is used a lot, which uses `system_info()`, which uses `get_linux_distro()` we add caching, by decorating the following functions with `@lru_cache`:

- get_architecture()
- _lsb_release()
- is_FreeBSD
- get_linux_distro
- system_info()
- _get_cmdline()

Since [functools](https://docs.python.org/3/library/functools.html) only exists in Python 3, only python 3 will benefit from this improvement. For python 2, our shim is just a pass-thru. Too bad, but, also… https://pythonclock.org/

The main motivation here was, at first, to cache more, following the style of _lsb_release.
That is now consolidated under this very same roof.

LP: #1815030
  • Loading branch information
igalic authored and blackboxsw committed Nov 25, 2019
1 parent aa935ae commit 2a135c4
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 24 deletions.
19 changes: 19 additions & 0 deletions cloudinit/tests/test_util.py
Expand Up @@ -387,6 +387,11 @@ def test_subp_exception_raises_to_caller(self, m_subp):
@mock.patch('os.path.exists')
class TestGetLinuxDistro(CiTestCase):

def setUp(self):
# python2 has no lru_cache, and therefore, no cache_clear()
if hasattr(util.get_linux_distro, "cache_clear"):
util.get_linux_distro.cache_clear()

@classmethod
def os_release_exists(self, path):
"""Side effect function"""
Expand All @@ -399,6 +404,12 @@ def redhat_release_exists(self, path):
if path == '/etc/redhat-release':
return 1

@classmethod
def freebsd_version_exists(self, path):
"""Side effect function """
if path == '/bin/freebsd-version':
return 1

@mock.patch('cloudinit.util.load_file')
def test_get_linux_distro_quoted_name(self, m_os_release, m_path_exists):
"""Verify we get the correct name if the os-release file has
Expand All @@ -417,6 +428,14 @@ def test_get_linux_distro_bare_name(self, m_os_release, m_path_exists):
dist = util.get_linux_distro()
self.assertEqual(('ubuntu', '16.04', 'xenial'), dist)

@mock.patch('cloudinit.util.subp')
def test_get_linux_freebsd(self, m_subp, m_path_exists):
"""Verify we get the correct name and release name on FreeBSD."""
m_path_exists.side_effect = TestGetLinuxDistro.freebsd_version_exists
m_subp.return_value = ("12.0-RELEASE-p10\n", '')
dist = util.get_linux_distro()
self.assertEqual(('freebsd', '12.0-RELEASE-p10', ''), dist)

@mock.patch('cloudinit.util.load_file')
def test_get_linux_centos6(self, m_os_release, m_path_exists):
"""Verify we get the correct name and release name on CentOS 6."""
Expand Down
47 changes: 29 additions & 18 deletions cloudinit/util.py
Expand Up @@ -50,6 +50,16 @@

from cloudinit.settings import (CFG_BUILTIN)

try:
from functools import lru_cache
except ImportError:
def lru_cache():
"""pass-thru replace for Python3's lru_cache()"""
def wrapper(f):
return f
return wrapper


_DNS_REDIRECT_IP = None
LOG = logging.getLogger(__name__)

Expand All @@ -68,17 +78,15 @@
['running-in-container'],
['lxc-is-container'])

PROC_CMDLINE = None

_LSB_RELEASE = {}


@lru_cache()
def get_architecture(target=None):
out, _ = subp(['dpkg', '--print-architecture'], capture=True,
target=target)
return out.strip()


@lru_cache()
def _lsb_release(target=None):
fmap = {'Codename': 'codename', 'Description': 'description',
'Distributor ID': 'id', 'Release': 'release'}
Expand Down Expand Up @@ -107,11 +115,7 @@ def lsb_release(target=None):
# do not use or update cache if target is provided
return _lsb_release(target)

global _LSB_RELEASE
if not _LSB_RELEASE:
data = _lsb_release()
_LSB_RELEASE.update(data)
return _LSB_RELEASE
return _lsb_release()


def target_path(target, path=None):
Expand Down Expand Up @@ -546,6 +550,7 @@ def is_ipv4(instr):
return len(toks) == 4


@lru_cache()
def is_FreeBSD():
return system_info()['variant'] == "freebsd"

Expand Down Expand Up @@ -595,6 +600,7 @@ def _parse_redhat_release(release_file=None):
return {}


@lru_cache()
def get_linux_distro():
distro_name = ''
distro_version = ''
Expand Down Expand Up @@ -622,6 +628,10 @@ def get_linux_distro():
flavor = match.groupdict()['codename']
if distro_name == 'rhel':
distro_name = 'redhat'
elif os.path.exists('/bin/freebsd-version'):
distro_name = 'freebsd'
distro_version, _ = subp(['uname', '-r'])
distro_version = distro_version.strip()
else:
dist = ('', '', '')
try:
Expand All @@ -642,6 +652,7 @@ def get_linux_distro():
return (distro_name, distro_version, flavor)


@lru_cache()
def system_info():
info = {
'platform': platform.platform(),
Expand Down Expand Up @@ -1371,14 +1382,8 @@ def load_file(fname, read_cb=None, quiet=False, decode=True):
return contents


def get_cmdline():
if 'DEBUG_PROC_CMDLINE' in os.environ:
return os.environ["DEBUG_PROC_CMDLINE"]

global PROC_CMDLINE
if PROC_CMDLINE is not None:
return PROC_CMDLINE

@lru_cache()
def _get_cmdline():
if is_container():
try:
contents = load_file("/proc/1/cmdline")
Expand All @@ -1393,10 +1398,16 @@ def get_cmdline():
except Exception:
cmdline = ""

PROC_CMDLINE = cmdline
return cmdline


def get_cmdline():
if 'DEBUG_PROC_CMDLINE' in os.environ:
return os.environ["DEBUG_PROC_CMDLINE"]

return _get_cmdline()


def pipe_in_out(in_fh, out_fh, chunk_size=1024, chunk_cb=None):
bytes_piped = 0
while True:
Expand Down
19 changes: 13 additions & 6 deletions tests/unittests/test_net.py
Expand Up @@ -4576,37 +4576,42 @@ def test_select_none_found_raises(self, m_eni_avail, m_sysc_avail):
priority=['sysconfig', 'eni'])

@mock.patch("cloudinit.net.renderers.netplan.available")
@mock.patch("cloudinit.net.renderers.sysconfig.available")
@mock.patch("cloudinit.net.renderers.sysconfig.available_sysconfig")
@mock.patch("cloudinit.net.renderers.sysconfig.available_nm")
@mock.patch("cloudinit.net.renderers.eni.available")
@mock.patch("cloudinit.net.renderers.sysconfig.util.get_linux_distro")
def test_sysconfig_selected_on_sysconfig_enabled_distros(self, m_distro,
m_eni, m_sys_nm,
m_sys_scfg,
m_sys_avail,
m_netplan):
"""sysconfig only selected on specific distros (rhel/sles)."""

# Ubuntu with Network-Manager installed
m_eni.return_value = False # no ifupdown (ifquery)
m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown
m_sys_nm.return_value = True # network-manager is installed
m_netplan.return_value = True # netplan is installed
m_eni.return_value = False # no ifupdown (ifquery)
m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown
m_sys_nm.return_value = True # network-manager is installed
m_netplan.return_value = True # netplan is installed
m_sys_avail.return_value = False # no sysconfig on Ubuntu
m_distro.return_value = ('ubuntu', None, None)
self.assertEqual('netplan', renderers.select(priority=None)[0])

# Centos with Network-Manager installed
m_eni.return_value = False # no ifupdown (ifquery)
m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown
m_sys_nm.return_value = True # network-manager is installed
m_netplan.return_value = False # netplan is not installed
m_netplan.return_value = False # netplan is not installed
m_sys_avail.return_value = True # sysconfig is available on centos
m_distro.return_value = ('centos', None, None)
self.assertEqual('sysconfig', renderers.select(priority=None)[0])

# OpenSuse with Network-Manager installed
m_eni.return_value = False # no ifupdown (ifquery)
m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown
m_sys_nm.return_value = True # network-manager is installed
m_netplan.return_value = False # netplan is not installed
m_netplan.return_value = False # netplan is not installed
m_sys_avail.return_value = True # sysconfig is available on opensuse
m_distro.return_value = ('opensuse', None, None)
self.assertEqual('sysconfig', renderers.select(priority=None)[0])

Expand All @@ -4625,6 +4630,8 @@ def test_sysconfig_available_uses_variant_mapping(self, m_distro, m_avail):
]
for (distro_name, distro_version, flavor) in distro_values:
m_distro.return_value = (distro_name, distro_version, flavor)
if hasattr(util.system_info, "cache_clear"):
util.system_info.cache_clear()
result = sysconfig.available()
self.assertTrue(result)

Expand Down

0 comments on commit 2a135c4

Please sign in to comment.