Hi, 2 notes inline.
On 03/31/2011 11:30 PM, Scott Seago wrote:
Signed-off-by: Scott Seago<sseago(a)redhat.com>
---
.../controllers/image_factory/builds_controller.rb | 36 ++++++++++++++++++++
src/app/models/image.rb | 11 ++++++
src/app/models/provider_image.rb | 6 +++
src/app/views/image_factory/templates/_builds.haml | 12 +++++-
src/config/routes.rb | 2 +-
src/spec/controllers/builds_controller_spec.rb | 14 ++++++++
6 files changed, 78 insertions(+), 3 deletions(-)
create mode 100644 src/app/views/image_factory/builds/index.haml
diff --git a/src/app/controllers/image_factory/builds_controller.rb
b/src/app/controllers/image_factory/builds_controller.rb
index b6efe81..a9901e5 100644
--- a/src/app/controllers/image_factory/builds_controller.rb
+++ b/src/app/controllers/image_factory/builds_controller.rb
@@ -1,6 +1,9 @@
class ImageFactory::BuildsController< ApplicationController
before_filter [:require_user], :except => [:update_status]
+ def index
+ end
+
def create
@tpl = Template.find(params[:template_id])
check_permission
@@ -43,6 +46,39 @@ class ImageFactory::BuildsController< ApplicationController
# FIXME: is @tpl defined here? do we need check_permission here?
end
+ def retry
+ @tpl = Template.find(params[:template_id])
+ check_permission
+ begin
+ if params[:image_id]
+ image = Image.find(params[:image_id])
+ if image
+ errors = {}
+ warnings = []
+ image.rebuild!
+ flash[:notice] = "Queued rebuild for image #{image.name}"
+ else
+ flash[:warning] = "could not find image #{image_id}"
+ end
+ elsif params[:provider_image_id]
+ provider_image = ProviderImage.find(params[:provider_image_id])
+ if provider_image
+ provider_image.retry_upload!
+ flash[:notice] = "Queued upload for provider image
#{provider_image.name}"
+ else
+ flash[:warning] = "could not find provider image
#{provider_image_id}"
+ end
+ else
+ flash[:warning] = 'You need to specify either a provider or a provider
image'
+ end
+ rescue
+ flash[:warning] = $!.message
+ logger.error $!.message
+ logger.error $!.backtrace.join("\n ")
+ end
+ redirect_to image_factory_template_path(@tpl, :details_tab => 'builds')
+ end
+
def update_status
image = Image.find_by_uuid(params[:uuid])
image = ProviderImage.find_by_uuid(params[:uuid]) unless image
diff --git a/src/app/models/image.rb b/src/app/models/image.rb
index f15f2da..f0434b8 100644
--- a/src/app/models/image.rb
+++ b/src/app/models/image.rb
@@ -84,6 +84,17 @@ class Image< ActiveRecord::Base
end
end
+ def rebuild!
+ # TODO: remove this check when we have UI for pushing images
+ if self.provider_type.providers.empty? or not self.provider_type.providers.detect
{|p| !p.provider_accounts.empty?}
+ raise "Error: A valid provider and provider account are required to create
and build templates"
+ end
+ self.status = STATE_QUEUED
+ self.save!
+ self.provider_images.each {|pimg| pimg.destroy}
+ Delayed::Job.enqueue(BuildJob.new(self.id))
+ end
+
def not_uploaded_providers
uploaded = self.provider_images
Provider.all(:conditions => {:provider_type_id =>
self.provider_type.id}).select do |p|
diff --git a/src/app/models/provider_image.rb b/src/app/models/provider_image.rb
index aee69a9..9cc0e23 100644
--- a/src/app/models/provider_image.rb
+++ b/src/app/models/provider_image.rb
@@ -37,6 +37,12 @@ class ProviderImage< ActiveRecord::Base
end
end
+ def retry_upload!
+ self.status = STATE_QUEUED
+ self.save!
+ Delayed::Job.enqueue(BuildJob.new(self.id))
PushJob should be called here.
+ end
+
def warehouse_bucket
'provider_images'
end
diff --git a/src/app/views/image_factory/builds/index.haml
b/src/app/views/image_factory/builds/index.haml
new file mode 100644
index 0000000..e69de29
diff --git a/src/app/views/image_factory/templates/_builds.haml
b/src/app/views/image_factory/templates/_builds.haml
index 65cbc5a..446ceb2 100644
--- a/src/app/views/image_factory/templates/_builds.haml
+++ b/src/app/views/image_factory/templates/_builds.haml
@@ -19,12 +19,20 @@ Builds for
- else
- @images.each do |img|
%li
- = "#{img.name}-#{img.template.architecture}: #{img.status}"
+ - form_tag(retry_image_factory_builds_path, { :method => :post }) do
+ = "#{img.name}-#{img.template.architecture}: #{img.status}"
+ = hidden_field_tag :template_id, @tpl.id
+ = hidden_field_tag :image_id, img.id
+ = submit_tag "Retry build", :name => "build"
- if img.status == Image::STATE_COMPLETED
%ul
- img.provider_images.each do |pimg|
%li
- = "#{pimg.provider.name}: #{pimg.status}"
+ - form_tag(retry_image_factory_builds_path, { :method => :post })
do
+ = "#{pimg.provider.name}: #{pimg.status}"
+ = hidden_field_tag :template_id, @tpl.id
+ = hidden_field_tag :provider_image_id, img.id
+ = submit_tag "Retry upload", :name => "build"
- unless @tpl.imported
- img.not_uploaded_providers.each do |provider|
%li
I think 'retry' action should be allowed only for failed jobs or at
least for not completed images / provider images. Also not sure what
happens if I run 'retry' on image which is building at the moment (but I
would expect that it finishes current build and pushes to warehouse,
then starts new build invoked by 'retry' action).
Both retry buttons can be done by links instead of forms, but if you
prefer this way, OK.
diff --git a/src/config/routes.rb b/src/config/routes.rb
index 313dd25..54c8ee6 100644
--- a/src/config/routes.rb
+++ b/src/config/routes.rb
@@ -46,7 +46,7 @@ ActionController::Routing::Routes.draw do |map|
r.resources :deployables, :collection => { :multi_destroy => :delete }
r.resources :templates, :collection => {:collections => :get, :add_selected
=> :get, :metagroup_packages => :get, :remove_package => :get, :multi_destroy
=> :delete}
r.connect "/builds/update_status.:format", :controller => :builds,
:action => :update_status
- r.resources :builds, :collection => { :delete => :delete, :upload =>
:get }
+ r.resources :builds, :collection => { :delete => :delete, :upload =>
:get, :retry => :post }
end
map.namespace 'admin' do |r|
diff --git a/src/spec/controllers/builds_controller_spec.rb
b/src/spec/controllers/builds_controller_spec.rb
index a3d7f4d..17fac9c 100644
--- a/src/spec/controllers/builds_controller_spec.rb
+++ b/src/spec/controllers/builds_controller_spec.rb
@@ -53,5 +53,19 @@ describe ImageFactory::BuildsController do
post :create, :template_id => @template.id, :target =>
ProviderType.find_by_codename("mock").id
end.should change(Image, :count).by(1)
end
+
+ it "retry build should update Image status" do
+ lambda do
+ post :create, :template_id => @template.id, :target =>
ProviderType.find_by_codename("mock").id
+ end.should change(Image, :count).by(1)
+ @template.images.size.should == 1
+ image = @template.images[0]
+ image.reload.status.should == "queued"
+ post :update_status, :uuid => image.uuid, :status => 'failed'
+ image.reload.status.should == "failed"
+ post :retry, :image_id => image.id, :template_id => @template.id
+ image.reload.status.should == "queued"
+
+ end
end
end