From: Greg Blomquist <gblomqui(a)redhat.com>
Conductor can now launch a deployable with multiple images in a single
deployable. Previously conductor was only able to launch deployables with the
same image because of a bug in the instance <-> provider account matching logic.
see
https://bugzilla.redhat.com/show_bug.cgi?id=767759 for more details.
---
src/app/models/deployment.rb | 104 +++++++++++++++-----
src/app/models/instance.rb | 27 -----
src/app/models/instance_observer.rb | 2 +-
src/app/util/taskomatic.rb | 13 +---
.../controllers/deployments_controller_spec.rb | 4 +-
5 files changed, 82 insertions(+), 68 deletions(-)
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 1a799b8..8c10913 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -173,26 +173,23 @@ class Deployment < ActiveRecord::Base
@launch_parameters = launch_parameters
end
- def launch(user, config_values = nil)
- # first, create all the instance objects,
- # if a config server is being used
- # find if there is at least a single account where they can all launch,
- # then generate the instance configs for the instances,
- # if a using a config server
- # then, for each instance send the instance configs to the config server,
- # and launch the instances
+ 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,
+ # then send the instance configs to the config server,
+ # finally, launch the instances
#
# TODO: need to be able to handle deployable-level errors
#
status = { :errors => {}, :successes => {} }
assembly_instances = {}
deployable_xml.assemblies.each do |assembly|
- # TODO: for now we try to start all instances even if some of them fails
begin
- task = nil
+ hw_profile = permissioned_frontend_hwprofile(user, assembly.hwp)
+ raise "Hardware Profile #{assembly.hwp} not found." unless hw_profile
Instance.transaction do
- hw_profile = permissioned_frontend_hwprofile(user, assembly.hwp)
- raise "Hardware Profile #{assembly.hwp} not found." unless
hw_profile
instance = Instance.create!(
:deployment => self,
:name => "#{name}/#{assembly.name}",
@@ -205,7 +202,6 @@ class Deployment < ActiveRecord::Base
:owner => user,
:hardware_profile => hw_profile
)
- # store the assembly's parameters
assembly.services.each do |service|
service.parameters.each do |parameter|
if not parameter.reference?
@@ -219,7 +215,6 @@ class Deployment < ActiveRecord::Base
end
end
end
-
assembly_instances[assembly.name] = instance
end
rescue
@@ -228,21 +223,25 @@ class Deployment < ActiveRecord::Base
status[:errors][assembly.name] = $!.message
end
end
- # figure out which config server to use
- # and, generate the instance configurations for the instances
+
+ account_matches, errors = common_provider_accounts_for(assembly_instances.values)
+ if account_matches.empty?
+ status[:errors][name] = errors
+ return status
+ end
+ account = account_matches.keys.first
+ instances_matches = account_matches[account]
+
if deployable_xml.requires_config_server?
- matches, errors = Instance.matches(assembly_instances.values)
- if matches.empty?
- #TODO:need to have a way to show the errors in a meaningful way
- status[:errors][name] = errors
- return status
- end
- found = matches.first
- config_server = found.provider_account.config_server
+ # the instance configurations need to be generated from the entire set of
+ # 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,
assembly_instances.values, config_server)
end
- # now actually do the launch
+
assembly_instances.each do |assembly_name, instance|
+ match = instances_matches[instance]
config = instance_configs[instance.uuid] if deployable_xml.requires_config_server?
begin
if deployable_xml.requires_config_server?
@@ -250,7 +249,7 @@ class Deployment < ActiveRecord::Base
instance.instance_config_xml = config.to_s
instance.save!
end
-
+ # create a taskomatic task
task = InstanceTask.create!({:user => user,
:task_target => instance,
:action => InstanceTask::ACTION_CREATE})
@@ -261,7 +260,7 @@ class Deployment < ActiveRecord::Base
raise I18n.t 'deployments.errors.config_server_connection'
end
end
- Taskomatic.create_instance(task)
+ Taskomatic.create_instance(task, match)
if task.state == Task::STATE_FAILED
status[:errors][assembly_name] = 'failed'
else
@@ -468,6 +467,57 @@ class Deployment < ActiveRecord::Base
end
end
+ # Find the provider accounts where all the given instances can be launched
+ # Returns a tuple of the form:
+ # [{account => {instance => match, instance => match},
+ # account => {instance => match, instance => match}},
+ # errors]
+ def common_provider_accounts_for(instances)
+ matches = nil
+ errors = []
+ instance_matches = {}
+ instances.each do |instance|
+ m, e = instance.matches
+ instance_matches[instance] = m if m and not m.empty?
+ errors << e
+ end
+ # this series of map-reductions takes the input:
+ # instance_matches
+ # {i1 => [m1, m2, m3], i2 => [m4, m5, m6]}
+ # and translates it into the form:
+ # {a1 => {i1 => m1, i2 => m4},
+ # a2 => {i1 => m2, i2 => m5},
+ # a3 => {i1 => m3, i2 => m6}}
+ # where iN is an instance, mN is a match, and aN is an account
+ account_matches = instance_matches.map do |instance, matches|
+ matches.map do |match|
+ {match.provider_account => {instance => match}}
+ end.reduce :merge
+ end.reduce do |account_map, instance_map|
+ account_map.merge!(instance_map) do |key, v1, v2|
+ v1.merge(v2)
+ end
+ end || []
+
+ # reject any accounts that cannot launch the entire deployment
+ account_matches.reject! do |account, instance_map|
+ # implies that one or more instance does not have a corresponding image
+ # pushed to this account
+ rejected = instance_map.size < instances.size
+
+ # also check that the account's quota can handle all the instances
+ if not rejected
+ if not account.quota.can_start? instances
+ errors <<
I18n.t('instances.errors.provider_account_quota_too_low', :match_provider_account
=> account)
+ rejected = true
+ end
+ end
+ rejected
+ end if not account_matches.empty?
+
+ [account_matches, errors]
+ end
+
def generate_uuid
self[:uuid] = UUIDTools::UUID.timestamp_create.to_s
end
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index fec47d1..d0e0f99 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -431,33 +431,6 @@ class Instance < ActiveRecord::Base
not deployment.instances.deployed.any? {|i| i != self}
end
- # find the list of possibles that will accommodate all of the instances
- def self.matches(instances)
- matches = nil
- errors = []
- instances.each do |instance|
- m, e = instance.matches
- if matches.nil?
- matches = m.dup
- else
- matches.delete_if {|match| not m.include?(match) }
- end
- errors << e
- end
- # For now, this only checks the account's quota to see whether all the
- # instances can launch there
- # TODO: Determine if there's more to check here
- matches.reject! do |match|
- rejected = false
- if !match.provider_account.quota.can_start? instances
- errors << I18n.t('instances.errors.provider_account_quota_too_low',
:match_provider_account => match.provider_account)
- rejected = true
- end
- rejected
- end
- [matches, errors]
- end
-
def stop
do_operation('stop')
end
diff --git a/src/app/models/instance_observer.rb b/src/app/models/instance_observer.rb
index 9d13fb0..c609538 100644
--- a/src/app/models/instance_observer.rb
+++ b/src/app/models/instance_observer.rb
@@ -57,7 +57,7 @@ class InstanceObserver < ActiveRecord::Observer
[provider_account, pool_family, pool, user].each do |parent|
if parent
# Since pool and pool_family are related, updating one can cause the other to
become stale
- parent.reload if parent.class == Pool
+ parent.reload if [Pool, ProviderAccount].include? parent.class
quota = parent.quota
if quota
if !RUNNING_STATES.include?(state_from) &&
RUNNING_STATES.include?(state_to)
diff --git a/src/app/util/taskomatic.rb b/src/app/util/taskomatic.rb
index 9750873..84c31d3 100644
--- a/src/app/util/taskomatic.rb
+++ b/src/app/util/taskomatic.rb
@@ -16,10 +16,8 @@
module Taskomatic
- def self.create_instance(task)
+ def self.create_instance(task, match)
begin
- match = matches(task.instance).first
-
task.state = Task::STATE_PENDING
task.save!
@@ -158,13 +156,4 @@ module Taskomatic
client_args.merge!({:user_data => instance.user_data}) if
instance.user_data.present?
client.create_instance(client_args)
end
-
- def self.matches(instance)
- matched, errors = instance.matches
- if matched.empty?
- raise "Could not find a matching backend provider, errors:
#{errors.join(', ')}"
- end
- matched
- end
-
end
diff --git a/src/spec/controllers/deployments_controller_spec.rb
b/src/spec/controllers/deployments_controller_spec.rb
index 4b1c4df..9e1f2b7 100644
--- a/src/spec/controllers/deployments_controller_spec.rb
+++ b/src/spec/controllers/deployments_controller_spec.rb
@@ -59,13 +59,15 @@ describe DeploymentsController do
describe "#create" do
before do
+ Factory.create(:front_hwp1)
+ Factory.create(:front_hwp2)
@deployment = Factory.build(:deployment)
Deployment.stub!(:new).and_return(@deployment)
post :create
end
it { response.should be_success }
- it { ActiveSupport::JSON.decode(response.body)["name"].should ==
@deployment.name }
+ it { json = ActiveSupport::JSON.decode(response.body)["name"].should ==
@deployment.name }
end
describe "#destroy" do
--
1.7.7.4