Reworked devserver so that update images generated are cached in directories named after

autoupdate.py

 # Copyright (c) 2009-2010 The Chromium OS Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 from buildutil import BuildObject
 from xml.dom import minidom
+
 import cherrypy
 import os
 import shutil
 import subprocess
-import tempfile
 import time
 

[DaleCurtis, 2010/11/15 22:38:04]

Why remove? Standard is two lines between top level definitions.

[dgarrett, 2010/11/15 23:32:39]

Done.

-
 def _LogMessage(message):
   cherrypy.log(message, 'UPDATE')
 
 
 class Autoupdate(BuildObject):
   """Class that contains functionality that handles Chrome OS update pings.
 
   Members:
     serve_only: Serve images from a pre-built image.zip file.  static_dir
       must be set to the location of the image.zip.
@@ skipping 74 matching lines @@
       return os.system('cd %s && unzip -n image.zip' % image_dir) == 0
 
   def _GetImageName(self):
     """Returns the name of the image that should be used."""
     if self.use_test_image:
       image_name = 'chromiumos_test_image.bin'
     else:
       image_name = 'chromiumos_image.bin'
     return image_name
 
-  def _IsImageNewerThanCached(self, image_path, cached_file_path):
-    """Returns true if the image is newer than the cached image."""
-    if os.path.exists(cached_file_path) and os.path.exists(image_path):
-      _LogMessage('Usable cached image found at %s.' % cached_file_path)
-      return os.path.getmtime(image_path) > os.path.getmtime(cached_file_path)
-    elif not os.path.exists(cached_file_path) and not os.path.exists(image_path):
-      raise Exception('Image does not exist and cached image missing')
-    else:
-      # Only one is missing, figure out which one.
-      if os.path.exists(image_path):
-        _LogMessage('No cached image found - image generation required.')
-        return True
-      else:
-        _LogMessage('Cached image found to serve at %s.' % cached_file_path)
-        return False
-
   def _GetSize(self, update_path):
     """Returns the size of the file given."""
     return os.path.getsize(update_path)
 
   def _GetHash(self, update_path):
     """Returns the sha1 of the file given."""
     cmd = ('cat %s | openssl sha1 -binary | openssl base64 | tr \'\\n\' \' \';'
            % update_path)
     return os.popen(cmd).read().rstrip()
 
@@ skipping 10 matching lines @@
   # it takes advantage of reduced I/O and multiple processors. Something like:
   # % tee < FILE > /dev/null \
   #     >( openssl dgst -sha256 -binary | openssl base64 ) \
   #     >( openssl sha1 -binary | openssl base64 )
   def _GetSHA256(self, update_path):
     """Returns the sha256 of the file given."""
     cmd = ('cat %s | openssl dgst -sha256 -binary | openssl base64' %
            update_path)
     return os.popen(cmd).read().rstrip()
 
+  def _GetMd5(self, update_path):
+    """Returns the md5 checksum of the file given."""
+    cmd = ("md5sum %s | awk '{print $1}'" % update_path)

[DaleCurtis, 2010/11/15 22:38:04]

Is there a reason you didn't use openssl like above? Seems more consistent.

[dgarrett, 2010/11/15 23:32:39]

Yes. Open SSL as used above generates binary values, not ascii strings. I needed
the ascii strings for the directory names. The first pass was written using
_GetHash().

On 2010/11/15 22:38:04, dalec wrote:

+    return os.popen(cmd).read().rstrip()
+
   def GetUpdatePayload(self, hash, sha256, size, url, is_delta_format):
     """Returns a payload to the client corresponding to a new update.
 
     Args:
       hash: hash of update blob
       sha256: SHA-256 hash of update blob
       size: size of update blob
       url: where to find update blob
     Returns:
       Xml string to be passed back to client.
@@ skipping 26 matching lines @@
       < gupdate xmlns = "http://www.google.com/update2/response" protocol = "2.0" >
         < daystart elapsed_seconds = "%s" />
         < app appid = "{%s}" status = "ok" >
           < ping status = "ok" />
           < updatecheck status = "noupdate" />
         </ app >
       </ gupdate >
     """
     return payload % (self._GetSecondsSinceMidnight(), self.app_id)
 
-  def GenerateUpdateFile(self, image_path):
+  def GenerateUpdateFile(self, src_image, image_path, static_image_dir):
     """Generates an update gz given a full path to an image.
 
     Args:
       image_path: Full path to image.
     Returns:
       Path to created update_payload or None on error.
     """
     image_dir = os.path.dirname(image_path)

[DaleCurtis, 2010/11/15 22:38:04]

Dead variable.

[dgarrett, 2010/11/15 23:32:39]

Done.

-    update_path = os.path.join(image_dir, 'update.gz')
+    update_path = os.path.join(static_image_dir, 'update.gz')
     patch_kernel_flag = '--patch_kernel'
     _LogMessage('Generating update image %s' % update_path)
 
     # Don't patch the kernel for vm images as they don't need the patch.
     if self.vm:
       patch_kernel_flag = ''
 
     mkupdate_command = (
         '%s/cros_generate_update_payload --image="%s" --output="%s" '
         '%s --noold_style --src_image="%s"' % (
             self.scripts_dir, image_path, update_path, patch_kernel_flag,
-            self.src_image))
+            src_image))
     _LogMessage(mkupdate_command)
     if os.system(mkupdate_command) != 0:
       _LogMessage('Failed to create base update file')
       return None
 
     return update_path
 
-  def GenerateStatefulFile(self, image_path):
-    """Generates a stateful update given a full path to an image.
+  def GenerateStatefulFile(self, image_path, static_image_dir):
+    """Generates a stateful update gz given a full path to an image.
 
     Args:
       image_path: Full path to image.
     Returns:
-      Path to created stateful update_payload.
+      Path to created stateful update_payload or None on error.
     Raises:
       A subprocess exception if the update generator fails to generate a
       stateful payload.
     """
-    work_dir = os.path.dirname(image_path)
-    output_gz = os.path.join(work_dir, 'stateful.tgz')
+

[DaleCurtis, 2010/11/15 22:38:04]

Extra blank line here and in most other methods. Style is no line between
docstring and first line of code. gpylint is a good test for these things. Even
if it's not 100% with CrOS style.

[dgarrett, 2010/11/15 23:32:39]

Done.

+    output_gz = os.path.join(static_image_dir, 'stateful.tgz')
     subprocess.check_call(
         ['%s/cros_generate_stateful_update_payload' % self.crosutils,
          '--image=%s' % image_path,
-         '--output_dir=%s' % work_dir,
+         '--output_dir=%s' % static_image_dir,
         ])
     return output_gz
 
-  def MoveImagesToStaticDir(self, update_path, stateful_update_path,
-                            static_image_dir):
-    """Moves gz files from their directories to serving directories.
+  def FindCachedUpdateImageSubDir(self, src_image, dest_image):
+    """Find directory to store a cached update.
 
-    Args:
-      update_path: full path to main update gz.
-      stateful_update_path: full path to stateful partition gz.
-      static_image_dir: where to put files.
-    Returns:
-      Returns True if the files were moved over successfully.
-    """
-    try:
-      shutil.copy(update_path, static_image_dir)
-      shutil.copy(stateful_update_path, static_image_dir)
-      os.remove(update_path)
-      os.remove(stateful_update_path)
-    except Exception:
-      _LogMessage('Failed to move %s and %s to %s' % (update_path,
-                                                    stateful_update_path,
-                                                    static_image_dir))
-      return False
+       Given one, or two images for an update, this finds which
+       cache directory should hold the udpate files, even if they don't exist
+       yet. The directory will be inside static_image_dir, and of the form:
 
-    return True
+       Non-delta updates:
+         cache/12345678
 
-  def GenerateUpdateImage(self, image_path, move_to_static_dir=False,
-                          static_image_dir=None):
-    """Generates an update payload based on the given image_path.
+       Delta updates:
+         cache/12345678_12345678
+       """
+
+    # If there is no src, we only have an image file, check image for changes
+    if not src_image:
+      return os.path.join('cache', self._GetMd5(dest_image))
+
+    # If we have src and dest, we are a delta, and check both for changes
+    return os.path.join('cache',
+                        "%s_%s" % (self._GetMd5(src_image),
+                                   self._GetMd5(dest_image)))
+
+  def GenerateUpdateImage(self, src_image, image_path, static_image_dir):
+    """Force generates an update payload based on the given image_path.
 
     Args:
       image_path: full path to the image.
       move_to_static_dir: Moves the files from their dir to the static dir.
       static_image_dir: the directory to move images to after generating.
     Returns:
-      True if the update payload was created successfully.
+      update filename (not directory) on success, or None
     """
+
+    update_file = None
+    stateful_update_file = None
+
+    # Actually do the generation
     _LogMessage('Generating update for image %s' % image_path)
-    update_path = self.GenerateUpdateFile(image_path)
-    if update_path:
-      stateful_update_path = self.GenerateStatefulFile(image_path)
-      if move_to_static_dir:
-        return self.MoveImagesToStaticDir(update_path, stateful_update_path,
+    update_file = self.GenerateUpdateFile(src_image,
+                                          image_path,
                                           static_image_dir)
-        return True
+
+    if update_file:
+      stateful_update_file = self.GenerateStatefulFile(image_path,
+                                                       static_image_dir)
+
+    if update_file and stateful_update_file:
+      return os.path.basename(update_file)
 
     _LogMessage('Failed to generate update')
-    return False
+
+    # Cleanup incomplete files, if they exist
+    if update_file and os.path.exists(update_file):
+      os.remove(update_file)
+
+    if stateful_update_file and os.path.exists(stateful_update_file):
+      os.remove(stateful_update_file)
+
+    return None

[DaleCurtis, 2010/11/15 22:38:04]

No need to explicitly return None, method will automatically return None.

[dgarrett, 2010/11/15 23:32:39]

True, I just considered it easier to understand, given that it returns a non
None value in the non-error case.

+
+  def GenerateUpdateImageWithCache(self, image_path, static_image_dir):
+    """Force generates an update payload based on the given image_path.
+
+    Args:
+      image_path: full path to the image.
+      move_to_static_dir: Moves the files from their dir to the static dir.
+      static_image_dir: the directory to move images to after generating.
+    Returns:
+      update filename (not directory) relative to static_image_dir on success,
+        or None
+    """
+
+    _LogMessage('Generating update for src %s image %s' % (self.src_image,
+                                                           image_path))
+
+    # Which sub_dir of static_image_dir should hold our cached update image
+    cache_sub_dir = self.FindCachedUpdateImageSubDir(self.src_image, image_path)
+    _LogMessage('Caching in sub_dir "%s"' % cache_sub_dir)
+
+    # Check for a cached image to use
+    if os.path.exists(os.path.join(static_image_dir,
+                                   cache_sub_dir,
+                                   'update.gz')):
+      return os.path.join(cache_sub_dir, 'update.gz')
+
+    full_cache_dir = os.path.join(static_image_dir, cache_sub_dir)
+    src_image = ""
+    dest_image = os.path.join(full_cache_dir, 'dest_image.bin')
+
+    # Create the directory for the cache values
+    if not os.path.exists(full_cache_dir):
+      os.makedirs(full_cache_dir)
+
+    gen_image = self.GenerateUpdateImage(self.src_image,
+                                         image_path,
+                                         full_cache_dir)
+
+    # If the generation worked
+    if gen_image:
+      return os.path.join(cache_sub_dir, gen_image)
+    else:

[DaleCurtis, 2010/11/15 22:38:04]

No return for None.

[dgarrett, 2010/11/15 23:32:39]

Same reasoning as above.

+      return None
+
 
   def GenerateLatestUpdateImage(self, board_id, client_version,
-                                static_image_dir=None):
+                                static_image_dir):
     """Generates an update using the latest image that has been built.
 
     This will only generate an update if the newest update is newer than that
     on the client or client_version is 'ForcedUpdate'.
 
     Args:
       board_id: Name of the board.
       client_version: Current version of the client or 'ForcedUpdate'
       static_image_dir: the directory to move images to after generating.
     Returns:
-      True if the update payload was created successfully.
+      Name of the update image relative to static_image_dir or None
     """
     latest_image_dir = self._GetLatestImageDir(board_id)
     latest_version = self._GetVersionFromDir(latest_image_dir)
     latest_image_path = os.path.join(latest_image_dir, self._GetImageName())
 
     _LogMessage('Preparing to generate update from latest built image %s.' %
               latest_image_path)
 
      # Check to see whether or not we should update.
     if client_version != 'ForcedUpdate' and not self._CanUpdate(
         client_version, latest_version):
       _LogMessage('no update')
-      return False
+      return None

[DaleCurtis, 2010/11/15 22:38:04]

Rework to avoid return None.

[dgarrett, 2010/11/15 23:32:39]

Again, I'm considering it better style that if you return an explicit value part
of the time, you should explicitly return None in the other cases to show that
you're doing it on purpose.

I can certainly understand why you might feel differently though.

 
-    cached_file_path = os.path.join(static_image_dir, 'update.gz')
-    if (os.path.exists(cached_file_path) and
-        not self._IsImageNewerThanCached(latest_image_path, cached_file_path)):
-      return True
-
-    return self.GenerateUpdateImage(latest_image_path, move_to_static_dir=True,
-                                    static_image_dir=static_image_dir)
+    return self.GenerateUpdateImageWithCache(latest_image_path,
+                                             static_image_dir=static_image_dir)
 
   def GenerateImageFromZip(self, static_image_dir):
     """Generates an update from an image zip file.
 
     This method assumes you have an image.zip in directory you are serving
     from.  If this file is newer than a previously cached file, it will unzip
     this file, create a payload and serve it.
 
     Args:
       static_image_dir: Directory where the zip file exists.
     Returns:
-      True if the update payload was created successfully.
+      Name of the update payload relative to static_image_dir if successful.
     """
     _LogMessage('Preparing to generate update from zip in %s.' % static_image_dir)
     image_path = os.path.join(static_image_dir, self._GetImageName())
-    cached_file_path = os.path.join(static_image_dir, 'update.gz')
     zip_file_path = os.path.join(static_image_dir, 'image.zip')
-    if not self._IsImageNewerThanCached(zip_file_path, cached_file_path):
-      return True
+
+    # TODO: Either work caching into this path before we unpack,
+    #       or remove zip support (sosa is considering). It
+    #       does currently cache, after the unpack.
 
     if not self._UnpackZip(static_image_dir):
       _LogMessage('unzip image.zip failed.')
-      return False
+      return None

[DaleCurtis, 2010/11/15 22:38:04]

Rework to:

if unpack zip:
  return GeneateUpdateImageWithCache(...)

_Log..(Fail)

So return None is unnecessary.

 
-    return self.GenerateUpdateImage(image_path, move_to_static_dir=False,
-                                    static_image_dir=None)
+    return self.GenerateUpdateImageWithCache(image_path,
+                                             static_image_dir=static_image_dir)
 
   def ImportFactoryConfigFile(self, filename, validate_checksums=False):
     """Imports a factory-floor server configuration file. The file should
     be in this format:
       config = [
         {
           'qual_ids': set([1, 2, 3, "x86-generic"]),
           'factory_image': 'generic-factory.gz',
           'factory_checksum': 'AtiI8B64agHVN+yeBAyiNMX3+HM=',
           'release_image': 'generic-release.gz',
@@ skipping 76 matching lines @@
       return self.GetNoUpdatePayload()
     url = '%s/static/%s' % (self.hostname, filename)
     is_delta_format = self._IsDeltaFormatFile(filename)
     _LogMessage('returning update payload ' + url)
     # Factory install is using memento updater which is using the sha-1 hash so
     # setting sha-256 to an empty string.
     return self.GetUpdatePayload(checksum, '', size, url, is_delta_format)
 
   def GenerateUpdatePayloadForNonFactory(self, board_id, client_version,
                                          static_image_dir):
-    """Generates an update for non-factory and returns True on success."""
-    if self.use_cached and os.path.exists(os.path.join(static_image_dir,
-                                                       'update.gz')):
-      _LogMessage('Using cached image regardless of timestamps.')
-      return True
+    """Generates an update for non-factory image.
+
+       Returns:
+         file name relative to static_image_dir on success.
+    """
+
+    if self.forced_image:
+      return self.GenerateUpdateImageWithCache(
+          self.forced_image,
+          static_image_dir=static_image_dir)
+    elif self.serve_only:
+      return self.GenerateImageFromZip(static_image_dir)
     else:
-      if self.forced_image:
-        has_built_image = self.GenerateUpdateImage(
-            self.forced_image, move_to_static_dir=True,
-            static_image_dir=static_image_dir)
-        return has_built_image
-      elif self.serve_only:
-        return self.GenerateImageFromZip(static_image_dir)
-      else:
-        if board_id:
-          return self.GenerateLatestUpdateImage(board_id,
-                                                client_version,
-                                                static_image_dir)
+      if board_id:
+        return self.GenerateLatestUpdateImage(board_id,
+                                              client_version,
+                                              static_image_dir)
 
-        _LogMessage('You must set --board for pre-generating latest update.')
-        return False
+      _LogMessage('You must set --board for pre-generating latest update.')
+      return None

[DaleCurtis, 2010/11/15 22:38:04]

No None.

 
   def PreGenerateUpdate(self):
     """Pre-generates an update.  Returns True on success."""
      # Does not work with factory config.
     assert(not self.factory_config)
     _LogMessage('Pre-generating the update payload.')
     # Does not work with labels so just use static dir.
     if self.GenerateUpdatePayloadForNonFactory(self.board, '0.0.0.0',
                                                self.static_dir):
-      # Force the devserver to use the pre-generated payload.
-      self.use_cached = True
       _LogMessage('Pre-generated update successfully.')
       return True
     else:
       _LogMessage('Failed to pre-generate update.')
       return False
 
   def HandleUpdatePing(self, data, label=None):
     """Handles an update ping from an update client.
 
     Args:
@@ skipping 41 matching lines @@
 
     # Separate logic as Factory requests have static url's that override
     # other options.
     if self.factory_config:
       return self.HandleFactoryRequest(board_id, channel)
     else:
       static_image_dir = self.static_dir
       if label:
         static_image_dir = os.path.join(static_image_dir, label)
 
-      if self.GenerateUpdatePayloadForNonFactory(board_id, client_version,
-                                                 static_image_dir):
-        filename = os.path.join(static_image_dir, 'update.gz')
+      update_name = self.GenerateUpdatePayloadForNonFactory(board_id,
+                                                            client_version,
+                                                            static_image_dir)
+      if update_name:
+        filename = os.path.join(static_image_dir, update_name)
         hash = self._GetHash(filename)
         sha256 = self._GetSHA256(filename)
         size = self._GetSize(filename)
         is_delta_format = self._IsDeltaFormatFile(filename)
         if label:
-          url = '%s/%s/update.gz' % (static_urlbase, label)
+          url = '%s/%s/%s' % (static_urlbase, label, update_name)
         else:
-          url = '%s/update.gz' % static_urlbase
+          url = '%s/%s' % (static_urlbase, update_name)
 
         _LogMessage('Responding to client to use url %s to get image.' % url)
         return self.GetUpdatePayload(hash, sha256, size, url, is_delta_format)
       else:
         return self.GetNoUpdatePayload()