On Wed, Sep 15, 2010 at 05:06:38PM +0100, [email protected] wrote: > From: martyntaylor <[email protected]> > > --- > src/app/controllers/instance_controller.rb | 1 + > src/app/controllers/users_controller.rb | 7 ++- > src/app/models/instance.rb | 1 + > src/app/models/instance_observer.rb | 30 ++++----- > src/app/models/quota.rb | 27 +++++--- > src/app/models/user.rb | 3 + > src/db/migrate/20090804142049_create_instances.rb | 1 + > src/db/migrate/20090917192602_create_users.rb | 1 + > src/spec/factories/instance.rb | 1 + > src/spec/factories/user.rb | 1 + > src/spec/models/instance_observer_spec.rb | 25 ++++--- > src/spec/models/quota_spec.rb | 68 > +++++++++++++++++++++ > 12 files changed, 128 insertions(+), 38 deletions(-) > create mode 100644 src/spec/models/quota_spec.rb > > diff --git a/src/app/controllers/instance_controller.rb > b/src/app/controllers/instance_controller.rb > index dcf0fa2..5c36be6 100644 > --- a/src/app/controllers/instance_controller.rb > +++ b/src/app/controllers/instance_controller.rb > @@ -106,6 +106,7 @@ class InstanceController < ApplicationController > def create > @instance = Instance.new(params[:instance]) > @instance.state = Instance::STATE_NEW > + @instance.owner_id = current_user > require_privilege(Privilege::INSTANCE_MODIFY, > Pool.find(@instance.pool_id)) > #FIXME: This should probably be in a transaction > diff --git a/src/app/controllers/users_controller.rb > b/src/app/controllers/users_controller.rb > index bb68542..8488509 100644 > --- a/src/app/controllers/users_controller.rb > +++ b/src/app/controllers/users_controller.rb > @@ -30,6 +30,11 @@ class UsersController < ApplicationController > def create > require_privilege(Privilege::USER_MODIFY) unless current_user.nil? > @user = User.new(params[:user]) > + > + #TODO Set Quota Values to SelfService Settings Default Quota > + @user_quota = Quota.new > + @user.quota_id = @user_quota.id > + > @registration = RegistrationService.new(@user) > if @registration.save > flash[:notice] = "User registered!" > @@ -65,4 +70,4 @@ class UsersController < ApplicationController > def index > @users = User.all > end > -end > +end > \ No newline at end of file > diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb > index 0212e24..7788b0c 100644 > --- a/src/app/models/instance.rb > +++ b/src/app/models/instance.rb > @@ -32,6 +32,7 @@ class Instance < ActiveRecord::Base > belongs_to :hardware_profile > belongs_to :image > belongs_to :realm > + belongs_to :owner, :class_name => "User", :foreign_key => "owner_id" > > validates_presence_of :pool_id > validates_presence_of :hardware_profile_id > diff --git a/src/app/models/instance_observer.rb > b/src/app/models/instance_observer.rb > index 70537ac..3ec83c8 100644 > --- a/src/app/models/instance_observer.rb > +++ b/src/app/models/instance_observer.rb > @@ -32,34 +32,32 @@ class InstanceObserver < ActiveRecord::Observer > end > > def update_quota(state_from, state_to, an_instance) > - > hwp = an_instance.hardware_profile > pool = an_instance.pool > cloud_account = an_instance.cloud_account > - > - [cloud_account, pool].each do |parent| > + user = an_instance.owner > + [cloud_account, pool, user].each do |parent| > 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 state_to == Instance::STATE_RUNNING > + quota.running_instances += 1 > + elsif state_from == Instance::STATE_RUNNING > + 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 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 > end > quota.save! > end > end > end > end > - > end > > -InstanceObserver.instance > +InstanceObserver.instance > \ No newline at end of file > diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb > index f882878..7b1b3f9 100644 > --- a/src/app/models/quota.rb > +++ b/src/app/models/quota.rb > @@ -23,6 +23,7 @@ class Quota < ActiveRecord::Base > > has_one :pool > has_one :cloud_account > + has_one :user > > QuotaResource = Struct.new(:name, :used, :max, :available, :unit) > > @@ -34,20 +35,26 @@ class Quota < ActiveRecord::Base > > RESOURCE_NAMES = [ RESOURCE_RUNNING_INSTANCES, RESOURCE_TOTAL_INSTANCES ] > > - def can_create_instance?(instance) > - potential_total_instances = total_instances + 1 > - if (Quota.no_limit(maximum_total_instances) || maximum_total_instances > >= potential_total_instances) > - return true > + def self.can_create_instance?(instance) > + [instance.owner, instance.pool, instance.cloud_account].each do |parent| > + quota = Quota.find(parent.quota_id) > + potential_total_instances = quota.total_instances + 1 > + if !Quota.no_limit(quota.maximum_total_instances) && > (quota.maximum_total_instances < potential_total_instances) > + return false > + end > end > - return false > + return true > end > > - def can_start_instance?(instance) > - potential_running_instances = running_instances + 1 > - if (Quota.no_limit(maximum_running_instances) || > maximum_running_instances >= potential_running_instances) > - return true > + def self.can_start_instance?(instance) > + [instance.owner, instance.pool, instance.cloud_account].each do |parent| > + quota = Quota.find(parent.quota_id) > + potential_running_instances = quota.running_instances + 1 > + if !Quota.no_limit(quota.maximum_running_instances) && > quota.maximum_running_instances < potential_running_instances > + return false > + end > end > - return false > + return true > end > > def quota_resources() > diff --git a/src/app/models/user.rb b/src/app/models/user.rb > index b08d03d..fc0cbdc 100644 > --- a/src/app/models/user.rb > +++ b/src/app/models/user.rb > @@ -24,4 +24,7 @@ class User < ActiveRecord::Base > > has_many :permissions > has_many :owned_pools, :class_name => "Pool", :foreign_key => "owner_id" > + has_many :owned_instances, :class_name => "Instance", :foreign_key => > "owner_id" > + > + belongs_to :quota > end > diff --git a/src/db/migrate/20090804142049_create_instances.rb > b/src/db/migrate/20090804142049_create_instances.rb > index 42706e1..d6bcbc9 100644 > --- a/src/db/migrate/20090804142049_create_instances.rb > +++ b/src/db/migrate/20090804142049_create_instances.rb > @@ -27,6 +27,7 @@ class CreateInstances < ActiveRecord::Migration > t.integer :hardware_profile_id, :null => false > t.integer :image_id, :null => false > t.integer :realm_id > + t.integer :owner_id > t.integer :pool_id, :null => false > t.integer :cloud_account_id > t.string :public_address > diff --git a/src/db/migrate/20090917192602_create_users.rb > b/src/db/migrate/20090917192602_create_users.rb > index 67706fb..1d9fd1d 100644 > --- a/src/db/migrate/20090917192602_create_users.rb > +++ b/src/db/migrate/20090917192602_create_users.rb > @@ -31,6 +31,7 @@ class CreateUsers < ActiveRecord::Migration > t.string :perishable_token, :null => false > t.string :first_name > t.string :last_name > + t.integer :quota_id > # Magic columns, just like ActiveRecord's created_at and updated_at. > # These are automatically maintained by Authlogic if they are present. > t.integer :login_count, :null => false, :default => 0 > diff --git a/src/spec/factories/instance.rb b/src/spec/factories/instance.rb > index 7533c64..e68ac0a 100644 > --- a/src/spec/factories/instance.rb > +++ b/src/spec/factories/instance.rb > @@ -5,6 +5,7 @@ Factory.define :instance do |i| > i.association :cloud_account, :factory => :mock_cloud_account > i.association :image, :factory => :image > i.association :pool, :factory => :pool > + i.association :owner, :factory => :user > i.state "running" > end > > diff --git a/src/spec/factories/user.rb b/src/spec/factories/user.rb > index 778aa30..e737a35 100644 > --- a/src/spec/factories/user.rb > +++ b/src/spec/factories/user.rb > @@ -5,6 +5,7 @@ Factory.define :user do |u| > u.password_confirmation 'secret' > u.first_name 'John' > u.last_name 'Smith' > + u.association :quota > end > > Factory.define :tuser, :parent => :user do |u| > diff --git a/src/spec/models/instance_observer_spec.rb > b/src/spec/models/instance_observer_spec.rb > index ae5ee0d..e4a2de2 100644 > --- a/src/spec/models/instance_observer_spec.rb > +++ b/src/spec/models/instance_observer_spec.rb > @@ -11,8 +11,11 @@ describe InstanceObserver do > @pool_quota = Factory :quota > @pool = Factory(:pool, :quota_id => @pool_quota.id) > > + @user_quota = Factory :quota > + @user = Factory(:user, :quota_id => @user_quota.id) > + > @hwp = Factory :mock_hwp1 > - @instance = Factory(:new_instance, :pool => @pool, :hardware_profile => > @hwp, :cloud_account_id => @cloud_account.id) > + @instance = Factory(:new_instance, :pool => @pool, :hardware_profile => > @hwp, :cloud_account_id => @cloud_account.id, :owner => @user) > end > > it "should set started at timestamp when instance goes to state pending" do > @@ -95,15 +98,15 @@ describe InstanceObserver do > @instance.acc_stopped_time.should <= 2 > end > > - it "should not update quota on pool and cloud account when an instance is > state new" do > - [...@cloud_account_quota, @pool_quota].each do |quota| > + it "should not update quota on pool, user and cloud account when an > instance is state new" do > + [...@cloud_account_quota, @pool_quota, @user_quota].each do |quota| > quota = Quota.find(quota) > quota.total_instances.should == 0 > end > end > > - it "should update quota on pool and cloud account when an instance fgoes > to state pending" do > - [...@cloud_account_quota, @pool_quota].each do |quota| > + it "should update quota on pool, user and cloud account when an instance > goes to state pending" do > + [...@cloud_account_quota, @pool_quota, @user_quota].each do |quota| > @instance.state = Instance::STATE_PENDING > @instance.save > > @@ -112,35 +115,35 @@ describe InstanceObserver do > end > end > > - it "should update cloud account and pool quota when an instance goes into > an inactive state" do > + it "should update cloud accoun, pool and user 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| > + [...@cloud_account_quota, @pool_quota, @user_quota].each do |quota| > quota = Quota.find(quota) > quota.total_instances.should == 0 > end > end > > - it "should update pool and cloud account quota when an instance state goes > to running" do > + it "should update pool, cloud account and user quota when an instance > state goes to running" do > @instance.state = Instance::STATE_RUNNING > @instance.save! > > - [...@cloud_account_quota, @pool_quota].each do |quota| > + [...@cloud_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 and cloud account quota when an instance state > goes from running to another active state" do > + it "should update a pool, cloud account and user 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| > + [...@cloud_account_quota, @pool_quota, @user_quota].each do |quota| > quota = Quota.find(quota.id) > > quota.running_instances.should == 0 > diff --git a/src/spec/models/quota_spec.rb b/src/spec/models/quota_spec.rb > new file mode 100644 > index 0000000..233bb84 > --- /dev/null > +++ b/src/spec/models/quota_spec.rb > @@ -0,0 +1,68 @@ > +require 'spec_helper' > + > +describe Quota do > + > + before(:each) do > + @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) > + > + @user_quota = Factory :quota > + @user = Factory(:user, :quota_id => @user_quota.id) > + > + @hwp = Factory :mock_hwp1 > + @instance = Factory(:new_instance, :pool => @pool, :hardware_profile => > @hwp, :cloud_account_id => @cloud_account.id, :owner => @user) > + end > + > + it "should return true when asking if an instance can be created/started > when there is sufficient quota space" do > + Quota.can_create_instance?(@instance).should == true > + Quota.can_start_instance?(@instance).should == true > + end > + > + it "should return false when asking if an instance can be created/started > when the user quota is reached" do > + @user_quota.total_instances = @user_quota.maximum_total_instances > + @user_quota.running_instances = @user_quota.maximum_running_instances > + @user_quota.save! > + > + Quota.can_create_instance?(@instance).should == false > + Quota.can_start_instance?(@instance).should == false > + end > + > + it "should return false when asking if an instance can be created/started > when the pool quota is reached" do > + @pool_quota.total_instances = @pool_quota.maximum_total_instances > + @pool_quota.running_instances = @pool_quota.maximum_running_instances > + @pool_quota.save! > + > + Quota.can_create_instance?(@instance).should == false > + Quota.can_start_instance?(@instance).should == false > + end > + > + it "should return false when asking if an instance can be created/started > when the cloud account quota is reached" do > + @cloud_account_quota.total_instances = > @cloud_account_quota.maximum_total_instances > + @cloud_account_quota.running_instances = > @cloud_account_quota.maximum_running_instances > + @cloud_account_quota.save! > + > + Quota.can_create_instance?(@instance).should == false > + Quota.can_start_instance?(@instance).should == false > + end > + > + it "should return false when asking if an instance can be created/started > when the all quotas are reached" do > + @user_quota.total_instances = @user_quota.maximum_total_instances > + @user_quota.running_instances = @user_quota.maximum_running_instances > + @user_quota.save! > + > + @pool_quota.total_instances = @pool_quota.maximum_total_instances > + @pool_quota.running_instances = @pool_quota.maximum_running_instances > + @pool_quota.save! > + > + @cloud_account_quota.total_instances = > @cloud_account_quota.maximum_total_instances > + @cloud_account_quota.running_instances = > @cloud_account_quota.maximum_running_instances > + @cloud_account_quota.save! > + > + Quota.can_create_instance?(@instance).should == false > + Quota.can_start_instance?(@instance).should == false > + end > + > +end > \ No newline at end of file > -- > 1.7.1.1 > > _______________________________________________ > deltacloud-devel mailing list > [email protected] > https://fedorahosted.org/mailman/listinfo/deltacloud-devel
Conditional ACK. Please push only after removing the whitespace/indentation/style edits in this patch. I am all for cleaning up those sorts of things, but patches need to stay on topic in this respect. Speaking only for myself, my train of thought needs all the help it can get staying on its tracks, and when a style edit happens in the middle of a patch it's like a non sequitur. Just an unnecessary distraction for whoever is trying to understand the code. Thankee!! Steve. _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
