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 | 72 ++++++++++++++++++++------------------
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, 95 insertions(+), 42 deletions(-)
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index 09438f7..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
+ 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?
+
+ build = image_build || image.latest_build
+ provider_images = build ? build.provider_images : []
possibles = []
-
- PoolFamily.all.each do |pool_family|
- if pool.pool_family.id != pool_family.id
+ 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
--
1.7.4