From: Jan Provaznik <jprovazn(a)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
--
1.7.4