From: Francesco Vollero fvollero@redhat.com

Hey guys, sorry for all this noise, but please throw away the latest 2 email i sent they included a mistake.

This patch its a review of the last patch of Image Imports. What i do here is: 1) Redesign the combobox design in a more slick way 2) Apply the jtomasek suggestion in cucumber test. 3) Fix a distraction bug in controller, uncommenting again the before_filter. --- .../image_factory/image_imports_controller.rb | 25 ++++++++++++++++ src/app/models/image.rb | 10 +++++- src/app/models/image_warehouse_object.rb | 1 + src/app/views/image_factory/image_imports/new.haml | 8 +++++ src/config/locales/en.yml | 1 + src/config/navigation.rb | 1 + src/config/routes.rb | 1 + src/features/image_import.feature | 30 ++++++++++++++++++++ 8 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 src/app/controllers/image_factory/image_imports_controller.rb create mode 100644 src/app/views/image_factory/image_imports/new.haml create mode 100644 src/features/image_import.feature

diff --git a/src/app/controllers/image_factory/image_imports_controller.rb b/src/app/controllers/image_factory/image_imports_controller.rb new file mode 100644 index 0000000..cd25f31 --- /dev/null +++ b/src/app/controllers/image_factory/image_imports_controller.rb @@ -0,0 +1,25 @@ +class ImageFactory::ImageImportsController < ApplicationController + before_filter :require_user + + def new + init_provider_vars + end + + def create + begin + Image.import(ProviderAccount.find(params[:provider_account_id]), params[:ami_id]) + flash[:notice]="Image successfully imported" + redirect_to image_factory_templates_path + rescue => e + init_provider_vars + flash.now[:error]=e.message + Rails.logger.error([e.message,e.backtrace].join("\n ")) + render :new + end + end + +protected +def init_provider_vars + @providers = Provider.list_for_user(current_user, Privilege::VIEW) +end +end diff --git a/src/app/models/image.rb b/src/app/models/image.rb index f15f2da..5e7c994 100644 --- a/src/app/models/image.rb +++ b/src/app/models/image.rb @@ -78,7 +78,7 @@ class Image < ActiveRecord::Base

# 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_COMPLETED + if not self.new_record? and self.status_changed? and self.status == STATE_COMPLETED safe_warehouse_sync upload_to_all_providers_with_account end @@ -126,6 +126,9 @@ class Image < ActiveRecord::Base end

def self.import(account, image_id) + if image_id.blank? + raise "image id should not be blank" + end unless raw_image = account.connect.image(image_id) raise "There is no image with '#{image_id}' id" end @@ -154,6 +157,8 @@ class Image < ActiveRecord::Base :provider_type_id => account.provider.provider_type_id, :template_id => template.id ) + image.generate_uuid + image.upload image.save!

rep = ProviderImage.new( @@ -162,9 +167,10 @@ class Image < ActiveRecord::Base :provider_image_key => image_id, :status => STATE_COMPLETED ) + rep.generate_uuid + rep.upload rep.save!

- template.upload end image end diff --git a/src/app/models/image_warehouse_object.rb b/src/app/models/image_warehouse_object.rb index 43f3b76..b88cc93 100644 --- a/src/app/models/image_warehouse_object.rb +++ b/src/app/models/image_warehouse_object.rb @@ -45,6 +45,7 @@ module ImageWarehouseObject # TODO: for now there is no way how it check if bucket exists in warehouse # so we try to create bucket everytime, if bucket exists, warehouse returns # 500 Internal server error + raise "uuid is not set" unless self.uuid warehouse.create_bucket(warehouse_bucket) rescue true # TODO: we delete existing object if it exists warehouse.bucket(warehouse_bucket).object(self.uuid).delete! rescue true diff --git a/src/app/views/image_factory/image_imports/new.haml b/src/app/views/image_factory/image_imports/new.haml new file mode 100644 index 0000000..cebc7a7 --- /dev/null +++ b/src/app/views/image_factory/image_imports/new.haml @@ -0,0 +1,8 @@ +- form_tag :image_factory_image_imports do + %fieldset.clear + = label_tag :provider_account_id, "Provider Account ID: " + = select_tag :provider_account_id, option_groups_from_collection_for_select(@providers,:provider_accounts, :name, :id,:name) + %fieldset.clear + = label_tag :ami_id, "Image ID: " + = text_field_tag :ami_id, params[:ami_id] + = restful_submit_tag "Import", 'create', image_factory_image_imports_path, 'POST', :class => "submit" diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml index 7a448de..7510a05 100644 --- a/src/config/locales/en.yml +++ b/src/config/locales/en.yml @@ -38,6 +38,7 @@ en: assistance_requests: Assistance Requests define: Define deployables: Deployables + image_imports: Image Imports basic_template: Template Builder Basic Workflow browse_packages: Browse Packages builds: Builds diff --git a/src/config/navigation.rb b/src/config/navigation.rb index bcf3b3b..b6944a5 100644 --- a/src/config/navigation.rb +++ b/src/config/navigation.rb @@ -10,6 +10,7 @@ SimpleNavigation::Configuration.run do |navigation| second_level.item :templates, t(:templates), image_factory_templates_path second_level.item :assemblies, t(:assemblies), image_factory_assemblies_path second_level.item :deployables, t(:deployables), image_factory_deployables_path + second_level.item :image_imports, t(:image_imports), new_image_factory_image_import_path end first_level.item :administration, t(:administration), admin_users_path, :highlights_on => //admin/ do |second_level| second_level.item :users, t(:users), admin_users_path diff --git a/src/config/routes.rb b/src/config/routes.rb index 313dd25..98bc4f8 100644 --- a/src/config/routes.rb +++ b/src/config/routes.rb @@ -43,6 +43,7 @@ ActionController::Routing::Routes.draw do |map|

map.namespace 'image_factory' do |r| r.resources :assemblies + r.resources :image_imports 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 diff --git a/src/features/image_import.feature b/src/features/image_import.feature new file mode 100644 index 0000000..8fc6f7a --- /dev/null +++ b/src/features/image_import.feature @@ -0,0 +1,30 @@ +Feature: Import Images + In order to use other images in my cloud infrastructure + As a user + I want to import images from other providers + + Background: + Given I am an authorised user + And I am logged in + And I am on the homepage + And there is a provider named "testprovider" + And there is a provider account named "provider1" + And There is a mock pulp repository + When I go to the new image factory image import page + Then I should be on the new image factory image import page + When I select "provider1" from "provider_account_id" + And I fill in "ami_id" with "img1" + + Scenario: Import a new image + Given I press "Import" + Then I should see "Image successfully imported" + And I should be on the image factory templates page + + Scenario: Import an already imported image + Given I press "Import" + When I go to the new image factory image import page + When I select "provider1" from "provider_account_id" + And I fill in "ami_id" with "img1" + And I press "Import" + Then I should see "Image 'img1' is already imported" + And I should be on the image factory image imports page