← Back to team overview

cloud-init-dev team mailing list archive

[Merge] ~smoser/cloud-init:fix/1766302-show-nics-not-up into cloud-init:master

 

Scott Moser has proposed merging ~smoser/cloud-init:fix/1766302-show-nics-not-up into cloud-init:master.

Commit message:
netinfo: fix netdev_pformat when a nic does not have an address assigned.

The last set of changes to netdev_pformat ended up dropping the output
of devices that were not up.  This adds back the 'down' interfaces to the
rendered output.

LP: #1766302

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1766302 in cloud-init (Ubuntu): "netdev_pformat does not show unconfigured devices"
  https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/1766302

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/343846

see commit message
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1766302-show-nics-not-up into cloud-init:master.
diff --git a/cloudinit/netinfo.py b/cloudinit/netinfo.py
index f090616..9b3a508 100644
--- a/cloudinit/netinfo.py
+++ b/cloudinit/netinfo.py
@@ -374,6 +374,8 @@ def netdev_pformat():
                 tbl.add_row(
                     [dev, data["up"], addr["ip"], ".", addr["scope6"],
                      data["hwaddr"]])
+            if len(data.get('ipv6')) + len(data.get('ipv4')) == 0:
+                tbl.add_row(dev, data["up"], ".", ".", ".", data["hwaddr"])
         netdev_s = tbl.get_string()
         max_len = len(max(netdev_s.splitlines(), key=len))
         header = util.center("Net device info", "+", max_len)
diff --git a/cloudinit/sources/DataSourceCloudStack.py b/cloudinit/sources/DataSourceCloudStack.py
index 0df545f..cfcdb1f 100644
--- a/cloudinit/sources/DataSourceCloudStack.py
+++ b/cloudinit/sources/DataSourceCloudStack.py
@@ -68,6 +68,10 @@ class DataSourceCloudStack(sources.DataSource):
 
     dsname = 'CloudStack'
 
+    # Setup read_url parameters per get_url_params.
+    url_max_wait = 120
+    url_timeout = 50
+
     def __init__(self, sys_cfg, distro, paths):
         sources.DataSource.__init__(self, sys_cfg, distro, paths)
         self.seed_dir = os.path.join(paths.seed_dir, 'cs')
@@ -80,28 +84,12 @@ class DataSourceCloudStack(sources.DataSource):
         self.metadata_address = "http://%s/"; % (self.vr_addr,)
         self.cfg = {}
 
-    def _get_url_settings(self):
-        mcfg = self.ds_cfg
-        max_wait = 120
-        try:
-            max_wait = int(mcfg.get("max_wait", max_wait))
-        except Exception:
-            util.logexc(LOG, "Failed to get max wait. using %s", max_wait)
+    def wait_for_metadata_service(self):
+        (max_wait, timeout, _retries) = self.get_url_params()
 
-        if max_wait == 0:
+        if max_wait <= 0:
             return False
 
-        timeout = 50
-        try:
-            timeout = int(mcfg.get("timeout", timeout))
-        except Exception:
-            util.logexc(LOG, "Failed to get timeout, using %s", timeout)
-
-        return (max_wait, timeout)
-
-    def wait_for_metadata_service(self):
-        (max_wait, timeout) = self._get_url_settings()
-
         urls = [uhelp.combine_url(self.metadata_address,
                                   'latest/meta-data/instance-id')]
         start_time = time.time()
diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
index 21e9ef8..057010a 100644
--- a/cloudinit/sources/DataSourceEc2.py
+++ b/cloudinit/sources/DataSourceEc2.py
@@ -59,6 +59,10 @@ class DataSourceEc2(sources.DataSource):
     # for extended metadata content. IPv6 support comes in 2016-09-02
     extended_metadata_versions = ['2016-09-02']
 
+    # Setup read_url parameters per get_url_params.
+    url_max_wait = 120
+    url_timeout = 50
+
     _cloud_platform = None
 
     _network_config = _unset  # Used for caching calculated network config v1
@@ -158,26 +162,10 @@ class DataSourceEc2(sources.DataSource):
         else:
             return self.metadata['instance-id']
 
-    def _get_url_settings(self):
-        mcfg = self.ds_cfg
-        max_wait = 120
-        try:
-            max_wait = int(mcfg.get("max_wait", max_wait))
-        except Exception:
-            util.logexc(LOG, "Failed to get max wait. using %s", max_wait)
-
-        timeout = 50
-        try:
-            timeout = max(0, int(mcfg.get("timeout", timeout)))
-        except Exception:
-            util.logexc(LOG, "Failed to get timeout, using %s", timeout)
-
-        return (max_wait, timeout)
-
     def wait_for_metadata_service(self):
         mcfg = self.ds_cfg
 
-        (max_wait, timeout) = self._get_url_settings()
+        (max_wait, timeout, _retries) = self.get_url_params()
         if max_wait <= 0:
             return False
 
diff --git a/cloudinit/sources/DataSourceOpenStack.py b/cloudinit/sources/DataSourceOpenStack.py
index fb166ae..eba78af 100644
--- a/cloudinit/sources/DataSourceOpenStack.py
+++ b/cloudinit/sources/DataSourceOpenStack.py
@@ -7,6 +7,8 @@
 import time
 
 from cloudinit import log as logging
+from cloudinit import net
+from cloudinit.net.dhcp import EphemeralDHCPv4, NoDHCPLeaseError
 from cloudinit import sources
 from cloudinit import url_helper
 from cloudinit import util
@@ -27,6 +29,12 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
 
     dsname = "OpenStack"
 
+    # Whether we want to get network configuration from the metadata service.
+    get_network_metadata = False
+
+    # Track the discovered fallback nic for use in configuration generation.
+    _fallback_interface = None
+
     def __init__(self, sys_cfg, distro, paths):
         super(DataSourceOpenStack, self).__init__(sys_cfg, distro, paths)
         self.metadata_address = None
@@ -40,33 +48,6 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
         mstr = "%s [%s,ver=%s]" % (root, self.dsmode, self.version)
         return mstr
 
-    def _get_url_settings(self):
-        # TODO(harlowja): this is shared with ec2 datasource, we should just
-        # move it to a shared location instead...
-        # Note: the defaults here are different though.
-
-        # max_wait < 0 indicates do not wait
-        max_wait = -1
-        timeout = 10
-        retries = 5
-
-        try:
-            max_wait = int(self.ds_cfg.get("max_wait", max_wait))
-        except Exception:
-            util.logexc(LOG, "Failed to get max wait. using %s", max_wait)
-
-        try:
-            timeout = max(0, int(self.ds_cfg.get("timeout", timeout)))
-        except Exception:
-            util.logexc(LOG, "Failed to get timeout, using %s", timeout)
-
-        try:
-            retries = int(self.ds_cfg.get("retries", retries))
-        except Exception:
-            util.logexc(LOG, "Failed to get retries. using %s", retries)
-
-        return (max_wait, timeout, retries)
-
     def wait_for_metadata_service(self):
         urls = self.ds_cfg.get("metadata_urls", [DEF_MD_URL])
         filtered = [x for x in urls if util.is_resolvable_url(x)]
@@ -86,7 +67,7 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
             md_urls.append(md_url)
             url2base[md_url] = url
 
-        (max_wait, timeout, _retries) = self._get_url_settings()
+        (max_wait, timeout, _retries) = self.get_url_params()
         start_time = time.time()
         avail_url = url_helper.wait_for_url(urls=md_urls, max_wait=max_wait,
                                             timeout=timeout)
@@ -100,13 +81,30 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
         return bool(avail_url)
 
     def _get_data(self):
+        if self.get_network_metadata:  # Setup networking in init-local stage.
+            try:
+                with EphemeralDHCPv4(self.fallback_interface):
+                    return util.log_time(
+                        logfunc=LOG.debug, msg='Crawl of metadata service',
+                        func=self._crawl_metadata)
+            except NoDHCPLeaseError:
+                return False
+        return util.log_time(
+            logfunc=LOG.debug, msg='Crawl of metadata service',
+            func=self._crawl_metadata)
+
+    def _crawl_metadata(self):
+        """Crawl metadata service when available.
+
+        @returns: True on success, False otherwise.
+        """
         try:
             if not self.wait_for_metadata_service():
                 return False
         except IOError:
             return False
 
-        (_max_wait, timeout, retries) = self._get_url_settings()
+        (_max_wait, timeout, retries) = self.get_url_params()
 
         try:
             results = util.log_time(LOG.debug,
@@ -145,11 +143,32 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource):
 
         return True
 
+    @property
+    def fallback_interface(self):
+        if self._fallback_interface is None:
+            # fallback_nic was used at one point, so restored objects may
+            # have an attribute there. respect that if found.
+            _legacy_fbnic = getattr(self, 'fallback_nic', None)
+            if _legacy_fbnic:
+                self._fallback_interface = _legacy_fbnic
+                self.fallback_nic = None
+            else:
+                self._fallback_interface = net.find_fallback_nic()
+                if self._fallback_interface is None:
+                    LOG.warning(
+                        "Did not find a fallback interface on OpenStack.")
+        return self._fallback_interface
+
     def check_instance_id(self, sys_cfg):
         # quickly (local check only) if self.instance_id is still valid
         return sources.instance_id_matches_system_uuid(self.get_instance_id())
 
 
+class DataSourceOpenStackLocal(DataSourceOpenStack):
+
+    get_network_metadata = True  # Get metadata network config if present
+
+
 def read_metadata_service(base_url, ssl_details=None,
                           timeout=5, retries=5):
     reader = openstack.MetadataReader(base_url, ssl_details=ssl_details,
@@ -159,6 +178,7 @@ def read_metadata_service(base_url, ssl_details=None,
 
 # Used to match classes to dependencies
 datasources = [
+    (DataSourceOpenStackLocal, (sources.DEP_FILESYSTEM,)),
     (DataSourceOpenStack, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)),
 ]
 
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index df0b374..c4d0de8 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -81,6 +81,11 @@ class DataSource(object):
     # Cached cloud_name as determined by _get_cloud_name
     _cloud_name = None
 
+    # read_url_params
+    url_max_wait = -1   # max_wait < 0 means do not wait
+    url_timeout = 10    # timeout for each metadata url read attempt
+    url_retries = 5     # number of times to retry url upon 404
+
     def __init__(self, sys_cfg, distro, paths, ud_proc=None):
         self.sys_cfg = sys_cfg
         self.distro = distro
@@ -149,6 +154,31 @@ class DataSource(object):
             'Subclasses of DataSource must implement _get_data which'
             ' sets self.metadata, vendordata_raw and userdata_raw.')
 
+    def get_url_params(self):
+        """Return the Datasource's prefered url_read parameters.
+
+        Subclasses may override url_max_wait, url_timeout, url_retries.
+
+        @return: A 3-tuple of  max_wait_seconds, timeout_seconds, num_retries.
+        """
+        try:
+            max_wait = int(self.ds_cfg.get("max_wait", self.url_max_wait))
+        except Exception:
+            util.logexc(LOG, "Failed to get max wait. using %s", max_wait)
+
+        try:
+            timeout = max(
+                0, int(self.ds_cfg.get("timeout", self.url_timeout)))
+        except Exception:
+            util.logexc(LOG, "Failed to get timeout, using %s", timeout)
+
+        try:
+            retries = int(self.ds_cfg.get("retries", self.url_retries))
+        except Exception:
+            util.logexc(LOG, "Failed to get retries. using %s", retries)
+
+        return (max_wait, timeout, retries)
+
     def get_userdata(self, apply_filter=False):
         if self.userdata is None:
             self.userdata = self.ud_proc.process(self.get_userdata_raw())
diff --git a/cloudinit/tests/test_netinfo.py b/cloudinit/tests/test_netinfo.py
index 2537c1c..336fd0f 100644
--- a/cloudinit/tests/test_netinfo.py
+++ b/cloudinit/tests/test_netinfo.py
@@ -4,7 +4,7 @@
 
 from copy import copy
 
-from cloudinit.netinfo import netdev_pformat, route_pformat
+from cloudinit.netinfo import netdev_info, netdev_pformat, route_pformat
 from cloudinit.tests.helpers import CiTestCase, mock, readResource
 
 
@@ -73,6 +73,51 @@ class TestNetInfo(CiTestCase):
 
     @mock.patch('cloudinit.netinfo.util.which')
     @mock.patch('cloudinit.netinfo.util.subp')
+    def test_netdev_info_nettools_down(self, m_subp, m_which):
+        """test netdev_info using nettools and down interfaces."""
+        m_subp.return_value = (
+            readResource("netinfo/new-ifconfig-output-down"), "")
+        m_which.side_effect = lambda x: x if x == 'ifconfig' else None
+        self.assertEqual(
+            {'eth0': {'ipv4': [], 'ipv6': [],
+                      'hwaddr': '00:16:3e:de:51:a6', 'up': False},
+             'lo': {'ipv4': [{'ip': '127.0.0.1', 'mask': '255.0.0.0'}],
+                    'ipv6': [{'ip': '::1/128', 'scope6': 'host'}],
+                    'hwaddr': '', 'up': True}},
+            netdev_info())
+
+    @mock.patch('cloudinit.netinfo.util.which')
+    @mock.patch('cloudinit.netinfo.util.subp')
+    def test_netdev_info_iproute_down(self, m_subp, m_which):
+        """Test netdev_info returns correct info with ip and down interfaces."""
+        m_subp.return_value = (
+            readResource("netinfo/sample-ipaddrshow-output-down"), "")
+        m_which.side_effect = lambda x: x if x == 'ip' else None
+        self.assertEqual(
+            {'lo': {'ipv4': [{'ip': '127.0.0.1', 'bcast': '',
+                              'mask': '255.0.0.0', 'scope': 'host'}],
+                    'ipv6': [{'ip': '::1/128', 'scope6': 'host'}],
+                    'hwaddr': '', 'up': True},
+             'eth0': {'ipv4': [], 'ipv6': [],
+                      'hwaddr': '00:16:3e:de:51:a6', 'up': False}},
+            netdev_info())
+
+    @mock.patch('cloudinit.netinfo.netdev_info')
+    def test_netdev_info_nettools_down(self, m_subp, m_which):
+        """test netdev_pformat when netdev_info returns 'down' interfaces."""
+        m_netdev_info.return_value = (
+            {'lo': {'ipv4': [{'ip': '127.0.0.1', 'mask': '255.0.0.0',
+                              'scope': 'host'}],
+                    'ipv6': [{'ip': '::1/128', 'scope6': 'host'}],
+                    'hwaddr': '', 'up': True},
+             'eth0': {'ipv4': [], 'ipv6': [],
+                      'hwaddr': '00:16:3e:de:51:a6', 'up': False}})
+
+        m_subp.return_value = (
+            readResource("netinfo/new-ifconfig-output-down"), "")
+
+    @mock.patch('cloudinit.netinfo.util.which')
+    @mock.patch('cloudinit.netinfo.util.subp')
     def test_route_nettools_pformat(self, m_subp, m_which):
         """route_pformat properly rendering nettools route info."""
 
diff --git a/tests/unittests/test_datasource/test_common.py b/tests/unittests/test_datasource/test_common.py
index ec33388..0d35dc2 100644
--- a/tests/unittests/test_datasource/test_common.py
+++ b/tests/unittests/test_datasource/test_common.py
@@ -40,6 +40,7 @@ DEFAULT_LOCAL = [
     OVF.DataSourceOVF,
     SmartOS.DataSourceSmartOS,
     Ec2.DataSourceEc2Local,
+    OpenStack.DataSourceOpenStackLocal,
 ]
 
 DEFAULT_NETWORK = [

Follow ups