Merge lp:~gpadgett/cloud-init/ovirt into lp:~cloud-init-dev/cloud-init/trunk

[Greg Padgett]

This branch contains fixes found while investigating integration of cloud-init into oVirt. They're in 3 categories:
 - compatibility with systemd configuration management (as used in Fedora 18)
 - workaround for a 2.6 kernel quirk which prevented 'blkid' from displaying /dev/sr0 in some cases
 - writing sysconfig files in typical convention, with a newline preceding EOF, to make some parsers happy

[Scott Moser (smoser)]

this looks fine to me, the only question i have is over 'blkid' usage and /dev/sr0.
I have one concern. At one point I was just trying 'mount /dev/sr0' and expecting that to time out quickly, but it turned out that that was taking 18 seconds to time out in a VM with an empty cd-rom. (see revision 350).
   I just want to make sure that we're not going todo something similar here. I suspect not, but would like to make sure that you've verified that boot speed is not affected significantly by this.

Reguarding the systemd changes, I largely have to blindly trust you.
Maybe Josh can sanity check?

[Greg Padgett (gpadgett)]

Thanks Scott. I tested the blkid calls to existing-but-empty and non-existing /dev/srX devices and didn't see any delays. Not as methodically, but I also did plenty of reboots with and without attached media and speed seemed fine. This was all on Fedora 18, RHEL 6.4, and Ubuntu 12.04 LTS--if you can think of other versions or tests to run I'd be more than happy to try it out.

I'd welcome a systemd sanity check as well. So far it seems good based on my testing and the "Changes for System Administrators" section of the F18 manual. You/others may notice that the code manually manipulates the /etc/localtime symlink instead of using timedatectl--this was done to avoid a timeout (dbus-related?) and errors that happen if timedatectl is called during startup.

[Joshua Harlow (harlowja)]

Since I don't have access to RHEL7 yet I am hoping it works there.

Otherwise seems fine to me.

[Greg Padgett (gpadgett)]

> Since I don't have access to RHEL7 yet I am hoping it works there.

I'm hoping the same, I don't see a reason why it wouldn't.

> Otherwise seems fine to me.

Great, thanks.

Is there anything else I can do to help get this one in?

=== modified file 'cloudinit/distros/rhel.py'
--- cloudinit/distros/rhel.py	2013-03-07 19:54:25 +0000
+++ cloudinit/distros/rhel.py	2013-03-26 22:14:53 +0000
@@ -47,8 +47,10 @@
     # See: http://tiny.cc/6r99fw
     clock_conf_fn = "/etc/sysconfig/clock"
     locale_conf_fn = '/etc/sysconfig/i18n'
+    systemd_locale_conf_fn = '/etc/locale.conf'
     network_conf_fn = "/etc/sysconfig/network"
     hostname_conf_fn = "/etc/sysconfig/network"
+    systemd_hostname_conf_fn = "/etc/hostname"
     network_script_tpl = '/etc/sysconfig/network-scripts/ifcfg-%s'
     resolve_conf_fn = "/etc/resolv.conf"
     tz_local_fn = "/etc/localtime"
@@ -143,21 +145,36 @@
             ]
             if not exists:
                 lines.insert(0, util.make_header())
-            util.write_file(fn, "\n".join(lines), 0644)
+            util.write_file(fn, "\n".join(lines) + "\n", 0644)
+
+    def _dist_uses_systemd(self):
+        # Fedora 18 and RHEL 7 were the first adopters in their series
+        (dist, vers) = util.system_info()['dist'][:2]
+        major = (int)(vers.split('.')[0])
+        return ((dist.startswith('Red Hat Enterprise Linux') and major >= 7)
+                or (dist.startswith('Fedora') and major >= 18))
 
     def apply_locale(self, locale, out_fn=None):
-        if not out_fn:
-            out_fn = self.locale_conf_fn
+        if self._dist_uses_systemd():
+            if not out_fn:
+                out_fn = self.systemd_locale_conf_fn
+            out_fn = self.systemd_locale_conf_fn
+        else:
+            if not out_fn:
+                out_fn = self.locale_conf_fn
         locale_cfg = {
             'LANG': locale,
         }
         self._update_sysconfig_file(out_fn, locale_cfg)
 
     def _write_hostname(self, hostname, out_fn):
-        host_cfg = {
-            'HOSTNAME': hostname,
-        }
-        self._update_sysconfig_file(out_fn, host_cfg)
+        if self._dist_uses_systemd():
+            util.subp(['hostnamectl', 'set-hostname', str(hostname)])
+        else:
+            host_cfg = {
+                'HOSTNAME': hostname,
+            }
+            self._update_sysconfig_file(out_fn, host_cfg)
 
     def _select_hostname(self, hostname, fqdn):
         # See: http://bit.ly/TwitgL
@@ -167,15 +184,25 @@
         return hostname
 
     def _read_system_hostname(self):
-        return (self.network_conf_fn,
-                self._read_hostname(self.network_conf_fn))
+        if self._dist_uses_systemd():
+            host_fn = self.systemd_hostname_conf_fn
+        else:
+            host_fn = self.hostname_conf_fn
+        return (host_fn, self._read_hostname(host_fn))
 
     def _read_hostname(self, filename, default=None):
-        (_exists, contents) = self._read_conf(filename)
-        if 'HOSTNAME' in contents:
-            return contents['HOSTNAME']
+        if self._dist_uses_systemd():
+            (out, _err) = util.subp(['hostname'])
+            if len(out):
+                return out
+            else:
+                return default
         else:
-            return default
+            (_exists, contents) = self._read_conf(filename)
+            if 'HOSTNAME' in contents:
+                return contents['HOSTNAME']
+            else:
+                return default
 
     def _read_conf(self, fn):
         exists = False
@@ -200,13 +227,19 @@
         if not os.path.isfile(tz_file):
             raise RuntimeError(("Invalid timezone %s,"
                                 " no file found at %s") % (tz, tz_file))
-        # Adjust the sysconfig clock zone setting
-        clock_cfg = {
-            'ZONE': str(tz),
-        }
-        self._update_sysconfig_file(self.clock_conf_fn, clock_cfg)
-        # This ensures that the correct tz will be used for the system
-        util.copy(tz_file, self.tz_local_fn)
+        if self._dist_uses_systemd():
+            # Currently, timedatectl complains if invoked during startup
+            # so for compatibility, create the link manually.
+            util.del_file(self.tz_local_fn)
+            util.sym_link(tz_file, self.tz_local_fn)
+        else:
+            # Adjust the sysconfig clock zone setting
+            clock_cfg = {
+                'ZONE': str(tz),
+            }
+            self._update_sysconfig_file(self.clock_conf_fn, clock_cfg)
+            # This ensures that the correct tz will be used for the system
+            util.copy(tz_file, self.tz_local_fn)
 
     def package_command(self, command, args=None, pkgs=None):
         if pkgs is None:
 
=== modified file 'cloudinit/sources/DataSourceConfigDrive.py'
--- cloudinit/sources/DataSourceConfigDrive.py	2013-03-07 21:27:47 +0000
+++ cloudinit/sources/DataSourceConfigDrive.py	2013-03-26 22:14:53 +0000
@@ -258,6 +258,9 @@
         * labeled with 'config-2'
     """
 
+    # Query optical drive to get it in blkid cache for 2.6 kernels
+    util.find_devs_with(path="/dev/sr0")
+
     by_fstype = (util.find_devs_with("TYPE=vfat") +
                  util.find_devs_with("TYPE=iso9660"))
     by_label = util.find_devs_with("LABEL=config-2")
 
=== modified file 'cloudinit/sources/DataSourceNoCloud.py'
--- cloudinit/sources/DataSourceNoCloud.py	2013-03-07 21:27:47 +0000
+++ cloudinit/sources/DataSourceNoCloud.py	2013-03-26 22:14:53 +0000
@@ -87,6 +87,9 @@
 
         label = self.ds_cfg.get('fs_label', "cidata")
         if label is not None:
+            # Query optical drive to get it in blkid cache for 2.6 kernels
+            util.find_devs_with(path="/dev/sr0")
+
             fslist = util.find_devs_with("TYPE=vfat")
             fslist.extend(util.find_devs_with("TYPE=iso9660"))
 
 
=== modified file 'cloudinit/util.py'
--- cloudinit/util.py	2013-03-19 13:32:04 +0000
+++ cloudinit/util.py	2013-03-26 22:14:53 +0000
@@ -408,6 +408,7 @@
         'release': platform.release(),
         'python': platform.python_version(),
         'uname': platform.uname(),
+        'dist': platform.linux_distribution(),
     }
 
 
 
=== modified file 'tests/unittests/test_datasource/test_configdrive.py'
--- tests/unittests/test_datasource/test_configdrive.py	2013-01-17 00:46:30 +0000
+++ tests/unittests/test_datasource/test_configdrive.py	2013-03-26 22:14:53 +0000
@@ -259,8 +259,9 @@
     def test_find_candidates(self):
         devs_with_answers = {}
 
-        def my_devs_with(criteria):
-            return devs_with_answers[criteria]
+        def my_devs_with(*args, **kwargs):
+            criteria = args[0] if len(args) else kwargs.pop('criteria', None)
+            return devs_with_answers.get(criteria, [])
 
         def my_is_partition(dev):
             return dev[-1] in "0123456789" and not dev.startswith("sr")