1. Update with smosers code review and comments (and put some of those comments into the files)

2. Rename consume() to consume_userdata() as it helps in figuring out what this does.
3. Fixup the tests due to #2

3 files changed, 49 insertions, 29 deletions

diff --git a/bin/cloud-init b/bin/cloud-init
index 0b879876..d3ef092f 100755
--- a/bin/cloud-init
+++ b/bin/cloud-init
@@ -34,13 +34,15 @@ if os.path.exists(os.path.join(possible_topdir, "cloudinit", "__init__.py")):
 
 from cloudinit import log as logging
 from cloudinit import netinfo
-from cloudinit import settings
 from cloudinit import sources
 from cloudinit import stages
 from cloudinit import templater
 from cloudinit import util
 from cloudinit import version
 
+from cloudinit.settings import (PER_INSTANCE, PER_ALWAYS, PER_ONCE,
+                                CLOUD_CONFIG)
+
 
 # Module section template
 MOD_SECTION_TPL = "cloud_%s_modules"
@@ -54,9 +56,9 @@ QUERY_DATA_TYPES = [
 
 # Frequency shortname to full name
 FREQ_SHORT_NAMES = {
-    'instance': settings.PER_INSTANCE,
-    'always': settings.PER_ALWAYS,
-    'once': settings.PER_ONCE,
+    'instance': PER_INSTANCE,
+    'always': PER_ALWAYS,
+    'once': PER_ONCE,
 }
 
 LOG = logging.getLogger()
@@ -111,8 +113,15 @@ def main_init(name, args):
         deps = [sources.DEP_FILESYSTEM]
 
     if not args.local:
-        # TODO: What is this for??
-        root_name = "%s.d" % (settings.CLOUD_CONFIG)
+        # See doc/kernel-cmdline.txt
+        #
+        # This is used in maas datasource, in "ephemeral" (read-only root)
+        # environment where the instance netboots to iscsi ro root.
+        # and the entity that controls the pxe config has to configure
+        # the maas datasource.
+        #
+        # Could be used elsewhere, only works on network based (not local).
+        root_name = "%s.d" % (CLOUD_CONFIG)
         target_fn = os.path.join(root_name, "91_kernel_cmdline_url.cfg")
         util.read_write_cmdline_url(target_fn)
 
@@ -194,22 +203,34 @@ def main_init(name, args):
         init.fetch()
     except sources.DataSourceNotFoundException:
         util.logexc(LOG, "No instance datasource found!")
-        # TODO: Return 0 or 1??
-        return 1
+        # In the case of cloud-init (net mode) it is a bit
+        # more likely that the user would consider it
+        # failure if nothing was found. When using
+        # upstart it will also mentions job failure
+        # in console log if exit code is != 0.
+        if args.local:
+            return 0
+        else:
+            return 1
     # Stage 6
     iid = init.instancify()
     LOG.debug("%s will now be targeting instance id: %s", name, iid)
     init.update()
     # Stage 7
     try:
+        # Attempt to consume the data per instance.
+        # This may run user-data handlers and/or perform
+        # url downloads and such as needed.
         (ran, _results) = init.cloudify().run('consume_userdata',
-                                             init.consume,
-                                             args=[settings.PER_INSTANCE],
-                                             freq=settings.PER_INSTANCE)
+                                             init.consume_userdata,
+                                             args=[PER_INSTANCE],
+                                             freq=PER_INSTANCE)
         if not ran:
-            # Just consume anything that is set to run per
-            # always if nothing ran in the per instance section
-            init.consume(settings.PER_ALWAYS)
+            # Just consume anything that is set to run per-always
+            # if nothing ran in the per-instance code
+            #
+            # TODO: should this always happen?? (even if the above runs?)
+            init.consume_userdata(PER_ALWAYS)
     except Exception:
         util.logexc(LOG, "Consuming user data failed!")
         return 1
diff --git a/cloudinit/stages.py b/cloudinit/stages.py
index 47f6e188..6689e4c9 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -192,13 +192,13 @@ class Init(object):
         cfg_list = self.cfg.get('datasource_list') or []
         return (cfg_list, pkg_list)
 
-    def _get_data_source(self, local_only=False):
+    def _get_data_source(self):
         if self.datasource:
             return self.datasource
         ds = self._restore_from_cache()
         if ds:
             LOG.debug("Restored from cache, datasource: %s", ds)
-        if not ds and not local_only:
+        if not ds:
             (cfg_list, pkg_list) = self._get_datasources()
             # Deep copy so that user-data handlers can not modify
             # (which will affect user-data handlers down the line...)
@@ -209,11 +209,10 @@ class Init(object):
                                                cfg_list,
                                                pkg_list)
             LOG.debug("Loaded datasource %s - %s", dsname, ds)
-        if ds:
-            self.datasource = ds
-            # Ensure we adjust our path members datasource
-            # now that we have one (thus allowing ipath to be used)
-            self.paths.datasource = ds
+        self.datasource = ds
+        # Ensure we adjust our path members datasource
+        # now that we have one (thus allowing ipath to be used)
+        self.paths.datasource = ds
         return ds
 
     def _get_instance_subdirs(self):
@@ -275,8 +274,8 @@ class Init(object):
                         "%s\n" % (previous_iid))
         return iid
 
-    def fetch(self, local_only=False):
-        return self._get_data_source(local_only)
+    def fetch(self):
+        return self._get_data_source()
 
     def instancify(self):
         return self._reflect_cur_instance()
@@ -312,7 +311,7 @@ class Init(object):
         ]
         return def_handlers
 
-    def consume(self, frequency=PER_INSTANCE):
+    def consume_userdata(self, frequency=PER_INSTANCE):
         cdir = self.paths.get_cpath("handlers")
         idir = self._get_ipath("handlers")
 
diff --git a/tests/unittests/test_userdata.py b/tests/unittests/test_userdata.py
index eeddde7d..861642b6 100644
--- a/tests/unittests/test_userdata.py
+++ b/tests/unittests/test_userdata.py
@@ -68,7 +68,7 @@ class TestConsumeUserData(MockerTestCase):
 
         log_file = self.capture_log(logging.WARNING)
         ci.fetch()
-        ci.consume()
+        ci.consume_userdata()
         self.assertIn(
             "Unhandled non-multipart (text/x-not-multipart) userdata:",
             log_file.getvalue())
@@ -85,7 +85,7 @@ class TestConsumeUserData(MockerTestCase):
 
         log_file = self.capture_log(logging.WARNING)
         ci.fetch()
-        ci.consume()
+        ci.consume_userdata()
         self.assertIn(
             "Unhandled unknown content-type (text/plain)",
             log_file.getvalue())
@@ -104,7 +104,7 @@ class TestConsumeUserData(MockerTestCase):
 
         log_file = self.capture_log(logging.WARNING)
         ci.fetch()
-        ci.consume()
+        ci.consume_userdata()
         self.assertEqual("", log_file.getvalue())
 
     def test_mime_text_x_shellscript(self):
@@ -122,7 +122,7 @@ class TestConsumeUserData(MockerTestCase):
 
         log_file = self.capture_log(logging.WARNING)
         ci.fetch()
-        ci.consume()
+        ci.consume_userdata()
         self.assertEqual("", log_file.getvalue())
 
     def test_mime_text_plain_shell(self):
@@ -140,5 +140,5 @@ class TestConsumeUserData(MockerTestCase):
 
         log_file = self.capture_log(logging.WARNING)
         ci.fetch()
-        ci.consume()
+        ci.consume_userdata()
         self.assertEqual("", log_file.getvalue())