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

Scott Seago sseago at redhat.com
Mon Feb 6 15:53:09 UTC 2012


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



More information about the aeolus-devel mailing list