[PATCH conductor] BZ 719383 - check resources before launching new deployment

[Jan Provazník]

From: Jan Provaznik

Now we try to find a launch match for each assembly in a deployment
before launch - errors are displayed to user on 'new' (deployment
overview) page
---
 src/app/controllers/deployments_controller.rb   |    7 +++++
 src/app/models/deployment.rb                    |   31 +++++++++++++++++++++++
 src/features/deployment.feature                 |   14 ++++++++++
 src/spec/factories/hardware_profile_property.rb |    8 +++---
 src/spec/models/deployment_spec.rb              |   11 ++++++++
 5 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/src/app/controllers/deployments_controller.rb b/src/app/controllers/deployments_controller.rb
index 1bdd12c..f11885c 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -46,6 +46,13 @@ class DeploymentsController < ApplicationController
     url = get_deployable_url
     respond_to do |format|
       if @deployment.accessible_and_valid_deployable_xml?(url)
+        errors = @deployment.check_assemblies_matches(current_user)
+        unless errors.empty?
+          flash[:error] = {
+            :summary => "It's not possible to launch some assemblies:",
+            :failures => errors
+          }
+        end
         format.js { render :partial => 'new' }
         format.html
         format.json { render :json => @deployment }
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 9a92f93..49b440a 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -183,4 +183,35 @@ class Deployment < ActiveRecord::Base
     # I REALLY want to get this via a join, but no dice...
     instances.first.provider_account.provider rescue nil
   end
+
+  # we try to create an instance for each assembly and check
+  # if a match is found
+  def check_assemblies_matches(user)
+    errs = {}
+    deployable_xml.assemblies.each do |assembly|
+      begin
+        hw_profile = HardwareProfile.frontend.find_by_name(assembly.hwp)
+        raise "Hardware Profile #{assembly.hwp} not found." unless hw_profile
+        instance = Instance.new(
+          :deployment => self,
+          :name => "#{name}/#{assembly.name}",
+          :frontend_realm => frontend_realm,
+          :pool => pool,
+          :image_uuid => assembly.image_id,
+          :image_build_uuid => assembly.image_build,
+          :assembly_xml => assembly.to_s,
+          :state => Instance::STATE_NEW,
+          :owner => user,
+          :hardware_profile => hw_profile
+        )
+        possibles, errors = instance.matches
+        if possibles.empty? and not errors.empty?
+          raise errors.join(", ")
+        end
+      rescue
+        errs[assembly.name] = $!.message
+      end
+    end
+    errs
+  end
 end
diff --git a/src/features/deployment.feature b/src/features/deployment.feature
index eda5123..2e7f368 100644
--- a/src/features/deployment.feature
+++ b/src/features/deployment.feature
@@ -182,3 +182,17 @@ Feature: Manage Deployments
     When I follow "testdeployment"
     And I press "Delete"
     Then I should see "The deployment testdeployment was successfully deleted"
+
+  Scenario: Launch a deployment which is not launchable
+    Given a pool "mockpool" exists
+    And there is "front_hwp1" conductor hardware profile
+    When I am viewing the pool "mockpool"
+    And I follow "New Deployment"
+    Then I should see "New Deployment"
+    When I fill in "deployable_url" with "http://localhost/deployables/deployable1.xml"
+    When I fill in "deployment_name" with "mynewdeployment"
+    When I press "Next"
+    Then I should see "Deployable details"
+    And I should see "It's not possible to launch some assemblies:"
+    And I should see "backend: Hardware Profile front_hwp2 not found."
+
diff --git a/src/spec/factories/hardware_profile_property.rb b/src/spec/factories/hardware_profile_property.rb
index c752d8e..67e2a3e 100644
--- a/src/spec/factories/hardware_profile_property.rb
+++ b/src/spec/factories/hardware_profile_property.rb
@@ -26,7 +26,7 @@ Factory.define :mock_hwp1_arch, :parent => :hardware_profile_property do |p|
   p.name 'architecture'
   p.kind 'fixed'
   p.unit 'label'
-  p.value 'i386'
+  p.value 'x86_64'
 end

Factory.define :mock_hwp2_memory, :parent => :hardware_profile_property do |p|
@@ -146,7 +146,7 @@ Factory.define :ec2_hwp1_arch, :parent => :hardware_profile_property do |p|
   p.name 'architecture'
   p.kind 'fixed'
   p.unit 'label'
-  p.value 'i386'
+  p.value 'x86_64'
 end

Factory.define :front_hwp3_memory, :parent => :hardware_profile_property do |p|
@@ -174,7 +174,7 @@ Factory.define :front_hwp3_arch, :parent => :hardware_profile_property do |p|
   p.name 'architecture'
   p.kind 'fixed'
   p.unit 'label'
-  p.value 'i386'
+  p.value 'x86_64'
 end

Factory.define :hwpp_range, :parent => :hardware_profile_property do |p|
@@ -204,5 +204,5 @@ Factory.define :hwpp_arch, :parent => :hardware_profile_property do |p|
   p.name 'architecture'
   p.kind 'fixed'
   p.unit 'label'
-  p.value 'i386'
+  p.value 'x86_64'
 end
diff --git a/src/spec/models/deployment_spec.rb b/src/spec/models/deployment_spec.rb
index 9482896..e6c423c 100644
--- a/src/spec/models/deployment_spec.rb
+++ b/src/spec/models/deployment_spec.rb
@@ -94,4 +94,15 @@ describe Deployment do
     @deployment.should be_destroyable
     expect { @deployment.destroy }.to change(Deployment, :count).by(-1)
   end
+
+  it "should return errors when checking assemblies matches which are not launchable" do
+    user = Factory(:user)
+    image_id = @deployment.deployable_xml.assemblies.first.image_id
+    provider_name = Image.find(image_id).latest_build.provider_images.first.provider_name
+    provider = Factory(:mock_provider, :name => provider_name)
+    @deployment.pool.pool_family.provider_accounts = [Factory(:mock_provider_account, :label => 'testaccount', :provider => provider)]
+    @deployment.check_assemblies_matches(user).should be_empty
+    @deployment.pool.pool_family.provider_accounts.destroy_all
+    @deployment.check_assemblies_matches(user).should_not be_empty
+  end
 end

[Tomas Sedovic]

On 07/18/2011 02:15 PM, [e-mail] wrote:
> From: Jan Provaznik
>
> Now we try to find a launch match for each assembly in a deployment
> before launch - errors are displayed to user on 'new' (deployment
> overview) page
>
> src/app/controllers/deployments_controller.rb   |    7 +++++
>   src/app/models/deployment.rb                    |   31 +++++++++++++++++++++++
>   src/features/deployment.feature                 |   14 ++++++++++
>   src/spec/factories/hardware_profile_property.rb |    8 +++---
>   src/spec/models/deployment_spec.rb              |   11 ++++++++
>   5 files changed, 67 insertions(+), 4 deletions(-)
>
> diff --git a/src/app/controllers/deployments_controller.rb b/src/app/controllers/deployments_controller.rb
> index 1bdd12c..f11885c 100644
> --- a/src/app/controllers/deployments_controller.rb
> +++ b/src/app/controllers/deployments_controller.rb
> @@ -46,6 +46,13 @@ class DeploymentsController<  ApplicationController
>       url = get_deployable_url
>       respond_to do |format|
>         if @deployment.accessible_and_valid_deployable_xml?(url)
>    errors = @deployment.check_assemblies_matches(current_user)
>    unless errors.empty?
>      flash[:error] = {
>        :summary =>  "It's not possible to launch some assemblies:",

To me, this message says that we've already tried to launch the 
deployment and it failed. What we want to say here is that when we try 
to launch the deployment, some of the assemblies *will* fail.

Would something along these lines work better?

* "It will not be possible to launch all assemblies"
* "Some assemblies will not be launched"
>        :failures =>  errors
>      }
>    end
>     format.js { render :partial =>  'new' }
>     format.html
>     format.json { render :json =>  @deployment }
>
> diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
> index 9a92f93..49b440a 100644
> --- a/src/app/models/deployment.rb
> +++ b/src/app/models/deployment.rb
> @@ -183,4 +183,35 @@ class Deployment<  ActiveRecord::Base
>       # I REALLY want to get this via a join, but no dice...
>       instances.first.provider_account.provider rescue nil
>     end
> # we try to create an instance for each assembly and check
> # if a match is found
> def check_assemblies_matches(user)
> errs = {}
> deployable_xml.assemblies.each do |assembly|
>  begin
>    hw_profile = HardwareProfile.frontend.find_by_name(assembly.hwp)
>    raise "Hardware Profile #{assembly.hwp} not found." unless hw_profile
>    instance = Instance.new(
>      :deployment =>  self,
>      :name =>  "#{name}/#{assembly.name}",
>      :frontend_realm =>  frontend_realm,
>      :pool =>  pool,
>      :image_uuid =>  assembly.image_id,
>      :image_build_uuid =>  assembly.image_build,
>      :assembly_xml =>  assembly.to_s,
>      :state =>  Instance::STATE_NEW,
>      :owner =>  user,
>      :hardware_profile =>  hw_profile
>    )
>    possibles, errors = instance.matches
>    if possibles.empty? and not errors.empty?
>      raise errors.join(", ")
>    end
>  rescue
>    errs[assembly.name] = $!.message
>  end
> end
> errs
> end
> end
>
> diff --git a/src/features/deployment.feature b/src/features/deployment.feature
> index eda5123..2e7f368 100644
> --- a/src/features/deployment.feature
> +++ b/src/features/deployment.feature
> @@ -182,3 +182,17 @@ Feature: Manage Deployments
>       When I follow "testdeployment"
>       And I press "Delete"
>       Then I should see "The deployment testdeployment was successfully deleted"
> Scenario: Launch a deployment which is not launchable
> Given a pool "mockpool" exists
> And there is "front_hwp1" conductor hardware profile
> When I am viewing the pool "mockpool"
> And I follow "New Deployment"
> Then I should see "New Deployment"
> When I fill in "deployable_url" with "http://localhost/deployables/deployable1.xml"
> When I fill in "deployment_name" with "mynewdeployment"
> When I press "Next"
> Then I should see "Deployable details"
> And I should see "It's not possible to launch some assemblies:"

If we do change the message, it should be reflected here as well: ^
> And I should see "backend: Hardware Profile front_hwp2 not found."

Trailing whitespace there ^
> diff --git a/src/spec/factories/hardware_profile_property.rb b/src/spec/factories/hardware_profile_property.rb
> index c752d8e..67e2a3e 100644
> --- a/src/spec/factories/hardware_profile_property.rb
> +++ b/src/spec/factories/hardware_profile_property.rb
> @@ -26,7 +26,7 @@ Factory.define :mock_hwp1_arch, :parent =>  :hardware_profile_property do |p|
>     p.name 'architecture'
>     p.kind 'fixed'
>     p.unit 'label'
> p.value 'i386'
>
> p.value 'x86_64'
> end
>
> Factory.define :mock_hwp2_memory, :parent =>  :hardware_profile_property do |p|
>
> @@ -146,7 +146,7 @@ Factory.define :ec2_hwp1_arch, :parent =>  :hardware_profile_property do |p|
>     p.name 'architecture'
>     p.kind 'fixed'
>     p.unit 'label'
> p.value 'i386'
>
> p.value 'x86_64'
> end
>
> Factory.define :front_hwp3_memory, :parent =>  :hardware_profile_property do |p|
>
> @@ -174,7 +174,7 @@ Factory.define :front_hwp3_arch, :parent =>  :hardware_profile_property do |p|
>     p.name 'architecture'
>     p.kind 'fixed'
>     p.unit 'label'
> p.value 'i386'
>
> p.value 'x86_64'
> end
>
> Factory.define :hwpp_range, :parent =>  :hardware_profile_property do |p|
>
> @@ -204,5 +204,5 @@ Factory.define :hwpp_arch, :parent =>  :hardware_profile_property do |p|
>     p.name 'architecture'
>     p.kind 'fixed'
>     p.unit 'label'
> p.value 'i386'
> p.value 'x86_64'
> end
>
> diff --git a/src/spec/models/deployment_spec.rb b/src/spec/models/deployment_spec.rb
> index 9482896..e6c423c 100644
> --- a/src/spec/models/deployment_spec.rb
> +++ b/src/spec/models/deployment_spec.rb
> @@ -94,4 +94,15 @@ describe Deployment do
>       @deployment.should be_destroyable
>       expect { @deployment.destroy }.to change(Deployment, :count).by(-1)
>     end
> it "should return errors when checking assemblies matches which are not launchable" do
> user = Factory(:user)
> image_id = @deployment.deployable_xml.assemblies.first.image_id
> provider_name = Image.find(image_id).latest_build.provider_images.first.provider_name
> provider = Factory(:mock_provider, :name =>  provider_name)
> @deployment.pool.pool_family.provider_accounts = [Factory(:mock_provider_account, :label =>  'testaccount', :provider =>  provider)]
> @deployment.check_assemblies_matches(user).should be_empty
> @deployment.pool.pool_family.provider_accounts.destroy_all
> @deployment.check_assemblies_matches(user).should_not be_empty
> end
> end

ACK, it works. I have a few nits, though. Please see them inline

Thomas