From: Jan Provaznik <jprovazn(a)redhat.com>
---
.../controllers/image_factory/builds_controller.rb | 5 +-
src/app/models/image.rb | 67 ++++++++++++++------
src/app/models/image_warehouse_object.rb | 9 ++-
src/app/models/provider.rb | 8 ---
src/app/models/provider_image.rb | 3 +
src/features/step_definitions/template_steps.rb | 2 +-
src/image_builder_service/image_builder_service | 10 +++-
src/spec/controllers/builds_controller_spec.rb | 8 +--
src/spec/controllers/templates_controller_spec.rb | 8 ---
src/spec/factories/image.rb | 6 ++
src/spec/models/image_spec.rb | 10 +--
11 files changed, 79 insertions(+), 57 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..55d3f9f 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,42 @@ 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
+ unless self.provider_type.build_supported
+ raise "Build is not supported on images with provider type
#{self.provider_type.name}"
end
+ 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 +178,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.push
+ 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 ee45869..e218970 100644
--- a/src/app/models/provider_image.rb
+++ b/src/app/models/provider_image.rb
@@ -13,6 +13,8 @@
#
class ProviderImage < ActiveRecord::Base
+ include ImageWarehouseObject
+
belongs_to :provider
belongs_to :image
has_one :icicle, :dependent => :destroy
@@ -20,6 +22,7 @@ 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
def push
# TODO: this is just stubbed upload call,
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/image_builder_service/image_builder_service
b/src/image_builder_service/image_builder_service
index d449a03..5167e10 100755
--- a/src/image_builder_service/image_builder_service
+++ b/src/image_builder_service/image_builder_service
@@ -113,7 +113,13 @@ 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.provider_images.first.provider.provider_accounts.first.build_credentials
+ creds = nil
+ image.provider_type.providers.each do |p|
+ unless p.provider_accounts.empty?
+ creds = p.provider_accounts.first.build_credentials
+ break
+ end
+ end
#TODO: switch this back to uri once ActiveBuild retrieves it properly
ab = @console.build_image(image.template.xml.to_xml, image.provider_type.codename,
image.uuid, creds)
if ab
@@ -167,6 +173,7 @@ class ImageBuilderService
@log.debug "Updating with status: " + new_status
@log.debug "========================================"
image.status = new_status
+ image.save!
if new_status == 'complete'
ri = image.provider_images.first
ri.provider_image_key = obj[:build].finished_image
@@ -180,7 +187,6 @@ class ImageBuilderService
@log.debug "========================================"
kick_condor
end
- image.save!
@log.debug "========================================"
@log.debug "database updated!"
@log.debug "========================================"
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/controllers/templates_controller_spec.rb
b/src/spec/controllers/templates_controller_spec.rb
index 65bb26c..db38d74 100644
--- a/src/spec/controllers/templates_controller_spec.rb
+++ b/src/spec/controllers/templates_controller_spec.rb
@@ -10,14 +10,6 @@ describe ImageFactory::TemplatesController do
activate_authlogic
end
- it "should allow a user with image_modify permission to create new template"
do
- UserSession.create(@admin)
- lambda do
- post :create, :tpl => { :name => 'template', :platform =>
'fedora' }
- end.should change(Template, :count).by(1)
- response.should redirect_to(image_factory_templates_path)
- end
-
it "should deny access to new template ui without image modify permission"
do
UserSession.create(@tuser)
get :new
diff --git a/src/spec/factories/image.rb b/src/spec/factories/image.rb
index c8b34e3..e2bcd25 100644
--- a/src/spec/factories/image.rb
+++ b/src/spec/factories/image.rb
@@ -6,4 +6,10 @@ Factory.define :image do |i|
i.status 'queued'
i.provider_type_id { ProviderType.find_by_codename("ec2").id }
i.association(:template)
+ i.after_build do |img|
+ if img.respond_to?(:stub!)
+ img.stub!(:build).and_return(true)
+ img.stub!(:upload).and_return(true)
+ end
+ end
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