Review API of select() to prepare removal of networkcachehelper

This also fixes a bug returning wrong metadata when several entries exists
in shadirs.

slapos/libnetworkcache.py

@@ -17,6 +17,7 @@ import ConfigParser
 import hashlib
 import httplib
 import json
+import logging
 import os
 import shutil
 import sys
@@ -33,6 +34,8 @@ TIMEOUT = 60
 # Same here. We just wait longer that, after having uploaded the file, the server digests it. It can take time.
 UPLOAD_TIMEOUT = 60 * 60
 
+logger = logging.getLogger('networkcache')
+logger.setLevel(logging.INFO)
 
 
 class NetworkcacheClient(object):
@@ -222,26 +225,33 @@ class NetworkcacheClient(object):
     '''
     return self._request('cache', sha512sum)
 
-  def select(self, key):
-    ''' Download a file from shacache by selecting the entry in shadir
-    Raise DirectoryNotFound if no trustable file is found.
+  def select(self, key, wanted_metadata_dict={}, required_key_list=frozenset()):
+    '''Return an iterator over shadir entries that match given criteria
     '''
-    for information_json, signature in self.select_generic(key,
-                                              self.signature_certificate_list):
-      break
-    else:
-      raise DirectoryNotFound('Could not find a trustable entry.')
+    required_key_test = frozenset(required_key_list).issubset
+    try:
+      data_list = self.select_generic(key, self.signature_certificate_list)
+    except (urllib2.HTTPError, DirectoryNotFound), e:
+      logger.info(str(e))
+      return
+    for information_json, signature in data_list:
       try:
         information_dict = json.loads(information_json)
       except Exception:
-      raise DirectoryNotFound('Failed to parse json-in-json response:\n%s'
-                              % traceback.format_exc())
+        logger.info('Failed to parse json-in-json response', exc_info=1)
+        continue
       try:
-      sha512 = information_dict['sha512']
-    except Exception:
-      raise DirectoryNotFound('No sha512 in directory response (%r):\n%s'
-                              % (information_dict, traceback.format_exc()))
-    return self.download(sha512)
+        len(information_dict['sha512'])
+      except StandardError:
+        logger.info('Bad or missing sha512 in directory response (%r)',
+                    information_dict)
+        continue
+      if required_key_test(information_dict):
+        for k, v in wanted_metadata_dict.iteritems():
+          if information_dict.get(k) != v:
+            break
+        else:
+          yield information_dict
 
   def select_generic(self, key, filter=True):
     ''' Select trustable entries from shadir.

slapos/libnetworkcachetests.py

@@ -4,6 +4,8 @@ import errno
 import hashlib
 import httplib
 import json
+import logging
+import logging.handlers
 import os
 import urllib2
 import random
@@ -20,6 +22,8 @@ import slapos.signature
 import sys
 from cStringIO import StringIO
 
+logging.basicConfig()
+
 
 class NCHandler(BaseHTTPServer.BaseHTTPRequestHandler):
   def __init__(self, request, address, server):
@@ -173,6 +177,8 @@ class OnlineMixin:
     self.test_string = str(random.random())
     self.test_data = StringIO(self.test_string)
     self.test_shasum = hashlib.sha512(self.test_string).hexdigest()
+    self.handler = logging.handlers.BufferingHandler(float('inf'))
+    slapos.libnetworkcache.logger.addHandler(self.handler)
 
   def tearDown(self):
     if not 'TEST_SHA_CACHE' in os.environ and not 'TEST_SHA_DIR' in os.environ:
@@ -184,13 +190,22 @@ class OnlineMixin:
       if self.thread is not None:
         self.thread.join()
       shutil.rmtree(self.tree)
+    slapos.libnetworkcache.logger.removeHandler(self.handler)
+    del self.handler
+
+  def assertLog(self, msg=None):
+    try:
+      self.assertTrue(self.handler.buffer.pop(0).message.startswith(msg))
+    except IndexError:
+      self.assertEqual(msg, None)
 
-  def assertDirectoryNotFound(self, msg, func, *args, **kw):
+  def select(self, nc, key, *args):
     try:
-      func(*args, **kw)
-      self.fail("DirectoryNotFound not raised")
-    except slapos.libnetworkcache.DirectoryNotFound, e:
-      self.assertTrue(str(e).startswith(msg))
+      return nc.download(nc.select(key).next()['sha512'])
+    except StopIteration:
+      for msg in args:
+        self.assertLog(msg)
+      self.assertLog()
 
   key = """-----BEGIN RSA PRIVATE KEY-----
 MIICXQIBAAKBgQDDrOO87nSiDcXOf+xGc4Iqcdjfwd0RTOxEkO9z8mPZVg2bTPwt
@@ -341,7 +356,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
     urlmd5 = str(random.random())
     key = 'somekey' + str(random.random())
     nc.upload(self.test_data, key, urlmd5=urlmd5, file_name='my file')
-    result = nc.select(key)
+    result = self.select(nc, key)
     self.assertEqual(result.read(), self.test_string)
 
   def test_upload_shadir_select_not_exists(self):
@@ -350,10 +365,8 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
     urlmd5 = str(random.random())
     key = 'somekey' + str(random.random())
     nc.upload(self.test_data, key, urlmd5=urlmd5, file_name='my file')
-    try:
-      nc.select('key_another_key' + str(random.random()))
-    except urllib2.HTTPError, error:
-      self.assertEqual(error.code, httplib.NOT_FOUND)
+    self.select(nc, 'key_another_key' + str(random.random()),
+                'HTTP Error 404: Not Found')
 
   def test_upload_shadir_no_filename(self):
     """Check scenario with shadir used, but not filename passed"""
@@ -385,6 +398,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
     nc = slapos.libnetworkcache.NetworkcacheClient(self.shacache, self.shadir)
     try:
       nc.download(self.test_shasum)
+      self.fail("HTTPError not raised")
     except urllib2.HTTPError, error:
       self.assertEqual(error.code, httplib.NOT_FOUND)
 
@@ -398,7 +412,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
     signed_nc = slapos.libnetworkcache.NetworkcacheClient(
       self.shacache, self.shadir, key_file.name, [self.certificate])
     signed_nc.upload(self.test_data, key, urlmd5=urlmd5, file_name=file_name)
-    result = signed_nc.select(key)
+    result = self.select(signed_nc, key)
     self.assertEqual(result.read(), self.test_string)
 
   def test_select_signed_content_several_certificates(self):
@@ -412,7 +426,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
       self.shacache, self.shadir, key_file.name,
       (self.alternate_certificate, self.certificate))
     signed_nc.upload(self.test_data, key, urlmd5=urlmd5, file_name=file_name)
-    result = signed_nc.select(key)
+    result = self.select(signed_nc, key)
     self.assertEqual(result.read(), self.test_string)
 
   def test_select_signed_content_multiple(self):
@@ -426,7 +440,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
       self.shacache, self.shadir, key_file.name, [self.certificate])
     signed_nc.upload(self.test_data, key, urlmd5=urlmd5, file_name=file_name)
     signed_nc.upload(self.test_data, key, urlmd5=urlmd5, file_name=file_name)
-    result = signed_nc.select(key)
+    result = self.select(signed_nc, key)
     self.assertEqual(result.read(), self.test_string)
 
   def test_select_no_entries(self):
@@ -440,8 +454,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
     f = os.path.join(self.tree, 'shadir', key)
     # now remove the entry from shacache
     open(f, 'w').write(json.dumps([]))
-    self.assertDirectoryNotFound('Could not find a trustable entry.',
-                                 nc.select, key)
+    self.assertEqual(self.select(nc, key), None)
 
   def test_select_no_json_response(self):
     key = 'somekey' + str(random.random())
@@ -454,8 +467,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
     with open(os.path.join(self.tree, 'shadir', key), 'w') as f:
       # now remove the entry from shacache
       f.write('This is not a json.')
-    self.assertDirectoryNotFound('Failed to parse json response',
-                                 nc.select, key)
+    self.select(nc, key, 'Failed to parse json response')
 
   def test_select_json_no_in_json_response(self):
     key = 'somekey' + str(random.random())
@@ -468,8 +480,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
     with open(os.path.join(self.tree, 'shadir', key), 'w') as f:
       # now remove the entry from shacache
       f.write(json.dumps([['This is not a json.', 'signature']]))
-    self.assertDirectoryNotFound('Failed to parse json-in-json response',
-                                 nc.select, key)
+    self.select(nc, key, 'Failed to parse json-in-json response')
 
   def test_select_json_in_json_no_dict(self):
     key = 'somekey' + str(random.random())
@@ -482,8 +493,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
     with open(os.path.join(self.tree, 'shadir', key), 'w') as f:
       # now remove the entry from shacache
       f.write(json.dumps([[json.dumps('This is a string'), 'signature']]))
-    self.assertDirectoryNotFound('No sha512 in directory response',
-                                 nc.select, key)
+    self.select(nc, key, 'Bad or missing sha512 in directory response')
 
   def test_select_signed_content_server_hacked(self):
     key = 'somekey' + str(random.random())
@@ -509,8 +519,7 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
     # ...but as he has no access to key, no way to sign data..
     # hacked_json[0][1] is still a good key
     open(f, 'w').write(json.dumps(hacked_json))
-    self.assertDirectoryNotFound('Could not find a trustable entry.',
-      signed_nc.select, key)
+    self.assertEqual(self.select(signed_nc, key), None)
 
   def test_DirectoryNotFound_non_trustable_entry(self):
     key_file = tempfile.NamedTemporaryFile()
@@ -526,18 +535,17 @@ class OnlineTest(OnlineMixin, unittest.TestCase):
       self.shacache, self.shadir, signature_certificate_list=[
           self.certificate])
     # when no signature is used, all works ok
-    selected = nc.select(key).read()
+    selected = self.select(nc, key).read()
     self.assertEqual(selected, self.test_string)
     # but when signature is used, networkcache will complain
-    self.assertDirectoryNotFound('Could not find a trustable entry.',
-      signed_nc.select, key)
+    self.assertEqual(self.select(signed_nc, key), None)
 
     # of course if proper key will be used to sign the content uploaded
     # into shacache all will work
     upload_nc = slapos.libnetworkcache.NetworkcacheClient(self.shacache,
       self.shadir, signature_private_key_file=key_file.name)
     upload_nc.upload(self.test_data, key, urlmd5=urlmd5, file_name=file_name)
-    selected = signed_nc.select(key).read()
+    selected = self.select(signed_nc, key).read()
     self.assertEqual(selected, self.test_string)
 
 

slapos/networkcachehelper.py

@@ -15,17 +15,11 @@
 # BBB: Deprecated. This file is ugly and must disappear.
 #      DO NOT EXTEND IT. Add methods to NetworkcacheClient class instead.
 
-import json
-import logging
 import os
 import shutil
 import urllib2
 from slapos.libnetworkcache import NetworkcacheClient, UploadError, \
-    DirectoryNotFound
-
-logging.basicConfig()
-logger = logging.getLogger('networkcachehelper')
-logger.setLevel(logging.INFO)
+    DirectoryNotFound, logger
 
 def __upload_network_cached(dir_url, cache_url,
     file_descriptor, directory_key,
@@ -57,16 +51,12 @@ def __upload_network_cached(dir_url, cache_url,
     shadir_cert_file = None
   if not shadir_key_file:
     shadir_key_file = None
-  try:
   nc = NetworkcacheClient(cache_url, dir_url,
       signature_private_key_file=signature_private_key_file,
       shacache_cert_file=shacache_cert_file,
       shacache_key_file=shacache_key_file,
       shadir_cert_file=shadir_cert_file,
       shadir_key_file=shadir_key_file)
-  except TypeError:
-    logger.warning('Incompatible version of networkcache, not using it.')
-    return False
 
   try:
     return nc.upload(file_descriptor, directory_key, **metadata_dict)
@@ -110,76 +100,29 @@ def helper_download_network_cached(dir_url, cache_url,
 
   return (file_descriptor, metadata) if succeeded, False otherwise.
   """
-  if not(dir_url and cache_url):
-    return False
-
-  if len(signature_certificate_list) == 0:
-    # convert [] into None in order to call nc nicely
-    signature_certificate_list = None
-
-  try:
+  if dir_url and cache_url:
     nc = NetworkcacheClient(cache_url, dir_url,
         signature_certificate_list=signature_certificate_list)
-  except TypeError:
-    logger.warning('Incompatible version of networkcache, not using it.')
-    return False
-
-  logger.info('Trying to download %s from network cache...' % directory_key)
-  try:
-    file_descriptor = None
-    json_entry_list = nc.select_generic(directory_key)
-    # For each entry shadir sent, chooses only the entry matching all
-    # wanted metadata, and having all wanted keys
-    matching_entry_list = []
-    for entry, _ in json_entry_list:
-      try:
-        tags = json.loads(entry)
-        match = True
-        for metadata_key, metadata_value in wanted_metadata_dict.items():
-          if tags.get(metadata_key) != metadata_value:
-            # Something doesn't match: not a good entry
-            match = False
-            break
-          # Now checks if all required keys are present
-          # i.e we want that entry contains a list key with not null
-          # corresponding value
-          for required_key in required_key_list:
-            if not tags.get(required_key):
-              match = False
-              break
-        if not match:
-          continue
-        # Everything match. Add it to list of matching entries
-        matching_entry_list.append(tags)
-      except Exception:
-        pass
-    if matching_entry_list:
-    # If a strategy is defined, call it to determine best entry
+    logger.info('Downloading %s...', directory_key)
+    result = nc.select(directory_key, wanted_metadata_dict, required_key_list)
     if strategy:
-        best_entry = strategy(matching_entry_list)
-        if not best_entry:
+      entry = None
+      result = list(result)
+      if result:
+        entry = strategy(result)
+        if not entry: # XXX: this should be the choice of 'strategy' function
           logger.info("Can't find best entry matching strategy, selecting "
               "random one between acceptable ones.")
-          best_entry = matching_entry_list[0]
+          entry = result[0]
     else:
-        best_entry = matching_entry_list[0]
-
-      # download best entry
-      file_descriptor = nc.download(best_entry.get('sha512'))
-      return file_descriptor, tags
-    else:
-      logger.info('No matching entry to download from network cache: %s'\
-          % directory_key)
-      return False
-
-  except (IOError, DirectoryNotFound), e:
-    if isinstance(e, urllib2.HTTPError) and e.code == 404:
-      logger.info('%s does not exist in network cache.' % directory_key)
+      entry = next(result, None)
+    if entry:
+      try:
+        return nc.download(entry['sha512']), entry
+      except urllib2.HTTPError, e:
+        logger.warning('Failed to download %s: %s', directory_key, e)
     else:
-      logger.warning('Failed to download from network cache %s: %s' % (
-          directory_key, str(e)))
-  return False
-
+      logger.info('No matching entry to download %s', directory_key)
 
 def helper_download_network_cached_to_file(dir_url, cache_url,
     signature_certificate_list,