[email protected] wrote:
> From: martyntaylor <[email protected]>
>
> ---
> src/app/models/instance_observer.rb | 41 ++++++++++++++++-
> src/app/models/quota.rb | 41 +++++++++++++++++
> src/app/models/task.rb | 7 ++-
> src/config/environment.rb | 1 +
> src/spec/factories/instance.rb | 5 +-
> src/spec/models/instance_observer_spec.rb | 68
> ++++++++++++++++++++++++++++-
> src/spec/models/instance_spec.rb | 10 ++---
> src/spec/models/task_spec.rb | 5 --
> 8 files changed, 159 insertions(+), 19 deletions(-)
>
> diff --git a/src/app/models/instance_observer.rb
> b/src/app/models/instance_observer.rb
> index fa5b3d2..a7ca75d 100644
> --- a/src/app/models/instance_observer.rb
> +++ b/src/app/models/instance_observer.rb
> @@ -1,11 +1,14 @@
> class InstanceObserver < ActiveRecord::Observer
>
>
We'll need to make sure everything here is free of threading/other race
conditions with all the incrementing/decrementing 'in use' values.
> + ACTIVE_STATES = [ Instance::STATE_NEW, Instance::STATE_PENDING,
> Instance::STATE_RUNNING, Instance::STATE_SHUTTING_DOWN,
> Instance::STATE_STOPPED ]
> +
> def before_save(an_instance)
> if an_instance.changed?
> change = an_instance.changes['state']
>
We might need to look at cloud_account_id too, although right now we
only move instances from 'no account' to 'some account' -- so I'm not
sure the logic around setting cloud accounts affects anything here.
> if change
> update_state_timestamps(change[1], an_instance)
> update_accumulative_state_time(change[0], an_instance)
> + update_quota(change[0], change[1], an_instance)
> end
> end
> end
> @@ -26,7 +29,41 @@ class InstanceObserver < ActiveRecord::Observer
> when Instance::STATE_SHUTTING_DOWN then
> an_instance.acc_shutting_down_time += Time.now -
> an_instance.time_last_shutting_down
> when Instance::STATE_STOPPED then an_instance.acc_stopped_time +=
> Time.now - an_instance.time_last_stopped
> end
> - end
> + end
> +
> + def update_quota(state_from, state_to, an_instance)
> +
> + hwp = HardwareProfile.find(an_instance.hardware_profile_id)
> +
>
A more concise way of doing the same (and avoiding the extra database
hit if the association has already loaded) would be:
hwp = an_instance.hardware_profile
> + pool = Pool.find(an_instance.pool_id)
>
pool= an_instance.pool
> + cloud_account = CloudAccount.find(an_instance.cloud_account_id)
> +
>
cloud_account = an_instance.cloud_account
> + [cloud_account, pool].each do |parent|
>
> + if Quota.exists?(parent.quota_id)
>
> + quota = Quota.find(parent.quota_id)
>
quota = parent.quota
if quota
>
> + if state_to == Instance::STATE_RUNNING
> + #TODO update running cpus
> + quota.running_instances += 1
> + quota.running_memory += hwp.memory
> + elsif state_from == Instance::STATE_RUNNING
> + #TODO update running cpus
> + quota.running_instances -= 1
> + quota.running_memory -= hwp.memory
> + end
>
if state_to == state_from then we should do nothing above here
> +
> + if !ACTIVE_STATES.include?(state_from) &&
> ACTIVE_STATES.include?(state_to)
> + quota.total_storage += hwp.storage
> + quota.total_instances += 1
> + elsif ACTIVE_STATES.include?(state_from) &&
> !ACTIVE_STATES.include?(state_to)
> + quota.total_storage -= hwp.storage
> + quota.total_instances -= 1
> + end
> +
> + quota.save!
> + end
> + end
> + end
> +
> end
>
> -InstanceObserver.instance
> \ No newline at end of file
> +InstanceObserver.instance
> diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
> index 23f9aee..57662a7 100644
> --- a/src/app/models/quota.rb
> +++ b/src/app/models/quota.rb
> @@ -22,4 +22,45 @@
> class Quota < ActiveRecord::Base
> has_one :pool
> has_one :cloud_account
> +
> + validates_presence_of :maximum_running_instances
> + validates_presence_of :maximum_running_memory
> + validates_presence_of :maximum_running_cpus
> +
> + validates_presence_of :maximum_total_storage
> + validates_presence_of :maximum_total_instances
> +
>
We need to be able to handle null values above. It should be possible,
for example, to define a quota that limits #instances but not other
parameters.
> + validates_numericality_of :maximum_running_instances
> + validates_numericality_of :maximum_running_memory
> + validates_numericality_of :maximum_running_cpus
> +
> + validates_numericality_of :maximum_total_storage
> + validates_numericality_of :maximum_total_instances
> +
> +
> + def can_create_instance?(instance)
> + hwp = HardwareProfile.find(instance.hardware_profile_id)
> +
>
hwp = instance.hardware_profile
> + if maximum_total_instances >= (total_instances + 1) &&
> maximum_total_storage >= (total_storage + hwp.storage)
> + return true
> + end
> + return false
> + end
> +
> + def can_start_instance?(instance)
> + hwp = HardwareProfile.find(instance.hardware_profile_id)
>
hwp = instance.hardware_profile
> + # TODO Add check for CPUs
> + if maximum_running_instances >= (running_instances + 1) &&
> maximum_running_memory >= (running_memory + hwp.memory)
> + return true
> + end
> + return false
> + end
> +
> + def validate
> + errors.add("maximum_running_instances", "cannot be less than the current
> running instances") if maximum_running_instances < running_instances
> + errors.add("maximum_running_memory", "cannot be less than the current
> running memory") if maximum_running_memory < running_memory
> + errors.add("maximum_running_cpus", "cannot be less than the current
> running CPUs") if maximum_running_cpus < running_cpus
> + errors.add("maximum_total_storage", "cannot be less than the current
> total storage") if maximum_total_storage < total_storage
> + errors.add("maximum_total_instances", "cannot be less than the current
> total instances") if maximum_total_instances < total_instances
>
> + end
> end
> diff --git a/src/app/models/task.rb b/src/app/models/task.rb
> index f3aaa63..feec26a 100644
> --- a/src/app/models/task.rb
> +++ b/src/app/models/task.rb
> @@ -43,7 +43,9 @@ class Task < ActiveRecord::Base
> FAILURE_PROVIDER_CONTACT_FAILED = "provider_contact_failed"
> FAILURE_PROVIDER_RETURNED_FAILED = "provider_returned_failed"
>
> - FAILURE_CODES = [FAILURE_PROVIDER_NOT_FOUND,
> FAILURE_PROVIDER_CONTACT_FAILED, FAILURE_PROVIDER_RETURNED_FAILED]
> + FAILURE_OVER_POOL_QUOTA = "exceeded_pool_quota"
> +
> + FAILURE_CODES = [FAILURE_PROVIDER_NOT_FOUND,
> FAILURE_PROVIDER_CONTACT_FAILED, FAILURE_PROVIDER_RETURNED_FAILED,
> FAILURE_OVER_POOL_QUOTA]
>
> validates_inclusion_of :failure_code,
> :in => FAILURE_CODES + [nil]
> @@ -114,7 +116,8 @@ class Task < ActiveRecord::Base
>
> def validate
> errors.add("created_at", "Task started but does not have the creation
> time set") if time_started and created_at.nil?
> - errors.add("time_started", "Task ends but does not have the start time
> set") if time_ended and time_started.nil?
> + # Removed check on time_started exisiting. if time_ended does. This can
> now occur, when the task fails before is starts. e.g. When Over Qutoa
> + #errors.add("time_started", "Task ends but does not have the start time
> set") if time_ended and time_started.nil?
> errors.add("time_ended", "Tasks ends before it's started") unless
> time_ended.nil? or time_started.nil? or time_ended > time_started
> errors.add("time_started", "Tasks starts before it's created") unless
> time_started.nil? or created_at.nil? or time_started > created_at
> end
> diff --git a/src/config/environment.rb b/src/config/environment.rb
> index 1721126..eefd3a4 100644
> --- a/src/config/environment.rb
> +++ b/src/config/environment.rb
> @@ -77,4 +77,5 @@ Rails::Initializer.run do |config|
> # config.i18n.load_path += Dir[Rails.root.join('my', 'locales',
> '*.{rb,yml}')]
> # config.i18n.default_locale = :de
>
> +
> end
> diff --git a/src/spec/factories/instance.rb b/src/spec/factories/instance.rb
> index 3a453c3..68225d0 100644
> --- a/src/spec/factories/instance.rb
> +++ b/src/spec/factories/instance.rb
> @@ -1,7 +1,8 @@
> Factory.define :instance do |i|
> i.sequence(:name) { |n| "instance#{n}" }
> i.sequence(:external_key) { |n| "key#{n}" }
> - i.hardware_profile_id 1
> + i.association :hardware_profile, :factory => :hardware_profile_auto
> + i.association :cloud_account, :factory => :mock_cloud_account
> i.image_id 1
> i.pool_id 1
> i.state "running"
> @@ -13,4 +14,4 @@ end
>
> Factory.define :new_instance, :parent => :instance do |i|
> i.state Instance::STATE_NEW
> -end
> \ No newline at end of file
> +end
> diff --git a/src/spec/models/instance_observer_spec.rb
> b/src/spec/models/instance_observer_spec.rb
> index 9e3dcc7..d713cb1 100644
> --- a/src/spec/models/instance_observer_spec.rb
> +++ b/src/spec/models/instance_observer_spec.rb
> @@ -4,7 +4,15 @@ describe InstanceObserver do
>
> before(:each) do
> @timestamp = Time.now
> - @instance = Factory :new_instance
> +
> + @cloud_account_quota = Factory :quota
> + @cloud_account = Factory(:mock_cloud_account, :quota_id =>
> @cloud_account_quota.id)
> +
> + @pool_quota = Factory :quota
> + @pool = Factory(:pool, :quota_id => @pool_quota.id)
> +
> + @hwp = Factory :mock_hwp1
> + @instance = Factory(:new_instance, :pool => @pool, :hardware_profile =>
> @hwp, :cloud_account_id => @cloud_account.id)
> end
>
> it "should set started at timestamp when instance goes to state pending" do
> @@ -55,7 +63,7 @@ describe InstanceObserver do
> sleep(1)
>
> @instance.state = Instance::STATE_SHUTTING_DOWN
> - @instance.save
> + @instance.save!
>
> @instance.acc_running_time.should >= 1
> @instance.acc_running_time.should <= 2
> @@ -86,4 +94,60 @@ describe InstanceObserver do
> @instance.acc_stopped_time.should >= 1
> @instance.acc_stopped_time.should <= 2
> end
> +
> + it "should update quota on pool and cloud account when a new instance is
> created" do
> + [...@cloud_account_quota, @pool_quota].each do |quota|
> + quota = Quota.find(quota)
> +
> + quota.total_instances.should == 1
> + quota.total_storage.should == @hwp.storage
> + end
> + end
> +
> + it "should update cloud account and pool quota when an instance goes into
> an inactive state" do
> + @instance.state = Instance::STATE_CREATE_FAILED
> + @instance.save!
> +
> + [...@cloud_account_quota, @pool_quota].each do |quota|
> + quota = Quota.find(quota)
> +
> + quota.total_instances.should == 0
> + quota.total_storage.should == 0
> + 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.should == @hwp.memory
> + #TODO test for cpus
> +
> + quota.total_storage.should == @hwp.storage
> + quota.total_instances.should == 1
> + 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.should == 0
> +
> + quota.total_storage.should == @hwp.storage
> + quota.total_instances.should == 1
> + end
> + end
> +
> end
> \ No newline at end of file
> diff --git a/src/spec/models/instance_spec.rb
> b/src/spec/models/instance_spec.rb
> index 0eaf837..78a0158 100644
> --- a/src/spec/models/instance_spec.rb
> +++ b/src/spec/models/instance_spec.rb
> @@ -2,7 +2,9 @@ require 'spec_helper'
>
> describe Instance do
> before(:each) do
> - @instance = Factory.build(:instance)
> + @quota = Factory :quota
> + @pool = Factory(:pool, :quota_id => @quota.id)
> + @instance = Factory.build(:instance, :pool_id => @pool.id)
> @actions = ['start', 'stop']
> end
>
> @@ -69,11 +71,7 @@ describe Instance do
> end
>
> it "should return action list" do
> - @instance.get_action_list.should eql([])
> -
> - InstanceTask.stub!(:valid_actions_for_instance_state).and_return(
> - @actions)
> - @instance.get_action_list.should eql(@actions)
> + @instance.get_action_list.should eql(["reboot", "stop"])
> end
>
> it "should be able to queue new actions" do
> diff --git a/src/spec/models/task_spec.rb b/src/spec/models/task_spec.rb
> index d392cb2..c5e7188 100644
> --- a/src/spec/models/task_spec.rb
> +++ b/src/spec/models/task_spec.rb
> @@ -43,11 +43,6 @@ describe Task do
> @task.should_not be_valid
> end
>
> - it "should have 'started' time set if it ended" do
> - @task.attributes = @valid_attributes.except :time_started
> - @task.should_not be_valid
> - end
> -
> it "should not be valid if it started before it was created" do
> @task.attributes = @valid_attributes
> @task.time_started = @task.created_at - 1.minute
>
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel
