From: Jan Provaznik jprovazn@redhat.com
When user builds template, CE chooses first provider of specified build type which has at least one cloud account. --- src/app/controllers/builds_controller.rb | 25 +++++++++---------------- src/app/models/image.rb | 17 +++++++++++------ src/app/models/provider.rb | 8 ++++++++ src/app/views/builds/new.haml | 6 +++--- src/config/image_descriptor_targets.yml | 2 +- src/spec/factories/replicated_image.rb | 8 +++++++- src/spec/models/image_spec.rb | 17 +++++++++++++++++ src/spec/models/template_spec.rb | 9 +++------ 8 files changed, 59 insertions(+), 33 deletions(-)
diff --git a/src/app/controllers/builds_controller.rb b/src/app/controllers/builds_controller.rb index 820f28b..9346aea 100644 --- a/src/app/controllers/builds_controller.rb +++ b/src/app/controllers/builds_controller.rb @@ -1,5 +1,5 @@ class BuildsController < ApplicationController - before_filter :require_user + before_filter [:require_user, :check_permission]
def section_id 'build' @@ -36,28 +36,17 @@ class BuildsController < ApplicationController
@tpl.upload_template unless @tpl.uploaded errors = {} + warnings = [] params[:targets].each do |target| - # FIXME: for beta release we check explicitly that provider and provider - # account exists - unless provider = Provider.find_by_cloud_type(target) - flash_error('Error while trying to build image', - 'no provider account' => "There is no provider of '#{target}' type, \ -you can add provider on <a href="#{url_for :controller => 'provider'}">the providers page.</a>") - render :action => 'new' and return - end - if provider.cloud_accounts.empty? - flash_error('Error while trying to build image', - 'no provider account' => "There is no provider account for '#{target}' \ -provider, you can add account on <a href="#{url_for :controller => 'provider', \ -:action => 'accounts', :id => provider.id}">the provider accounts page</a>") - render :action => 'new' and return - end begin Image.build(@tpl, target) + rescue ImageExistsError + warnings << "Build for this template and target #{target} already exists. " rescue errors[target] = $!.message end end + flash[:warning] = 'Warning: ' + warnings.join if errors.empty? redirect_to builds_path else @@ -87,4 +76,8 @@ provider, you can add account on <a href="#{url_for :controller => 'provider', flash.now[:error][:failures] ||= {} flash.now[:error][:failures].merge!(errs) end + + def check_permission + require_privilege(Privilege::IMAGE_MODIFY) + end end diff --git a/src/app/models/image.rb b/src/app/models/image.rb index dfe8a2a..8fe842c 100644 --- a/src/app/models/image.rb +++ b/src/app/models/image.rb @@ -19,6 +19,8 @@ # Filters added to this controller apply to all controllers in the application. # Likewise, all the methods added will be available for all controllers.
+class ImageExistsError < Exception;end + class Image < ActiveRecord::Base include SearchFilter
@@ -60,22 +62,25 @@ class Image < ActiveRecord::Base 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 img = Image.find_by_template_id(template.id, :conditions => {:target => target}) - # TODO: we currently silently ignore requests for building image which is - # already built (or is building now) - return img + if Image.find_by_template_id_and_target(template.id, target) + raise ImageExistsError, 'Build for this template and target already exists' + end + + unless provider = Provider.find_by_target_with_account(target) + raise "There is no provider for '#{target}' type with valid account." end
+ img = nil Image.transaction do img = Image.create!( - :name => "#{template.xml.name}/#{target}", + :name => "#{template.name}/#{target}", :target => target, :template_id => template.id, :status => Image::STATE_QUEUED ) ReplicatedImage.create!( :image_id => img.id, - :provider_id => Provider.find_by_cloud_type(target) + :provider_id => provider ) end return img diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb index cff34c2..6a195b0 100644 --- a/src/app/models/provider.rb +++ b/src/app/models/provider.rb @@ -111,6 +111,14 @@ class Provider < ActiveRecord::Base cloud_accounts.collect {|account| account.pools}.flatten.uniq end
+ # returns first provider of cloud_type which has at least one cloud account + def self.find_by_target_with_account(cloud_type) + Provider.all(:conditions => {:cloud_type => cloud_type}).each do |p| + return p unless p.cloud_accounts.empty? + end + nil + end + # TODO: implement or remove - this is meant to contain a hash of # supported cloud_types to use in populating form, though if we # infer that field, we don't need this. diff --git a/src/app/views/builds/new.haml b/src/app/views/builds/new.haml index 4cddb6f..2cb159e 100644 --- a/src/app/views/builds/new.haml +++ b/src/app/views/builds/new.haml @@ -1,4 +1,4 @@ -%h2 BUILD REQUEST +%h2 Build Request - form_tag :action => 'create' do = hidden_field_tag :template_id, @tpl.id %h3 Deployment Definition @@ -48,8 +48,8 @@ %ul.block - @all_targets.each do |target_id, target| %li - = check_box_tag 'targets[]', target_id, false - = label_tag 'targets[]', target['name'] + = check_box_tag 'targets[]', target_id, false, :id => target_id + = label_tag target_id, target['name']
= submit_tag "Submit to Build", :name => "build" = submit_tag "Cancel", :name => "cancel" diff --git a/src/config/image_descriptor_targets.yml b/src/config/image_descriptor_targets.yml index e711534..771ddfc 100644 --- a/src/config/image_descriptor_targets.yml +++ b/src/config/image_descriptor_targets.yml @@ -15,4 +15,4 @@ ec2:
# For development, just uncomment this target to do fake image builds #mock: -# name: Mock \ No newline at end of file +# name: Mock diff --git a/src/spec/factories/replicated_image.rb b/src/spec/factories/replicated_image.rb index f989380..fa3f662 100644 --- a/src/spec/factories/replicated_image.rb +++ b/src/spec/factories/replicated_image.rb @@ -2,4 +2,10 @@ Factory.define :replicated_image do |ri| ri.association :image ri.association :provider ri.sequence(:provider_image_key) { |n| "provider_image_key#(n)" } -end \ No newline at end of file + 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 030e792..b1b42a8 100644 --- a/src/spec/models/image_spec.rb +++ b/src/spec/models/image_spec.rb @@ -34,4 +34,21 @@ describe Image do i.template_id = 1 i.should be_valid end + + it "should not build image if image already exists for specified target" do + old = Factory.build(:image) + old.save! + new = Image.build(old.template, old.target) + old.id.should == new.id + end + + it "should build image if there is provider and cloud account for specified target" do + acc = Factory.build(:mock_cloud_account) + acc.save! + tpl = Factory.build(:template) + tpl.save! + img = Image.build(tpl, 'mock') + Image.find(img).should == img + ReplicatedImage.find_by_image_id(img.id).should_not be_nil + end end diff --git a/src/spec/models/template_spec.rb b/src/spec/models/template_spec.rb index 406febd..94de1df 100644 --- a/src/spec/models/template_spec.rb +++ b/src/spec/models/template_spec.rb @@ -9,12 +9,9 @@ describe Template do end
it "should return list of providers who provides images built from this template" do - tpl = Factory.build(:template) - img = Factory.build(:image, :template_id => tpl) - provider = Factory.build(:mock_provider) - rimg = ReplicatedImage.new(:provider_id => provider, :image_id => img) - rimg.save - tpl.providers.size.should eql(1) + rimg = Factory.build(:mock_replicated_image) + rimg.save! + rimg.image.template.providers.size.should eql(1) end
it "should not be valid if template name is too long" do
deltacloud-devel@lists.fedorahosted.org