On Tue, 28 Jun 2011 12:20:08 +0200
jprovazn(a)redhat.com wrote:
From: Jan Provaznik <jprovazn(a)redhat.com>
Most important change is that quota.running_instances now includes
'pending' and 'shutting_down' instances too. Till now only
'running'
instances were counted. Also an instance is set as 'pending' once
it's passed to condor in condormatic_instance_create. Above changes
are important because of (mostly) removing atomic problem with quota
check: running instances quota is checked before passing job to
condor, but instances running state is set in dbomatic (after
instance is really running) -> in this time quota check didn't work
properly. ---
src/app/models/instance_observer.rb | 21
++++++++++----------- src/app/util/condormatic.rb |
3 ++- src/spec/models/instance_observer_spec.rb | 20
+++++++++++++++++++- 3 files changed, 31 insertions(+), 13
deletions(-)
diff --git a/src/app/models/instance_observer.rb
b/src/app/models/instance_observer.rb index eedf813..800da10 100644
--- a/src/app/models/instance_observer.rb
+++ b/src/app/models/instance_observer.rb
@@ -1,6 +1,7 @@
class InstanceObserver < ActiveRecord::Observer
ACTIVE_STATES = [ Instance::STATE_PENDING,
Instance::STATE_RUNNING, Instance::STATE_SHUTTING_DOWN,
Instance::STATE_STOPPED ]
+ RUNNING_STATES = [ Instance::STATE_PENDING,
Instance::STATE_RUNNING, Instance::STATE_SHUTTING_DOWN ]
def before_save(an_instance)
if an_instance.changed?
@@ -42,18 +43,16 @@ class InstanceObserver < ActiveRecord::Observer
if parent
quota = parent.quota
if quota
- if state_to == Instance::STATE_RUNNING
- quota.running_instances += 1
- elsif state_from == Instance::STATE_RUNNING
- quota.running_instances -= 1
- end
+ if !RUNNING_STATES.include?(state_from) &&
RUNNING_STATES.include?(state_to)
+ quota.running_instances += 1
+ elsif RUNNING_STATES.include?(state_from)
&& !RUNNING_STATES.include?(state_to)
+ quota.running_instances -= 1
+ end
- if state_from != nil
- if !ACTIVE_STATES.include?(state_from) &&
ACTIVE_STATES.include?(state_to)
- quota.total_instances += 1
- elsif ACTIVE_STATES.include?(state_from)
&& !ACTIVE_STATES.include?(state_to)
- quota.total_instances -= 1
- end
+ if !ACTIVE_STATES.include?(state_from) &&
ACTIVE_STATES.include?(state_to)
+ quota.total_instances += 1
+ elsif ACTIVE_STATES.include?(state_from)
&& !ACTIVE_STATES.include?(state_to)
+ quota.total_instances -= 1
end
quota.save!
end
diff --git a/src/app/util/condormatic.rb b/src/app/util/condormatic.rb
index 107ad6e..4b42dee 100644
--- a/src/app/util/condormatic.rb
+++ b/src/app/util/condormatic.rb
@@ -67,7 +67,7 @@ def match(instance)
# 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_create_instance?(instance, match.account)
+ if Quota.can_start_instance?(instance, match.account)
return match
end
end
@@ -167,6 +167,7 @@ def condormatic_instance_create(task)
instance.state = Instance::STATE_CREATE_FAILED
else
task.state = Task::STATE_PENDING
+ instance.state = Instance::STATE_PENDING
end
instance.save!
task.save!
diff --git a/src/spec/models/instance_observer_spec.rb
b/src/spec/models/instance_observer_spec.rb index 767712b..69d702c
100644 --- a/src/spec/models/instance_observer_spec.rb
+++ b/src/spec/models/instance_observer_spec.rb
@@ -139,7 +139,10 @@ describe InstanceObserver do
end
end
- it "should update a pool, cloud account and user quota when an
instance state goes from running to another active state" do
+ it "should not update pool, cloud account and user quota when an
instance state goes from pending to running to shutting down" do
+ @instance.state = Instance::STATE_PENDING
+ @instance.save!
+
@instance.state = Instance::STATE_RUNNING
@instance.save!
@@ -148,6 +151,21 @@ describe InstanceObserver do
[@provider_account_quota, @pool_quota, @user_quota].each do
|quota| quota = Quota.find(quota.id)
+ quota.running_instances.should == 1
+ quota.total_instances.should == 1
+ end
+ end
+
+
+ it "should update a pool, cloud account and user quota when an
instance state goes from running to stopped state" do
+ @instance.state = Instance::STATE_RUNNING
+ @instance.save!
+
+ @instance.state = Instance::STATE_STOPPED
+ @instance.save!
+
+ [@provider_account_quota, @pool_quota, @user_quota].each do
|quota|
+ quota = Quota.find(quota.id)
quota.running_instances.should == 0
quota.total_instances.should == 1
ACK.