[PATCH conductor 1/3] BZ 768089: Refactor provider account matching to facilitate stubbing (revised)
Scott Seago
sseago at redhat.com
Tue Dec 20 18:13:15 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..111cbd9 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
+
+ def provider_images_for_match(provider)
+ the_build = build
+ (the_build ? the_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..8079119 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(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