[PATCH conductor 1/3] BZ 768089: Refactor provider account matching to facilitate stubbing

Scott Seago sseago at redhat.com
Tue Dec 20 06:13:22 UTC 2011


Instance.match was one long 70 line method, which wasn't conducive to stubbing out parts of it for rspec tests.
---
 src/app/models/instance.rb         |   51 +++++++-----------------------------
 src/app/models/provider_account.rb |   31 ++++++++++++++++++++++
 src/app/util/taskomatic.rb         |   11 +++++---
 3 files changed, 48 insertions(+), 45 deletions(-)

diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index 08aa3b6..083c01f 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -184,6 +184,15 @@ class Instance < ActiveRecord::Base
     Aeolus::Image::Warehouse::ImageBuild.find(image_build_uuid) if image_build_uuid
   end
 
+  def build
+    image_build || (image.nil? ? nil : image.latest_pushed_build)
+  end
+
+  # defined as class method to facilitate stubbing out for testing
+  def self.provider_images_for_match(image, provider)
+    (image.build ? image.build.provider_images : []).select {|pi| pi.provider_name == provider.name}
+  end
+
   def assembly_xml
     @assembly_xml ||= AssemblyXML.new(self[:assembly_xml].to_s)
   end
@@ -346,50 +355,10 @@ class Instance < ActiveRecord::Base
     errors << I18n.t('instances.errors.image_not_found', :b_uuid=> image_build_uuid, :i_uuid => image_uuid) if image_build.nil? and image.nil?
     return [[], errors] unless errors.empty?
 
-    build = image_build || image.latest_pushed_build
-    provider_images = build ? build.provider_images : []
     matched = []
     pool.pool_family.provider_accounts.each do |account|
-      unless account.provider.enabled?
-        errors << I18n.t('instances.errors.must_be_enabled', :account_name => account.name)
-        next
-      end
+      account.instance_matches(self, matched, errors)
 
-      # match_provider_hardware_profile returns a single provider
-      # hardware_profile that can satisfy the input hardware_profile
-      hwp = HardwareProfile.match_provider_hardware_profile(account.provider,
-                                                            hardware_profile)
-      unless hwp
-        errors << I18n.t('instances.errors.hw_profile_match_not_found', :account_name => account.name)
-        next
-      end
-      account_images = provider_images.select {|pi| pi.provider_name == account.provider.name}
-      if account_images.empty?
-        errors << I18n.t('instances.errors.image_not_pushed_to_provider', :account_name => account.name)
-        next
-      end
-      if account.quota.reached?
-        errors << I18n.t('instances.errors.provider_account_quota_reached', :account_name => account.name)
-        next
-      end
-      if requires_config_server? and account.config_server.nil?
-        errors << I18n.t('instances.errors.no_config_server_available', :account_name => account.name)
-        next
-      end
-      account_images.each do |pi|
-        if not frontend_realm.nil?
-          brealms = frontend_realm.realm_backend_targets.select {|brealm_target| brealm_target.target_provider == account.provider}
-          if brealms.empty?
-            errors << I18n.t('instances.errors.realm_not_mapped', :frontend_realm_name => frontend_realm.name)
-            next
-          end
-          brealms.each do |brealm_target|
-            matched << Match.new(pool.pool_family, account, hwp, pi, brealm_target.target_realm)
-          end
-        else
-          matched << Match.new(pool.pool_family, account, hwp, pi, nil)
-        end
-      end
     end
 
     [matched, errors]
diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb
index 88ac8d4..0ba423b 100644
--- a/src/app/models/provider_account.rb
+++ b/src/app/models/provider_account.rb
@@ -338,6 +338,37 @@ class ProviderAccount < ActiveRecord::Base
     end
   end
 
+  def instance_matches(instance, matched, errors)
+    if !provider.enabled?
+      errors << I18n.t('instances.errors.must_be_enabled', :account_name => name)
+    elsif quota.reached?
+      errors << I18n.t('instances.errors.provider_account_quota_reached', :account_name => name)
+    # match_provider_hardware_profile returns a single provider
+    # hardware_profile that can satisfy the input hardware_profile
+    elsif !(hwp = HardwareProfile.match_provider_hardware_profile(provider, instance.hardware_profile))
+      errors << I18n.t('instances.errors.hw_profile_match_not_found', :account_name => name)
+    elsif (account_images = Instance.provider_images_for_match(instance,provider)).empty?
+      errors << I18n.t('instances.errors.image_not_pushed_to_provider', :account_name => name)
+    elsif instance.requires_config_server? and config_server.nil?
+      errors << I18n.t('instances.errors.no_config_server_available', :account_name => name)
+    else
+      account_images.each do |pi|
+        if not instance.frontend_realm.nil?
+          brealms = instance.frontend_realm.realm_backend_targets.select {|brealm_target| brealm_target.target_provider == provider}
+          if brealms.empty?
+            errors << I18n.t('instances.errors.realm_not_mapped', :frontend_realm_name => instance.frontend_realm.name)
+            next
+          end
+          brealms.each do |brealm_target|
+            matched << Instance::Match.new(instance.pool.pool_family, self, hwp, pi, brealm_target.target_realm)
+          end
+        else
+          matched << Instance::Match.new(instance.pool.pool_family, self, hwp, pi, nil)
+        end
+      end
+    end
+  end
+
   private
 
   def self.apply_search_filter(search)
diff --git a/src/app/util/taskomatic.rb b/src/app/util/taskomatic.rb
index 9750873..3629706 100644
--- a/src/app/util/taskomatic.rb
+++ b/src/app/util/taskomatic.rb
@@ -32,10 +32,7 @@ module Taskomatic
 
       task.state = Task::STATE_RUNNING
       task.save!
-
-      Rails.logger.info "Task instance create completed with key #{dcloud_instance.id} and state #{dcloud_instance.state}"
-      task.instance.external_key = dcloud_instance.id
-      task.instance.state = dcloud_to_instance_state(dcloud_instance.state)
+      handle_instance_state(task.instance,dcloud_instance)
       task.instance.save!
     rescue HttpException => ex
       task.failure_code = Task::FAILURE_PROVIDER_CONTACT_FAILED
@@ -48,6 +45,12 @@ module Taskomatic
     end
   end
 
+  def self.handle_instance_state(instance, dcloud_instance)
+      Rails.logger.info "Task instance create completed with key #{dcloud_instance.id} and state #{dcloud_instance.state}"
+      instance.external_key = dcloud_instance.id
+      instance.state = dcloud_to_instance_state(dcloud_instance.state)
+  end
+
   def self.do_action(task, action)
     task.time_started = Time.now
 
-- 
1.7.6.4




More information about the aeolus-devel mailing list