This patch is intended to fix the bug that appeared in the demo where, pending states did not update the quota running totals.
This can cause over quota issues, since two instances can be in state pending, with the provider and the aggregator with only one available running instance in the quota, once the two instances start in the provider, they would push the quota over its limit. The aggregator previously would allow this state, with these changes it should not. Since we are treating "Pending" as a running state to keep consistency "Shutting Down" is now also classed as a running state.
From: martyntaylor mtaylor@redhat.com
--- src/app/models/instance_observer.rb | 36 ++++++------ src/spec/models/instance_observer_spec.rb | 88 +++++++++++++++++++---------- 2 files changed, 77 insertions(+), 47 deletions(-)
diff --git a/src/app/models/instance_observer.rb b/src/app/models/instance_observer.rb index 610d742..ec13282 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_RUNNING, Instance::STATE_PENDING, Instance::STATE_SHUTTING_DOWN ]
def before_save(an_instance) if an_instance.changed? @@ -41,24 +42,25 @@ class InstanceObserver < ActiveRecord::Observer if parent quota = parent.quota if quota - if state_to == Instance::STATE_RUNNING - quota.running_instances += 1 - quota.running_memory = quota.running_memory.to_f + hwp.memory.value.to_f - quota.running_cpus = quota.running_cpus.to_f + hwp.cpu.value.to_f - elsif state_from == Instance::STATE_RUNNING - quota.running_instances -= 1 - quota.running_memory = quota.running_memory.to_f - hwp.memory.value.to_f - quota.running_cpus = quota.running_cpus.to_f - hwp.cpu.value.to_f - end + quota.lock! + if RUNNING_STATES.include?(state_to) && !RUNNING_STATES.include?(state_from) + quota.running_instances += 1 + quota.running_memory = quota.running_memory.to_f + hwp.memory.value.to_f + quota.running_cpus = quota.running_cpus.to_f + hwp.cpu.value.to_f + elsif !RUNNING_STATES.include?(state_to) && RUNNING_STATES.include?(state_from) + quota.running_instances -= 1 + quota.running_memory = quota.running_memory.to_f - hwp.memory.value.to_f + quota.running_cpus = quota.running_cpus.to_f - hwp.cpu.value.to_f + end
- if state_from != nil - if !ACTIVE_STATES.include?(state_from) && ACTIVE_STATES.include?(state_to) - quota.total_storage = quota.total_storage.to_f + hwp.storage.value.to_f - quota.total_instances += 1 + if state_from != nil + if !ACTIVE_STATES.include?(state_from) && ACTIVE_STATES.include?(state_to) + quota.total_storage = quota.total_storage.to_f + hwp.storage.value.to_f + quota.total_instances += 1 elsif ACTIVE_STATES.include?(state_from) && !ACTIVE_STATES.include?(state_to) - quota.total_storage = quota.total_storage.to_f - hwp.storage.value.to_f - quota.total_instances -= 1 - end + quota.total_storage = quota.total_storage.to_f - hwp.storage.value.to_f + quota.total_instances -= 1 + end end quota.save! end @@ -68,4 +70,4 @@ class InstanceObserver < ActiveRecord::Observer
end
-InstanceObserver.instance +InstanceObserver.instance \ No newline at end of file diff --git a/src/spec/models/instance_observer_spec.rb b/src/spec/models/instance_observer_spec.rb index 6bcb4d0..828ebdb 100644 --- a/src/spec/models/instance_observer_spec.rb +++ b/src/spec/models/instance_observer_spec.rb @@ -104,7 +104,7 @@ describe InstanceObserver do end end
- it "should update quota on pool and cloud account when an instance fgoes to state pending" do + it "should update quota on pool and cloud account when an instance goes to state pending" do [@cloud_account_quota, @pool_quota].each do |quota| @instance.state = Instance::STATE_PENDING @instance.save @@ -128,38 +128,66 @@ describe InstanceObserver do end end
- it "should update pool and cloud account quota when an instance state goes to running" do - @instance.state = Instance::STATE_RUNNING - @instance.save! - - [@cloud_account_quota, @pool_quota].each do |quota| - quota = Quota.find(quota.id) - quota.running_instances.should == 1 - quota.running_memory.to_f.should == @hwp.memory.value.to_f - quota.running_cpus.to_f.should == @hwp.cpu.value.to_f - - quota.total_storage.to_f.should == @hwp.storage.value.to_f - quota.total_instances.should == 1 + it "should update pool and cloud account quota when an instance state goes to a running state" do + [Instance::STATE_PENDING, Instance::STATE_RUNNING, Instance::STATE_SHUTTING_DOWN ].each do |state| + [@cloud_account_quota, @pool_quota].each do |quota| + # Reset Quota + quota = Quota.find(quota.id) + quota.running_instances = 0 + quota.running_memory = 0 + quota.running_cpus = 0 + + quota.total_storage= 0 + quota.total_instances = 0 + quota.save! + + # Create Instance + @instance = Factory(:new_instance, :pool => @pool, :hardware_profile => @hwp, :cloud_account_id => @cloud_account.id) + @instance.state = state + @instance.save! + + # Check Quota + quota = Quota.find(quota.id) + quota.running_instances.should == 1 + quota.running_memory.to_f.should == @hwp.memory.value.to_f + quota.running_cpus.to_f.should == @hwp.cpu.value.to_f + + quota.total_storage.to_f.should == @hwp.storage.value.to_f + quota.total_instances.should == 1 + end end end
- it "should update a pool and cloud account quota when an instance state goes from running to another active state" do - @instance.state = Instance::STATE_RUNNING - @instance.save! - - @instance.state = Instance::STATE_SHUTTING_DOWN - @instance.save! - - [@cloud_account_quota, @pool_quota].each do |quota| - quota = Quota.find(quota.id) - - #TODO test for cpus - quota.running_instances.should == 0 - quota.running_memory.to_f.should == 0.to_f - - quota.total_storage.to_f.should == @hwp.storage.value.to_f - quota.total_instances.should == 1 + it "should update a pool and cloud account quota when an instance state goes from a running state to a non running state" do + [Instance::STATE_PENDING, Instance::STATE_RUNNING, Instance::STATE_SHUTTING_DOWN ].each do |state| + [@cloud_account_quota, @pool_quota].each do |quota| + # Reset Quota + quota = Quota.find(quota.id) + quota.running_instances = 0 + quota.running_memory = 0 + quota.running_cpus = 0 + + quota.total_storage= 0 + quota.total_instances = 0 + quota.save! + + # Create Instance + @instance = Factory(:new_instance, :pool => @pool, :hardware_profile => @hwp, :cloud_account_id => @cloud_account.id) + @instance.state = state + @instance.save! + + @instance.state = Instance::STATE_STOPPED + @instance.save! + + # Check Quota + quota = Quota.find(quota.id) + quota.running_cpus.to_f.should == 0 + quota.running_instances.should == 0 + quota.running_memory.to_f.should == 0.to_f + + quota.total_storage.to_f.should == @hwp.storage.value.to_f + quota.total_instances.should == 1 + end end end - end \ No newline at end of file
deltacloud-devel@lists.fedorahosted.org