cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Chad Smith has proposed merging ~chad.smith/cloud-init:bug/1776701-openstack-local-no-probe-on-ec2 into cloud-init:master.

Commit message:
openstack: avoid unneeded metadata probe on non-openstack platforms

OpenStack datasource is now discovered in init-local stage. In order to probe whether OpenStack metadata is present, it performs a costly sandboxed dhclient setup and metadata probe against http://169.254.169.254 for openstack data. While cloud-init properly detect non-OpenStack on EC2, it spends precious time probing the metadata service also resulting in a confusing WARNING log about 'metadata not present'. To avoid the wasted cycles, and confusing warning, get_data will call a detect_openstack function to quickly determine whether the platform looks like OpenStack before
trying to setup network to probe and crawl the metadata service.

LP: #1776701


Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/347937
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:bug/1776701-openstack-local-no-probe-on-ec2 into cloud-init:master.

diff --git a/cloudinit/sources/DataSourceOpenStack.py b/cloudinit/sources/DataSourceOpenStack.py
index 1a12a3f..e7b0b41 100644
--- a/cloudinit/sources/DataSourceOpenStack.py
+++ b/cloudinit/sources/DataSourceOpenStack.py
@@ -4,6 +4,8 @@
 #
 # This file is part of cloud-init. See LICENSE file for license information.
 
+import os
+import re
 import time
 
 from cloudinit import log as logging
@@ -114,6 +116,8 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
             False when unable to contact metadata service or when metadata
             format is invalid or disabled.
         """
+        if not detect_openstack():
+            return False
         if self.perform_dhcp_setup:  # Setup networking in init-local stage.
             try:
                 with EphemeralDHCPv4(self.fallback_interface):
@@ -205,6 +209,25 @@ def read_metadata_service(base_url, ssl_details=None,
     return reader.read_v2()
 
 
+def detect_openstack():
+    """Return True when a potential OpenStack platform is detected."""
+    cpu_arch = os.uname()[4]
+    if not re.match(r'i.86|x86_64', cpu_arch):
+        return True  # Non-Intel cpus don't properly report dmi product names
+    product_name = util.read_dmi_data('system-product-name')
+    if product_name in ('OpenStack Nova', 'OpenStack Compute'):
+        return True
+    elif util.read_dmi_data('chassis-asset-tag') == 'OpenTelekomCloud':
+        return True
+    elif os.path.exists('/proc/1/environ'):
+        environ_content = util.load_file('/proc/1/environ')
+        env_items = re.split(r'\0|=', environ_content)
+        pid1_environ = dict(zip(env_items[::2], env_items[1::2]))
+        if pid1_environ.get('product_name') == 'OpenStack Nova':
+            return True
+    return False
+
+
 # Used to match classes to dependencies
 datasources = [
     (DataSourceOpenStackLocal, (sources.DEP_FILESYSTEM,)),
diff --git a/tests/unittests/test_datasource/test_openstack.py b/tests/unittests/test_datasource/test_openstack.py
index fad73b2..53c7a3a 100644
--- a/tests/unittests/test_datasource/test_openstack.py
+++ b/tests/unittests/test_datasource/test_openstack.py
@@ -69,6 +69,8 @@ EC2_VERSIONS = [
     'latest',
 ]
 
+MOCK_PATH = 'cloudinit.sources.DataSourceOpenStack.'
+
 
 # TODO _register_uris should leverage test_ec2.register_mock_metaserver.
 def _register_uris(version, ec2_files, ec2_meta, os_files):
@@ -231,7 +233,10 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase):
         ds_os = ds.DataSourceOpenStack(
             settings.CFG_BUILTIN, None, helpers.Paths({'run_dir': self.tmp}))
         self.assertIsNone(ds_os.version)
-        found = ds_os.get_data()
+        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.assertEqual(2, ds_os.version)
         md = dict(ds_os.metadata)
@@ -260,7 +265,10 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase):
             'broadcast-address': '192.168.2.255'}]
 
         self.assertIsNone(ds_os_local.version)
-        found = ds_os_local.get_data()
+        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_local.get_data()
         self.assertTrue(found)
         self.assertEqual(2, ds_os_local.version)
         md = dict(ds_os_local.metadata)
@@ -284,7 +292,10 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase):
                                        None,
                                        helpers.Paths({'run_dir': self.tmp}))
         self.assertIsNone(ds_os.version)
-        found = ds_os.get_data()
+        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.assertIsNone(ds_os.version)
         self.assertIn(
@@ -306,15 +317,16 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase):
             'timeout': 0,
         }
         self.assertIsNone(ds_os.version)
-        found = ds_os.get_data()
+        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.assertIsNone(ds_os.version)
 
     def test_network_config_disabled_by_datasource_config(self):
         """The network_config can be disabled from datasource config."""
-        mock_path = (
-            'cloudinit.sources.DataSourceOpenStack.openstack.'
-            'convert_net_json')
+        mock_path = MOCK_PATH + 'openstack.convert_net_json'
         ds_os = ds.DataSourceOpenStack(
             settings.CFG_BUILTIN, None, helpers.Paths({'run_dir': self.tmp}))
         ds_os.ds_cfg = {'apply_network_config': False}
@@ -327,9 +339,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase):
 
     def test_network_config_from_network_json(self):
         """The datasource gets network_config from network_data.json."""
-        mock_path = (
-            'cloudinit.sources.DataSourceOpenStack.openstack.'
-            'convert_net_json')
+        mock_path = MOCK_PATH + 'openstack.convert_net_json'
         example_cfg = {'version': 1, 'config': []}
         ds_os = ds.DataSourceOpenStack(
             settings.CFG_BUILTIN, None, helpers.Paths({'run_dir': self.tmp}))
@@ -345,9 +355,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase):
 
     def test_network_config_cached(self):
         """The datasource caches the network_config property."""
-        mock_path = (
-            'cloudinit.sources.DataSourceOpenStack.openstack.'
-            'convert_net_json')
+        mock_path = MOCK_PATH + 'openstack.convert_net_json'
         example_cfg = {'version': 1, 'config': []}
         ds_os = ds.DataSourceOpenStack(
             settings.CFG_BUILTIN, None, helpers.Paths({'run_dir': self.tmp}))
@@ -374,7 +382,10 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase):
             'timeout': 0,
         }
         self.assertIsNone(ds_os.version)
-        found = ds_os.get_data()
+        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.assertIsNone(ds_os.version)
 
@@ -438,4 +449,97 @@ class TestVendorDataLoading(test_helpers.TestCase):
         data = {'foo': 'bar', 'cloud-init': ['VD_1', 'VD_2']}
         self.assertEqual(self.cvj(data), data['cloud-init'])
 
+
+@test_helpers.mock.patch(MOCK_PATH + 'os.uname')
+class TestDetectOpenStack(test_helpers.CiTestCase):
+
+    def test_detect_openstack_non_intel_x86(self, m_uname):
+        """Return True on non-intel platforms because dmi isn't conclusive."""
+        non_x86_intel_arch = ['ia64', '9000/800', 'arm64v71']
+        for cpu_arch in non_x86_intel_arch:
+            m_uname.return_value = [0, 1, 2, 3, cpu_arch]
+            self.assertTrue(
+                ds.detect_openstack(), 'Expected detect_openstack == True')
+
+    @test_helpers.mock.patch(MOCK_PATH + 'util.load_file')
+    @test_helpers.mock.patch(MOCK_PATH + 'util.os.path.exists')
+    @test_helpers.mock.patch(MOCK_PATH + 'util.read_dmi_data')
+    def test_not_detect_openstack_intel_x86_ec2(self, m_dmi, m_exists,
+                                                m_load_file, m_uname):
+        """Return False on EC2 platforms."""
+        m_exists.return_value = True
+        m_uname.return_value = [0, 1, 2, 3, 'x86_64']
+        # No product_name in proc/1/environ
+        m_load_file.return_value = 'HOME=/\x00init=/sbin/init\x00'
+
+        def fake_dmi_read(dmi_key):
+            if dmi_key == 'system-product-name':
+                return 'HVM domU'  # Nothing 'openstackish' on EC2
+            if dmi_key == 'chassis-asset-tag':
+                return ''  # Empty string on EC2
+            assert False, 'Unexpected dmi read of %s' % dmi_key
+
+        m_dmi.side_effect = fake_dmi_read
+        self.assertFalse(
+            ds.detect_openstack(), 'Expected detect_openstack == False on EC2')
+        m_exists.assert_called_with('/proc/1/environ')
+        m_load_file.assert_called_with('/proc/1/environ')
+
+    @test_helpers.mock.patch(MOCK_PATH + 'util.read_dmi_data')
+    def test_detect_openstack_intel_product_name_compute(self, m_dmi, m_uname):
+        """Return True on OpenStack compute and nova instances."""
+        m_uname.return_value = [0, 1, 2, 3, 'x86_64']
+
+        openstack_product_names = ['OpenStack Nova', 'OpenStack Compute']
+
+        for product_name in openstack_product_names:
+            m_dmi.return_value = product_name
+            self.assertTrue(
+                ds.detect_openstack(), 'Failed to detect_openstack')
+
+    @test_helpers.mock.patch(MOCK_PATH + 'util.read_dmi_data')
+    def test_detect_openstack_opentelekomcloud_chassis_asset_tag(self, m_dmi,
+                                                                 m_uname):
+        """Return True on OpenStack reporting OpenTelekomCloud asset-tag."""
+        m_uname.return_value = [0, 1, 2, 3, 'i386']
+
+        def fake_dmi_read(dmi_key):
+            if dmi_key == 'system-product-name':
+                return 'HVM domU'  # Nothing 'openstackish' on OpenTelekomCloud
+            if dmi_key == 'chassis-asset-tag':
+                return 'OpenTelekomCloud'
+            assert False, 'Unexpected dmi read of %s' % dmi_key
+
+        m_dmi.side_effect = fake_dmi_read
+        self.assertTrue(
+            ds.detect_openstack(),
+            'Expected detect_openstack == True on OpenTelekomCloud')
+
+    @test_helpers.mock.patch(MOCK_PATH + 'util.load_file')
+    @test_helpers.mock.patch(MOCK_PATH + 'util.os.path.exists')
+    @test_helpers.mock.patch(MOCK_PATH + 'util.read_dmi_data')
+    def test_detect_openstack_by_proc_1_environ(self, m_dmi, m_exists,
+                                                m_load_file, m_uname):
+        """Return True when nova product_name specified in /proc/1/environ."""
+        m_uname.return_value = [0, 1, 2, 3, 'i586']
+        m_exists.return_value = True  # for /proc/1/environ
+        # Nova product_name in proc/1/environ
+        m_load_file.return_value = (
+            'HOME=/\x00init=/sbin/init\x00product_name=OpenStack Nova\x00')
+
+        def fake_dmi_read(dmi_key):
+            if dmi_key == 'system-product-name':
+                return 'HVM domU'  # Nothing 'openstackish'
+            if dmi_key == 'chassis-asset-tag':
+                return ''  # Nothin 'openstackish'
+            assert False, 'Unexpected dmi read of %s' % dmi_key
+
+        m_dmi.side_effect = fake_dmi_read
+        self.assertTrue(
+            ds.detect_openstack(),
+            'Expected detect_openstack == True on OpenTelekomCloud')
+        m_exists.assert_called_with('/proc/1/environ')
+        m_load_file.assert_called_with('/proc/1/environ')
+
+
 # vi: ts=4 expandtab