[PATCH conductor 0/1] Add images from Factroy to API::ImagesController#index

[Petr Blaho]

https://www.aeolusproject.org/redmine/issues/2497

[PATCH conductor] Add images from Factory to API::ImagesController#index output

[Petr Blaho]

From: Petr Blaho <pblaho(a)redhat.com&gt;

https://www.aeolusproject.org/redmine/issues/2497
---
 src/app/controllers/api/images_controller.rb       |   27 ++++-
 src/app/views/api/images/_image.xml.haml           |    2 +-
 src/spec/controllers/api/images_controller_spec.rb |  116 +++++++++++++++++---
 3 files changed, 122 insertions(+), 23 deletions(-)

diff --git a/src/app/controllers/api/images_controller.rb
b/src/app/controllers/api/images_controller.rb
index 8f6957d..fddd11b 100644
--- a/src/app/controllers/api/images_controller.rb
+++ b/src/app/controllers/api/images_controller.rb
@@ -21,6 +21,7 @@
 
 module Api
   class ImagesController < ApplicationController
+
     before_filter :require_user_api
 
     respond_to :xml
@@ -28,6 +29,7 @@ module Api
 
     def index
       @images = Aeolus::Image::Warehouse::Image.all
+      @images += factory_images
       respond_with(@images)
     end
 
@@ -90,20 +92,33 @@ module Api
     end
 
     private
+
+    def factory_images
+
+      active_states = ['FAILED', 'COMPLETED']
+
+      factory_builds = Aeolus::Image::Factory::Builder.first.builders.select{|build|
!active_states.include?(build.status) && build.operation ==
'build'}.group_by(&:image_id)
+
+      factory_image_collection = factory_builds.map{|image_id, builds|
Aeolus::Image::Factory::Image.new(:id => image_id, :image_builds =>
builds.map{|build| {:id => build.build_id, :status => build.status}}, :name =>
nil, :description => nil, :os => Aeolus::Image::Warehouse::Image::OS.new)}
+
+      return [ factory_image_collection ].flatten
+
+    end
+
     def process_post(body)
       doc = Nokogiri::XML CGI.unescapeHTML(body)
       if !doc.xpath("/image/targets").empty? &&
!doc.xpath("/image/tdl/template").empty?
         { :type => :build, :params => { :template =>
doc.xpath("/image/tdl/template").to_s,
-                                        :targets =>
doc.xpath("/image/targets").text }
+          :targets => doc.xpath("/image/targets").text }
         }
       elsif !doc.xpath("/image/target_name").empty? &&
!doc.xpath("/image/target_identifier").empty? &&
-                 !doc.xpath("/image/image_descriptor").empty? &&
!doc.xpath("/image/provider_name").empty?
+        !doc.xpath("/image/image_descriptor").empty? &&
!doc.xpath("/image/provider_name").empty?
 
         { :type => :import, :params => { :target_name =>
doc.xpath("/image/target_name").text,
-                                         :targets =>
doc.xpath("/image/target_name").text,
-                                         :target_identifier =>
doc.xpath("/image/target_identifier").text,
-                                         :image_descriptor =>
doc.xpath("/image/image_descriptor").children.first.to_s,
-                                         :provider_name =>
doc.xpath("/image/provider_name").text }
+          :targets => doc.xpath("/image/target_name").text,
+          :target_identifier => doc.xpath("/image/target_identifier").text,
+          :image_descriptor =>
doc.xpath("/image/image_descriptor").children.first.to_s,
+          :provider_name => doc.xpath("/image/provider_name").text }
         }
       else
         { :type => :failed }
diff --git a/src/app/views/api/images/_image.xml.haml
b/src/app/views/api/images/_image.xml.haml
index f19b1e6..a1fa87f 100644
--- a/src/app/views/api/images/_image.xml.haml
+++ b/src/app/views/api/images/_image.xml.haml
@@ -7,4 +7,4 @@
   %arch= image.os.arch
   %builds{:type => 'xs:list'}
     - image.image_builds.each do |bld|
-      %build{:id => bld.id, :href => api_build_url(bld.id)}
+      %build{:id => bld.id, :href => api_build_url(bld.id), :status =>
bld.respond_to?(:status) ? bld.status : 'COMPLETED'}
diff --git a/src/spec/controllers/api/images_controller_spec.rb
b/src/spec/controllers/api/images_controller_spec.rb
index c1861d7..3e879b0 100644
--- a/src/spec/controllers/api/images_controller_spec.rb
+++ b/src/spec/controllers/api/images_controller_spec.rb
@@ -38,8 +38,14 @@ describe Api::ImagesController do
                     :image_builds => [@build],
                     :build => @build,
                     :provider_images => [mock(ProviderImage, :target_identifier =>
"ami-1234567", :provider => "provider")] * 2
-                    )
-      Aeolus::Image::Warehouse::ImageBuild.stub(:where).and_return([@build])
+                   )
+                   @factory_image = mock(Aeolus::Image::Factory::Image,
+                                         :id => '9',
+                                         :name => nil,
+                                         :description => nil,
+                                         :os => mock( :name => nil, :version =>
nil, :arch => nil ),
+                                         :image_builds => [ @build ] )
+                  
Aeolus::Image::Warehouse::ImageBuild.stub(:where).and_return([@build])
     end
 
     context "when authenticated as admin" do
@@ -57,19 +63,22 @@ describe Api::ImagesController do
           before(:each) do
             @image_collection = [@image, @image, @image]
             Aeolus::Image::Warehouse::Image.stub(:all).and_return(@image_collection)
+
+            @factory_image_collection = [ @factory_image, @factory_image ]
+            controller.stub(:factory_images).and_return(@factory_image_collection)
             get :index
           end
 
           it { response.should be_success }
           it { response.headers['Content-Type'].should
include("application/xml") }
-          it "should have 3 images" do
+          it "should have 5 images" do
             resp = Hash.from_xml(response.body)
-            resp['images']['image'].size.should
be_equal((a)image_collection.size)
+            resp['images']['image'].size.should be_equal( (
@image_collection + @factory_image_collection ).size )
           end
 
           it "should have images with correct attributes" do
             resp = Hash.from_xml(response.body)
-            @image_collection.each_with_index do |image, index|
+            ( @image_collection + @factory_image_collection ).each_with_index do |image,
index|
               resp['images']['image'][index]['name'].should ==
image.name
               resp['images']['image'][index]['id'].should ==
image.id
               resp['images']['image'][index]['os'].should ==
image.os.name
@@ -86,6 +95,7 @@ describe Api::ImagesController do
 
           before(:each) do
             Aeolus::Image::Warehouse::Image.stub(:all).and_return([@image])
+            controller.stub(:factory_images).and_return([])
             get :index
           end
 
@@ -108,6 +118,7 @@ describe Api::ImagesController do
           before(:each) do
             send_and_accept_xml
             Aeolus::Image::Warehouse::Image.stub(:all).and_return([])
+            controller.stub(:factory_images).and_return([])
             get :index
           end
 
@@ -225,7 +236,7 @@ describe Api::ImagesController do
             resp['image']['id'].should == @image.id
             resp['image']['build']['target_images'].should ==
"\n"
             resp['image']['build']['id'].should == @build.id
-         end
+          end
         end
 
         context "when trying to import image" do
@@ -255,7 +266,7 @@ describe Api::ImagesController do
             resp['image']['id'].should == @image.id
             resp['image']['build']['target_images'].should ==
"\n"
             resp['image']['build']['id'].should == @build.id
-         end
+          end
 
         end
 
@@ -306,15 +317,15 @@ describe Api::ImagesController do
 
         end
 
-#        context "when image is not found" do
-#          before(:each) do
-#            Aeolus::Image::Warehouse::Image.stub(:find).and_return(nil)
-#            delete :destroy, :id => @image.id
-#          end
-#
-#          it { response.status.should == 404}
-#          it { response.headers['Content-Type'].should
include("application/xml") }
-#        end
+        #        context "when image is not found" do
+        #          before(:each) do
+        #            Aeolus::Image::Warehouse::Image.stub(:find).and_return(nil)
+        #            delete :destroy, :id => @image.id
+        #          end
+        #
+        #          it { response.status.should == 404}
+        #          it { response.headers['Content-Type'].should
include("application/xml") }
+        #        end
 
 
       end
@@ -377,6 +388,7 @@ describe Api::ImagesController do
         end
         it { response.headers['Content-Type'].should
include("application/xml") }
       end
+
     end
   end
 
@@ -396,4 +408,76 @@ describe Api::ImagesController do
     it_behaves_like "Api::ImagesController responding with XML"
   end
 
+  describe '#factory_images' do
+    subject { controller.send(:factory_images) }
+    before(:each) do
+      @completed_builder = mock(Aeolus::Image::Factory::Builder,
+                                :image_id => '3',
+                                :build_id => '8',
+                                :_type => 'builder',
+                                :status => 'COMPLETED')
+      @building_builder = mock(Aeolus::Image::Factory::Builder,
+                               :image_id => '13',
+                               :build_id => '28',
+                               :operation => 'build',
+                               :status => 'BUILDING')
+      @building_builder_2 = mock(Aeolus::Image::Factory::Builder,
+                                 :image_id => '13',
+                                 :build_id => '42',
+                                 :operation => 'build',
+                                 :status => 'BUILDING')
+      @building_builder_3 = mock(Aeolus::Image::Factory::Builder,
+                                 :image_id => '17',
+                                 :build_id => '33',
+                                 :operation => 'build',
+                                 :status => 'BUILDING')
+      @failed_builder = mock(Aeolus::Image::Factory::Builder,
+                             :image_id => '11',
+                             :build_id => '12',
+                             :operation => 'build',
+                             :status => 'FAILED')
+      @pusher = mock(Aeolus::Image::Factory::Builder,
+                     :image_id => '19',
+                     :build_id => '15',
+                     :operation => 'push',
+                     :status => 'BUILDING')
+
+
+      @builders = [ @completed_builder, @building_builder, @building_builder_2,
@building_builder_3, @failed_builder, @pusher ]
+      @main_builder = mock(Aeolus::Image::Factory::Builder,
+                           :builders => @builders)
+      Aeolus::Image::Factory::Builder.stub(:first).and_return(@main_builder)
+
+    end
+
+    context "its return value" do
+      it "should contain building images from Factory" do
+        subject.select{|image| image.id == @building_builder.image_id}.count be_eq(1)
+        subject.select{|image| image.id == @building_builder_2.image_id}.count be_eq(1)
+        subject.select{|image| image.id == @building_builder_3.image_id}.count be_eq(1)
+      end
+
+      it "should not contain completed image" do
+        subject.select{|image| image.id == @completed_builder.image_id}.should be_empty
+      end
+
+      it "should not contain failed image" do
+        subject.select{|image| image.id ==
@failed_builder.image_id}.map(&:image_builds).select{|build| build.id ==
@failed_builder.build_id}.should be_empty
+      end
+
+      it "should not contain image with pusher type" do
+        subject.select{|image| image.id == @pusher.image_id}.should be_empty
+      end
+
+      it "should contain correct data for building images" do
+        subject.select{|image| image.id ==
@building_builder.image_id}.map(&:image_builds).flatten.select{|build| build.id ==
@building_builder.build_id}.size.should == 1
+        subject.select{|image| image.id ==
@building_builder_2.image_id}.map(&:image_builds).flatten.select{|build| build.id ==
@building_builder_2.build_id}.size.should == 1
+        subject.select{|image| image.id ==
@building_builder_3.image_id}.map(&:image_builds).flatten.select{|build| build.id ==
@building_builder_3.build_id}.size.should == 1
+      end
+
+    end
+
+  end
+
+
 end
-- 
1.7.6.4

[PATCH conductor] Add images from Factory to API::ImagesController#index output

[Matt Wagner]

On Thu, Dec 01, 2011 at 05:43:00PM +0100, pblaho(a)redhat.com wrote:
>  From: Petr Blaho <pblaho(a)redhat.com&gt;
>  
>  https://www.aeolusproject.org/redmine/issues/2497
>  ---
>   src/app/controllers/api/images_controller.rb       |   27 ++++-
>   src/app/views/api/images/_image.xml.haml           |    2 +-
>   src/spec/controllers/api/images_controller_spec.rb |  116 +++++++++++++++++---
>   3 files changed, 122 insertions(+), 23 deletions(-) 
I don't believe this is necessarily. It causes duplicates to show up for
me. I kicked off a RHEVM build (since EC2 and Mock complete essentially
instantly), and then tried this patch -- the new build showed up twice
in the list, with one occurrence in "BUILDING" and the other in
"COMPLETE" state. Same UUIDs for the image and the build both times.

After poking around and speaking with the Factory folks, it seems that
they create the image and build in Warehouse once Factory starts a
build, which would explain the duplication. (It was further explained
that the target image is created in Warehouse after the build completes,
and the provider image is created after the push finishes.)

-- Matt