[PATCH conductor] Warehouse sync update

[Jan Provazník]

From: Jan Provaznik <jprovazn(a)redhat.com&gt;

sync code moved to models. Image, ProviderImage, Icicle and Template now has
warehouse_sync method. When you call this method attributes are updated from
warehouse, note that save call is not part of this method - you need to save
explicitly (this is because of after_save hooks for image and provider_iamge
call warehouse_sync so there could be recursion)
---
 src/app/models/icicle.rb                 |   15 +++
 src/app/models/image.rb                  |   36 ++++++--
 src/app/models/image_warehouse_object.rb |   49 ++++++++---
 src/app/models/provider_image.rb         |   31 +++++--
 src/app/models/template.rb               |   15 ++--
 src/app/util/image_descriptor_xml.rb     |   12 +--
 src/lib/warehouse_client.rb              |    4 +
 src/warehouse_sync/warehouse_sync-server |    2 +-
 src/warehouse_sync/warehouse_sync.rb     |  145 ++++-------------------------
 9 files changed, 144 insertions(+), 165 deletions(-)

diff --git a/src/app/models/icicle.rb b/src/app/models/icicle.rb
index 2bad5d9..a086285 100644
--- a/src/app/models/icicle.rb
+++ b/src/app/models/icicle.rb
@@ -12,7 +12,22 @@
 #
 
 class Icicle < ActiveRecord::Base
+  include ImageWarehouseObject
+
   validates_presence_of :uuid
   validates_uniqueness_of :uuid
   belongs_to :provider_image
+
+  def self.create_or_update(uuid)
+    icicle = Icicle.find_by_uuid(uuid) || Icicle.new(:uuid => uuid)
+    icicle.warehouse_sync
+    icicle.log_changes
+    icicle.save! if icicle.changed?
+    icicle
+  end
+
+  def warehouse_sync
+    obj = warehouse.bucket('icicles').object(self.uuid)
+    self.xml = obj.body
+  end
 end
diff --git a/src/app/models/image.rb b/src/app/models/image.rb
index 7a8aae4..f15f2da 100644
--- a/src/app/models/image.rb
+++ b/src/app/models/image.rb
@@ -79,15 +79,8 @@ class Image < ActiveRecord::Base
   # TODO: for now when build is finished we call upload automatically for all providers
   def after_update
     if self.status_changed? and self.status == STATE_COMPLETED
-      # TODO: use after_commit callback in rails 3 - it's better to have it outside
-      # update transaction
-      begin
-        invoke_sync
-        upload_to_all_providers_with_account
-      rescue => e
-        logger.error e.message
-        logger.error e.backtrace.join("\n  ")
-      end
+      safe_warehouse_sync
+      upload_to_all_providers_with_account
     end
   end
 
@@ -176,6 +169,31 @@ class Image < ActiveRecord::Base
     image
   end
 
+  def warehouse_bucket
+    'images'
+  end
+
+  def warehouse_sync
+    bucket = warehouse.bucket(warehouse_bucket)
+    raise WarehouseObjectNotFoundError unless bucket.include?(self.uuid)
+    obj = bucket.object(self.uuid)
+    attrs = obj.attrs([:uuid, :target, :template])
+    unless attrs[:target]
+      raise "target uuid is not set"
+    end
+    unless ptype = ProviderType.find_by_codename(attrs[:target])
+      raise "provider type #{attrs[:target]} not found"
+    end
+    unless attrs[:template]
+      raise "template uuid is not set"
+    end
+    unless tpl = Template.find_by_uuid(attrs[:template])
+      raise "Template with uuid #{attrs[:template]} not found"
+    end
+    self.provider_type_id = ptype.id
+    self.template_id = tpl.id
+  end
+
   private
 
   def upload_to_all_providers_with_account
diff --git a/src/app/models/image_warehouse_object.rb
b/src/app/models/image_warehouse_object.rb
index 40a7c3b..43f3b76 100644
--- a/src/app/models/image_warehouse_object.rb
+++ b/src/app/models/image_warehouse_object.rb
@@ -19,6 +19,8 @@
 
 require 'warehouse_client'
 
+class WarehouseObjectNotFoundError < Exception;end
+
 module ImageWarehouseObject
 
   WAREHOUSE_CONFIG =
YAML.load_file("#{RAILS_ROOT}/config/image_warehouse.yml")
@@ -40,32 +42,57 @@ module ImageWarehouseObject
   # TODO: it would be nice not call upload, when save was invoked by warehouse
   # sync script
   def upload
-    whouse = Warehouse::Client.new(WAREHOUSE_CONFIG['baseurl'])
     # TODO: for now there is no way how it check if bucket exists in warehouse
     # so we try to create bucket everytime, if bucket exists, warehouse returns
     # 500 Internal server error
-    whouse.create_bucket(warehouse_bucket) rescue true
+    warehouse.create_bucket(warehouse_bucket) rescue true
     # TODO: we delete existing object if it exists
-    whouse.bucket(warehouse_bucket).object(self.uuid).delete! rescue true
-    whouse.bucket(warehouse_bucket).create_object(self.uuid, warehouse_body,
warehouse_attrs)
+    warehouse.bucket(warehouse_bucket).object(self.uuid).delete! rescue true
+    warehouse.bucket(warehouse_bucket).create_object(self.uuid, warehouse_body,
warehouse_attrs)
   end
 
   def delete_in_warehouse
-    whouse = Warehouse::Client.new(WAREHOUSE_CONFIG['baseurl'])
     begin
-      whouse.bucket(warehouse_bucket).object(self.uuid).delete!
+      warehouse.bucket(warehouse_bucket).object(self.uuid).delete!
     rescue
       logger.error "failed to delete #{self.uuid} in warehouse: #{$!}"
     end
   end
 
+  def warehouse
+    @warehouse ||= Warehouse::Client.new(WAREHOUSE_CONFIG['baseurl'])
+  end
+
+  def warehouse_url
+    "#{WAREHOUSE_CONFIG['baseurl']}/#{warehouse_bucket}/#{self.uuid}"
+  end
+
+  def safe_warehouse_sync
+    warehouse_sync
+    log_changes
+  rescue RestClient::ResourceNotFound, WarehouseObjectNotFoundError
+    logger.error "Failed to fetch #{self.class.class_name} with uuid #{self.uuid} -
not found in warehouse"
+  rescue => e
+    logger.error "Failed to sync #{self.class.class_name} with uuid #{self.uuid}:
#{e.message}"
+    logger.error e.backtrace.join("\n   ")
+  end
+
+  def log_changes
+    if self.new_record?
+      logger.info "#{self.class.class_name} #{self.uuid} is not in DB yet"
+    elsif self.changed?
+      logger.info "#{self.class.class_name} #{self.uuid} has been changed:"
+      self.changed.each do |attr|
+        logger.info "old #{attr}: #{self.send(attr + '_was')}"
+        logger.info "new #{attr}: #{self[attr]}"
+      end
+    else
+      logger.info "#{self.class.class_name} #{self.uuid} is without changes"
+    end
+  end
+
   def generate_uuid
     # TODO: generate real uuid here, e.g. with some ruby uuid generator
     self.uuid ||= UUIDTools::UUID.timestamp_create.to_s
   end
-
-  def invoke_sync
-    # TODO: invoke warehouse_sync here (after switching to eventmachine)
-    # pass self.uuid to sync only changed object
-  end
 end
diff --git a/src/app/models/provider_image.rb b/src/app/models/provider_image.rb
index 67390dd..aee69a9 100644
--- a/src/app/models/provider_image.rb
+++ b/src/app/models/provider_image.rb
@@ -33,12 +33,31 @@ class ProviderImage < ActiveRecord::Base
 
   def after_update
     if self.status_changed? and self.status == STATE_COMPLETED
-      begin
-        invoke_sync
-      rescue => e
-        logger.error e.message
-        logger.error e.backtrace.join("\n  ")
-      end
+      safe_warehouse_sync
     end
   end
+
+  def warehouse_bucket
+    'provider_images'
+  end
+
+  def warehouse_sync
+    bucket = warehouse.bucket(warehouse_bucket)
+    raise WarehouseObjectNotFoundError unless bucket.include?(self.uuid)
+    obj = bucket.object(self.uuid)
+    attrs = obj.attrs([:uuid, :image, :icicle, :target_identifier])
+    unless attrs[:image]
+      raise "image uuid is not set"
+    end
+    unless img = Image.find_by_uuid(attrs[:image])
+      raise "image with uuid #{attrs[:image]} not found"
+    end
+    begin
+      self.icicle = Icicle.create_or_update(attrs[:icicle])
+    rescue
+      logger.error "Failed to fetch icicle '#{attrs[:icicle]}', setting
icicle to nil: #{$!.message}"
+    end
+    self.image_id = img.id
+    self.provider_image_key = attrs[:target_identifier]
+  end
 end
diff --git a/src/app/models/template.rb b/src/app/models/template.rb
index 3147e16..1b09dae 100644
--- a/src/app/models/template.rb
+++ b/src/app/models/template.rb
@@ -78,9 +78,11 @@ class Template < ActiveRecord::Base
   def update_from_xml
     self.name = xml.name
     self.summary = xml.description
-    self.platform = xml.platform
-    self.platform_version = xml.platform_version
-    self.architecture = xml.architecture
+    self.platform_hash = {
+      :platform => xml.platform,
+      :version => xml.platform_version,
+      :architecture => xml.architecture,
+    }
   end
 
   def providers
@@ -157,8 +159,9 @@ class Template < ActiveRecord::Base
     'templates'
   end
 
-  def warehouse_url
-    baseurl =
YAML.load_file("#{RAILS_ROOT}/config/image_warehouse.yml")['baseurl'] +
"/templates/"
-    self.uuid ? baseurl + self.uuid : baseurl
+  def warehouse_sync
+    obj = warehouse.bucket(warehouse_bucket).object(self.uuid)
+    xml = obj.body
+    update_from_xml
   end
 end
diff --git a/src/app/util/image_descriptor_xml.rb b/src/app/util/image_descriptor_xml.rb
index 8476770..02634b2 100644
--- a/src/app/util/image_descriptor_xml.rb
+++ b/src/app/util/image_descriptor_xml.rb
@@ -52,8 +52,7 @@ class ImageDescriptorXML
   end
 
   def platform
-    node = @root.at_xpath('/template/os')
-    return node ? node['name'] : nil
+    return get_node_text('os/name')
   end
 
   def platform=(platform_hash)
@@ -80,17 +79,16 @@ class ImageDescriptorXML
   end
 
   def platform_version
-    node = @root.at_xpath('/image/os/version')
-    return node ? node.content : nil
+    return get_node_text('os/version')
   end
 
   def architecture
-    node = @root.at_xpath('/template/os/arch')
-    return node ? node.content : nil
+    return get_node_text('os/arch')
   end
 
   def architecture=(str)
-    get_or_create_node('os')['architecture'] = str
+    node = get_or_create_node('arch', get_or_create_node('os'))
+    node.content = str
   end
 
   def services=(services)
diff --git a/src/lib/warehouse_client.rb b/src/lib/warehouse_client.rb
index 9431e47..12b8d9d 100644
--- a/src/lib/warehouse_client.rb
+++ b/src/lib/warehouse_client.rb
@@ -99,6 +99,10 @@ module Warehouse
     def create_object(key, body, attrs)
       BucketObject.create(@connection, key, self, body, attrs)
     end
+
+    def include?(key)
+      object_names.include?(key)
+    end
   end
 
   class Connection
diff --git a/src/warehouse_sync/warehouse_sync-server
b/src/warehouse_sync/warehouse_sync-server
index 7fc15f5..603a520 100755
--- a/src/warehouse_sync/warehouse_sync-server
+++ b/src/warehouse_sync/warehouse_sync-server
@@ -61,7 +61,7 @@ else
 end
 
 logger = Logger.new(LOG_FILE)
-logger.level = Logger::DEBUG
+logger.level = Logger::INFO
 logger.datetime_format = "%Y-%m-%d %H:%M:%S"
 logger.info "Warehouse_sync starting up"
 
diff --git a/src/warehouse_sync/warehouse_sync.rb b/src/warehouse_sync/warehouse_sync.rb
index f18c1e9..5868db9 100644
--- a/src/warehouse_sync/warehouse_sync.rb
+++ b/src/warehouse_sync/warehouse_sync.rb
@@ -11,143 +11,38 @@ class WarehouseSync
     @uri = opts[:uri]
     @delay = opts[:delay] || 10*60
     @logger = opts[:logger]
+    ActiveRecord::Base.logger = @logger
     @whouse = Warehouse::Client.new(@uri)
   end
 
   def run
     while true
       begin
-        @logger.debug "---------------------------------------"
-        pull_templates
-        pull_images
-        pull_provider_images
+        @logger.info "***** Runnning warehouse sync"
+        # TODO: I disabled template sync - templates should never
+        # be changed by anything else than conductor
+        #(a)logger.info "*** syncing templates"
+        #Template.all.each do |i|
+        #  i.safe_warehouse_sync
+        #  i.save! if i.changed?
+        #end
+        @logger.info "*** syncing images"
+        Image.all.each do |i|
+          i.safe_warehouse_sync
+          i.save! if i.changed?
+        end
+        @logger.info "*** syncing provider images"
+        ProviderImage.all.each do |i|
+          i.safe_warehouse_sync
+          i.save! if i.changed?
+        end
       rescue => e
         @logger.error e.message
         @logger.error "backtrace:\n" + e.backtrace.join("\n   ")
       ensure
-        @logger.debug "sleep #{@delay}"
+        @logger.info "sleep #{@delay}"
         sleep @delay
       end
     end
   end
-
-  def pull_templates
-    @logger.debug "*** Getting templates"
-    @whouse.bucket('templates').objects.each do |bucket_obj|
-      safely_process(bucket_obj) do |obj|
-        attrs = obj.attrs([:uuid])
-        #tpl = Template.find_by_uuid(attrs[:uuid]) || Template.new(:uuid =>
attrs[:uuid])
-        unless tpl = Template.find_by_uuid(attrs[:uuid])
-          raise NotFoundError, "Template with uuid #{attrs[:uuid]} not found"
-        end
-        tpl.xml = obj.body
-        tpl.update_from_xml
-        update_changes(tpl)
-      end
-    end
-  end
-
-  def pull_images
-    @logger.debug "*** Getting images"
-    @whouse.bucket('images').objects.each do |bucket_obj|
-      safely_process(bucket_obj) do |obj|
-        attrs = obj.attrs([:uuid, :target, :template])
-        #img = Image.find_by_uuid(attrs[:uuid]) || Image.new(:uuid => attrs[:uuid])
-        unless img = Image.find_by_uuid(attrs[:uuid])
-          raise NotFoundError, "image with uuid #{attrs[:uuid]} not found"
-        end
-        unless attrs[:target]
-          raise "target uuid is not set"
-        end
-        unless ptype = ProviderType.find_by_codename(attrs[:target])
-          raise "provider type #{attrs[:target]} not found"
-        end
-        unless attrs[:template]
-          raise "template uuid is not set"
-        end
-        unless tpl = Template.find_by_uuid(attrs[:template])
-          raise "Template with uuid #{attrs[:template]} not found"
-        end
-        img.provider_type_id = ptype.id
-        img.template_id = tpl.id
-        update_changes(img)
-      end
-    end
-  end
-
-  def pull_provider_images
-    @logger.debug "*** Getting provider images"
-    @whouse.bucket('provider_images').objects.each do |bucket_obj|
-      safely_process(bucket_obj) do |obj|
-        attrs = obj.attrs([:uuid, :image, :icicle, :target_identifier])
-        # we don't allow create non-existing ProviderImage in conductor because
-        # we don't know provider_id (provider attribute contains only url or
-        # string which is not unique in conductor)
-        unless pimg = ProviderImage.find_by_uuid(attrs[:uuid])
-          raise NotFoundError, "provider image with uuid #{attrs[:uuid]} not
found"
-        end
-        unless attrs[:image]
-          raise "image uuid is not set"
-        end
-        unless img = Image.find_by_uuid(attrs[:image])
-          raise "image with uuid #{attrs[:image]} not found"
-        end
-        unless attrs[:icicle]
-          raise "icicle uuid is not set"
-        end
-        pimg.image_id = img.id
-        pimg.icicle = pull_provider_image_icicle(attrs[:icicle])
-        pimg.provider_image_key = attrs[:target_identifier]
-        update_changes(pimg)
-      end
-    end
-  end
-
-  private
-
-  def pull_provider_image_icicle(uuid)
-    @logger.debug "  getting provider image icicle with uuid #{uuid}"
-    begin
-      whouse_icicle = @whouse.bucket('icicles').object(uuid)
-      icicle = Icicle.find_by_uuid(uuid) || Icicle.new(:uuid => uuid)
-      icicle.xml = whouse_icicle.body
-      icicle.uuid = uuid
-      update_changes(icicle)
-      icicle
-    rescue
-      @logger.debug "  skipping image icicle with uuid #{uuid}: #{$!.message}"
-      nil
-    end
-  end
-
-  def safely_process(obj)
-    begin
-      yield obj
-    rescue NotFoundError => e
-      @logger.error "Skipping #{obj.key} - not found in DB"
-    rescue => e
-      @logger.error "Error while processing #{obj.key} (skipping):
#{e.message}"
-      @logger.error e.backtrace.join("\n   ")
-    end
-  end
-
-  def update_changes(obj)
-    if obj.new_record?
-      @logger.debug "#{obj.class.class_name} #{obj.uuid} is not in DB, saving"
-      obj.save!
-    elsif obj.changed?
-      @logger.debug "#{obj.class.class_name} #{obj.uuid} has been changed:"
-      log_changes(obj)
-      obj.save!
-    else
-      @logger.debug "#{obj.class.class_name} #{obj.uuid} is without changes"
-    end
-  end
-
-  def log_changes(obj)
-    obj.changed.each do |attr|
-      @logger.debug "old #{attr}: #{obj.send(attr + '_was')}"
-      @logger.debug "new #{attr}: #{obj[attr]}"
-    end
-  end
 end
-- 
1.7.4