cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Chad Smith has proposed merging ~chad.smith/cloud-init:set-hostname-before-network into cloud-init:master.

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

For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/339720

set_hostname: When present in metadata, set it before network bringup.

When instance metadata/user-data provides hostname information, run
cc_set_hostname in the init-local stage before network comes up.

This prevents an initial DHCP request which would leak the stock
cloud-image seed hostname before the user-data/metadata provided hostname
was processed. A leaked cloud-image hostname adversely affects Dynamic
DNS which would continue reallocate 'ubuntu' hostname in DNS to every
instance brought up by cloud-init. These instances would only update DNS
to the cloud-init configured hostname upon DHCP lease renewal.

LP: #1746455

-- 
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:set-hostname-before-network into cloud-init:master.

diff --git a/cloudinit/cloud.py b/cloudinit/cloud.py
index ba61678..6d12c43 100644
--- a/cloudinit/cloud.py
+++ b/cloudinit/cloud.py
@@ -78,8 +78,9 @@ class Cloud(object):
     def get_locale(self):
         return self.datasource.get_locale()
 
-    def get_hostname(self, fqdn=False):
-        return self.datasource.get_hostname(fqdn=fqdn)
+    def get_hostname(self, fqdn=False, metadata_only=False):
+        return self.datasource.get_hostname(
+            fqdn=fqdn, metadata_only=metadata_only)
 
     def device_name_to_device(self, name):
         return self.datasource.device_name_to_device(name)
diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
index d2f1b77..120c3cc 100644
--- a/cloudinit/cmd/main.py
+++ b/cloudinit/cmd/main.py
@@ -40,6 +40,7 @@ from cloudinit.settings import (PER_INSTANCE, PER_ALWAYS, PER_ONCE,
 
 from cloudinit import atomic_helper
 
+from cloudinit.config import cc_set_hostname
 from cloudinit.dhclient_hook import LogDhclient
 
 
@@ -354,7 +355,17 @@ def main_init(name, args):
     LOG.debug("[%s] %s will now be targeting instance id: %s. new=%s",
               mode, name, iid, init.is_new_instance())
 
-    init.apply_network_config(bring_up=bool(mode != sources.DSMODE_LOCAL))
+    if mode == sources.DSMODE_LOCAL:
+        # Before network comes up, set any configured hostname to allow
+        # dhcp clients to advertize this hostname to any DDNS services
+        # lp:1746455.
+        cloud = init.cloudify()
+        (hostname, _fqdn) = util.get_hostname_fqdn(
+            init.cfg, cloud, metadata_only=True)
+        if hostname:  # Either metadata or user-data provided hostname content
+            cc_set_hostname.handle(
+                'pre-network-set-hostname', init.cfg, cloud, LOG, None)
+    init.apply_network_config(bring_up=bring_up)
 
     if mode == sources.DSMODE_LOCAL:
         if init.datasource.dsmode != mode:
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index a05ca2f..d1a213f 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -276,21 +276,34 @@ class DataSource(object):
             return "iid-datasource"
         return str(self.metadata['instance-id'])
 
-    def get_hostname(self, fqdn=False, resolve_ip=False):
+    def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False):
+        """Get hostname or fqdn from the datasource. Look it up if desired.
+
+        @param fqdn: Boolean, set True to return hostname with domain.
+        @param resolve_ip: Boolean, set True to attempt to resolve an ipv4
+            address provided in local-hostname metadata.
+        @param metadata_only: Boolean, set True to avoid looking up hostname
+            if metadata doesn't have local-hostname present.
+
+        @return: hostname or qualified hostname. Optionally return None when
+            metadata_only is True and local-hostname data is not available.
+        """
         defdomain = "localdomain"
         defhost = "localhost"
         domain = defdomain
 
         if not self.metadata or 'local-hostname' not in self.metadata:
+            if metadata_only:
+                return None
             # this is somewhat questionable really.
             # the cloud datasource was asked for a hostname
             # and didn't have one. raising error might be more appropriate
             # but instead, basically look up the existing hostname
             toks = []
             hostname = util.get_hostname()
-            fqdn = util.get_fqdn_from_hosts(hostname)
-            if fqdn and fqdn.find(".") > 0:
-                toks = str(fqdn).split(".")
+            hosts_fqdn = util.get_fqdn_from_hosts(hostname)
+            if hosts_fqdn and hosts_fqdn.find(".") > 0:
+                toks = str(hosts_fqdn).split(".")
             elif hostname and hostname.find(".") > 0:
                 toks = str(hostname).split(".")
             elif hostname:
diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py
index af15115..6639a3c 100644
--- a/cloudinit/sources/tests/test_init.py
+++ b/cloudinit/sources/tests/test_init.py
@@ -7,7 +7,7 @@ import stat
 from cloudinit.helpers import Paths
 from cloudinit.sources import (
     INSTANCE_JSON_FILE, DataSource)
-from cloudinit.tests.helpers import CiTestCase, skipIf
+from cloudinit.tests.helpers import CiTestCase, skipIf, mock
 from cloudinit.user_data import UserDataProcessor
 from cloudinit import util
 
@@ -108,6 +108,74 @@ class TestDataSource(CiTestCase):
         self.assertEqual('userdata_raw', datasource.userdata_raw)
         self.assertEqual('vendordata_raw', datasource.vendordata_raw)
 
+    def test_get_hostname_strips_local_hostname_without_domain(self):
+        """Datasource.get_hostname strips metadata local-hostname of domain."""
+        tmp = self.tmp_dir()
+        datasource = DataSourceTestSubclassNet(
+            self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
+        self.assertTrue(datasource.get_data())
+        self.assertEqual(
+           'test-subclass-hostname', datasource.metadata['local-hostname'])
+        self.assertEqual('test-subclass-hostname', datasource.get_hostname())
+        datasource.metadata['local-hostname'] = 'hostname.my.domain.com'
+        self.assertEqual('hostname', datasource.get_hostname())
+
+    def test_get_hostname_with_fqdn_returns_local_hostname_with_domain(self):
+        """Datasource.get_hostname with fqdn set gets qualified hostname."""
+        tmp = self.tmp_dir()
+        datasource = DataSourceTestSubclassNet(
+            self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
+        self.assertTrue(datasource.get_data())
+        datasource.metadata['local-hostname'] = 'hostname.my.domain.com'
+        self.assertEqual(
+            'hostname.my.domain.com', datasource.get_hostname(fqdn=True))
+
+    def test_get_hostname_without_metadata_uses_system_hostname(self):
+        """Datasource.gethostname runs util.get_hostname when no metadata."""
+        tmp = self.tmp_dir()
+        datasource = DataSourceTestSubclassNet(
+            self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
+        self.assertEqual({}, datasource.metadata)
+        mock_fqdn = 'cloudinit.sources.util.get_fqdn_from_hosts'
+        with mock.patch('cloudinit.sources.util.get_hostname') as m_gethost:
+            with mock.patch(mock_fqdn) as m_fqdn:
+                m_gethost.return_value = 'systemhostname.domain.com'
+                m_fqdn.return_value = None  # No maching fqdn in /etc/hosts
+                self.assertEqual('systemhostname', datasource.get_hostname())
+                self.assertEqual(
+                    'systemhostname.domain.com',
+                    datasource.get_hostname(fqdn=True))
+
+    def test_get_hostname_without_metadata_returns_none(self):
+        """Datasource.gethostname returns None when metadata_only and no MD."""
+        tmp = self.tmp_dir()
+        datasource = DataSourceTestSubclassNet(
+            self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
+        self.assertEqual({}, datasource.metadata)
+        mock_fqdn = 'cloudinit.sources.util.get_fqdn_from_hosts'
+        with mock.patch('cloudinit.sources.util.get_hostname') as m_gethost:
+            with mock.patch(mock_fqdn) as m_fqdn:
+                self.assertIsNone(datasource.get_hostname(metadata_only=True))
+                self.assertIsNone(
+                    datasource.get_hostname(fqdn=True, metadata_only=True))
+        self.assertEqual([], m_gethost.call_args_list)
+        self.assertEqual([], m_fqdn.call_args_list)
+
+    def test_get_hostname_without_metadata_prefers_etc_hosts(self):
+        """Datasource.gethostname prefers /etc/hosts to util.get_hostname."""
+        tmp = self.tmp_dir()
+        datasource = DataSourceTestSubclassNet(
+            self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
+        self.assertEqual({}, datasource.metadata)
+        mock_fqdn = 'cloudinit.sources.util.get_fqdn_from_hosts'
+        with mock.patch('cloudinit.sources.util.get_hostname') as m_gethost:
+            with mock.patch(mock_fqdn) as m_fqdn:
+                m_gethost.return_value = 'systemhostname.domain.com'
+                m_fqdn.return_value = 'fqdnhostname.domain.com'
+                self.assertEqual('fqdnhostname', datasource.get_hostname())
+                self.assertEqual('fqdnhostname.domain.com',
+                                 datasource.get_hostname(fqdn=True))
+
     def test_get_data_write_json_instance_data(self):
         """get_data writes INSTANCE_JSON_FILE to run_dir as readonly root."""
         tmp = self.tmp_dir()
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 02dc2ce..f883fd9 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1026,9 +1026,16 @@ def dos2unix(contents):
     return contents.replace('\r\n', '\n')
 
 
-def get_hostname_fqdn(cfg, cloud):
-    # return the hostname and fqdn from 'cfg'.  If not found in cfg,
-    # then fall back to data from cloud
+def get_hostname_fqdn(cfg, cloud, metadata_only=False):
+    """Get hostname and fqdn from config if present  and fallback to cloud.
+
+    @param cfg: Dictionary of merged user-data configuration (from init.cfg).
+    @param cloud: Cloud instance from init.cloudify().
+    @param metadata_only: Boolean, set True to only query user-data and
+        cloud meta-data, returning None if not present in meta-data.
+    @return: a Tuple of strings <hostname>, <fqdn>. Values can be none when
+        metadata_only is True and no cfg or metadata provides hostname info.
+    """
     if "fqdn" in cfg:
         # user specified a fqdn.  Default hostname then is based off that
         fqdn = cfg['fqdn']
@@ -1042,11 +1049,11 @@ def get_hostname_fqdn(cfg, cloud):
         else:
             # no fqdn set, get fqdn from cloud.
             # get hostname from cfg if available otherwise cloud
-            fqdn = cloud.get_hostname(fqdn=True)
+            fqdn = cloud.get_hostname(fqdn=True, metadata_only=metadata_only)
             if "hostname" in cfg:
                 hostname = cfg['hostname']
             else:
-                hostname = cloud.get_hostname()
+                hostname = cloud.get_hostname(metadata_only=metadata_only)
     return (hostname, fqdn)