From: Jan Provaznik jprovazn@redhat.com
--- .../controllers/image_factory/builds_controller.rb | 5 +- src/app/models/image.rb | 75 ++++++++++++++----- src/app/models/image_warehouse_object.rb | 9 ++- src/app/models/provider.rb | 8 -- src/app/models/provider_image.rb | 29 ++++++++ src/features/step_definitions/template_steps.rb | 2 +- src/spec/controllers/builds_controller_spec.rb | 8 +-- src/spec/factories/image.rb | 1 + src/spec/models/image_spec.rb | 10 +-- 9 files changed, 100 insertions(+), 47 deletions(-)
diff --git a/src/app/controllers/image_factory/builds_controller.rb b/src/app/controllers/image_factory/builds_controller.rb index ac0a3e2..035bb38 100644 --- a/src/app/controllers/image_factory/builds_controller.rb +++ b/src/app/controllers/image_factory/builds_controller.rb @@ -23,17 +23,18 @@ class ImageFactory::BuildsController < ApplicationController return end
- @tpl.upload unless @tpl.uploaded errors = {} warnings = [] params[:targets].each do |target_id| begin target = ProviderType.find(target_id) - Image.build(@tpl, target) + Image.create_and_build!(@tpl, target) rescue ImageExistsError warnings << $!.message rescue errors[target ? target.name : target_id] = $!.message + logger.error $!.message + logger.error $!.backtrace.join("\n ") end end flash[:warning] = 'Warning: ' + warnings.join unless warnings.empty? diff --git a/src/app/models/image.rb b/src/app/models/image.rb index cfa15e4..55e8f6e 100644 --- a/src/app/models/image.rb +++ b/src/app/models/image.rb @@ -40,6 +40,7 @@ class ImageExistsError < Exception;end
class Image < ActiveRecord::Base include SearchFilter + include ImageWarehouseObject
before_save :generate_uuid
@@ -57,6 +58,8 @@ class Image < ActiveRecord::Base validates_presence_of :template_id validates_presence_of :provider_type_id
+ validates_uniqueness_of :template_id, :scope => :provider_type_id + SEARCHABLE_COLUMNS = %w(name)
STATE_QUEUED = 'queued' @@ -73,31 +76,50 @@ class Image < ActiveRecord::Base self.uuid ||= "image-#{self.template_id}-#{Time.now.to_f.to_s}" end
- def self.build(template, target) - # FIXME: This will need to be enhanced to handle multiple - # providers of same type, only one is supported right now - if Image.find_by_template_id_and_provider_type_id(template.id, target.id) - raise ImageExistsError, "An attempted build of this template for the target '#{target.name}' already exists" + # TODO: for now when build is finished we call upload automatically for all providers + def after_update + if self.status_changed? and self.status == STATE_COMPLETE + # TODO: use after_commit callback in rails 3 - it's better to have it outside + # update transaction + begin + invoke_sync + upload_to_all_providers_with_account + rescue => e + logger.error e.message + logger.error e.backtrace.join("\n ") + end end + end
- unless provider = Provider.find_by_target_with_account(target.id) - raise "There is no provider for '#{target.name}' type with valid account." + def build + # TODO: this is just stubbed upload call, + # when new image_builder_service is done, replace + # with real request to image_builder_service + begin + unless self.provider_type.build_supported + raise "Build is not supported on images with provider type #{self.provider_type.name}" + end + # put build request to image_builder_service here + self.status = STATE_COMPLETE + rescue + # TODO - associate with event model and create event from exception + self.status = STATE_FAILED end + save! + end
- img = nil - Image.transaction do - img = Image.create!( - :name => "#{template.name}/#{target.codename}", - :provider_type_id => target.id, - :template_id => template.id, - :status => Image::STATE_QUEUED - ) - ProviderImage.create!( - :image_id => img.id, - :provider_id => provider - ) + def self.create_and_build!(template, provider_type) + if Image.find_by_template_id_and_provider_type_id(template.id, provider_type.id) + raise ImageExistsError, "An attempted build of this template for the target '#{provider_type.name}' already exists" end - return img + img = Image.create!( + :name => "#{template.name}/#{provider_type.codename}", + :provider_type_id => provider_type.id, + :template_id => template.id, + :status => Image::STATE_QUEUED + ) + img.delay.build + img end
def self.single_import(providername, username, password, image_id) @@ -164,6 +186,19 @@ class Image < ActiveRecord::Base
private
+ def upload_to_all_providers_with_account + provider_type.providers.each do |p| + # upload only to providers with account + unless p.provider_accounts.empty? + ProviderImage.create!( + :uuid => UUIDTools::UUID.timestamp_create.to_s, + :image => self, + :provider => p + ).delay.upload + end + end + end + def self.get_account(providername, username, password) unless provider = Provider.find_by_name(providername) raise "There is not provider with name '#{providername}'" diff --git a/src/app/models/image_warehouse_object.rb b/src/app/models/image_warehouse_object.rb index 7717ccd..40a7c3b 100644 --- a/src/app/models/image_warehouse_object.rb +++ b/src/app/models/image_warehouse_object.rb @@ -27,13 +27,12 @@ module ImageWarehouseObject @xml ||= ImageDescriptorXML.new(self[:xml].to_s) end
- # this should be overriden in a model if we want to save additional attrs - # for the model + # this should be overriden in a model if model wants to save additional attrs def warehouse_attrs {:uuid => self.uuid, :object_type => self.class.class_name} end
- # this should be overriden in a model if we want to save body + # this should be overriden in a model if model wants to save a body def warehouse_body nil end @@ -65,4 +64,8 @@ module ImageWarehouseObject self.uuid ||= UUIDTools::UUID.timestamp_create.to_s end
+ def invoke_sync + # TODO: invoke warehouse_sync here (after switching to eventmachine) + # pass self.uuid to sync only changed object + end end diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb index b933fdb..d7d77ea 100644 --- a/src/app/models/provider.rb +++ b/src/app/models/provider.rb @@ -129,14 +129,6 @@ class Provider < ActiveRecord::Base cloud_accounts.collect {|account| account.pools}.flatten.uniq end
- # returns first provider of provider_type which has at least one cloud account - def self.find_by_target_with_account(provider_type) - Provider.all(:conditions => {:provider_type_id => provider_type}).each do |p| - return p unless p.provider_accounts.empty? - end - nil - end - # TODO: implement or remove - this is meant to contain a hash of # supported provider_types to use in populating form, though if we # infer that field, we don't need this. diff --git a/src/app/models/provider_image.rb b/src/app/models/provider_image.rb index c2d9e2e..7d54a37 100644 --- a/src/app/models/provider_image.rb +++ b/src/app/models/provider_image.rb @@ -14,6 +14,8 @@ #
class ProviderImage < ActiveRecord::Base + include ImageWarehouseObject + belongs_to :provider belongs_to :image belongs_to :icicle @@ -21,9 +23,36 @@ class ProviderImage < ActiveRecord::Base validates_presence_of :provider_id validates_presence_of :image_id validates_uniqueness_of :uuid, :allow_nil => true + validates_uniqueness_of :image_id, :scope => :provider_id
after_destroy :destroy_orphaned_icicles
+ def upload + # TODO: this is just stubbed upload call, + # when new image_builder_service is done, replace + # with real request to image_builder_service + begin + # put upload request to image_builder_service here + self.uploaded = true + self.registered = true + rescue + self.uploaded = false + self.registered = false + end + save! + end + + def after_update + if self.uploaded_changed? and self.uploaded == true + begin + invoke_sync + rescue => e + logger.error e.message + logger.error e.backtrace.join("\n ") + end + end + end + def destroy_orphaned_icicles if self.icicle and self.icicle.provider_images.empty? self.icicle.destroy diff --git a/src/features/step_definitions/template_steps.rb b/src/features/step_definitions/template_steps.rb index 3491eb4..79d2e96 100644 --- a/src/features/step_definitions/template_steps.rb +++ b/src/features/step_definitions/template_steps.rb @@ -98,7 +98,7 @@ Given /^there is Amazon AWS provider account$/ do end
Given /^there is Amazon AWS build for this template$/ do - Image.build(@template, ProviderType.find_by_codename("ec2")) + Image.create_and_build!(@template, ProviderType.find_by_codename("ec2")) end
Given /^there is an imported template$/ do diff --git a/src/spec/controllers/builds_controller_spec.rb b/src/spec/controllers/builds_controller_spec.rb index b0f1885..1bcfa5c 100644 --- a/src/spec/controllers/builds_controller_spec.rb +++ b/src/spec/controllers/builds_controller_spec.rb @@ -24,14 +24,8 @@ describe ImageFactory::BuildsController do
it "should create a new Image" do lambda do - post :create, :template_id => @template.id, :targets => ProviderType.find_by_codename("mock").id + post :create, :template_id => @template.id, :targets => [ProviderType.find_by_codename("mock").id] end.should change(Image, :count).by(1) end - - it "should create a new ProviderImage" do - lambda do - post :create, :template_id => @template.id, :targets => ProviderType.find_by_codename("mock").id - end.should change(ProviderImage, :count).by(1) - end end end diff --git a/src/spec/factories/image.rb b/src/spec/factories/image.rb index c8b34e3..d794dbd 100644 --- a/src/spec/factories/image.rb +++ b/src/spec/factories/image.rb @@ -6,4 +6,5 @@ Factory.define :image do |i| i.status 'queued' i.provider_type_id { ProviderType.find_by_codename("ec2").id } i.association(:template) + i.after_build {|img| img.stub!(:build).and_return(true) if img.respond_to?(:stub!)} end diff --git a/src/spec/models/image_spec.rb b/src/spec/models/image_spec.rb index d364919..9635f20 100644 --- a/src/spec/models/image_spec.rb +++ b/src/spec/models/image_spec.rb @@ -35,10 +35,9 @@ describe Image do i.should be_valid end
- it "should not build image if image already exists for specified target" do - old = Factory.build(:image) - old.save! - lambda {Image.build(old.template, old.provider_type)}.should raise_error(ImageExistsError) + it "should not create and build image if image already exists for specified target" do + old = Factory(:image) + lambda {Image.create_and_build!(old.template, old.provider_type)}.should raise_error(ImageExistsError) end
it "should build image if there is provider and cloud account for specified target" do @@ -47,9 +46,8 @@ describe Image do acc.save! tpl = Factory.build(:template) tpl.save! - img = Image.build(tpl, ProviderType.find_by_codename("mock")) + img = Image.create_and_build!(tpl, ProviderType.find_by_codename("mock")) Image.find(img).should == img - ProviderImage.find_by_image_id(img.id).should_not be_nil end
it "should import image" do