On Wed, 29 Jun 2011 13:32:43 +0200
jprovazn(a)redhat.com wrote:
From: Jan Provaznik <jprovazn(a)redhat.com>
Condormatic instance start method now returns information why
a match wasn't found. This method is still not nice and errors output
can be confusing if there are many matching errors, but should be
sufficient for now.
---
src/app/models/instance.rb | 70
++++++++++++++++++++------------------
src/app/models/provider_image.rb | 4 ++
src/app/models/quota.rb | 4 ++
src/app/util/condormatic.rb | 18 +++++----
src/spec/models/instance_spec.rb | 39 +++++++++++++++++++++ 5 files
changed, 94 insertions(+), 41 deletions(-)
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index afe4e30..aba031b 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -307,47 +307,51 @@ class Instance < ActiveRecord::Base
instances
end
- def match
- possibles = []
+ def matches
+ errors = []
+ if pool.pool_family.provider_accounts.empty?
+ errors << 'There are no provider accounts associated with pool
family of selected pool.'
+ end
+ errors << 'Pool quota reached' if pool.quota.reached?
+ errors << 'Pool family quota reached' if
pool.pool_family.quota.reached?
+ errors << 'User quota reached' if owner.quota.reached?
+ return [[], errors] unless errors.empty?
- PoolFamily.all.each do |pool_family|
- if pool.pool_family.id != pool_family.id
+ build = image_build || image.latest_build
+ provider_images = build ? build.provider_images : []
+ possibles = []
+ pool.pool_family.provider_accounts.each do |account|
+ # 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)
+ account_images = provider_images.select {|pi| pi.provider ==
account.provider}
+ if account_images.empty?
+ errors << "#{account.name}: image is not pushed to this
provider account" next
end
-
- image_build = image_build || image.latest_build
- provider_images = image_build ? image_build.provider_images :
[]
- pool_family.provider_accounts.each do |account|
- # 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) -
- provider_images.select {|pi| pi.provider ==
account.provider}.each do |pi|
- if not frontend_realm.nil?
- frontend_realm.realm_backend_targets.each do
|brealm_target|
- if brealm_target.target_provider == account.provider
- possibles << Possible.new(pool_family, account, hwp,
pi,
- brealm_target.target_realm)
- end
- end
- else
- possibles << Possible.new(pool_family, account, hwp, pi,
nil)
+ if account.quota.reached?
+ errors << "#{account.name}: provider account quota reached"
+ next
+ end
+ account_images.each do |pi|
+ if not frontend_realm.nil?
+ blreams = frontend_realm.realm_backend_targets.select
{|brealm_target| brealm_target.target_provider == account.provider}
+ if brealms.empty?
+ errors << "Realm #{rontend_realm.nam} is not mapped to
any provider or provider realm"
+ next
end
+ brealms.each do |brealm_target|
+ possibles << Possible.new(pool.pool_family, account,
hwp, pi,
+ brealm_target.target_realm)
+ end
+ else
+ possibles << Possible.new(pool.pool_family, account, hwp,
pi, nil) end
end
end
- possibles.each do |match|
- # FIXME: we should have something smarter here that prioritizes
- # and/or chooses the "cheapest" possibility. For now, just
return the
- # first that fits under quota
- if Quota.can_start_instance?(self, match.account)
- return match
- end
- end
-
- return nil
+ [possibles, errors]
end
named_scope :with_hardware_profile, lambda {
diff --git a/src/app/models/provider_image.rb
b/src/app/models/provider_image.rb index f98370b..30dc408 100644
--- a/src/app/models/provider_image.rb
+++ b/src/app/models/provider_image.rb
@@ -17,6 +17,10 @@ class ProviderImage < WarehouseModel
TargetImage.find(@target_image) if @target_image
end
+ def provider_name
+ @provider
+ end
+
def provider
Provider.find_by_name(@provider)
end
diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
index b33560c..9583e5f 100644
--- a/src/app/models/quota.rb
+++ b/src/app/models/quota.rb
@@ -100,6 +100,10 @@ class Quota < ActiveRecord::Base
return true
end
+ def reached?
+ !Quota.no_limit(maximum_running_instances) && running_instances
>= maximum_running_instances
+ end
+
def quota_resources()
quota_resources = {"running_instances" =>
QuotaResource.new("Running Instances", running_instances,
maximum_running_instances, nil, ""), "total_instances" =>
QuotaResource.new("Total Instances", total_instances,
maximum_total_instances, nil, "")} diff --git
a/src/app/util/condormatic.rb b/src/app/util/condormatic.rb index
a3f3b24..24a849c 100644 --- a/src/app/util/condormatic.rb +++
b/src/app/util/condormatic.rb @@ -61,12 +61,12 @@ end
def condormatic_instance_create(task)
instance = task.instance
- found = instance.match
+ matches, errors = instance.matches
+ found = matches.first
begin
if found.nil?
- Rails.logger.error "Couldn't find a match!"
- raise ("Could not find a matching backend provider")
+ raise "Could not find a matching backend provider, errors:
#{errors.join(', ')}" end
job_name = "job_#{instance.name}_#{instance.id}"
@@ -117,17 +117,19 @@ def condormatic_instance_create(task)
Rails.logger.error "$? (return value?) is #{$?}"
raise ("Error calling condor_submit: #{out}") if $? != 0
+ task.state = Task::STATE_PENDING
+ instance.state = Instance::STATE_PENDING
rescue Exception => ex
Rails.logger.error ex.message
Rails.logger.error ex.backtrace
task.state = Task::STATE_FAILED
instance.state = Instance::STATE_CREATE_FAILED
- else
- task.state = Task::STATE_PENDING
- instance.state = Instance::STATE_PENDING
+ # exception is raised after ensure block
+ raise ex
+ ensure
+ instance.save!
+ task.save!
end
- instance.save!
- task.save!
end
def condormatic_instance_stop(task)
diff --git a/src/spec/models/instance_spec.rb
b/src/spec/models/instance_spec.rb index f86cf8e..de2154c 100644
--- a/src/spec/models/instance_spec.rb
+++ b/src/spec/models/instance_spec.rb
@@ -120,4 +120,43 @@ describe Instance do
instance.get_action_list.should be_empty
end
+ it "shouldn't return any matches if pool quota is reached" do
+ @quota.maximum_running_instances = 1
+ @quota.running_instances = 1
+ @quota.save!
+ @instance.matches.last.should include('Pool quota reached')
+ end
+
+ it "shouldn't return any matches if pool family quota is reached"
do
+ quota = @pool.pool_family.quota
+ quota.maximum_running_instances = 1
+ quota.running_instances = 1
+ quota.save!
+ @instance.matches.last.should include('Pool family quota
reached')
+ end
+
+ it "shouldn't return any matches if user quota is reached" do
+ quota = @instance.owner.quota
+ quota.maximum_running_instances = 1
+ quota.running_instances = 1
+ quota.save!
+ @instance.matches.last.should include('User quota reached')
+ end
+
+ it "shouldn't return any matches if there are no provider accounts
associated with pool family" do
+ @instance.pool.pool_family.provider_accounts = []
+ @instance.matches.last.should include('There are no provider
accounts associated with pool family of selected pool.')
+ end
+
+ it "shouldn't match provider accounts where image is not pushed" do
+ @pool.pool_family.provider_accounts =
[Factory(:mock_provider_account, :label => 'testaccount')]
+ @instance.matches.last.should include('testaccount: image is not
pushed to this provider account')
+ end
+
+ it "should return a match if all requirements are satisfied" do
+ build = @instance.image_build || @instance.image.latest_build
+ provider = Factory(:mock_provider, :name =>
build.provider_images.first.provider_name)
+ @pool.pool_family.provider_accounts =
[Factory(:mock_provider_account, :label => 'testaccount', :provider
=> provider)]
+ @instance.matches.first.should_not be_empty
+ end
end
ACK