On 07/18/2011 02:15 PM, jprovazn(a)redhat.com wrote:
From: Jan Provaznik<jprovazn(a)redhat.com>
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
--
No trees were killed to send this message, but a large number of
electrons were terribly inconvenienced.