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

When instance meta-data provides hostname information, run
cc_set_hostname in the init-local or init-net stage before network
comes up.

Prevent an initial DHCP request which leaks the stock cloud-image default
hostname before the meta-data provided hostname was processed.
A leaked cloud-image hostname adversely affects Dynamic DNS which
would 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.

This branch extends the get_hostname methods in datasource, cloud and
util to limit results to metadata_only to avoid extra cost of querying
the distro for hostname information if metadata does not provide that
information.

LP: #1746455

9 files changed, 449 insertions, 22 deletions

diff --git a/cloudinit/cloud.py b/cloudinit/cloud.py
index ba616781..6d12c437 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 fcddd75c..3f2dbb93 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
 
 
@@ -352,6 +353,11 @@ 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())
 
+    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.
+        _maybe_set_hostname(init, stage='local', retry_stage='network')
     init.apply_network_config(bring_up=bool(mode != sources.DSMODE_LOCAL))
 
     if mode == sources.DSMODE_LOCAL:
@@ -368,6 +374,7 @@ def main_init(name, args):
     init.setup_datasource()
     # update fully realizes user-data (pulling in #include if necessary)
     init.update()
+    _maybe_set_hostname(init, stage='init-net', retry_stage='modules:config')
     # Stage 7
     try:
         # Attempt to consume the data per instance.
@@ -681,6 +688,24 @@ def status_wrapper(name, args, data_d=None, link_d=None):
     return len(v1[mode]['errors'])
 
 
+def _maybe_set_hostname(init, stage, retry_stage):
+    """Call set-hostname if metadata, vendordata or userdata provides it.
+
+    @param stage: String representing current stage in which we are running.
+    @param retry_stage: String represented logs upon error setting hostname.
+    """
+    cloud = init.cloudify()
+    (hostname, _fqdn) = util.get_hostname_fqdn(
+        init.cfg, cloud, metadata_only=True)
+    if hostname:  # meta-data or user-data hostname content
+        try:
+            cc_set_hostname.handle('set-hostname', init.cfg, cloud, LOG, None)
+        except cc_set_hostname.SetHostnameError as e:
+            LOG.debug(
+                'Failed setting hostname in %s stage. Will'
+                ' retry in %s stage. Error: %s.', stage, retry_stage, str(e))
+
+
 def main_features(name, args):
     sys.stdout.write('\n'.join(sorted(version.FEATURES)) + '\n')
 
diff --git a/cloudinit/cmd/tests/test_main.py b/cloudinit/cmd/tests/test_main.py
new file mode 100644
index 00000000..dbe421c0
--- /dev/null
+++ b/cloudinit/cmd/tests/test_main.py
@@ -0,0 +1,161 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+from collections import namedtuple
+import copy
+import os
+from six import StringIO
+
+from cloudinit.cmd import main
+from cloudinit.util import (
+    ensure_dir, load_file, write_file, yaml_dumps)
+from cloudinit.tests.helpers import (
+    FilesystemMockingTestCase, wrap_and_call)
+
+mypaths = namedtuple('MyPaths', 'run_dir')
+myargs = namedtuple('MyArgs', 'debug files force local reporter subcommand')
+
+
+class TestMain(FilesystemMockingTestCase):
+
+    with_logs = True
+
+    def setUp(self):
+        super(TestMain, self).setUp()
+        self.new_root = self.tmp_dir()
+        self.cloud_dir = self.tmp_path('var/lib/cloud/', dir=self.new_root)
+        os.makedirs(self.cloud_dir)
+        self.replicateTestRoot('simple_ubuntu', self.new_root)
+        self.cfg = {
+            'datasource_list': ['None'],
+            'runcmd': ['ls /etc'],  # test ALL_DISTROS
+            'system_info': {'paths': {'cloud_dir': self.cloud_dir,
+                                      'run_dir': self.new_root}},
+            'write_files': [
+                {
+                    'path': '/etc/blah.ini',
+                    'content': 'blah',
+                    'permissions': 0o755,
+                },
+            ],
+            'cloud_init_modules': ['write-files', 'runcmd'],
+        }
+        cloud_cfg = yaml_dumps(self.cfg)
+        ensure_dir(os.path.join(self.new_root, 'etc', 'cloud'))
+        self.cloud_cfg_file = os.path.join(
+            self.new_root, 'etc', 'cloud', 'cloud.cfg')
+        write_file(self.cloud_cfg_file, cloud_cfg)
+        self.patchOS(self.new_root)
+        self.patchUtils(self.new_root)
+        self.stderr = StringIO()
+        self.patchStdoutAndStderr(stderr=self.stderr)
+
+    def test_main_init_run_net_stops_on_file_no_net(self):
+        """When no-net file is present, main_init does not process modules."""
+        stop_file = os.path.join(self.cloud_dir, 'data', 'no-net')  # stop file
+        write_file(stop_file, '')
+        cmdargs = myargs(
+            debug=False, files=None, force=False, local=False, reporter=None,
+            subcommand='init')
+        (item1, item2) = wrap_and_call(
+            'cloudinit.cmd.main',
+            {'util.close_stdin': True,
+             'netinfo.debug_info': 'my net debug info',
+             'util.fixup_output': ('outfmt', 'errfmt')},
+            main.main_init, 'init', cmdargs)
+        # We should not run write_files module
+        self.assertFalse(
+            os.path.exists(os.path.join(self.new_root, 'etc/blah.ini')),
+            'Unexpected run of write_files module produced blah.ini')
+        self.assertEqual([], item2)
+        # Instancify is called
+        instance_id_path = 'var/lib/cloud/data/instance-id'
+        self.assertFalse(
+            os.path.exists(os.path.join(self.new_root, instance_id_path)),
+            'Unexpected call to datasource.instancify produced instance-id')
+        expected_logs = [
+            "Exiting. stop file ['{stop_file}'] existed\n".format(
+                stop_file=stop_file),
+            'my net debug info'  # netinfo.debug_info
+        ]
+        for log in expected_logs:
+            self.assertIn(log, self.stderr.getvalue())
+
+    def test_main_init_run_net_runs_modules(self):
+        """Modules like write_files are run in 'net' mode."""
+        cmdargs = myargs(
+            debug=False, files=None, force=False, local=False, reporter=None,
+            subcommand='init')
+        (item1, item2) = wrap_and_call(
+            'cloudinit.cmd.main',
+            {'util.close_stdin': True,
+             'netinfo.debug_info': 'my net debug info',
+             'util.fixup_output': ('outfmt', 'errfmt')},
+            main.main_init, 'init', cmdargs)
+        self.assertEqual([], item2)
+        # Instancify is called
+        instance_id_path = 'var/lib/cloud/data/instance-id'
+        self.assertEqual(
+            'iid-datasource-none\n',
+            os.path.join(load_file(
+                os.path.join(self.new_root, instance_id_path))))
+        # modules are run (including write_files)
+        self.assertEqual(
+            'blah', load_file(os.path.join(self.new_root, 'etc/blah.ini')))
+        expected_logs = [
+            'network config is disabled by fallback',  # apply_network_config
+            'my net debug info',  # netinfo.debug_info
+            'no previous run detected'
+        ]
+        for log in expected_logs:
+            self.assertIn(log, self.stderr.getvalue())
+
+    def test_main_init_run_net_calls_set_hostname_when_metadata_present(self):
+        """When local-hostname metadata is present, call cc_set_hostname."""
+        self.cfg['datasource'] = {
+            'None': {'metadata': {'local-hostname': 'md-hostname'}}}
+        cloud_cfg = yaml_dumps(self.cfg)
+        write_file(self.cloud_cfg_file, cloud_cfg)
+        cmdargs = myargs(
+            debug=False, files=None, force=False, local=False, reporter=None,
+            subcommand='init')
+
+        def set_hostname(name, cfg, cloud, log, args):
+            self.assertEqual('set-hostname', name)
+            updated_cfg = copy.deepcopy(self.cfg)
+            updated_cfg.update(
+                {'def_log_file': '/var/log/cloud-init.log',
+                 'log_cfgs': [],
+                 'syslog_fix_perms': ['syslog:adm', 'root:adm', 'root:wheel'],
+                 'vendor_data': {'enabled': True, 'prefix': []}})
+            updated_cfg.pop('system_info')
+
+            self.assertEqual(updated_cfg, cfg)
+            self.assertEqual(main.LOG, log)
+            self.assertIsNone(args)
+
+        (item1, item2) = wrap_and_call(
+            'cloudinit.cmd.main',
+            {'util.close_stdin': True,
+             'netinfo.debug_info': 'my net debug info',
+             'cc_set_hostname.handle': {'side_effect': set_hostname},
+             'util.fixup_output': ('outfmt', 'errfmt')},
+            main.main_init, 'init', cmdargs)
+        self.assertEqual([], item2)
+        # Instancify is called
+        instance_id_path = 'var/lib/cloud/data/instance-id'
+        self.assertEqual(
+            'iid-datasource-none\n',
+            os.path.join(load_file(
+                os.path.join(self.new_root, instance_id_path))))
+        # modules are run (including write_files)
+        self.assertEqual(
+            'blah', load_file(os.path.join(self.new_root, 'etc/blah.ini')))
+        expected_logs = [
+            'network config is disabled by fallback',  # apply_network_config
+            'my net debug info',  # netinfo.debug_info
+            'no previous run detected'
+        ]
+        for log in expected_logs:
+            self.assertIn(log, self.stderr.getvalue())
+
+# vi: ts=4 expandtab
diff --git a/cloudinit/config/cc_set_hostname.py b/cloudinit/config/cc_set_hostname.py
index aa3dfe5f..3d2b2da3 100644
--- a/cloudinit/config/cc_set_hostname.py
+++ b/cloudinit/config/cc_set_hostname.py
@@ -32,22 +32,51 @@ will be used.
     hostname: <fqdn/hostname>
 """
 
+import os
+
+
+from cloudinit.atomic_helper import write_json
 from cloudinit import util
 
 
+class SetHostnameError(Exception):
+    """Raised when the distro runs into an exception when setting hostname.
+
+    This may happen if we attempt to set the hostname early in cloud-init's
+    init-local timeframe as certain services may not be running yet.
+    """
+    pass
+
+
 def handle(name, cfg, cloud, log, _args):
     if util.get_cfg_option_bool(cfg, "preserve_hostname", False):
         log.debug(("Configuration option 'preserve_hostname' is set,"
                    " not setting the hostname in module %s"), name)
         return
-
     (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud)
+    # Check for previous successful invocation of set-hostname
+
+    # set-hostname artifact file accounts for both hostname and fqdn
+    # deltas. As such, it's format is different than cc_update_hostname's
+    # previous-hostname file which only contains the base hostname.
+    # TODO consolidate previous-hostname and set-hostname artifact files and
+    # distro._read_hostname implementation so we only validate  one artifact.
+    prev_fn = os.path.join(cloud.get_cpath('data'), "set-hostname")
+    prev_hostname = {}
+    if os.path.exists(prev_fn):
+        prev_hostname = util.load_json(util.load_file(prev_fn))
+    hostname_changed = (hostname != prev_hostname.get('hostname') or
+                        fqdn != prev_hostname.get('fqdn'))
+    if not hostname_changed:
+        log.debug('No hostname changes. Skipping set-hostname')
+        return
+    log.debug("Setting the hostname to %s (%s)", fqdn, hostname)
     try:
-        log.debug("Setting the hostname to %s (%s)", fqdn, hostname)
         cloud.distro.set_hostname(hostname, fqdn)
-    except Exception:
-        util.logexc(log, "Failed to set the hostname to %s (%s)", fqdn,
-                    hostname)
-        raise
+    except Exception as e:
+        msg = "Failed to set the hostname to %s (%s)" % (fqdn, hostname)
+        util.logexc(log, msg)
+        raise SetHostnameError("%s: %s" % (msg, e))
+    write_json(prev_fn, {'hostname': hostname, 'fqdn': fqdn})
 
 # vi: ts=4 expandtab
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index a05ca2f6..df0b374a 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 meta-data.
+        @param metadata_only: Boolean, set True to avoid looking up hostname
+            if meta-data 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 af151154..5065083c 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/tests/test_util.py b/cloudinit/tests/test_util.py
index c3e2e404..d30643dc 100644
--- a/cloudinit/tests/test_util.py
+++ b/cloudinit/tests/test_util.py
@@ -16,6 +16,25 @@ MOUNT_INFO = [
 ]
 
 
+class FakeCloud(object):
+
+    def __init__(self, hostname, fqdn):
+        self.hostname = hostname
+        self.fqdn = fqdn
+        self.calls = []
+
+    def get_hostname(self, fqdn=None, metadata_only=None):
+        myargs = {}
+        if fqdn is not None:
+            myargs['fqdn'] = fqdn
+        if metadata_only is not None:
+            myargs['metadata_only'] = metadata_only
+        self.calls.append(myargs)
+        if fqdn:
+            return self.fqdn
+        return self.hostname
+
+
 class TestUtil(CiTestCase):
 
     def test_parse_mount_info_no_opts_no_arg(self):
@@ -67,3 +86,58 @@ class TestShellify(CiTestCase):
                        "'echo' 'hi' 'sis'", ""]),
             util.shellify(["echo hi mom", ["echo", "hi dad"],
                            ('echo', 'hi', 'sis')]))
+
+
+class TestGetHostnameFqdn(CiTestCase):
+
+    def test_get_hostname_fqdn_from_only_cfg_fqdn(self):
+        """When cfg only has the fqdn key, derive hostname and fqdn from it."""
+        hostname, fqdn = util.get_hostname_fqdn(
+            cfg={'fqdn': 'myhost.domain.com'}, cloud=None)
+        self.assertEqual('myhost', hostname)
+        self.assertEqual('myhost.domain.com', fqdn)
+
+    def test_get_hostname_fqdn_from_cfg_fqdn_and_hostname(self):
+        """When cfg has both fqdn and hostname keys, return them."""
+        hostname, fqdn = util.get_hostname_fqdn(
+            cfg={'fqdn': 'myhost.domain.com', 'hostname': 'other'}, cloud=None)
+        self.assertEqual('other', hostname)
+        self.assertEqual('myhost.domain.com', fqdn)
+
+    def test_get_hostname_fqdn_from_cfg_hostname_with_domain(self):
+        """When cfg has only hostname key which represents a fqdn, use that."""
+        hostname, fqdn = util.get_hostname_fqdn(
+            cfg={'hostname': 'myhost.domain.com'}, cloud=None)
+        self.assertEqual('myhost', hostname)
+        self.assertEqual('myhost.domain.com', fqdn)
+
+    def test_get_hostname_fqdn_from_cfg_hostname_without_domain(self):
+        """When cfg has a hostname without a '.' query cloud.get_hostname."""
+        mycloud = FakeCloud('cloudhost', 'cloudhost.mycloud.com')
+        hostname, fqdn = util.get_hostname_fqdn(
+            cfg={'hostname': 'myhost'}, cloud=mycloud)
+        self.assertEqual('myhost', hostname)
+        self.assertEqual('cloudhost.mycloud.com', fqdn)
+        self.assertEqual(
+            [{'fqdn': True, 'metadata_only': False}], mycloud.calls)
+
+    def test_get_hostname_fqdn_from_without_fqdn_or_hostname(self):
+        """When cfg has neither hostname nor fqdn cloud.get_hostname."""
+        mycloud = FakeCloud('cloudhost', 'cloudhost.mycloud.com')
+        hostname, fqdn = util.get_hostname_fqdn(cfg={}, cloud=mycloud)
+        self.assertEqual('cloudhost', hostname)
+        self.assertEqual('cloudhost.mycloud.com', fqdn)
+        self.assertEqual(
+            [{'fqdn': True, 'metadata_only': False},
+             {'metadata_only': False}], mycloud.calls)
+
+    def test_get_hostname_fqdn_from_passes_metadata_only_to_cloud(self):
+        """Calls to cloud.get_hostname pass the metadata_only parameter."""
+        mycloud = FakeCloud('cloudhost', 'cloudhost.mycloud.com')
+        hostname, fqdn = util.get_hostname_fqdn(
+            cfg={}, cloud=mycloud, metadata_only=True)
+        self.assertEqual(
+            [{'fqdn': True, 'metadata_only': True},
+             {'metadata_only': True}], mycloud.calls)
+
+# vi: ts=4 expandtab
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 083a8efe..4504f053 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1025,9 +1025,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 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']
@@ -1041,11 +1048,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)
 
 
diff --git a/tests/unittests/test_handler/test_handler_set_hostname.py b/tests/unittests/test_handler/test_handler_set_hostname.py
index abdc17e7..d09ec23a 100644
--- a/tests/unittests/test_handler/test_handler_set_hostname.py
+++ b/tests/unittests/test_handler/test_handler_set_hostname.py
@@ -11,6 +11,7 @@ from cloudinit.tests import helpers as t_help
 
 from configobj import ConfigObj
 import logging
+import os
 import shutil
 from six import BytesIO
 import tempfile
@@ -19,14 +20,18 @@ LOG = logging.getLogger(__name__)
 
 
 class TestHostname(t_help.FilesystemMockingTestCase):
+
+    with_logs = True
+
     def setUp(self):
         super(TestHostname, self).setUp()
         self.tmp = tempfile.mkdtemp()
+        util.ensure_dir(os.path.join(self.tmp, 'data'))
         self.addCleanup(shutil.rmtree, self.tmp)
 
     def _fetch_distro(self, kind):
         cls = distros.fetch(kind)
-        paths = helpers.Paths({})
+        paths = helpers.Paths({'cloud_dir': self.tmp})
         return cls(kind, {}, paths)
 
     def test_write_hostname_rhel(self):
@@ -34,7 +39,7 @@ class TestHostname(t_help.FilesystemMockingTestCase):
             'hostname': 'blah.blah.blah.yahoo.com',
         }
         distro = self._fetch_distro('rhel')
-        paths = helpers.Paths({})
+        paths = helpers.Paths({'cloud_dir': self.tmp})
         ds = None
         cc = cloud.Cloud(ds, paths, {}, distro, None)
         self.patchUtils(self.tmp)
@@ -51,7 +56,7 @@ class TestHostname(t_help.FilesystemMockingTestCase):
             'hostname': 'blah.blah.blah.yahoo.com',
         }
         distro = self._fetch_distro('debian')
-        paths = helpers.Paths({})
+        paths = helpers.Paths({'cloud_dir': self.tmp})
         ds = None
         cc = cloud.Cloud(ds, paths, {}, distro, None)
         self.patchUtils(self.tmp)
@@ -65,7 +70,7 @@ class TestHostname(t_help.FilesystemMockingTestCase):
             'hostname': 'blah.blah.blah.suse.com',
         }
         distro = self._fetch_distro('sles')
-        paths = helpers.Paths({})
+        paths = helpers.Paths({'cloud_dir': self.tmp})
         ds = None
         cc = cloud.Cloud(ds, paths, {}, distro, None)
         self.patchUtils(self.tmp)
@@ -74,4 +79,48 @@ class TestHostname(t_help.FilesystemMockingTestCase):
             contents = util.load_file(distro.hostname_conf_fn)
             self.assertEqual('blah', contents.strip())
 
+    def test_multiple_calls_skips_unchanged_hostname(self):
+        """Only new hostname or fqdn values will generate a hostname call."""
+        distro = self._fetch_distro('debian')
+        paths = helpers.Paths({'cloud_dir': self.tmp})
+        ds = None
+        cc = cloud.Cloud(ds, paths, {}, distro, None)
+        self.patchUtils(self.tmp)
+        cc_set_hostname.handle(
+            'cc_set_hostname', {'hostname': 'hostname1.me.com'}, cc, LOG, [])
+        contents = util.load_file("/etc/hostname")
+        self.assertEqual('hostname1', contents.strip())
+        cc_set_hostname.handle(
+            'cc_set_hostname', {'hostname': 'hostname1.me.com'}, cc, LOG, [])
+        self.assertIn(
+            'DEBUG: No hostname changes. Skipping set-hostname\n',
+            self.logs.getvalue())
+        cc_set_hostname.handle(
+            'cc_set_hostname', {'hostname': 'hostname2.me.com'}, cc, LOG, [])
+        contents = util.load_file("/etc/hostname")
+        self.assertEqual('hostname2', contents.strip())
+        self.assertIn(
+            'Non-persistently setting the system hostname to hostname2',
+            self.logs.getvalue())
+
+    def test_error_on_distro_set_hostname_errors(self):
+        """Raise SetHostnameError on exceptions from distro.set_hostname."""
+        distro = self._fetch_distro('debian')
+
+        def set_hostname_error(hostname, fqdn):
+            raise Exception("OOPS on: %s" % fqdn)
+
+        distro.set_hostname = set_hostname_error
+        paths = helpers.Paths({'cloud_dir': self.tmp})
+        ds = None
+        cc = cloud.Cloud(ds, paths, {}, distro, None)
+        self.patchUtils(self.tmp)
+        with self.assertRaises(cc_set_hostname.SetHostnameError) as ctx_mgr:
+            cc_set_hostname.handle(
+                'somename', {'hostname': 'hostname1.me.com'}, cc, LOG, [])
+        self.assertEqual(
+            'Failed to set the hostname to hostname1.me.com (hostname1):'
+            ' OOPS on: hostname1.me.com',
+            str(ctx_mgr.exception))
+
 # vi: ts=4 expandtab