Merge lp:~vlastimil-holer/cloud-init/opennebula into lp:~cloud-init-dev/cloud-init/trunk

[Vlastimil Holer]

I'm resubmitting another review proposal for OpenNebula support in cloud-init

* pep8/pylint/tests were fixed
* shell output parser has been replaced by simple direct RE parser
* fixed DSMODE fetching from contextualization disk
* minor fixes, cleanups
* use unittests.helpers.populate_dir (wrapped by populate_context_dir which generates content)

Please let us know what else would be good to fix. Thanks to Javier Fontan <email address hidden>

[Joshua Harlow (harlowja)]

Just a few comments, otherwise seems cool.

Is 'NonConfigDriveDir' (exception afaik) supposed to be brought in somewhere?

Should this ds class be derived from the config drive datasource?
 - This is due to the new overload that was just added there
   for mapping device names to actual devices. I noticed this
   code does not have that function either (without it fstab
   will not be adjusted). So perhaps its better to inherit
   if the functionality is the same?? Perhaps override getdata()
   but still use the rest of the parent?

For capturing 'except subprocess.CalledProcessError as exc:' you might want to catch the one in 'util.py' instead (its not the same exception - although maybe it should be).

Is it possible add more comments as to what the context.sh parsing is doing?
Ie: ''comm -23 <(set | sort -u) <(echo "$VARS") | egrep -v "^(VARS|PIPESTATUS|_)="'' (???)

context_sh[key.lower()]=r.group(1).\
205 + replace('\\\\','\\').\
206 + replace('\\t','\t').\
207 + replace('\\n','\n').\
208 + replace("\\'","'")

Should this just be re.group(1).decode('string_escape')?
>>> print c
\n\tblahblah\n\'
>>> print c.decode('string_escape')

 blahblah
'

review: Needs Fixing

[Joshua Harlow (harlowja)]

Also some tests would be super-great :-)

[Vlastimil Holer (vlastimil-holer)]

> Just a few comments, otherwise seems cool.
>
> Is 'NonConfigDriveDir' (exception afaik) supposed to be brought in somewhere?

Fixed, leftover from ConfigDrive class.

> Should this ds class be derived from the config drive datasource?
> - This is due to the new overload that was just added there
> for mapping device names to actual devices. I noticed this
> code does not have that function either (without it fstab
> will not be adjusted). So perhaps its better to inherit
> if the functionality is the same?? Perhaps override getdata()
> but still use the rest of the parent?

If I understand right, it's not any problem here. OpenNebula's metadata
aren't strictly specified as in OpenStack or EC2. They all are just user
specified and metadata I currently depend on were just mentioned in
their documentation or example scripts, I haven't seen anything like
block device mappings so far. Since there are no default metadata for
device mapping, I think it's better not to invent this functionality now.

You can check here:
http://opennebula.org/documentation:rel3.8:context_overview
All metadata are just optional and user specified, it's common
to have there SSH public key or static network configuration.

> For capturing 'except subprocess.CalledProcessError as exc:' you might want to
> catch the one in 'util.py' instead (its not the same exception - although
> maybe it should be).

Fixed.

> Is it possible add more comments as to what the context.sh parsing is doing?
> Ie: ''comm -23 <(set | sort -u) <(echo "$VARS") | egrep -v
> "^(VARS|PIPESTATUS|_)="'' (???)

Explanation added into code. I know this construction isn't very nice,
but IMHO it's easiest way how to read context variables. Bash dumps
them just as easy parsable (single line) way.

> context_sh[key.lower()]=r.group(1).\
> 205 + replace('\\\\','\\').\
> 206 + replace('\\t','\t').\
> 207 + replace('\\n','\n').\
> 208 + replace("\\'","'")
>
> Should this just be re.group(1).decode('string_escape')?
> >>> print c
> \n\tblahblah\n\'
> >>> print c.decode('string_escape')
>
> blahblah
> '

Yes, thanks, fixed.

> Also some tests would be super-great :-)

On my TODO list, will do that soon.

[Joshua Harlow (harlowja)]

Ok, one last comment.

Is the modification of "cloudinit/distros/__init__.py" still needed or is that just leftover code that got picked up in the review?

[Vlastimil Holer (vlastimil-holer)]

> Ok, one last comment.
>
> Is the modification of "cloudinit/distros/__init__.py" still needed or is that
> just leftover code that got picked up in the review?

You are right, this was leftover after Javi's first version of static network configuration.
Currently we are already generating full Debian style network configuration including
"dns-search|nameservers" interface options. These are on RHEL systems automatically
converted and applied into /etc/resolv.conf inside your code already.

Sorry fot that. Change has been uncommited.

[Scott Moser (smoser)]

Some thoughts:
 * run ./tools/run-pep8 tests/unittests/test_datasource/test_opennebula.py
   and ./tools/run-pylint tests/unittests/test_datasource/test_opennebula.py
   You'll see lots of (sorry) nit-picky things to fix.

 * how does 'user_dsmode' get set? I guess through 'context.sh' ?

 * I guess I might like to see the reading of context.sh in its own method for
   easier testing.

 * populate_dir (copied to your test) is now in unittests/helpers.py

The one thing I really do not like here is that context.sh is explicitly
executed (by bash), as opposed to being parsed. The issue is that I can
essentially do *anything* here. I could attach a disk labeled CDROM, with
context.sh and execute arbitrary code as root.

This isn't terribly a security issue, since executing code as root is generally
cloud-init's goal, but its very inconsistent with other datasources.

Also, fwiw, I've been wanting to explicitly add a "execute something as root
really early" hook. The goal would be to allow you to tweak an image as early
as possible, even fiddling with or fixing cloud-init.

So, please fix the pep8 and pylint, and, are people actually expecting code to be executed? Ie, do they think they can build/provide a context.sh that executes code?

review: Needs Fixing

[Javi Fontan (jfontan)]

"context.sh" is generated by OpenNebula and is a shell script with shell variables. It is meant to be executed by some init scripts in the VM to get that information. It should not contain any "executable" code, at least OpenNebula does not add code to it.

The problem with parsing that file is that some variables can be multiline and and parsing is not that straightforward. For example:

# Context variables generated by OpenNebula
DISK_ID='1'
ETH0_DNS='10.0.0.1'
ETH0_GATEWAY='10.0.0.1'
ETH0_IP='10.0.0.72'
ETH0_MASK='255.255.255.0'
FILES_DS='/var/lib/one/datastores/2/9c92ad910c0b30a411ccdfc141bc7a25:'\''jfopet.sh'\'' '
FILES_URL='http://10.0.0.11/files/installer.tar.gz'
INIT_SCRIPTS='jfopet.sh'
MANIFEST='ubuntu-kvm1'
SSH_PUBLIC_KEY='ssh-dss AAAAB3NzaC1kc3MAAACBANprdUVFRaADZXZaAm2elpRaUGCMETLLuYYJCDUZPb0Dh/V7KeM2/a3rFIDA0s5sVK2XQNqLHyy0U8xA/0R8dplmg7BDckkAzhrDVpEGnQE1fk/xPd1t7u+yeVqpbrfyAmfmyE2P980mhBoWalbeV/f7SmHUP8RiQ2hlAWUxr7I5AAAAFQDLHhFndbcA7svGd/yfY6nU4ubodwAAAIAoXLKlckmZur1pc9TN4XoHa+Fl/6Qpu0XO7Ai/tu8dqHlN+FpVk8BQNnokwE+EZBARLIL0JCjHpT9b5aEvlpRz3TuDa6az8wvJRlScNufmVrf0ls1WCFDHujSLzd3aOkKct35Aamf1amP/NPE2aGne/tPS7HQ3TCf5E2MAQzYVJKSARwAAAIAaVqvU7LfGMEw0hEXr7fuJCMHh4FmPvejiOoSUz2GOU5bceasRitdulCQJbNiVtY6U0S+qQ0X8rvAnG934p4zS9TtgKIhplr146fkbYnNjCaAM0rNVvTh2SzEEKJiG2G1d3wyNuO8wpPhIiJ1OVZrGwkVyWwiNzC2sWXAXldQ9Hg== user1@host
ssh-dss AAAAB3NzaC1kc3MAAACBANBWTQmm4GtkAiPpA20DK1TQd1n7UoC5BymTHlGzFmj+BsQdn6ZZbihV/roNVo1roJhb2hQq/WuQR50D72vMIjrC6t1DkofFBL0iz+mb7JdmNxbE7cnOHMxEr/cd4ds4EwzpBKiQzt8NNcz/zbSadHQtBd0+u1G5nvm4MXHNVYQnAAAAFQDbEN554Or4vd7D+2wzjs3c1nNOowAAAIAdRlTrRG1YmceKHh3urcltniIoo8FrNudwCShbHTCOQbM+KkMUTtw5qwWFuJP6HdaLjmUehqxqDWeWGp8c2y5yMee0JR5cx+iMwhg1Q2o4S6c+zgWdUYIVkuPgYOOR2GMCPdl9mwcxtVvpHD59UFlPh16oLzakUCSxkro5V/LUXxuywAAAIBtYvxfwI5Cl0xq3/KQI7giNef05EXIgK+KwYu3xC6fs+7NxQYFzMsSEQhEJI62J091Kh0RFpFPdGPECIQolt8j4ltymKM9+pfgiE7oBXSxkW44XadBnCWCGU5B4gTnz84VKECWuu2J9Z4cn44hKYt1uj0SxxzExnB1X51kUN+Z5Q== user2@host'

I don't think creating a parser for these files is worth the job.

[Scott Moser (smoser)]

Vlastimil, thanks.
  One thing that I thought about was the 'parsing'. If the content in 'config.sh' is actually shell , then it makes most sense to have shell execute it rather than parsing with regex. Parsing with regex just seems brittle, and is only going to cover very basic things.
  I put together 'parse_shell_config' at https://gist.github.com/smoser/6466708
  That basically reads bash variables into a dict after executing a given file.

  This would execute the file, which is what I was originally opposed to. I don't miss the irony there :).
  The gist there has the ability to use 'sudo' to change to a different user. We could use 'nobody' by default and allow it to be configured. By parsing with shell, and running as nobody, we should be pretty adverse to any complexities that might be present in the 'config.sh'.

[Vlastimil Holer (vlastimil-holer)]

Scott, it looks great, I definitely love your code style!

From my point of view, I more agree with your former opinion not to use shell. I originally chose that option because I got normalized output with each variable on single line very easy. Also in the past it could be possible to reference variables from other variables, but this is not true since OpenNebula 4.0 (May 2013). Context variables are now single quoted and so it's impossible to reference other variables or execute external commands. Context.sh is now just plain key='value' file, which is (from historic reasons when people wrote their own contextualization shell scripts) usually sourced from shell. Now I see the only benefit in more reliable parsing then our horrible regexp.

So, it's up to you what you prefer to leave in your project at last. Both will work fine.

Also we can make a simple parser using e.g. pyparsing, but it means new dependency into cloud-init.

[Scott Moser (smoser)]

OK.
 I took a further look.
 I'm fine to merge this.
 Could you make the following changes:
  * use the bash parsing at https://gist.github.com/smoser/6466708
  * allow configuration to set the user to drop privledges to (None indicate no change).
    That configuration would be in the datasource config (datasource:OpenNebula:parseuser) or some name.
  * default that user to 'Nobody'.
  * there is one pylint (at least on my machine) error, complaining about the toks.split() use toks = str(toks).split('.'). Its not a real issue, but just make pylint happy by wrapping 'toks' in str().

Other than that I think it looks good.
Sooner is better to get those changes in. I want to make a upload to ubuntu sometime this week, and I'm shooting for 0.7.3 probably end of next week.

Thanks!

[lp:~vlastimil-holer/cloud-init/opennebula]

687. By Vlastimil Holer on 2013-09-09

Replace RE context.sh parser with Scott's rewrite of bash dumper. Upper case context variable names.

688. By Vlastimil Holer on 2013-09-09

Fix pylint complain on toks.split('.')

689. By Vlastimil Holer on 2013-09-10

Configurable OpenNebula::parseuser. Seed search dir+dev merge.
Eat shell parser error output. Few tests for tests for get_data.

690. By Vlastimil Holer on 2013-09-10

Update OpenNebula documentation (parseuser, more fs. labels, K-V single quotes)

691. By Vlastimil Holer on 2013-09-10

Detect invalid system user for test

[Vlastimil Holer (vlastimil-holer)]

I have used your bash parsing, the only change I made there was
stderr redirection for invalid (or empty) context scripts:

> sp = subprocess.Popen(cmd, stdin=subprocess.PIPE,
> stdout=subprocess.PIPE,
> stderr=subprocess.PIPE)

I thought you'll move the function somewhere into your
libraries after merge.

* unprivileged user is configurable in
  OpenNebula:parseuser, defaults to "nobody"

* pylint complain fixed

But, I have added some more unit tests around get_data,
which were missing, and other cleanups. So, please look
once again if it's OK for you at least for now. Or what
should I fix before the end of week.

Also I didn't have time to check the new code in real,
I'll let you know if it's ok ASAP.

[lp:~vlastimil-holer/cloud-init/opennebula]

692. By Vlastimil Holer on 2013-09-10

Fix detection of ETHx_IP context variable, add test.

[Vlastimil Holer (vlastimil-holer)]

Now looks good after fix in commit 692, tested on Debian 6.

[Scott Moser (smoser)]

Vlastimil, can you please merge from
 lp:~smoser/cloud-init/opennebula
and test that?

Just a few cleanups there.

Thanks.

[lp:~vlastimil-holer/cloud-init/opennebula]

693. By Vlastimil Holer on 2013-09-10

Merged lp:~smoser/cloud-init/opennebula

694. By Vlastimil Holer on 2013-09-10

All fake util.find_devs_with set before try-finally section

[Vlastimil Holer (vlastimil-holer)]

I don't know why have you moved one save of original util.find_devs_with before try-finally section, but I have moved the rest to be same.

I have quickly tested on real systems and works. Thanks!

[Scott Moser (smoser)]

because I hit that one. probably in error, but the finally didn't account for it not being defined.

On Sep 10, 2013, at 7:28 PM, Vlastimil Holer <email address hidden> wrote:

> I don't know why have you moved one save of original util.find_devs_with before try-finally section, but I have moved the rest to be same.
>
> I have quickly tested on real systems and works. Thanks!
> --
> https://code.launchpad.net/~vlastimil-holer/cloud-init/opennebula/+merge/184278
> You are requested to review the proposed merge of lp:~vlastimil-holer/cloud-init/opennebula into lp:cloud-init.
>

=== modified file 'cloudinit/settings.py'
--- cloudinit/settings.py	2013-07-18 21:37:18 +0000
+++ cloudinit/settings.py	2013-09-10 23:15:34 +0000
@@ -31,6 +31,7 @@
     'datasource_list': [
         'NoCloud',
         'ConfigDrive',
+        'OpenNebula',
         'Azure',
         'AltCloud',
         'OVF',

=== added file 'cloudinit/sources/DataSourceOpenNebula.py'
--- cloudinit/sources/DataSourceOpenNebula.py	1970-01-01 00:00:00 +0000
+++ cloudinit/sources/DataSourceOpenNebula.py	2013-09-10 23:15:34 +0000
@@ -0,0 +1,442 @@
+# vi: ts=4 expandtab
+#
+#    Copyright (C) 2012 Canonical Ltd.
+#    Copyright (C) 2012 Yahoo! Inc.
+#    Copyright (C) 2012-2013 CERIT Scientific Cloud
+#    Copyright (C) 2012-2013 OpenNebula.org
+#
+#    Author: Scott Moser <[e-mail]>
+#    Author: Joshua Harlow <[e-mail]>
+#    Author: Vlastimil Holer <[e-mail]>
+#    Author: Javier Fontan <[e-mail]>
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License version 3, as
+#    published by the Free Software Foundation.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+#    You should have received a copy of the GNU General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import os
+import pwd
+import re
+import string  # pylint: disable=W0402
+
+from cloudinit import log as logging
+from cloudinit import sources
+from cloudinit import util
+
+LOG = logging.getLogger(__name__)
+
+DEFAULT_IID = "iid-dsopennebula"
+DEFAULT_MODE = 'net'
+DEFAULT_PARSEUSER = 'nobody'
+CONTEXT_DISK_FILES = ["context.sh"]
+VALID_DSMODES = ("local", "net", "disabled")
+
+
+class DataSourceOpenNebula(sources.DataSource):
+    def __init__(self, sys_cfg, distro, paths):
+        sources.DataSource.__init__(self, sys_cfg, distro, paths)
+        self.dsmode = 'local'
+        self.seed = None
+        self.seed_dir = os.path.join(paths.seed_dir, 'opennebula')
+
+    def __str__(self):
+        root = sources.DataSource.__str__(self)
+        return "%s [seed=%s][dsmode=%s]" % (root, self.seed, self.dsmode)
+
+    def get_data(self):
+        defaults = {"instance-id": DEFAULT_IID}
+        results = None
+        seed = None
+
+        # decide parseuser for context.sh shell reader
+        parseuser = DEFAULT_PARSEUSER
+        if 'parseuser' in self.ds_cfg:
+            parseuser = self.ds_cfg.get('parseuser')
+
+        candidates = [self.seed_dir]
+        candidates.extend(find_candidate_devs())
+        for cdev in candidates:
+            try:
+                if os.path.isdir(self.seed_dir):
+                    results = read_context_disk_dir(cdev, asuser=parseuser)
+                elif cdev.startswith("/dev"):
+                    results = util.mount_cb(cdev, read_context_disk_dir,
+                                            data=parseuser)
+            except NonContextDiskDir:
+                continue
+            except BrokenContextDiskDir as exc:
+                raise exc
+            except util.MountFailedError:
+                LOG.warn("%s was not mountable" % cdev)
+
+            if results:
+                seed = cdev
+                LOG.debug("found datasource in %s", cdev)
+                break
+
+        if not seed:
+            return False
+
+        # merge fetched metadata with datasource defaults
+        md = results['metadata']
+        md = util.mergemanydict([md, defaults])
+
+        # check for valid user specified dsmode
+        user_dsmode = results['metadata'].get('DSMODE', None)
+        if user_dsmode not in VALID_DSMODES + (None,):
+            LOG.warn("user specified invalid mode: %s", user_dsmode)
+            user_dsmode = None
+
+        # decide dsmode
+        if user_dsmode:
+            dsmode = user_dsmode
+        elif self.ds_cfg.get('dsmode'):
+            dsmode = self.ds_cfg.get('dsmode')
+        else:
+            dsmode = DEFAULT_MODE
+
+        if dsmode == "disabled":
+            # most likely user specified
+            return False
+
+        # apply static network configuration only in 'local' dsmode
+        if ('network-interfaces' in results and self.dsmode == "local"):
+            LOG.debug("Updating network interfaces from %s", self)
+            self.distro.apply_network(results['network-interfaces'])
+
+        if dsmode != self.dsmode:
+            LOG.debug("%s: not claiming datasource, dsmode=%s", self, dsmode)
+            return False
+
+        self.seed = seed
+        self.metadata = md
+        self.userdata_raw = results.get('userdata')
+        return True
+
+    def get_hostname(self, fqdn=False, resolve_ip=None):
+        if resolve_ip is None:
+            if self.dsmode == 'net':
+                resolve_ip = True
+            else:
+                resolve_ip = False
+        return sources.DataSource.get_hostname(self, fqdn, resolve_ip)
+
+
+class DataSourceOpenNebulaNet(DataSourceOpenNebula):
+    def __init__(self, sys_cfg, distro, paths):
+        DataSourceOpenNebula.__init__(self, sys_cfg, distro, paths)
+        self.dsmode = 'net'
+
+
+class NonContextDiskDir(Exception):
+    pass
+
+
+class BrokenContextDiskDir(Exception):
+    pass
+
+
+class OpenNebulaNetwork(object):
+    REG_DEV_MAC = re.compile(
+                    r'^\d+: (eth\d+):.*?link\/ether (..:..:..:..:..:..) ?',
+                    re.MULTILINE | re.DOTALL)
+
+    def __init__(self, ip, context):
+        self.ip = ip
+        self.context = context
+        self.ifaces = self.get_ifaces()
+
+    def get_ifaces(self):
+        return self.REG_DEV_MAC.findall(self.ip)
+
+    def mac2ip(self, mac):
+        components = mac.split(':')[2:]
+        return [str(int(c, 16)) for c in components]
+
+    def get_ip(self, dev, components):
+        var_name = dev.upper() + '_IP'
+        if var_name in self.context:
+            return self.context[var_name]
+        else:
+            return '.'.join(components)
+
+    def get_mask(self, dev):
+        var_name = dev.upper() + '_MASK'
+        if var_name in self.context:
+            return self.context[var_name]
+        else:
+            return '255.255.255.0'
+
+    def get_network(self, dev, components):
+        var_name = dev.upper() + '_NETWORK'
+        if var_name in self.context:
+            return self.context[var_name]
+        else:
+            return '.'.join(components[:-1]) + '.0'
+
+    def get_gateway(self, dev):
+        var_name = dev.upper() + '_GATEWAY'
+        if var_name in self.context:
+            return self.context[var_name]
+        else:
+            return None
+
+    def get_dns(self, dev):
+        var_name = dev.upper() + '_DNS'
+        if var_name in self.context:
+            return self.context[var_name]
+        else:
+            return None
+
+    def get_domain(self, dev):
+        var_name = dev.upper() + '_DOMAIN'
+        if var_name in self.context:
+            return self.context[var_name]
+        else:
+            return None
+
+    def gen_conf(self):
+        global_dns = []
+        if 'DNS' in self.context:
+            global_dns.append(self.context['DNS'])
+
+        conf = []
+        conf.append('auto lo')
+        conf.append('iface lo inet loopback')
+        conf.append('')
+
+        for i in self.ifaces:
+            dev = i[0]
+            mac = i[1]
+            ip_components = self.mac2ip(mac)
+
+            conf.append('auto ' + dev)
+            conf.append('iface ' + dev + ' inet static')
+            conf.append('  address ' + self.get_ip(dev, ip_components))
+            conf.append('  network ' + self.get_network(dev, ip_components))
+            conf.append('  netmask ' + self.get_mask(dev))
+
+            gateway = self.get_gateway(dev)
+            if gateway:
+                conf.append('  gateway ' + gateway)
+
+            domain = self.get_domain(dev)
+            if domain:
+                conf.append('  dns-search ' + domain)
+
+            # add global DNS servers to all interfaces
+            dns = self.get_dns(dev)
+            if global_dns or dns:
+                all_dns = global_dns
+                if dns:
+                    all_dns.append(dns)
+                conf.append('  dns-nameservers ' + ' '.join(all_dns))
+
+            conf.append('')
+
+        return "\n".join(conf)
+
+
+def find_candidate_devs():
+    """
+    Return a list of devices that may contain the context disk.
+    """
+    combined = []
+    for f in ('LABEL=CONTEXT', 'LABEL=CDROM', 'TYPE=iso9660'):
+        devs = util.find_devs_with(f)
+        devs.sort()
+        for d in devs:
+            if d not in combined:
+                combined.append(d)
+
+    return combined
+
+
+def switch_user_cmd(user):
+    return ['sudo', '-u', user]
+
+
+def parse_shell_config(content, keylist=None, bash=None, asuser=None,
+                       switch_user_cb=None):
+
+    if isinstance(bash, str):
+        bash = [bash]
+    elif bash is None:
+        bash = ['bash', '-e']
+
+    if switch_user_cb is None:
+        switch_user_cb = switch_user_cmd
+
+    # allvars expands to all existing variables by using '${!x*}' notation
+    # where x is lower or upper case letters or '_'
+    allvars = ["${!%s*}" % x for x in string.letters + "_"]
+
+    keylist_in = keylist
+    if keylist is None:
+        keylist = allvars
+        keylist_in = []
+
+    setup = '\n'.join(('__v="";', '',))
+
+    def varprinter(vlist):
+        # output '\0'.join(['_start_', key=value NULL for vars in vlist]
+        return '\n'.join((
+            'printf "%s\\0" _start_',
+            'for __v in %s; do' % ' '.join(vlist),
+            '   printf "%s=%s\\0" "$__v" "${!__v}";',
+            'done',
+            ''
+        ))
+
+    # the rendered 'bcmd' is bash syntax that does
+    # setup: declare variables we use (so they show up in 'all')
+    # varprinter(allvars): print all variables known at beginning
+    # content: execute the provided content
+    # varprinter(keylist): print all variables known after content
+    #
+    # output is then a null terminated array of:
+    #   literal '_start_'
+    #   key=value (for each preset variable)
+    #   literal '_start_'
+    #   key=value (for each post set variable)
+    bcmd = ('unset IFS\n' +
+            setup +
+            varprinter(allvars) +
+            '{\n%s\n\n:\n} > /dev/null\n' % content +
+            'unset IFS\n' +
+            varprinter(keylist) + "\n")
+
+    cmd = []
+    if asuser is not None:
+        cmd = switch_user_cb(asuser)
+
+    cmd.extend(bash)
+
+    (output, _error) = util.subp(cmd, data=bcmd)
+
+    # exclude vars in bash that change on their own or that we used
+    excluded = ("RANDOM", "LINENO", "_", "__v")
+    preset = {}
+    ret = {}
+    target = None
+    output = output[0:-1]  # remove trailing null
+
+    # go through output.  First _start_ is for 'preset', second for 'target'.
+    # Add to target only things were changed and not in volitile
+    for line in output.split("\x00"):
+        try:
+            (key, val) = line.split("=", 1)
+            if target is preset:
+                target[key] = val
+            elif (key not in excluded and
+                  (key in keylist_in or preset.get(key) != val)):
+                ret[key] = val
+        except ValueError:
+            if line != "_start_":
+                raise
+            if target is None:
+                target = preset
+            elif target is preset:
+                target = ret
+
+    return ret
+
+
+def read_context_disk_dir(source_dir, asuser=None):
+    """
+    read_context_disk_dir(source_dir):
+    read source_dir and return a tuple with metadata dict and user-data
+    string populated.  If not a valid dir, raise a NonContextDiskDir
+    """
+    found = {}
+    for af in CONTEXT_DISK_FILES:
+        fn = os.path.join(source_dir, af)
+        if os.path.isfile(fn):
+            found[af] = fn
+
+    if not found:
+        raise NonContextDiskDir("%s: %s" % (source_dir, "no files found"))
+
+    context = {}
+    results = {'userdata': None, 'metadata': {}}
+
+    if "context.sh" in found:
+        if asuser is not None:
+            try:
+                pwd.getpwnam(asuser)
+            except KeyError as e:
+                raise BrokenContextDiskDir("configured user '%s' "
+                                           "does not exist", asuser)
+        try:
+            with open(os.path.join(source_dir, 'context.sh'), 'r') as f:
+                content = f.read().strip()
+
+            context = parse_shell_config(content, asuser=asuser)
+        except util.ProcessExecutionError as e:
+            raise BrokenContextDiskDir("Error processing context.sh: %s" % (e))
+        except IOError as e:
+            raise NonContextDiskDir("Error reading context.sh: %s" % (e))
+    else:
+        raise NonContextDiskDir("Missing context.sh")
+
+    if not context:
+        return results
+
+    results['metadata'] = context
+
+    # process single or multiple SSH keys
+    ssh_key_var = None
+    if "SSH_KEY" in context:
+        ssh_key_var = "SSH_KEY"
+    elif "SSH_PUBLIC_KEY" in context:
+        ssh_key_var = "SSH_PUBLIC_KEY"
+
+    if ssh_key_var:
+        lines = context.get(ssh_key_var).splitlines()
+        results['metadata']['public-keys'] = [l for l in lines
+            if len(l) and not l.startswith("#")]
+
+    # custom hostname -- try hostname or leave cloud-init
+    # itself create hostname from IP address later
+    for k in ('HOSTNAME', 'PUBLIC_IP', 'IP_PUBLIC', 'ETH0_IP'):
+        if k in context:
+            results['metadata']['local-hostname'] = context[k]
+            break
+
+    # raw user data
+    if "USER_DATA" in context:
+        results['userdata'] = context["USER_DATA"]
+    elif "USERDATA" in context:
+        results['userdata'] = context["USERDATA"]
+
+    # generate static /etc/network/interfaces
+    # only if there are any required context variables
+    # http://opennebula.org/documentation:rel3.8:cong#network_configuration
+    for k in context.keys():
+        if re.match(r'^ETH\d+_IP$', k):
+            (out, _) = util.subp(['/sbin/ip', 'link'])
+            net = OpenNebulaNetwork(out, context)
+            results['network-interfaces'] = net.gen_conf()
+            break
+
+    return results
+
+
+# Used to match classes to dependencies
+datasources = [
+    (DataSourceOpenNebula, (sources.DEP_FILESYSTEM, )),
+    (DataSourceOpenNebulaNet, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)),
+]
+
+
+# Return a list of data sources that match this set of dependencies
+def get_datasource_list(depends):
+    return sources.list_from_depends(depends, datasources)

=== modified file 'cloudinit/sources/__init__.py'
--- cloudinit/sources/__init__.py	2013-07-23 17:10:33 +0000
+++ cloudinit/sources/__init__.py	2013-09-10 23:15:34 +0000
@@ -53,9 +53,16 @@
         self.userdata = None
         self.metadata = None
         self.userdata_raw = None
+
+        # find the datasource config name.
+        # remove 'DataSource' from classname on front, and remove 'Net' on end.
+        # Both Foo and FooNet sources expect config in cfg['sources']['Foo']
         name = type_utils.obj_name(self)
         if name.startswith(DS_PREFIX):
             name = name[len(DS_PREFIX):]
+        if name.endswith('Net'):
+            name = name[0:-3]
+
         self.ds_cfg = util.get_cfg_by_path(self.sys_cfg,
                                           ("datasource", name), {})
         if not ud_proc:
@@ -144,7 +151,7 @@
             return "iid-datasource"
         return str(self.metadata['instance-id'])
 
-    def get_hostname(self, fqdn=False):
+    def get_hostname(self, fqdn=False, resolve_ip=False):
         defdomain = "localdomain"
         defhost = "localhost"
         domain = defdomain
@@ -168,7 +175,14 @@
             # make up a hostname (LP: #475354) in format ip-xx.xx.xx.xx
             lhost = self.metadata['local-hostname']
             if util.is_ipv4(lhost):
-                toks = ["ip-%s" % lhost.replace(".", "-")]
+                toks = []
+                if resolve_ip:
+                    toks = util.gethostbyaddr(lhost)
+
+                if toks:
+                    toks = str(toks).split('.')
+                else:
+                    toks = ["ip-%s" % lhost.replace(".", "-")]
             else:
                 toks = lhost.split(".")
 

=== modified file 'cloudinit/util.py'
--- cloudinit/util.py	2013-09-07 07:07:26 +0000
+++ cloudinit/util.py	2013-09-10 23:15:34 +0000
@@ -955,6 +955,13 @@
     return hostname
 
 
+def gethostbyaddr(ip):
+    try:
+        return socket.gethostbyaddr(ip)[0]
+    except socket.herror:
+        return None
+
+
 def is_resolvable_url(url):
     """determine if this url is resolvable (existing or ip)."""
     return (is_resolvable(urlparse.urlparse(url).hostname))

=== modified file 'doc/rtd/topics/datasources.rst'
--- doc/rtd/topics/datasources.rst	2013-02-06 07:58:49 +0000
+++ doc/rtd/topics/datasources.rst	2013-09-10 23:15:34 +0000
@@ -141,6 +141,12 @@
 .. include:: ../../sources/configdrive/README.rst
 
 ---------------------------
+OpenNebula
+---------------------------
+
+.. include:: ../../sources/opennebula/README.rst
+
+---------------------------
 Alt cloud
 ---------------------------
 

=== added directory 'doc/sources/opennebula'
=== added file 'doc/sources/opennebula/README.rst'
--- doc/sources/opennebula/README.rst	1970-01-01 00:00:00 +0000
+++ doc/sources/opennebula/README.rst	2013-09-10 23:15:34 +0000
@@ -0,0 +1,142 @@
+The `OpenNebula`_ (ON) datasource supports the contextualization disk.
+
+  See `contextualization overview`_, `contextualizing VMs`_ and
+  `network configuration`_ in the public documentation for
+  more information.
+
+OpenNebula's virtual machines are contextualized (parametrized) by
+CD-ROM image, which contains a shell script *context.sh* with
+custom variables defined on virtual machine start. There are no
+fixed contextualization variables, but the datasource accepts
+many used and recommended across the documentation.
+
+Datasource configuration
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Datasource accepts following configuration options.
+
+::
+    
+    dsmode:
+      values: local, net, disabled
+      default: net
+
+Tells if this datasource will be processed in 'local' (pre-networking) or
+'net' (post-networking) stage or even completely 'disabled'.
+
+::
+    
+    parseuser:
+      default: nobody
+
+Unprivileged system user used for contextualization script
+processing.
+
+Contextualization disk
+~~~~~~~~~~~~~~~~~~~~~~
+
+The following criteria are required:
+
+1. Must be formatted with `iso9660`_ filesystem
+   or have a *filesystem* label of **CONTEXT** or **CDROM**
+2. Must contain file *context.sh* with contextualization variables.
+   File is generated by OpenNebula, it has a KEY='VALUE' format and
+   can be easily read by bash 
+
+Contextualization variables
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+There are no fixed contextualization variables in OpenNebula, no standard.
+Following variables were found on various places and revisions of
+the OpenNebula documentation. Where multiple similar variables are
+specified, only first found is taken.
+
+::
+    
+    DSMODE
+
+Datasource mode configuration override. Values: local, net, disabled.
+
+::
+
+    DNS
+    ETH<x>_IP
+    ETH<x>_NETWORK
+    ETH<x>_MASK
+    ETH<x>_GATEWAY
+    ETH<x>_DOMAIN
+    ETH<x>_DNS
+
+Static `network configuration`_.
+
+::
+    
+    HOSTNAME
+
+Instance hostname.
+
+::
+    
+    PUBLIC_IP
+    IP_PUBLIC
+    ETH0_IP
+
+If no hostname has been specified, cloud-init will try to create hostname
+from instance's IP address in 'local' dsmode. In 'net' dsmode, cloud-init 
+tries to resolve one of its IP addresses to get hostname.
+
+::
+    
+    SSH_KEY
+    SSH_PUBLIC_KEY
+
+One or multiple SSH keys (separated by newlines) can be specified.
+
+::
+    
+    USER_DATA
+    USERDATA
+
+cloud-init user data.
+
+Example configuration
+~~~~~~~~~~~~~~~~~~~~~
+
+This example cloud-init configuration (*cloud.cfg*) enables
+OpenNebula datasource only in 'net' mode.
+
+::
+    
+    disable_ec2_metadata: True
+    datasource_list: ['OpenNebula']
+    datasource:
+      OpenNebula:
+        dsmode: net
+        parseuser: nobody
+
+Example VM's context section
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+::
+    
+    CONTEXT=[
+      PUBLIC_IP="$NIC[IP]",
+      SSH_KEY="$USER[SSH_KEY] 
+    $USER[SSH_KEY1] 
+    $USER[SSH_KEY2] ",
+      USER_DATA="#cloud-config
+    # see https://help.ubuntu.com/community/CloudInit
+    
+    packages: []
+    
+    mounts:
+    - [vdc,none,swap,sw,0,0]
+    runcmd:
+    - echo 'Instance has been configured by cloud-init.' | wall
+    " ]
+
+.. _OpenNebula: http://opennebula.org/
+.. _contextualization overview: http://opennebula.org/documentation:documentation:context_overview
+.. _contextualizing VMs: http://opennebula.org/documentation:documentation:cong
+.. _network configuration: http://opennebula.org/documentation:documentation:cong#network_configuration
+.. _iso9660: https://en.wikipedia.org/wiki/ISO_9660

=== added file 'tests/unittests/test_datasource/test_opennebula.py'
--- tests/unittests/test_datasource/test_opennebula.py	1970-01-01 00:00:00 +0000
+++ tests/unittests/test_datasource/test_opennebula.py	2013-09-10 23:15:34 +0000
@@ -0,0 +1,267 @@
+from cloudinit.sources import DataSourceOpenNebula as ds
+from cloudinit import helpers
+from cloudinit import util
+from mocker import MockerTestCase
+from tests.unittests.helpers import populate_dir
+
+import os
+import pwd
+
+TEST_VARS = {
+    'VAR1': 'single',
+    'VAR2': 'double word',
+    'VAR3': 'multi\nline\n',
+    'VAR4': "'single'",
+    'VAR5': "'double word'",
+    'VAR6': "'multi\nline\n'",
+    'VAR7': 'single\\t',
+    'VAR8': 'double\\tword',
+    'VAR9': 'multi\\t\nline\n',
+    'VAR10': '\\',  # expect \
+    'VAR11': '\'',  # expect '
+    'VAR12': '$',   # expect $
+}
+
+INVALID_CONTEXT = ';'
+USER_DATA = '#cloud-config\napt_upgrade: true'
+SSH_KEY = 'ssh-rsa AAAAB3NzaC1....sIkJhq8wdX+4I3A4cYbYP ubuntu@server-460-%i'
+HOSTNAME = 'foo.example.com'
+PUBLIC_IP = '10.0.0.3'
+
+CMD_IP_OUT = '''\
+1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN
+    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
+2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP qlen 1000
+    link/ether 02:00:0a:12:01:01 brd ff:ff:ff:ff:ff:ff
+'''
+
+
+class TestOpenNebulaDataSource(MockerTestCase):
+    parsed_user = None
+
+    def setUp(self):
+        super(TestOpenNebulaDataSource, self).setUp()
+        self.tmp = self.makeDir()
+        self.paths = helpers.Paths({'cloud_dir': self.tmp})
+
+        # defaults for few tests
+        self.ds = ds.DataSourceOpenNebula
+        self.seed_dir = os.path.join(self.paths.seed_dir, "opennebula")
+        self.sys_cfg = {'datasource': {'OpenNebula': {'dsmode': 'local'}}}
+
+        # we don't want 'sudo' called in tests. so we patch switch_user_cmd
+        def my_switch_user_cmd(user):
+            self.parsed_user = user
+            return []
+
+        self.switch_user_cmd_real = ds.switch_user_cmd
+        ds.switch_user_cmd = my_switch_user_cmd
+
+    def tearDown(self):
+        ds.switch_user_cmd = self.switch_user_cmd_real
+        super(TestOpenNebulaDataSource, self).tearDown()
+
+    def test_get_data_non_contextdisk(self):
+        orig_find_devs_with = util.find_devs_with
+        try:
+            # dont' try to lookup for CDs
+            util.find_devs_with = lambda n: []
+            dsrc = self.ds(sys_cfg=self.sys_cfg, distro=None, paths=self.paths)
+            ret = dsrc.get_data()
+            self.assertFalse(ret)
+        finally:
+            util.find_devs_with = orig_find_devs_with
+
+    def test_get_data_broken_contextdisk(self):
+        orig_find_devs_with = util.find_devs_with
+        try:
+            # dont' try to lookup for CDs
+            util.find_devs_with = lambda n: []
+            populate_dir(self.seed_dir, {'context.sh': INVALID_CONTEXT})
+            dsrc = self.ds(sys_cfg=self.sys_cfg, distro=None, paths=self.paths)
+            self.assertRaises(ds.BrokenContextDiskDir, dsrc.get_data)
+        finally:
+            util.find_devs_with = orig_find_devs_with
+
+    def test_get_data_invalid_identity(self):
+        orig_find_devs_with = util.find_devs_with
+        try:
+            # generate non-existing system user name
+            sys_cfg = self.sys_cfg
+            invalid_user = 'invalid'
+            while not sys_cfg['datasource']['OpenNebula'].get('parseuser'):
+                try:
+                    pwd.getpwnam(invalid_user)
+                    invalid_user += 'X'
+                except KeyError:
+                    sys_cfg['datasource']['OpenNebula']['parseuser'] = \
+                        invalid_user
+
+            # dont' try to lookup for CDs
+            util.find_devs_with = lambda n: []
+            populate_context_dir(self.seed_dir, {'KEY1': 'val1'})
+            dsrc = self.ds(sys_cfg=sys_cfg, distro=None, paths=self.paths)
+            self.assertRaises(ds.BrokenContextDiskDir, dsrc.get_data)
+        finally:
+            util.find_devs_with = orig_find_devs_with
+
+    def test_get_data(self):
+        orig_find_devs_with = util.find_devs_with
+        try:
+            # dont' try to lookup for CDs
+            util.find_devs_with = lambda n: []
+            populate_context_dir(self.seed_dir, {'KEY1': 'val1'})
+            dsrc = self.ds(sys_cfg=self.sys_cfg, distro=None, paths=self.paths)
+            ret = dsrc.get_data()
+            self.assertTrue(ret)
+        finally:
+            util.find_devs_with = orig_find_devs_with
+
+    def test_seed_dir_non_contextdisk(self):
+        self.assertRaises(ds.NonContextDiskDir, ds.read_context_disk_dir,
+                          self.seed_dir)
+
+    def test_seed_dir_empty1_context(self):
+        populate_dir(self.seed_dir, {'context.sh': ''})
+        results = ds.read_context_disk_dir(self.seed_dir)
+
+        self.assertEqual(results['userdata'], None)
+        self.assertEqual(results['metadata'], {})
+
+    def test_seed_dir_empty2_context(self):
+        populate_context_dir(self.seed_dir, {})
+        results = ds.read_context_disk_dir(self.seed_dir)
+
+        self.assertEqual(results['userdata'], None)
+        self.assertEqual(results['metadata'], {})
+
+    def test_seed_dir_broken_context(self):
+        populate_dir(self.seed_dir, {'context.sh': INVALID_CONTEXT})
+
+        self.assertRaises(ds.BrokenContextDiskDir,
+                          ds.read_context_disk_dir,
+                          self.seed_dir)
+
+    def test_context_parser(self):
+        populate_context_dir(self.seed_dir, TEST_VARS)
+        results = ds.read_context_disk_dir(self.seed_dir)
+
+        self.assertTrue('metadata' in results)
+        self.assertEqual(TEST_VARS, results['metadata'])
+
+    def test_ssh_key(self):
+        public_keys = ['first key', 'second key']
+        for c in range(4):
+            for k in ('SSH_KEY', 'SSH_PUBLIC_KEY'):
+                my_d = os.path.join(self.tmp, "%s-%i" % (k, c))
+                populate_context_dir(my_d, {k: '\n'.join(public_keys)})
+                results = ds.read_context_disk_dir(my_d)
+
+                self.assertTrue('metadata' in results)
+                self.assertTrue('public-keys' in results['metadata'])
+                self.assertEqual(public_keys,
+                                 results['metadata']['public-keys'])
+
+            public_keys.append(SSH_KEY % (c + 1,))
+
+    def test_user_data(self):
+        for k in ('USER_DATA', 'USERDATA'):
+            my_d = os.path.join(self.tmp, k)
+            populate_context_dir(my_d, {k: USER_DATA})
+            results = ds.read_context_disk_dir(my_d)
+
+            self.assertTrue('userdata' in results)
+            self.assertEqual(USER_DATA, results['userdata'])
+
+    def test_hostname(self):
+        for k in ('HOSTNAME', 'PUBLIC_IP', 'IP_PUBLIC', 'ETH0_IP'):
+            my_d = os.path.join(self.tmp, k)
+            populate_context_dir(my_d, {k: PUBLIC_IP})
+            results = ds.read_context_disk_dir(my_d)
+
+            self.assertTrue('metadata' in results)
+            self.assertTrue('local-hostname' in results['metadata'])
+            self.assertEqual(PUBLIC_IP, results['metadata']['local-hostname'])
+
+    def test_network_interfaces(self):
+        populate_context_dir(self.seed_dir, {'ETH0_IP': '1.2.3.4'})
+        results = ds.read_context_disk_dir(self.seed_dir)
+
+        self.assertTrue('network-interfaces' in results)
+
+    def test_find_candidates(self):
+        def my_devs_with(criteria):
+            return {
+                "LABEL=CONTEXT": ["/dev/sdb"],
+                "LABEL=CDROM": ["/dev/sr0"],
+                "TYPE=iso9660": ["/dev/vdb"],
+            }.get(criteria, [])
+
+        orig_find_devs_with = util.find_devs_with
+        try:
+            util.find_devs_with = my_devs_with
+            self.assertEqual(["/dev/sdb", "/dev/sr0", "/dev/vdb"],
+                             ds.find_candidate_devs())
+        finally:
+            util.find_devs_with = orig_find_devs_with
+
+
+class TestOpenNebulaNetwork(MockerTestCase):
+
+    def setUp(self):
+        super(TestOpenNebulaNetwork, self).setUp()
+
+    def test_lo(self):
+        net = ds.OpenNebulaNetwork('', {})
+        self.assertEqual(net.gen_conf(), u'''\
+auto lo
+iface lo inet loopback
+''')
+
+    def test_eth0(self):
+        net = ds.OpenNebulaNetwork(CMD_IP_OUT, {})
+        self.assertEqual(net.gen_conf(), u'''\
+auto lo
+iface lo inet loopback
+
+auto eth0
+iface eth0 inet static
+  address 10.18.1.1
+  network 10.18.1.0
+  netmask 255.255.255.0
+''')
+
+    def test_eth0_override(self):
+        context = {
+            'DNS': '1.2.3.8',
+            'ETH0_IP': '1.2.3.4',
+            'ETH0_NETWORK': '1.2.3.0',
+            'ETH0_MASK': '255.255.0.0',
+            'ETH0_GATEWAY': '1.2.3.5',
+            'ETH0_DOMAIN': 'example.com',
+            'ETH0_DNS': '1.2.3.6 1.2.3.7'
+        }
+
+        net = ds.OpenNebulaNetwork(CMD_IP_OUT, context)
+        self.assertEqual(net.gen_conf(), u'''\
+auto lo
+iface lo inet loopback
+
+auto eth0
+iface eth0 inet static
+  address 1.2.3.4
+  network 1.2.3.0
+  netmask 255.255.0.0
+  gateway 1.2.3.5
+  dns-search example.com
+  dns-nameservers 1.2.3.8 1.2.3.6 1.2.3.7
+''')
+
+
+def populate_context_dir(path, variables):
+    data = "# Context variables generated by OpenNebula\n"
+    for (k, v) in variables.iteritems():
+        data += ("%s='%s'\n" % (k.upper(), v.replace(r"'", r"'\''")))
+    populate_dir(path, {'context.sh': data})
+
+# vi: ts=4 expandtab