Re: [PATCH aeolus] Renamed ReplicatedImage to ProviderImage

From: Martyn Taylor&lt;mtaylor(a)redhat.com&gt; --- .../controllers/resources/instances_controller.rb | 4 ++-- src/app/models/image.rb | 10 +++++----- src/app/models/provider.rb | 2 +- src/app/models/provider_image.rb | 7 +++++++ src/app/models/replicated_image.rb | 7 ------- src/app/models/template.rb | 2 +- src/app/util/condormatic.rb | 14 +++++++------- ...31_rename_replicated_image_to_provider_image.rb | 9 +++++++++ src/features/provider.feature | 4 ++-- src/features/step_definitions/instance_steps.rb | 2 +- src/features/step_definitions/provider_steps.rb | 8 ++++---- src/image_builder_service/image_builder_service | 4 ++-- src/spec/controllers/builds_controller_spec.rb | 4 ++-- src/spec/factories/provider_image.rb | 11 +++++++++++ src/spec/factories/replicated_image.rb | 11 ----------- src/spec/models/image_spec.rb | 4 ++-- src/spec/models/template_spec.rb | 2 +- 17 files changed, 57 insertions(+), 48 deletions(-) create mode 100644 src/app/models/provider_image.rb delete mode 100644 src/app/models/replicated_image.rb create mode 100644 src/db/migrate/20110207110131_rename_replicated_image_to_provider_image.rb create mode 100644 src/spec/factories/provider_image.rb delete mode 100644 src/spec/factories/replicated_image.rb diff --git a/src/app/controllers/resources/instances_controller.rb b/src/app/controllers/resources/instances_controller.rb index bf5e021..d1be6a5 100644 --- a/src/app/controllers/resources/instances_controller.rb +++ b/src/app/controllers/resources/instances_controller.rb @@ -34,8 +34,8 @@ class Resources::InstancesController< ApplicationController # => TODO: add TEMPLATE_* permissions @templates = Template.paginate( :page => params[:page] || 1, - :include => {:images => :replicated_images}, - :conditions => "replicated_images.uploaded = 't'" + :include => {:images => :provider_images}, + :conditions => "provider_images.uploaded = 't'" ) end diff --git a/src/app/models/image.rb b/src/app/models/image.rb index 8962bf1..2cb15b7 100644 --- a/src/app/models/image.rb +++ b/src/app/models/image.rb @@ -30,8 +30,8 @@ class Image< ActiveRecord::Base @@per_page = 15 belongs_to :template, :counter_cache => true - has_many :replicated_images, :dependent => :destroy - has_many :providers, :through => :replicated_images + has_many :provider_images, :dependent => :destroy + has_many :providers, :through => :provider_images validates_presence_of :name validates_length_of :name, :maximum => 1024 @@ -78,7 +78,7 @@ class Image< ActiveRecord::Base :template_id => template.id, :status => Image::STATE_QUEUED ) - ReplicatedImage.create!( + ProviderImage.create!( :image_id => img.id, :provider_id => provider ) @@ -108,7 +108,7 @@ class Image< ActiveRecord::Base raise "There is no image with '#{image_id}' id" end - if ReplicatedImage.find_by_provider_id_and_provider_image_key(account.provider.id, image_id) + if ProviderImage.find_by_provider_id_and_provider_image_key(account.provider.id, image_id) raise "Image '#{image_id}' is already imported" end @@ -134,7 +134,7 @@ class Image< ActiveRecord::Base ) image.save! - rep = ReplicatedImage.new( + rep = ProviderImage.new( :image_id => image.id, :provider_id => account.provider.id, :provider_image_key => image_id, diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb index 1e89958..31eb7bb 100644 --- a/src/app/models/provider.rb +++ b/src/app/models/provider.rb @@ -31,7 +31,7 @@ class Provider< ActiveRecord::Base has_many :cloud_accounts, :dependent => :destroy has_many :hardware_profiles, :dependent => :destroy - has_many :replicated_images, :dependent => :destroy + has_many :provider_images, :dependent => :destroy has_many :realms, :dependent => :destroy validates_presence_of :name diff --git a/src/app/models/provider_image.rb b/src/app/models/provider_image.rb new file mode 100644 index 0000000..054972a --- /dev/null +++ b/src/app/models/provider_image.rb @@ -0,0 +1,7 @@ +class ProviderImage< ActiveRecord::Base + belongs_to :provider + belongs_to :image + + validates_presence_of :provider_id + validates_presence_of :image_id +end diff --git a/src/app/models/replicated_image.rb b/src/app/models/replicated_image.rb deleted file mode 100644 index ace66fc..0000000 --- a/src/app/models/replicated_image.rb +++ /dev/null @@ -1,7 +0,0 @@ -class ReplicatedImage< ActiveRecord::Base - belongs_to :provider - belongs_to :image - - validates_presence_of :provider_id - validates_presence_of :image_id -end diff --git a/src/app/models/template.rb b/src/app/models/template.rb index b2ef852..4e8ad27 100644 --- a/src/app/models/template.rb +++ b/src/app/models/template.rb @@ -71,7 +71,7 @@ class Template< ActiveRecord::Base def providers # TODO: rewrite cleanly - ReplicatedImage.all( + ProviderImage.all( :include => [:image, :provider], :conditions => {:images => {:template_id => self.id}} ).map {|p| p.provider} diff --git a/src/app/util/condormatic.rb b/src/app/util/condormatic.rb index 8b4e224..57ad075 100644 --- a/src/app/util/condormatic.rb +++ b/src/app/util/condormatic.rb @@ -210,24 +210,24 @@ def condormatic_classads_sync Rails.logger.info "Syncing classads.." ads = [] providers.each { |provider| - # The replicated image entry gets put in the database as soon as we ask + # The provider image entry gets put in the database as soon as we ask # to have the image built, so we only want to generate classads for it if # it is ready to be used. When ready it will have an image key assigned # to it. - replicated_images = provider.replicated_images.find(:all, + provider_images = provider.provider_images.find(:all, :conditions => ['provider_image_key IS NOT NULL']) accounts = provider.cloud_accounts hardware_profiles = provider.hardware_profiles realms = provider.realms accounts.each { |account| - replicated_images.each { |replicated_img| + provider_images.each { |provider_img| hardware_profiles.each { |hwp| ads += realms.collect { |realm| - [account, replicated_img, hwp, realm] } } } } + [account, provider_img, hwp, realm] } } } } } ads.each { |ad| - account, replicated_image, hwp, realm = *ad + account, provider_image, hwp, realm = *ad pipe = IO.popen("condor_advertise UPDATE_STARTD_AD 2>&1", "w+") @@ -236,10 +236,10 @@ def condormatic_classads_sync pipe.puts 'Requirements=true' pipe.puts "\n# Stuff needed to match:" pipe.puts "hardwareprofile=\"#{hwp.aggregator_hardware_profiles[0].id}\"" - pipe.puts "image=\"#{replicated_image.image.template.id}\"" + pipe.puts "image=\"#{provider_image.image.template.id}\"" pipe.puts "realm=\"#{realm.frontend_realms[0].id}\"" pipe.puts "\n# Backend info to complete this job:" - pipe.puts "image_key=\"#{replicated_image.provider_image_key}\"" + pipe.puts "image_key=\"#{provider_image.provider_image_key}\"" pipe.puts "hardwareprofile_key=\"#{hwp.external_key}\"" pipe.puts "realm_key=\"#{realm.external_key}\"" pipe.puts "provider_url=\"#{account.provider.url}\"" diff --git a/src/db/migrate/20110207110131_rename_replicated_image_to_provider_image.rb b/src/db/migrate/20110207110131_rename_replicated_image_to_provider_image.rb new file mode 100644 index 0000000..0e3f60c --- /dev/null +++ b/src/db/migrate/20110207110131_rename_replicated_image_to_provider_image.rb @@ -0,0 +1,9 @@ +class RenameReplicatedImageToProviderImage< ActiveRecord::Migration + def self.up + rename_table :replicated_images, :provider_images + end + + def self.down + rename_table :provider_images, :replicated_images + end +end diff --git a/src/features/provider.feature b/src/features/provider.feature index c2f6da5..2cdc80b 100644 --- a/src/features/provider.feature +++ b/src/features/provider.feature @@ -60,7 +60,7 @@ Feature: Manage Providers Scenario: Delete a provider Given I am on the homepage And there is a provider named "provider1" - And this provider has 5 replicated images + And this provider has 5 provider images And this provider has 5 hardware profiles And this provider has a realm And this provider has a cloud account @@ -68,7 +68,7 @@ Feature: Manage Providers And I check "provider1" provider And I press "Delete" And there should not exist a provider named "provider1" - And there should not be any replicated images + And there should not be any provider images And there should not be any hardware profiles And there should not be a cloud account And there should not be a realm diff --git a/src/features/step_definitions/instance_steps.rb b/src/features/step_definitions/instance_steps.rb index 4324891..956d3de 100644 --- a/src/features/step_definitions/instance_steps.rb +++ b/src/features/step_definitions/instance_steps.rb @@ -48,7 +48,7 @@ Given /^there is a "([^"]*)" running instance$/ do |name| end Given /^there is an uploaded image for a template$/ do - Factory :replicated_image + Factory :provider_image end Given /^there is "([^"]*)" aggregator hardware profile$/ do |name| diff --git a/src/features/step_definitions/provider_steps.rb b/src/features/step_definitions/provider_steps.rb index bc56276..927fb33 100644 --- a/src/features/step_definitions/provider_steps.rb +++ b/src/features/step_definitions/provider_steps.rb @@ -36,8 +36,8 @@ Given /^there are these providers:$/ do |table| end end -Given /^this provider has (\d+) replicated images$/ do |number| - number.to_i.times { |i| Factory(:replicated_image, :provider => @provider) } +Given /^this provider has (\d+) provider images$/ do |number| + number.to_i.times { |i| Factory(:provider_image, :provider => @provider) } end Given /^this provider has (\d+) hardware profiles$/ do |number| @@ -53,8 +53,8 @@ Given /^this provider has a cloud account$/ do Factory(:mock_cloud_account, :provider => @provider) end -Then /^there should not be any replicated images$/ do - ReplicatedImage.find(:all, :conditions => { :provider_id => @provider.id} ).size.should == 0 +Then /^there should not be any provider images$/ do + ProviderImage.find(:all, :conditions => { :provider_id => @provider.id} ).size.should == 0 end Then /^there should not be any hardware profiles$/ do diff --git a/src/image_builder_service/image_builder_service b/src/image_builder_service/image_builder_service index 0ffb74d..c5e4822 100755 --- a/src/image_builder_service/image_builder_service +++ b/src/image_builder_service/image_builder_service @@ -121,7 +121,7 @@ class ImageBuilderService # FIXME: this should be contained elsewhere (probably Image model) so we # can keep logic out of here. Also, this currently only handles one # account, we will need to be able to specify at some point. - creds = image.replicated_images.first.provider.cloud_accounts.first.build_credentials + creds = image.provider_images.first.provider.cloud_accounts.first.build_credentials #TODO: switch this back to uri once ActiveBuild retrieves it properly ab = @console.build_image(image.template.xml.to_xml, image.target, image.uuid, creds) if ab @@ -176,7 +176,7 @@ class ImageBuilderService @log.debug "========================================" image.status = new_status if new_status == 'complete' - ri = image.replicated_images.first + ri = image.provider_images.first ri.provider_image_key = obj[:build].finished_image ri.uploaded =true ri.registered=true diff --git a/src/spec/controllers/builds_controller_spec.rb b/src/spec/controllers/builds_controller_spec.rb index 31b47f5..560290b 100644 --- a/src/spec/controllers/builds_controller_spec.rb +++ b/src/spec/controllers/builds_controller_spec.rb @@ -28,10 +28,10 @@ describe ImageFactory::BuildsController do end.should change(Image, :count).by(1) end - it "should create a new ReplicatedImage" do + it "should create a new ProviderImage" do lambda do post :create, :template_id => @template.id, :targets => ["mock"] - end.should change(ReplicatedImage, :count).by(1) + end.should change(ProviderImage, :count).by(1) end end end diff --git a/src/spec/factories/provider_image.rb b/src/spec/factories/provider_image.rb new file mode 100644 index 0000000..55c2281 --- /dev/null +++ b/src/spec/factories/provider_image.rb @@ -0,0 +1,11 @@ +Factory.define :provider_image do |ri| + ri.association :image + ri.association :provider + ri.sequence(:provider_image_key) { |n| "provider_image_key#(n)" } + ri.uploaded true + ri.registered true +end + +Factory.define :mock_provider_image, :parent => :provider_image do |i| + i.provider { |p| p.association(:mock_provider) } +end diff --git a/src/spec/factories/replicated_image.rb b/src/spec/factories/replicated_image.rb deleted file mode 100644 index fa3f662..0000000 --- a/src/spec/factories/replicated_image.rb +++ /dev/null @@ -1,11 +0,0 @@ -Factory.define :replicated_image do |ri| - ri.association :image - ri.association :provider - ri.sequence(:provider_image_key) { |n| "provider_image_key#(n)" } - ri.uploaded true - ri.registered true -end - -Factory.define :mock_replicated_image, :parent => :replicated_image do |i| - i.provider { |p| p.association(:mock_provider) } -end diff --git a/src/spec/models/image_spec.rb b/src/spec/models/image_spec.rb index dbd5ba8..e16d43f 100644 --- a/src/spec/models/image_spec.rb +++ b/src/spec/models/image_spec.rb @@ -49,7 +49,7 @@ describe Image do tpl.save! img = Image.build(tpl, 'mock') Image.find(img).should == img - ReplicatedImage.find_by_image_id(img.id).should_not be_nil + ProviderImage.find_by_image_id(img.id).should_not be_nil end it "should import image" do @@ -73,6 +73,6 @@ describe Image do img.template.uploaded.should be_true end.should change(Image, :count).by(1) end.should change(Template, :count).by(1) - end.should change(ReplicatedImage, :count).by(1) + end.should change(ProviderImage, :count).by(1) end end diff --git a/src/spec/models/template_spec.rb b/src/spec/models/template_spec.rb index 4a42996..2249bf3 100644 --- a/src/spec/models/template_spec.rb +++ b/src/spec/models/template_spec.rb @@ -9,7 +9,7 @@ describe Template do end it "should return list of providers who provides images built from this template" do - rimg = Factory.build(:mock_replicated_image) + rimg = Factory.build(:mock_provider_image) rimg.save! rimg.image.template.providers.size.should eql(1) end