On 02/06/2012 09:01 AM, Scott Seago wrote:
On 02/03/2012 10:10 AM, jprovazn(a)redhat.com wrote:
> From: Jan Provaznik<jprovazn(a)redhat.com>
>
> TODO: status of current availability is not displayed anywhere in UI yet
> ---
> src/app/models/provider.rb | 23
> +++++++++++++++++--
> src/app/models/provider_account.rb | 7 ++++-
> .../20120202121555_add_realm_available_column.rb | 11 +++++++++
> src/spec/factories/provider.rb | 7 ++++++
> src/spec/factories/realm.rb | 1 -
> src/spec/models/instance_spec.rb | 19
> ++++++++++++++++
> 6 files changed, 62 insertions(+), 6 deletions(-)
> create mode 100644
> src/db/migrate/20120202121555_add_realm_available_column.rb
>
> diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb
> index 01c0d3e..498304d 100644
> --- a/src/app/models/provider.rb
> +++ b/src/app/models/provider.rb
> @@ -134,6 +134,15 @@ class Provider< ActiveRecord::Base
>
> def populate_realms
> reload
> +
> + # if the provider is not running, mark as unavailable and don't
> refresh its
> + # realms
> + unless valid_framework?
> + update_attribute(:available, false)
> + logger.warn "provider #{name} is not available"
> + return
> + end
> +
> conductor_acct_realms = {}
> conductor_acct_realm_ids = {}
> deltacloud_realms = []
> @@ -141,7 +150,12 @@ class Provider< ActiveRecord::Base
> dc_acct_realm_ids = {}
> self.transaction do
> provider_accounts.each do |acct|
> - client = acct.connect
> + # if account is not accessible (but provider is running)
> + # account's realms will be removed
> + unless client = acct.connect
> + logger.warn "provider account #{acct.name} is not available"
> + next
> + end
> dc_acct_realms[acct.label] = client.realms
> dc_acct_realm_ids[acct.label] =
> dc_acct_realms[acct.label].collect{|r| r.id}
> dc_acct_realms[acct.label].each do |dc_realm|
> @@ -172,16 +186,19 @@ class Provider< ActiveRecord::Base
>
> # Add anything in Deltacloud to Conductor if it's not already
> there
> deltacloud_realms.each do |d_realm|
> - unless conductor_realm_ids.include?(d_realm.id)
> + if ar_realm = conductor_realms.detect {|r| r.external_key ==
> d_realm.id}
> + ar_realm.available = d_realm.state == 'AVAILABLE'
> + else
> ar_realm = Realm.new(:external_key => d_realm.id,
> :name => d_realm.name ?
> d_realm.name : d_realm.id,
> :provider_id => id)
You need to set ar_realm.available in the new realm too, otherwise a
new unavailable realm will initially show up as available.
And one more comment. We'd better downcase our check, since it seems the
providers are inconsistent with usage of 'available' vs 'AVAILABLE'.
> - ar_realm.save!
> end
> + ar_realm.save!
> end
>
> # add any new provider account realm mappings
> provider_accounts.each do |acct|
> + next unless dc_acct_realms[acct.label]
> dc_acct_realms[acct.label].each do |d_realm|
> unless
> conductor_acct_realm_ids[acct.label].include?(d_realm.id)
> acct.realms<< realms.where("external_key" =>
d_realm.id)
> diff --git a/src/app/models/provider_account.rb
> b/src/app/models/provider_account.rb
> index 89009aa..82b44c4 100644
> --- a/src/app/models/provider_account.rb
> +++ b/src/app/models/provider_account.rb
> @@ -352,13 +352,16 @@ class ProviderAccount< ActiveRecord::Base
> else
> account_images.each do |pi|
> if not instance.frontend_realm.nil?
> - brealms =
> instance.frontend_realm.realm_backend_targets.select {|brealm_target|
> brealm_target.target_provider == provider}
> + brealms =
> instance.frontend_realm.realm_backend_targets.select {|brealm_target|
> brealm_target.target_provider == provider and
> brealm_target.target_provider.available}
Instead of checking provider availablity here, we should check at the
beginning of the match method:
if !provider.enabled?
errors << I18n.t('instances.errors.must_be_enabled',
:account_name => name)
perhaps with a new 'if' clause checking availability and returning
instances.errors.provider_unavailable. Otherwise when realm is blank
we won't be verifying provider availability.
> if brealms.empty?
> errors<< I18n.t('instances.errors.realm_not_mapped',
> :frontend_realm_name => instance.frontend_realm.name)
> next
> end
> brealms.each do |brealm_target|
> - if (brealm_target.target_realm.nil? ||
> realms.include?(brealm_target.target_realm))
> + # add match if realm is mapped to provider or if it's
> mapped to
> + # backend realm which is available and is accessible for
> this
> + # provider account
> + if (brealm_target.target_realm.nil? ||
> (brealm_target.target_realm.available&&
> realms.include?(brealm_target.target_realm)))
> matched<<
> Instance::Match.new(instance.pool.pool_family, self, hwp, pi,
> brealm_target.target_realm)
> end
> end
...
Scott
Conditional ACK for the patchset -- fix the 3 issues above, and test
availability-setting with ec2, and then go ahead and push.
Scott