On 01/11/11 14:04 +0100, jprovazn(a)redhat.com wrote:
From: Jan Provaznik <jprovazn(a)redhat.com>
---
src/app/models/deployment.rb | 3 ++-
src/app/models/instance.rb | 17 +++++++----------
src/app/models/pool.rb | 4 ----
src/app/models/pool_family.rb | 4 ++--
src/app/models/provider_account.rb | 4 ----
src/config/locales/en.yml | 4 ++++
src/spec/models/instance_spec.rb | 21 ++++++++++++++++-----
7 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index e75c113..c83fcca 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -93,7 +93,8 @@ class Deployment < ActiveRecord::Base
end
def pool_must_be_enabled
- errors.add(:pool, "must be enabled") unless pool and pool.enabled?
+ errors.add(:pool, I18n.t('pools.errors.must_be_enabled')) unless pool and
pool.enabled?
+ errors.add(:pool, I18n.t('pools.errors.providers_disabled')) if pool and
pool.pool_family.all_providers_disabled?
end
def object_list
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index a83b2f5..9d9c634 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -163,17 +163,9 @@ class Instance < ActiveRecord::Base
return_val
end
- def enabled?
- pool and pool.enabled? and (provider_account.nil? or provider_account.enabled?)
- end
-
def pool_and_account_enabled_validation
- return if enabled?
-
- errors.add(:pool, 'must be enabled') unless pool and pool.enabled?
- unless provider_account.nil? or provider_account.enabled?
- errors.add(:provider_account, 'must be enabled')
- end
+ errors.add(:pool, I18n.t('pools.errors.must_be_enabled')) unless pool and
pool.enabled?
+ errors.add(:pool, I18n.t('pools.errors.providers_disabled')) if pool and
pool.pool_family.all_providers_disabled?
end
@@ -338,6 +330,11 @@ class Instance < ActiveRecord::Base
provider_images = build ? build.provider_images : []
matched = []
pool.pool_family.provider_accounts.each do |account|
+ unless account.provider.enabled?
+ errors << I18n.t('instances.errors.must_be_enabled', :account_name
=> account.name)
+ next
+ end
+
# 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,
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index ce8d83b..14e8a6d 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -77,10 +77,6 @@ class Pool < ActiveRecord::Base
instances.all? {|i| i.destroyable? }
end
- def enabled?
- self.enabled and pool_family and pool_family.enabled?
- end
-
# TODO: Implement Alerts and Updates
def statistics
# TODO - Need to set up cache invalidation before this is safe
diff --git a/src/app/models/pool_family.rb b/src/app/models/pool_family.rb
index 26d5efb..4cad050 100644
--- a/src/app/models/pool_family.rb
+++ b/src/app/models/pool_family.rb
@@ -65,7 +65,7 @@ class PoolFamily < ActiveRecord::Base
self != PoolFamily.default
end
- def enabled?
- provider_accounts.blank? or provider_accounts.any? {|acc| acc.enabled?}
+ def all_providers_disabled?
+ !provider_accounts.empty? and !provider_accounts.any? {|acc| acc.provider.enabled?}
end
end
diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb
index 38083c2..4e3242e 100644
--- a/src/app/models/provider_account.rb
+++ b/src/app/models/provider_account.rb
@@ -125,10 +125,6 @@ class ProviderAccount < ActiveRecord::Base
instances.empty? || instances.all? { |i| i.destroyable? }
end
- def enabled?
- provider and provider.enabled?
- end
-
def connect
begin
opts = {:username => credentials_hash['username'],
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index c62c67a..666c057 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -334,6 +334,7 @@ en:
provider_account_quota_too_low: "%{match_provider_account} quota limit too low
to launch deployable"
stop_invalid: "stop is an invalid action."
stop_not_be_performed: "stop cannot be performed on this instance."
+ must_be_enabled: "%{account_name}: provider must be enabled"
headers:
vm_name: VM NAME
status: STATUS
@@ -406,6 +407,9 @@ en:
instance_or: instance or
unlimited: unlimited
pool_enabled: Pool enabled
+ errors:
+ providers_disabled: has all associated providers disabled
+ must_be_enabled: must be enabled
hardware_profiles:
hardware: Hardware
hardware_profile: Hardware Profile
diff --git a/src/spec/models/instance_spec.rb b/src/spec/models/instance_spec.rb
index 4ffdae0..2ac70f9 100644
--- a/src/spec/models/instance_spec.rb
+++ b/src/spec/models/instance_spec.rb
@@ -230,16 +230,27 @@ describe Instance do
export_string.include?("created").should be_true
end
- it "should not be launchable if its provider is disabled" do
- instance = FactoryGirl.build(:instance_with_disabled_provider)
- instance.should_not be_enabled
- instance.should_not be_valid
+ it "should not be launchable on disabled provider account" do
+ instance = FactoryGirl.create(:instance)
+ pf = instance.pool.pool_family
+ pf.provider_accounts = [FactoryGirl.create(:disabled_provider_account)]
+ errors = instance.matches.last
+ errors.find {|e| e =~ /provider must be enabled/}.should_not be_nil
end
it "should not be launchable if its pool is disabled" do
instance = FactoryGirl.build(:instance_in_disabled_pool)
- instance.should_not be_enabled
instance.should_not be_valid
+ instance.errors[:pool].should_not be_empty
+ instance.errors[:pool].first.should == "must be enabled"
+ end
+
+ it "should not be launchable if its pool's providers are all disabled"
do
+ instance = FactoryGirl.build(:instance)
+ instance.pool.pool_family.provider_accounts <<
FactoryGirl.create(:disabled_provider_account)
+ instance.should_not be_valid
+ instance.errors[:pool].should_not be_empty
+ instance.errors[:pool].first.should == "has all associated providers
disabled"
end
context "When more instances of deployment are starting" do
--
1.7.6
ACK and pushed.