cloud-init-dev team mailing list archive

[Garrett Holmstrom <gholms@xxxxxxxxxxxxxxxxx>]

Garrett Holmstrom has proposed merging lp:~gholms/cloud-init/hostname-refactor into lp:cloud-init.

Requested reviews:
  cloud init development team (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~gholms/cloud-init/hostname-refactor/+merge/125869

All distros use the same logic in their set_hostname and update_hostname methods, so we refactor that into the base Distro class, replace the file names that were previously hardcoded with class variables, and require distros to instead implement _read_hostname and _write_hostname.

After switching to systemd, Fedora switched from the RHEL-like sysconfig mechanism for configuring its hostname to the Debian-like /etc/hostname mechanism, so we move the code that does the latter to the base Distro class by the names _read_bare_hostname and _write_bare_hostname so distros' _read_hostname and _write_hostname functions, respectively, can invoke them.
-- 
https://code.launchpad.net/~gholms/cloud-init/hostname-refactor/+merge/125869
Your team cloud init development team is requested to review the proposed merge of lp:~gholms/cloud-init/hostname-refactor into lp:cloud-init.

=== modified file 'cloudinit/distros/__init__.py'
--- cloudinit/distros/__init__.py	2012-09-18 17:27:41 +0000
+++ cloudinit/distros/__init__.py	2012-09-23 00:37:20 +0000
@@ -48,6 +48,7 @@
     __metaclass__ = abc.ABCMeta
     default_user = None
     default_user_groups = None
+    default_hostname_conf_file = '/etc/hostname'
 
     def __init__(self, name, cfg, paths):
         self._paths = paths
@@ -95,13 +96,68 @@
     def get_option(self, opt_name, default=None):
         return self._cfg.get(opt_name, default)
 
-    @abc.abstractmethod
     def set_hostname(self, hostname):
-        raise NotImplementedError()
+        conf_file = self.get_option('hostname_conf_file',
+                                    default=self.default_hostname_conf_file)
+        out_fn = self._paths.join(False, conf_file)
+        self._write_hostname(hostname, out_fn)
+        if out_fn == conf_file:
+            # We only do this if we are running in non-adjusted root mode
+            LOG.debug("Setting hostname to %s", hostname)
+            util.subp(['hostname', hostname])
 
-    @abc.abstractmethod
     def update_hostname(self, hostname, prev_hostname_fn):
-        raise NotImplementedError()
+        conf_file = self.get_option('hostname_conf_file',
+                                    default=self.default_hostname_conf_file)
+        hostname_prev = self._read_hostname(prev_hostname_fn)
+        read_fn = self._paths.join(True, conf_file)
+        hostname_in_conf = self._read_hostname(read_fn)
+        update_files = []
+        if not hostname_prev or hostname_prev != hostname:
+            update_files.append(prev_hostname_fn)
+        if (not hostname_in_conf or
+            (hostname_in_conf == hostname_prev
+             and hostname_in_conf != hostname)):
+            write_fn = self._paths.join(False, conf_file)
+            update_files.append(write_fn)
+        for fn in update_files:
+            try:
+                self._write_hostname(hostname, fn)
+            except:
+                util.logexc(LOG, "Failed to write hostname %s to %s",
+                            hostname, fn)
+        if (hostname_in_conf and hostname_prev and
+            hostname_in_conf != hostname_prev):
+            LOG.debug(("Hostname in %s differs from that in %s; assuming "
+                       "hostname is user-maintained"), prev_hostname_fn, conf_file)
+        if conf_file in update_files:
+            # We only do this if we are running in non-adjusted root mode
+            LOG.debug("Setting hostname to %s", hostname)
+            util.subp(["hostname", hostname])
+
+    @abc.abstractmethod
+    def _write_hostname(self, hostname, filename):
+        raise NotImplementedError()
+
+    def _write_bare_hostname(self, hostname, out_fn):
+        # "" gives trailing newline.
+        util.write_file(out_fn, "%s\n" % str(hostname), 0644)
+
+    @abc.abstractmethod
+    def _read_hostname(self, filename, default=None):
+        raise NotImplementedError()
+
+    def _read_bare_hostname(self, filename, default=None):
+        contents = util.load_file(filename, quiet=True)
+        for line in contents.splitlines():
+            c_pos = line.find("#")
+            # Handle inline comments
+            if c_pos != -1:
+                line = line[0:c_pos]
+            line_c = line.strip()
+            if line_c:
+                return line_c
+        return default
 
     @abc.abstractmethod
     def package_command(self, cmd, args=None):

=== modified file 'cloudinit/distros/debian.py'
--- cloudinit/distros/debian.py	2012-09-19 19:07:03 +0000
+++ cloudinit/distros/debian.py	2012-09-23 00:37:20 +0000
@@ -57,56 +57,11 @@
         net_fn = self._paths.join(False, "/etc/network/interfaces")
         util.write_file(net_fn, settings)
 
-    def set_hostname(self, hostname):
-        out_fn = self._paths.join(False, "/etc/hostname")
-        self._write_hostname(hostname, out_fn)
-        if out_fn == '/etc/hostname':
-            # Only do this if we are running in non-adjusted root mode
-            LOG.debug("Setting hostname to %s", hostname)
-            util.subp(['hostname', hostname])
-
     def _write_hostname(self, hostname, out_fn):
-        # "" gives trailing newline.
-        util.write_file(out_fn, "%s\n" % str(hostname), 0644)
-
-    def update_hostname(self, hostname, prev_fn):
-        hostname_prev = self._read_hostname(prev_fn)
-        read_fn = self._paths.join(True, "/etc/hostname")
-        hostname_in_etc = self._read_hostname(read_fn)
-        update_files = []
-        if not hostname_prev or hostname_prev != hostname:
-            update_files.append(prev_fn)
-        if (not hostname_in_etc or
-            (hostname_in_etc == hostname_prev and
-             hostname_in_etc != hostname)):
-            write_fn = self._paths.join(False, "/etc/hostname")
-            update_files.append(write_fn)
-        for fn in update_files:
-            try:
-                self._write_hostname(hostname, fn)
-            except:
-                util.logexc(LOG, "Failed to write hostname %s to %s",
-                            hostname, fn)
-        if (hostname_in_etc and hostname_prev and
-            hostname_in_etc != hostname_prev):
-            LOG.debug(("%s differs from /etc/hostname."
-                        " Assuming user maintained hostname."), prev_fn)
-        if "/etc/hostname" in update_files:
-            # Only do this if we are running in non-adjusted root mode
-            LOG.debug("Setting hostname to %s", hostname)
-            util.subp(['hostname', hostname])
+        return self._write_bare_hostname(hostname, out_fn)
 
     def _read_hostname(self, filename, default=None):
-        contents = util.load_file(filename, quiet=True)
-        for line in contents.splitlines():
-            c_pos = line.find("#")
-            # Handle inline comments
-            if c_pos != -1:
-                line = line[0:c_pos]
-            line_c = line.strip()
-            if line_c:
-                return line_c
-        return default
+        return self._read_bare_hostname(filename, default=default)
 
     def _get_localhost_ip(self):
         # Note: http://www.leonardoborda.com/blog/127-0-1-1-ubuntu-debian/

=== modified file 'cloudinit/distros/fedora.py'
--- cloudinit/distros/fedora.py	2012-09-17 18:23:51 +0000
+++ cloudinit/distros/fedora.py	2012-09-23 00:37:20 +0000
@@ -30,3 +30,10 @@
 class Distro(rhel.Distro):
     distro_name  = 'fedora'
     default_user = 'ec2-user'
+    default_hostname_conf_file = '/etc/hostname'
+
+    def _write_hostname(self, hostname, out_fn):
+        return self._write_bare_hostname(hostname, out_fn)
+
+    def _read_hostname(self, filename, default=None):
+        return self._read_bare_hostname(filename, default=default)

=== modified file 'cloudinit/distros/rhel.py'
--- cloudinit/distros/rhel.py	2012-09-19 20:06:58 +0000
+++ cloudinit/distros/rhel.py	2012-09-23 00:37:20 +0000
@@ -57,6 +57,7 @@
 
 
 class Distro(distros.Distro):
+    default_hostname_conf_file = '/etc/sysconfig/network'
 
     def __init__(self, name, cfg, paths):
         distros.Distro.__init__(self, name, cfg, paths)
@@ -128,14 +129,6 @@
         if nameservers or searchservers:
             self._write_resolve(nameservers, searchservers)
 
-    def set_hostname(self, hostname):
-        out_fn = self._paths.join(False, '/etc/sysconfig/network')
-        self._write_hostname(hostname, out_fn)
-        if out_fn == '/etc/sysconfig/network':
-            # Only do this if we are running in non-adjusted root mode
-            LOG.debug("Setting hostname to %s", hostname)
-            util.subp(['hostname', hostname])
-
     def apply_locale(self, locale, out_fn=None):
         if not out_fn:
             out_fn = self._paths.join(False, '/etc/sysconfig/i18n')
@@ -151,33 +144,6 @@
         w_contents = "\n".join(contents.write())
         util.write_file(out_fn, w_contents, 0644)
 
-    def update_hostname(self, hostname, prev_file):
-        hostname_prev = self._read_hostname(prev_file)
-        read_fn = self._paths.join(True, "/etc/sysconfig/network")
-        hostname_in_sys = self._read_hostname(read_fn)
-        update_files = []
-        if not hostname_prev or hostname_prev != hostname:
-            update_files.append(prev_file)
-        if (not hostname_in_sys or
-            (hostname_in_sys == hostname_prev
-             and hostname_in_sys != hostname)):
-            write_fn = self._paths.join(False, "/etc/sysconfig/network")
-            update_files.append(write_fn)
-        for fn in update_files:
-            try:
-                self._write_hostname(hostname, fn)
-            except:
-                util.logexc(LOG, "Failed to write hostname %s to %s",
-                            hostname, fn)
-        if (hostname_in_sys and hostname_prev and
-            hostname_in_sys != hostname_prev):
-            LOG.debug(("%s differs from /etc/sysconfig/network."
-                        " Assuming user maintained hostname."), prev_file)
-        if "/etc/sysconfig/network" in update_files:
-            # Only do this if we are running in non-adjusted root mode
-            LOG.debug("Setting hostname to %s", hostname)
-            util.subp(['hostname', hostname])
-
     def _read_hostname(self, filename, default=None):
         (_exists, contents) = self._read_conf(filename)
         if 'HOSTNAME' in contents: