Improve error message when a cookbook doesn't define any recipes in m…

…etadata.json

Fix: keypair-file should be None if empty in auth.cfg, otherwise fabric will try to use the empty keyfile ""

littlechef/chef.py

@@ -17,6 +17,7 @@
 """
 import os
 import simplejson as json
+import time
 
 from fabric.api import *
 from fabric.contrib.files import append, exists
@@ -54,19 +55,15 @@ def _synchronize_node(configfile, cookbook_paths, node_work_path):
     """Performs the Synchronize step of a Chef run:
     Uploads needed cookbooks and all roles to a node
     """
-    # Clean up node
-    for path in ['roles'] + cookbook_paths:
-        with hide('stdout'):
-            sudo('rm -rf {0}/{1}'.format(node_work_path, path))
-
-    cookbooks = []
+    # Read node.json
     with open(configfile, 'r') as f:
         try:
             node = json.loads(f.read())
         except json.decoder.JSONDecodeError as e:
             msg = 'Little Chef found the following error in'
             msg += ' "{0}":\n                {1}'.format(configfile, str(e))
             abort(msg)
+    cookbooks = []
     # Fetch cookbooks needed for recipes
     for recipe in lib.get_recipes_in_node(node):
         recipe = recipe.split('::')[0]
@@ -107,7 +104,6 @@ def _synchronize_node(configfile, cookbook_paths, node_work_path):
                             print "Warning: Possible error because of missing",
                             print "dependency for cookbook {0}".format(recipe['name'])
                             print "         Cookbook '{0}' not found".format(dep)
-                            import time
                             time.sleep(1)
 
     cookbooks_by_path = {}
@@ -117,8 +113,12 @@ def _synchronize_node(configfile, cookbook_paths, node_work_path):
             if os.path.exists(path):
                 cookbooks_by_path[path] = cookbook
 
-    print "Uploading cookbooks... ({0})".format(
-            ", ".join(c for c in cookbooks))
+    # Clean up node
+    for path in ['roles'] + cookbook_paths:
+        with hide('stdout'):
+            sudo('rm -rf {0}/{1}'.format(node_work_path, path))
+
+    print "Uploading cookbooks... ({0})".format(", ".join(c for c in cookbooks))
     _upload_and_unpack([p for p in cookbooks_by_path.keys()], node_work_path)
 
     print "Uploading roles..."

littlechef/lib.py

@@ -70,31 +70,45 @@ def get_recipes_in_cookbook(name, cookbook_paths):
     """Gets the name of all recipes present in a cookbook"""
     recipes = []
     path = None
+    exists = False
     for cookbook_path in cookbook_paths:
-        path = '{0}/{1}/metadata.json'.format(cookbook_path, name)
+        path = os.path.join(cookbook_path, name)
+        exists = exists or os.path.exists(path)
         try:
-            with open(path, 'r') as f:
+            with open(os.path.join(path, 'metadata.json'), 'r') as f:
                 try:
                     cookbook = json.loads(f.read())
-                    for recipe in cookbook.get('recipes', []):
-                        recipes.append(
-                            {
-                                'name': recipe,
-                                'description': cookbook['recipes'][recipe],
-                                'version': cookbook.get('version'),
-                                'dependencies': cookbook.get('dependencies').keys(),
-                                'attributes': cookbook.get('attributes').keys(),
-                            }
-                        )
                 except json.decoder.JSONDecodeError, e:
                     msg = "Little Chef found the following error in your"
-                    msg += " {0}.json file:\n  {1}".format(path, e)
+                    msg += " {0}.json file:\n  {1}".format(
+                        os.path.join(path, 'metadata.json'), e)
                     abort(msg)
+                # Add each recipe defined in the cookbook
+                for recipe in cookbook.get('recipes', []):
+                    recipes.append(
+                        {
+                            'name': recipe,
+                            'description': cookbook['recipes'][recipe],
+                            'version': cookbook.get('version'),
+                            'dependencies': cookbook.get('dependencies', {}).keys(),
+                            'attributes': cookbook.get('attributes', {}).keys(),
+                        }
+                    )
+                if not recipes:
+                    msg = 'Cookbook "{0}"\'s metadata.json'.format(name)
+                    msg += ' doesn\'t define any recipes'
+                    abort(msg)
+            # If cookbook metadata.json found, don't try next cookbook path
+            # metadata.json in site-cookbooks has preference
             break
         except IOError:
-            None
+            # metadata.json was not found, try next cookbook_path
+            pass
     if not recipes:
-        abort('Unable to find cookbook "{0}" with metadata.json'.format(name))
+        if exists:
+            abort('Cookbook "{0}" has no metadata.json'.format(name))
+        else:
+            abort('Unable to find cookbook "{0}"'.format(name))
     return recipes
 
 

littlechef/littlechef.py

@@ -276,11 +276,12 @@ def _readconfig():
 
     # Allow password OR keypair-file not to be present
     try:
-        env.password = config.get('userinfo', 'password')
+        env.password = config.get('userinfo', 'password') or None
     except ConfigParser.NoOptionError:
         pass
     try:
-        env.key_filename = config.get('userinfo', 'keypair-file')
+        #If keypair-file is empty, assign None or fabric will try to read key ""
+        env.key_filename = config.get('userinfo', 'keypair-file') or None
     except ConfigParser.NoOptionError:
         pass
 

tests/auth.cfg

@@ -1,4 +1,4 @@
 [userinfo]
-user = "testuser"
-password = "testpass"
+user = testuser
+password = testpass
 keypair-file = 

tests/tests.py

@@ -64,6 +64,16 @@ def test_list_recipes_detailed_site_cookbooks(self):
         resp, error = self.execute(['../cook', 'list_recipes_detailed'])
         self.assertTrue('0.8.4' in resp)
 
+    def test_no_metadata(self):
+        """Should abort if cookbook has no metadata.json"""
+        cookbooks_path = os.path.dirname(os.path.abspath(__file__))
+        bad_cookbook = os.path.join(cookbooks_path, 'cookbooks', 'bad_cookbook')
+        os.mkdir(bad_cookbook)
+        resp, error = self.execute(['../cook', 'list_recipes'])
+        os.rmdir(bad_cookbook)
+        expected = 'Fatal error: Cookbook "bad_cookbook" has no metadata.json'
+        self.assertTrue(expected in error)
+
 
 class NodeTest(BaseTest):
     def test_list_nodes(self):