add prefer_fqdn_over_hostname config option (#859)

the above option allows the user to control the behavior of a distro
hostname selection if both short hostname and FQDN are supplied.
If `prefer_fqdn_over_hostname` is true the FQDN will be selected as
hostname; if false the hostname will be selected

LP: #1921004

7 files changed, 105 insertions, 28 deletions

diff --git a/cloudinit/config/cc_set_hostname.py b/cloudinit/config/cc_set_hostname.py
index d4017478..5a59dc32 100644
--- a/cloudinit/config/cc_set_hostname.py
+++ b/cloudinit/config/cc_set_hostname.py
@@ -19,7 +19,10 @@ A hostname and fqdn can be provided by specifying a full domain name under the
 key, and the fqdn of the cloud wil be used. If a fqdn specified with the
 ``hostname`` key, it will be handled properly, although it is better to use
 the ``fqdn`` config key. If both ``fqdn`` and ``hostname`` are set,
-it is distro dependent whether ``hostname`` or ``fqdn`` is used.
+it is distro dependent whether ``hostname`` or ``fqdn`` is used,
+unless the ``prefer_fqdn_over_hostname`` option is true and fqdn is set
+it will force the use of FQDN in all distros, and if false then it will
+force the hostname use.
 
 This module will run in the init-local stage before networking is configured
 if the hostname is set by metadata or user data on the local system.
@@ -38,6 +41,7 @@ based on initial hostname.
 **Config keys**::
 
     preserve_hostname: <true/false>
+    prefer_fqdn_over_hostname: <true/false>
     fqdn: <fqdn>
     hostname: <fqdn/hostname>
 """
@@ -62,6 +66,14 @@ def handle(name, cfg, cloud, log, _args):
         log.debug(("Configuration option 'preserve_hostname' is set,"
                    " not setting the hostname in module %s"), name)
         return
+
+    # Set prefer_fqdn_over_hostname value in distro
+    hostname_fqdn = util.get_cfg_option_bool(cfg,
+                                             "prefer_fqdn_over_hostname",
+                                             None)
+    if hostname_fqdn is not None:
+        cloud.distro.set_option('prefer_fqdn_over_hostname', hostname_fqdn)
+
     (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud)
     # Check for previous successful invocation of set-hostname
 
diff --git a/cloudinit/config/cc_update_hostname.py b/cloudinit/config/cc_update_hostname.py
index d5f4eb5a..f4120356 100644
--- a/cloudinit/config/cc_update_hostname.py
+++ b/cloudinit/config/cc_update_hostname.py
@@ -27,6 +27,7 @@ is set, then the hostname will not be altered.
 **Config keys**::
 
     preserve_hostname: <true/false>
+    prefer_fqdn_over_hostname: <true/false>
     fqdn: <fqdn>
     hostname: <fqdn/hostname>
 """
@@ -45,6 +46,13 @@ def handle(name, cfg, cloud, log, _args):
                    " not updating the hostname in module %s"), name)
         return
 
+    # Set prefer_fqdn_over_hostname value in distro
+    hostname_fqdn = util.get_cfg_option_bool(cfg,
+                                             "prefer_fqdn_over_hostname",
+                                             None)
+    if hostname_fqdn is not None:
+        cloud.distro.set_option('prefer_fqdn_over_hostname', hostname_fqdn)
+
     (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud)
     try:
         prev_fn = os.path.join(cloud.get_cpath('data'), "previous-hostname")
diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 220bd11f..8b8a647d 100755
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -79,6 +79,7 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta):
     shutdown_options_map = {'halt': '-H', 'poweroff': '-P', 'reboot': '-r'}
 
     _ci_pkl_version = 1
+    prefer_fqdn = False
 
     def __init__(self, name, cfg, paths):
         self._paths = paths
@@ -131,6 +132,9 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta):
     def get_option(self, opt_name, default=None):
         return self._cfg.get(opt_name, default)
 
+    def set_option(self, opt_name, value=None):
+        self._cfg[opt_name] = value
+
     def set_hostname(self, hostname, fqdn=None):
         writeable_hostname = self._select_hostname(hostname, fqdn)
         self._write_hostname(writeable_hostname, self.hostname_conf_fn)
@@ -259,6 +263,9 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta):
     def _select_hostname(self, hostname, fqdn):
         # Prefer the short hostname over the long
         # fully qualified domain name
+        if util.get_cfg_option_bool(self._cfg, "prefer_fqdn_over_hostname",
+                                    self.prefer_fqdn) and fqdn:
+            return fqdn
         if not hostname:
             return fqdn
         return hostname
diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py
index dde34d41..9659843f 100644
--- a/cloudinit/distros/freebsd.py
+++ b/cloudinit/distros/freebsd.py
@@ -27,12 +27,7 @@ class Distro(cloudinit.distros.bsd.BSD):
     pkg_cmd_remove_prefix = ["pkg", "remove"]
     pkg_cmd_update_prefix = ["pkg", "update"]
     pkg_cmd_upgrade_prefix = ["pkg", "upgrade"]
-
-    def _select_hostname(self, hostname, fqdn):
-        # Should be FQDN if available. See rc.conf(5) in FreeBSD
-        if fqdn:
-            return fqdn
-        return hostname
+    prefer_fqdn = True  # See rc.conf(5) in FreeBSD
 
     def _get_add_member_to_group_cmd(self, member_name, group_name):
         return ['pw', 'usermod', '-n', member_name, '-G', group_name]
diff --git a/cloudinit/distros/rhel.py b/cloudinit/distros/rhel.py
index c72f7c17..0c00a531 100644
--- a/cloudinit/distros/rhel.py
+++ b/cloudinit/distros/rhel.py
@@ -50,6 +50,10 @@ class Distro(distros.Distro):
         }
     }
 
+    # Should be fqdn if we can use it
+    # See: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/5/html/deployment_guide/ch-sysconfig  # noqa: E501
+    prefer_fqdn = True
+
     def __init__(self, name, cfg, paths):
         distros.Distro.__init__(self, name, cfg, paths)
         # This will be used to restrict certain
@@ -91,13 +95,6 @@ class Distro(distros.Distro):
             }
             rhel_util.update_sysconfig_file(out_fn, host_cfg)
 
-    def _select_hostname(self, hostname, fqdn):
-        # Should be fqdn if we can use it
-        # See: https://www.centos.org/docs/5/html/Deployment_Guide-en-US/ch-sysconfig.html#s2-sysconfig-network # noqa
-        if fqdn:
-            return fqdn
-        return hostname
-
     def _read_system_hostname(self):
         if self.uses_systemd():
             host_fn = self.systemd_hostname_conf_fn
diff --git a/tests/integration_tests/modules/test_set_hostname.py b/tests/integration_tests/modules/test_set_hostname.py
index 2bfa403d..e7f7f6b6 100644
--- a/tests/integration_tests/modules/test_set_hostname.py
+++ b/tests/integration_tests/modules/test_set_hostname.py
@@ -24,6 +24,13 @@ hostname: cloudinit1
 fqdn: cloudinit2.i9n.cloud-init.io
 """
 
+USER_DATA_PREFER_FQDN = """\
+#cloud-config
+prefer_fqdn_over_hostname: {}
+hostname: cloudinit1
+fqdn: cloudinit2.test.io
+"""
+
 
 @pytest.mark.ci
 class TestHostname:
@@ -33,6 +40,16 @@ class TestHostname:
         hostname_output = client.execute("hostname")
         assert "cloudinit2" in hostname_output.strip()
 
+    @pytest.mark.user_data(USER_DATA_PREFER_FQDN.format(True))
+    def test_prefer_fqdn(self, client):
+        hostname_output = client.execute("hostname")
+        assert "cloudinit2.test.io" in hostname_output.strip()
+
+    @pytest.mark.user_data(USER_DATA_PREFER_FQDN.format(False))
+    def test_prefer_short_hostname(self, client):
+        hostname_output = client.execute("hostname")
+        assert "cloudinit1" in hostname_output.strip()
+
     @pytest.mark.user_data(USER_DATA_FQDN)
     def test_hostname_and_fqdn(self, client):
         hostname_output = client.execute("hostname")
diff --git a/tests/unittests/test_handler/test_handler_set_hostname.py b/tests/unittests/test_handler/test_handler_set_hostname.py
index 58abf51a..73641b70 100644
--- a/tests/unittests/test_handler/test_handler_set_hostname.py
+++ b/tests/unittests/test_handler/test_handler_set_hostname.py
@@ -15,6 +15,7 @@ import os
 import shutil
 import tempfile
 from io import BytesIO
+from unittest import mock
 
 LOG = logging.getLogger(__name__)
 
@@ -29,14 +30,53 @@ class TestHostname(t_help.FilesystemMockingTestCase):
         util.ensure_dir(os.path.join(self.tmp, 'data'))
         self.addCleanup(shutil.rmtree, self.tmp)
 
-    def _fetch_distro(self, kind):
+    def _fetch_distro(self, kind, conf=None):
         cls = distros.fetch(kind)
         paths = helpers.Paths({'cloud_dir': self.tmp})
-        return cls(kind, {}, paths)
+        conf = {} if conf is None else conf
+        return cls(kind, conf, paths)
 
-    def test_write_hostname_rhel(self):
+    def test_debian_write_hostname_prefer_fqdn(self):
         cfg = {
-            'hostname': 'blah.blah.blah.yahoo.com',
+            'hostname': 'blah',
+            'prefer_fqdn_over_hostname': True,
+            'fqdn': 'blah.yahoo.com',
+        }
+        distro = self._fetch_distro('debian', cfg)
+        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',
+                               cfg, cc, LOG, [])
+        contents = util.load_file("/etc/hostname")
+        self.assertEqual('blah.yahoo.com', contents.strip())
+
+    @mock.patch('cloudinit.distros.Distro.uses_systemd', return_value=False)
+    def test_rhel_write_hostname_prefer_hostname(self, m_uses_systemd):
+        cfg = {
+            'hostname': 'blah',
+            'prefer_fqdn_over_hostname': False,
+            'fqdn': 'blah.yahoo.com',
+        }
+        distro = self._fetch_distro('rhel', cfg)
+        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',
+                               cfg, cc, LOG, [])
+        contents = util.load_file("/etc/sysconfig/network", decode=False)
+        n_cfg = ConfigObj(BytesIO(contents))
+        self.assertEqual(
+            {'HOSTNAME': 'blah'},
+            dict(n_cfg))
+
+    @mock.patch('cloudinit.distros.Distro.uses_systemd', return_value=False)
+    def test_write_hostname_rhel(self, m_uses_systemd):
+        cfg = {
+            'hostname': 'blah',
+            'fqdn': 'blah.blah.blah.yahoo.com'
         }
         distro = self._fetch_distro('rhel')
         paths = helpers.Paths({'cloud_dir': self.tmp})
@@ -45,15 +85,16 @@ class TestHostname(t_help.FilesystemMockingTestCase):
         self.patchUtils(self.tmp)
         cc_set_hostname.handle('cc_set_hostname',
                                cfg, cc, LOG, [])
-        if not distro.uses_systemd():
-            contents = util.load_file("/etc/sysconfig/network", decode=False)
-            n_cfg = ConfigObj(BytesIO(contents))
-            self.assertEqual({'HOSTNAME': 'blah.blah.blah.yahoo.com'},
-                             dict(n_cfg))
+        contents = util.load_file("/etc/sysconfig/network", decode=False)
+        n_cfg = ConfigObj(BytesIO(contents))
+        self.assertEqual(
+            {'HOSTNAME': 'blah.blah.blah.yahoo.com'},
+            dict(n_cfg))
 
     def test_write_hostname_debian(self):
         cfg = {
-            'hostname': 'blah.blah.blah.yahoo.com',
+            'hostname': 'blah',
+            'fqdn': 'blah.blah.blah.yahoo.com',
         }
         distro = self._fetch_distro('debian')
         paths = helpers.Paths({'cloud_dir': self.tmp})
@@ -65,7 +106,8 @@ class TestHostname(t_help.FilesystemMockingTestCase):
         contents = util.load_file("/etc/hostname")
         self.assertEqual('blah', contents.strip())
 
-    def test_write_hostname_sles(self):
+    @mock.patch('cloudinit.distros.Distro.uses_systemd', return_value=False)
+    def test_write_hostname_sles(self, m_uses_systemd):
         cfg = {
             'hostname': 'blah.blah.blah.suse.com',
         }
@@ -75,9 +117,8 @@ class TestHostname(t_help.FilesystemMockingTestCase):
         cc = cloud.Cloud(ds, paths, {}, distro, None)
         self.patchUtils(self.tmp)
         cc_set_hostname.handle('cc_set_hostname', cfg, cc, LOG, [])
-        if not distro.uses_systemd():
-            contents = util.load_file(distro.hostname_conf_fn)
-            self.assertEqual('blah', contents.strip())
+        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."""