Resolves https://bugzilla.redhat.com/show_bug.cgi?id=765714 --- src/app/controllers/api/images_controller.rb | 7 +++- src/app/controllers/images_controller.rb | 6 ++- src/config/locales/en.yml | 2 + src/lib/exceptions.rb | 4 ++ src/lib/image.rb | 8 +++- src/spec/controllers/api/images_controller_spec.rb | 41 +++++++++++++++++++- 6 files changed, 64 insertions(+), 4 deletions(-)
diff --git a/src/app/controllers/api/images_controller.rb b/src/app/controllers/api/images_controller.rb index db82553..3d3c448 100644 --- a/src/app/controllers/api/images_controller.rb +++ b/src/app/controllers/api/images_controller.rb @@ -68,7 +68,12 @@ module Api account = ProviderAccount.find_by_label(req[:params][:provider_account_name]) raise(Aeolus::Conductor::API::ProviderAccountNotFound.new(404, t("api.error_messages.provider_account_not_found", :name => req[:params][:provider_account_name]))) unless account.present? - @image = Image.import(account, req[:params][:target_identifier], req[:params][:image_descriptor]) + begin + @image = Image.import(account, req[:params][:target_identifier], req[:params][:image_descriptor]) + rescue Aeolus::Conductor::UI::ImageNotFound + raise(Aeolus::Conductor::API::ImageNotFound.new(404, t("api.error_messages.image_not_found_on_provider", + :image => req[:params][:target_identifier]))) + end respond_with(@image) end rescue ActiveResource::BadRequest => e diff --git a/src/app/controllers/images_controller.rb b/src/app/controllers/images_controller.rb index 2adce72..a3d1e20 100644 --- a/src/app/controllers/images_controller.rb +++ b/src/app/controllers/images_controller.rb @@ -97,7 +97,11 @@ class ImagesController < ApplicationController redirect_to image_url(image.id) and return rescue Exception => e logger.error "Caught exception importing image: #{e.message}" - flash[:error] = t("images.import.image_not_imported") + if e.is_a?(Aeolus::Conductor::UI::ImageNotFound) + flash[:error] = t('images.not_on_provider') + else + flash[:error] = t("images.import.image_not_imported") + end redirect_to new_image_url(:tab => 'import') end end diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml index 9f4742f..d478e03 100644 --- a/src/config/locales/en.yml +++ b/src/config/locales/en.yml @@ -738,6 +738,7 @@ en: make_deployable: Automatically make "%{name}" deployable. back: Back save_template: Save Template + not_on_provider: The requested image was not found on the provider. template_xml: errors: xml_parse_error: Failed to parse XML. @@ -1180,6 +1181,7 @@ en: error_messages: build_not_found: Could not find Build %{build} image_not_found: Could not find Image %{image} + image_not_found_on_provider: "Could not find Image %{image} on provider" in_grabbing_target_images: "In grabbing list of target images: %{error}" invalid_parameters_for_account_and_target_image: "Invalid parameters for Account: %{account} TargetImage: %{targetimage}" no_image_build_or_target_image: "Invalid Parameters: No Image, Build or TargetImage provided in request" diff --git a/src/lib/exceptions.rb b/src/lib/exceptions.rb index 2068fe6..6c11681 100644 --- a/src/lib/exceptions.rb +++ b/src/lib/exceptions.rb @@ -42,5 +42,9 @@ module Aeolus class TargetImageStatusNotFound < Error; end class TargetNotFound < Error; end end + + module UI + class ImageNotFound < StandardError; end + end end end diff --git a/src/lib/image.rb b/src/lib/image.rb index 2c8db1a..d903e94 100644 --- a/src/lib/image.rb +++ b/src/lib/image.rb @@ -16,8 +16,14 @@
class Image # Given a ProviderAccount and an AMI/image ID on a provider, plus an optional XML string, use aeolus-image - # to import the image. Returns an Aeolus::Image::Factory::Image or returns any raised exceptions + # to import the image. Returns an Aeolus::Image::Factory::Image or allows exceptions to bubble up def self.import(provider_account, image_id, xml=nil) + # Verify that the image exists prior to import + conn = provider_account.connect + img = conn.image(image_id) rescue nil + raise Aeolus::Conductor::UI::ImageNotFound unless img.present? + # We have the image name in the cloud provider, so we might as well use it + xml ||= "<image><name>#{img.name}</name></image>" if img.name.present? provider = provider_account.provider account_id = provider_account.credentials_hash['username'] Aeolus::Image.import(provider.name, provider.provider_type.deltacloud_driver, image_id, account_id, xml) diff --git a/src/spec/controllers/api/images_controller_spec.rb b/src/spec/controllers/api/images_controller_spec.rb index 09481c8..d3fb24f 100644 --- a/src/spec/controllers/api/images_controller_spec.rb +++ b/src/spec/controllers/api/images_controller_spec.rb @@ -257,11 +257,15 @@ describe Api::ImagesController do # We previously stubbed this out to return nil... That's inappropriate here: Aeolus::Image::Warehouse::Image.stub(:find).and_return(@image) @provider_image.stub(:set_attr) + @deltacloud_connection = mock("Deltacloud::API") + @provider_account.stub(:connect).and_return(@deltacloud_connection) + @deltacloud_image = mock("Deltacloud::API::Stateful::Image", + :name => 'mock-image') + @deltacloud_connection.stub(:image).and_return(@deltacloud_image)
request.env['RAW_POST_DATA'] = xml.to_xml post :create end - it { response.response_code == 200 } it { response.headers['Content-Type'].should include("application/xml") } it "should have an image with correct attributes" do @@ -273,6 +277,41 @@ describe Api::ImagesController do
end
+ context "when trying to import image that does not exist" do + before(:each) do + xml = Nokogiri::XML::Builder.new do + image { + target_identifier "tid2" + image_descriptor { + child "c3" + child "c4" + } + provider_account_name "mock2" + } + end + Aeolus::Image::Factory::Image.stub(:new).and_return(@image) + ProviderAccount.stub(:find_by_label).and_return(@provider_account) + @provider_account.stub(:provider).and_return(@provider) + @image.stub(:save!) + # We previously stubbed this out to return nil... That's inappropriate here: + Aeolus::Image::Warehouse::Image.stub(:find).and_return(@image) + @provider_image.stub(:set_attr) + @deltacloud_connection = mock("Deltacloud::API") + @provider_account.stub(:connect).and_return(@deltacloud_connection) + @deltacloud_connection.stub(:image).and_raise("no image") + + request.env['RAW_POST_DATA'] = xml.to_xml + post :create + end + it { response.response_code == 404 } + it { response.headers['Content-Type'].should include("application/xml") } + it "should include an error" do + resp = Hash.from_xml(response.body) + resp['error']['message'].should == "Could not find Image tid2 on provider" + end + + end + end
describe "#destroy" do