From: Jan Provaznik <jprovazn(a)redhat.com>
https://www.aeolusproject.org/redmine/issues/3204
---
src/app/controllers/deployments_controller.rb | 94 ++++++++--------
src/app/models/deployment.rb | 120 +++++++++++---------
src/config/locales/en.yml | 1 +
.../controllers/deployments_controller_spec.rb | 1 +
src/spec/models/deployment_spec.rb | 25 ++--
5 files changed, 126 insertions(+), 115 deletions(-)
diff --git a/src/app/controllers/deployments_controller.rb
b/src/app/controllers/deployments_controller.rb
index 3cdc9c6..e982b85 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -116,63 +116,49 @@ class DeploymentsController < ApplicationController
end
def create
- launch_parameters_encoded = params.delete(:launch_parameters_encoded)
- unless launch_parameters_encoded.blank?
- decoded_parameters = JSON.load(Base64.decode64(launch_parameters_encoded))
- params[:deployment][:launch_parameters] = decoded_parameters
- end
@deployment = Deployment.new(params[:deployment])
@pool = @deployment.pool
require_privilege(Privilege::CREATE, Deployment, @pool)
+
+ if params[:launch_parameters_encoded].present?
+ @deployment.launch_parameters = JSON.load(
+ Base64.decode64(params[:launch_parameters_encoded]))
+ end
+
init_new_deployment_attrs
- @deployment.deployable_xml = @deployable.xml if @deployable &&
@deployable.xml
- if params.has_key?(:deployable_id)
- require_privilege(Privilege::USE, @deployable)
+ require_privilege(Privilege::USE, @deployable)
+ @deployment.deployable_xml = @deployable.xml
+ @deployment.owner = current_user
+
+ if params.delete(:commit) == 'back'
+ load_assemblies_services
+ view = @deployment.launch_parameters.blank? ?
+ 'launch_new' : 'launch_time_params'
+ render view
+ return
end
- img, img2, missing, d_errors = @deployable.get_image_details
- respond_to do |format|
- unless d_errors.empty?
- @pool = @deployment.pool
- flash.now[:warning] = t "deployments.flash.warning.failed_to_launch"
- flash[:error] = d_errors
- format.html { render :action => 'overview' }
- format.js { launch_new }
- format.json { render :json => d_errors, :status => :unprocessable_entity }
- else
+ return unless check_deployable_images
- @deployment.owner = current_user
- load_assemblies_services
- if params.delete(:commit) == 'back'
- view = launch_parameters_encoded.blank? ? 'launch_new' :
'launch_time_params'
- render view and return
+ respond_to do |format|
+ if @deployment.create_and_launch(current_user)
+ format.html do
+ flash[:notice] = t "deployments.flash.notice.launched"
+ redirect_to deployment_path(@deployment)
end
-
- if @deployment.save
- status = @deployment.launch(current_user)
- if status[:errors].empty?
- flash[:notice] = t "deployments.flash.notice.launched"
- else
- flash[:error] = {
- :summary =>
t("deployments.flash.error.failed_to_launch_assemblies"),
- :failures => status[:errors],
- :successes => status[:successes]
- }
- end
- format.html { redirect_to deployment_path(@deployment) }
- format.js do
- @deployment_properties = @deployment.properties
- render :partial => 'properties'
- end
- format.json { render :json => @deployment, :status => :created }
- else
- # We need @pool to re-display the form
- @pool = @deployment.pool
+ format.js do
+ @deployment_properties = @deployment.properties
+ render :partial => 'properties'
+ end
+ format.json { render :json => @deployment, :status => :created }
+ else
+ # TODO: put deployment's errors into flash or display inside page?
+ format.html do
flash.now[:warning] = t "deployments.flash.warning.failed_to_launch"
- init_new_deployment_attrs
- format.html { render :action => 'overview' }
- format.js { launch_new }
- format.json { render :json => @deployment.errors, :status =>
:unprocessable_entity }
+ render :action => 'overview'
end
+ format.js { render :partial => 'overview' }
+ format.json { render :json => @deployment.errors,
+ :status => :unprocessable_entity }
end
end
end
@@ -433,4 +419,16 @@ class DeploymentsController < ApplicationController
end
end
+ def check_deployable_images
+ image_details, images, missing_images, deployable_errors =
@deployable.get_image_details
+ return true if deployable_errors.empty?
+ respond_to do |format|
+ flash.now[:warning] = t "deployments.flash.warning.failed_to_launch"
+ flash[:error] = deployable_errors
+ format.html { render :action => 'overview' }
+ format.js { render :partial => 'overview' }
+ format.json { render :json => deployable_errors, :status =>
:unprocessable_entity }
+ end
+ false
+ end
end
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 4ad2082..d7956ab 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -206,26 +206,33 @@ class Deployment < ActiveRecord::Base
@launch_parameters = launch_parameters
end
- def launch(user)
- # first, create the instance and instance_parameter objects
- # then, find a single provider account where all instances can launch
- # then, if a config server is being used
- # then generate the instance configs for each instance,
- # finally, launch the instances
- #
- # TODO: need to be able to handle deployable-level errors
- #
+ def create_and_launch(user)
+ begin
+ rollback_active_record_state! do
+ # deployment's attrs are not reset on rollback,
+ # rollback_active_record_state! takes care of this
+ transaction do
+ create_instances_with_params!(user)
+ launch!(user)
+ end
+ end
+ return true
+ rescue
+ errors.add(:base, $!.message)
+ logger.error $!.message
+ logger.error $!.backtrace.join("\n ")
+ end
+ false
+ end
+
+ def launch!(user)
self.state = STATE_PENDING
save!
- status = { :errors => {}, :successes => {} }
- status[:errors] = create_instances_with_params(user)
- all_inst_match, account, errors = find_match_with_common_account
+ all_inst_match, account, errs = find_match_with_common_account
unless all_inst_match
- status[:errors][name] = errors
- # set CREATE_FAILED for all newly created instances
- instances.each {|i| i.update_attribute(:state, Instance::STATE_CREATE_FAILED)}
- return status
+ raise I18n.t('deployments.errors.match_not_found',
+ :errors => errs.join(", "))
end
if deployable_xml.requires_config_server?
@@ -233,24 +240,34 @@ class Deployment < ActiveRecord::Base
# instances (and not each individual instance) in order to do parameter
# dependency resolution across the set
config_server = account.config_server
- instance_configs = ConfigServerUtil.instance_configs(self, instances,
config_server)
+ instance_configs = ConfigServerUtil.instance_configs(self,
+ instances,
+ config_server)
else
config_server = nil
instance_configs = {}
end
+ # TODO: in follow up patch there should be only delayed job call
+ # on this place
+ # For now, we just call DC create intance method on foreground, if an error
+ # occurs (for example DC API is not running, or config server is not
+ # running), then CREATE_FAILED is set in instance.launch!
instances.each do |instance|
match = all_inst_match.find{|m| m.instance.id == instance.id}
begin
- instance.launch!(match, user, config_server, instance_configs[instance.uuid])
- status[:successes][instance.name] = 'launched'
+ instance.launch!(match,
+ user,
+ config_server,
+ instance_configs[instance.uuid])
rescue
- status[:errors][instance.name] = $!.message.to_s.split("\n").first
+ errors.add(:base,
+ "#{instance.name}:
#{$!.message.to_s.split("\n").first}")
logger.error $!.message
logger.error $!.backtrace.join("\n ")
end
end
- status
+ true
end
def self.list(order_field, order_dir)
@@ -535,47 +552,40 @@ class Deployment < ActiveRecord::Base
self[:pool_family_id] = pool.pool_family_id
end
- def create_instances_with_params(user)
+ def create_instances_with_params!(user)
errors = {}
deployable_xml.assemblies.each do |assembly|
- begin
- hw_profile = permissioned_frontend_hwprofile(user, assembly.hwp)
- raise I18n.t('deployments.flash.error.no_hwp_permission', :hwp =>
assembly.hwp) unless hw_profile
- Instance.transaction do
- instance = Instance.create!(
- :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
- )
- assembly.services.each do |service|
- service.parameters.each do |parameter|
- if not parameter.reference?
- param = InstanceParameter.create!(
- :instance => instance,
- :service => service.name,
- :name => parameter.name,
- :type => parameter.type,
- :value => parameter.value
- )
- end
+ hw_profile = permissioned_frontend_hwprofile(user, assembly.hwp)
+ raise I18n.t('deployments.flash.error.no_hwp_permission', :hwp =>
assembly.hwp) unless hw_profile
+ Instance.transaction do
+ instance = Instance.create!(
+ :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
+ )
+ assembly.services.each do |service|
+ service.parameters.each do |parameter|
+ if not parameter.reference?
+ param = InstanceParameter.create!(
+ :instance => instance,
+ :service => service.name,
+ :name => parameter.name,
+ :type => parameter.type,
+ :value => parameter.value
+ )
end
end
- self.instances << instance
end
- rescue
- logger.error $!.message
- logger.error $!.backtrace.join("\n ")
- errors[assembly.name] = $!.message.to_s.split("\n").first
+ self.instances << instance
end
end
- errors
end
def set_new_state
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index beb8505..793f448 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -308,6 +308,7 @@ en:
cannot_stop: "stop cannot be performed on this instance."
all_stopped: "All instances must be stopped or running"
not_valid_deployable_xml: "seems to be not valid deployable XML: %{msg}"
+ match_not_found: "Match not found: %{errors}"
preset_filters:
all_deployments: All Deployments
instances:
diff --git a/src/spec/controllers/deployments_controller_spec.rb
b/src/spec/controllers/deployments_controller_spec.rb
index 54b1477..69057ba 100644
--- a/src/spec/controllers/deployments_controller_spec.rb
+++ b/src/spec/controllers/deployments_controller_spec.rb
@@ -65,6 +65,7 @@ describe DeploymentsController do
@catalog = Factory.create(:catalog)
@deployable = Factory.create(:deployable, :catalogs => [@catalog])
Deployment.stub!(:new).and_return(@deployment)
+ Deployment.any_instance.stub(:create_and_launch).and_return(true)
post :create, :deployable_id => @deployable.id
end
diff --git a/src/spec/models/deployment_spec.rb b/src/spec/models/deployment_spec.rb
index f382eda..4b188e0 100644
--- a/src/spec/models/deployment_spec.rb
+++ b/src/spec/models/deployment_spec.rb
@@ -222,11 +222,11 @@ describe Deployment do
end
it "should launch instances when launching deployment" do
- @deployment.save!
@deployment.instances.should be_empty
Taskomatic.stub!(:create_instance!).and_return(true)
- @deployment.launch(@user_for_launch)[:errors].should be_empty
+ @deployment.create_and_launch(@user_for_launch)
+ @deployment.errors.should be_empty
@deployment.instances.count.should == 2
end
@@ -238,7 +238,8 @@ describe Deployment do
Taskomatic.stub!(:create_dcloud_instance).and_return(true)
Taskomatic.stub!(:handle_dcloud_error).and_return(true)
Taskomatic.stub!(:handle_instance_state).and_return(true)
- @deployment.launch(@user_for_launch)[:errors].should be_empty
+ @deployment.create_and_launch(@user_for_launch)
+ @deployment.errors.should be_empty
@deployment.reload
@deployment.instances.count.should == 2
@deployment.instances[0].provider_account.should == @provider_account1
@@ -246,7 +247,8 @@ describe Deployment do
@provider_account1.priority = 30
@provider_account1.save!
deployment2 = Factory.create(:deployment, :pool_id => @pool.id)
- deployment2.launch(@user_for_launch)[:errors].should be_empty
+ deployment2.create_and_launch(@user_for_launch)
+ deployment2.errors.should be_empty
deployment2.reload
deployment2.instances.count.should == 2
deployment2.instances[0].provider_account.should == @provider_account2
@@ -264,28 +266,27 @@ describe Deployment do
Taskomatic.stub!(:create_dcloud_instance).and_return(true)
Taskomatic.stub!(:handle_dcloud_error).and_return(true)
Taskomatic.stub!(:handle_instance_state).and_return(true)
- @deployment.launch(@user_for_launch)[:errors].should be_empty
+ @deployment.create_and_launch(@user_for_launch)
+ @deployment.errors.should be_empty
@deployment.reload
@deployment.instances.count.should == 2
@deployment.instances[0].provider_account.should == @provider_account2
@deployment.instances[1].provider_account.should == @provider_account2
end
- it "should set create_failed status for instances if match not found" do
- @deployment.save!
+ it "should not create deployment with instances if match not found" do
@deployment.instances.should be_empty
@deployment.pool.pool_family.provider_accounts.destroy_all
Taskomatic.stub!(:create_instance!).and_return(true)
- @deployment.launch(@user_for_launch)
- @deployment.instances.should_not be_empty
- @deployment.instances.each {|i| i.state.should == Instance::STATE_CREATE_FAILED}
+ @deployment.create_and_launch(@user_for_launch)
+ @deployment.errors.should_not be_empty
+ lambda { Deployment.find((a)deployment.id) }.should
raise_error(ActiveRecord::RecordNotFound)
end
it "should set create_failed status for instances if instance's launch
raises an exception" do
- @deployment.save!
@deployment.instances.should be_empty
Taskomatic.stub!(:create_dcloud_instance).and_raise("an exception")
- @deployment.launch(@user_for_launch)
+ @deployment.create_and_launch(@user_for_launch)
@deployment.reload
@deployment.instances.should_not be_empty
@deployment.instances.each {|i| i.state.should == Instance::STATE_CREATE_FAILED}
--
1.7.7.6