[PATCH conductor 1/3] BZ 717987 (1) - Added realms available flag

Jan Provazník jprovazn at redhat.com
Mon Feb 6 19:30:32 UTC 2012


On 02/06/2012 04:53 PM, Scott Seago wrote:
> On 02/06/2012 09:01 AM, Scott Seago wrote:
>> On 02/03/2012 10:10 AM, jprovazn at redhat.com wrote:
>>> From: Jan Provaznik<jprovazn at 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

Pushed (with the issues fixed), thanks for the review.

Jan



More information about the aeolus-devel mailing list