From: Jan Provaznik <jprovazn(a)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 | 21 +++++----------------
src/app/models/image.rb | 10 +++++++---
src/app/models/provider.rb | 8 ++++++++
src/app/views/builds/new.haml | 4 ++--
src/config/image_descriptor_targets.yml | 2 +-
src/spec/factories/replicated_image.rb | 9 +++++++++
src/spec/models/image_spec.rb | 17 +++++++++++++++++
src/spec/models/template_spec.rb | 9 +++------
8 files changed, 52 insertions(+), 28 deletions(-)
create mode 100644 src/spec/factories/replicated_image.rb
diff --git a/src/app/controllers/builds_controller.rb
b/src/app/controllers/builds_controller.rb
index 820f28b..6ffdd98 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'
@@ -37,21 +37,6 @@ class BuildsController < ApplicationController
@tpl.upload_template unless @tpl.uploaded
errors = {}
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
@@ -87,4 +72,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..1619289 100644
--- a/src/app/models/image.rb
+++ b/src/app/models/image.rb
@@ -60,22 +60,26 @@ 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})
+ if img = Image.find_by_template_id_and_target(template.id, target)
# TODO: we currently silently ignore requests for building image which is
# already built (or is building now)
return img
end
+ unless provider = Provider.find_by_target_with_account(target)
+ raise "There is no provider for '#{target}' type with valid
account."
+ end
+
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 c203d1f..ba8ecd9 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..06523a9 100644
--- a/src/app/views/builds/new.haml
+++ b/src/app/views/builds/new.haml
@@ -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
new file mode 100644
index 0000000..e1294ed
--- /dev/null
+++ b/src/spec/factories/replicated_image.rb
@@ -0,0 +1,9 @@
+Factory.define :replicated_image do |i|
+ i.association(:image)
+ i.uploaded true
+ i.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
--
1.7.2.3