openstack
/
fuel-astute
Code Issues Proposed changes

task_deploy: no implicit conversion of String into Integer 

Now Astute will not calculate fault tolerance groups and
critical node uids twice.

Change-Id: I3bf2dd0ffc0fc74fd9c670bd50b32e3285ae7e2a
Closes-Bug: #1669499
This commit is contained in:
Vladimir Sharshov (warpc) 2017-03-02 19:16:31 +03:00
parent 48ee1f7467
commit 892894bfb5
2 changed files with 54 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
lib/astute/task_deployment.rb
View File

@ -25,6 +25,7 @@ module Astute
'failed' => {'status' => 'error', 'error_type' => 'deploy'}
}
attr_reader :ctx, :cluster_class, :node_class
def initialize(context, cluster_class=TaskCluster, node_class=TaskNode)
@ctx = context
@cluster_class = cluster_class
@ -95,7 +96,7 @@ module Astute
support_virtual_node(tasks_graph)
unzip_graph(tasks_graph, tasks_directory)
cluster = @cluster_class.new
cluster = cluster_class.new
cluster.node_concurrency.maximum = Astute.config.max_nodes_per_call
cluster.stop_condition { Thread.current[:gracefully_stop] }
@ -107,21 +108,23 @@ module Astute
NODE_STATUSES_TRANSITIONS
)
offline_uids = fail_offline_nodes(
tasks_graph,
cluster.node_statuses_transitions
)
setup_fault_tolerance_behavior(
tasks_metadata['fault_tolerance_groups'],
cluster,
tasks_graph.keys
)
critical_uids = critical_node_uids(cluster.fault_tolerance_groups)
offline_uids = detect_offline_nodes(tasks_graph.keys)
fail_offline_nodes(
:offline_uids => offline_uids,
:critical_uids => critical_uids,
:node_statuses_transitions => cluster.node_statuses_transitions
)
tasks_graph.keys.each do |node_id|
node = @node_class.new(node_id, cluster)
node.context = @ctx
node = node_class.new(node_id, cluster)
node.context = ctx
node.set_critical if critical_uids.include?(node_id)
node.set_as_sync_point if sync_point?(node_id)
node.set_status_failed if offline_uids.include?(node_id)
@ -254,11 +257,11 @@ module Astute
def report_deploy_result(result)
if result[:success] && result.fetch(:failed_nodes, []).empty?
@ctx.report('status' => 'ready', 'progress' => 100)
ctx.report('status' => 'ready', 'progress' => 100)
elsif result[:success] && result.fetch(:failed_nodes, []).present?
@ctx.report('status' => 'ready', 'progress' => 100)
ctx.report('status' => 'ready', 'progress' => 100)
else
@ctx.report(
ctx.report(
'status' => 'error',
'progress' => 100,
'error' => result[:status]
@ -270,7 +273,7 @@ module Astute
return unless Astute.config.enable_graph_file
graph_file = File.join(
Astute.config.graph_dot_dir,
"graph-#{@ctx.task_id}.dot"
"graph-#{ctx.task_id}.dot"
)
File.open(graph_file, 'w') { |f| f.write(deployment.to_dot) }
Astute.logger.info("Check graph into file #{graph_file}")
@ -308,30 +311,32 @@ module Astute
critical_nodes
end
def fail_offline_nodes(tasks_graph, node_statuses_transitions)
offline_uids = detect_offline_nodes(tasks_graph.keys)
if offline_uids.present?
nodes = offline_uids.map do |uid|
{'uid' => uid,
'error_msg' => 'Node is not ready for deployment: '\
'mcollective has not answered'
}.merge(node_statuses_transitions.fetch('failed', {}))
end
def fail_offline_nodes(args={})
critical_uids = args.fetch(:critical_uids, [])
offline_uids = args.fetch(:offline_uids, [])
node_statuses_transitions = args.fetch(:node_statuses_transitions, {})
@ctx.report_and_update_status(
'nodes' => nodes,
'error' => 'Node is not ready for deployment'
)
return if offline_uids.blank?
missing_required = critical_node_uids(tasks_graph) & offline_uids
if missing_required.present?
error_message = "Critical nodes are not available for deployment: " \
"#{missing_required}"
raise Astute::DeploymentEngineError, error_message
end
nodes = offline_uids.map do |uid|
{'uid' => uid,
'error_msg' => 'Node is not ready for deployment: '\
'mcollective has not answered'
}.merge(node_statuses_transitions.fetch('failed', {}))
end
ctx.report_and_update_status(
'nodes' => nodes,
'error' => 'Node is not ready for deployment'
)
missing_required = critical_uids & offline_uids
if missing_required.present?
error_message = "Critical nodes are not available for deployment: " \
"#{missing_required}"
raise Astute::DeploymentEngineError, error_message
end
offline_uids
end
def detect_offline_nodes(uids)
@ -344,7 +349,7 @@ module Astute
if uids.present?
Astute.config.mc_retries.times.each do
systemtype = MClient.new(
@ctx,
ctx,
"systemtype",
uids,
_check_result=false,
@ -368,7 +373,7 @@ module Astute
node_report = cluster.nodes.inject([]) do |node_progress, node|
node_progress += [{'uid' => node[0].to_s, 'progress' => 100}]
end
@ctx.report('nodes' => node_report)
ctx.report('nodes' => node_report)
end
end

30
spec/unit/task_deployment_spec.rb
View File

@ -143,7 +143,7 @@ describe Astute::TaskDeployment do
describe '#deploy' do
it 'should run deploy' do
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
@ -155,7 +155,7 @@ describe Astute::TaskDeployment do
end
it 'should not raise error if deployment info not provided' do
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
@ -218,7 +218,7 @@ describe Astute::TaskDeployment do
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
task_deployment.expects(:fail_offline_nodes).returns([])
task_deployment.expects(:detect_offline_nodes).returns([])
Astute::TaskCluster.any_instance.stubs(:run).returns({:success => true})
task_deployment.deploy(
@ -232,7 +232,7 @@ describe Astute::TaskDeployment do
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
task_deployment.expects(:fail_offline_nodes).returns([])
task_deployment.expects(:detect_offline_nodes).returns([])
Astute::TaskCluster.any_instance.stubs(:run).returns({:success => true})
task_deployment.deploy(
@ -245,7 +245,7 @@ describe Astute::TaskDeployment do
Astute::TaskPreDeploymentActions.any_instance.stubs(:process)
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
Astute::TaskCluster.any_instance.stubs(:run).returns({:success => true})
Astute::TaskCluster.any_instance.expects(:stop_condition)
@ -261,7 +261,7 @@ describe Astute::TaskDeployment do
before(:each) do
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
Astute::TaskCluster.any_instance.stubs(:run).returns({:success => true})
Deployment::Concurrency::Counter.any_instance
.stubs(:maximum=).with(
@ -334,7 +334,7 @@ describe Astute::TaskDeployment do
context 'dry_run' do
it 'should not run actual deployment if dry_run is set to True' do
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
@ -350,7 +350,7 @@ describe Astute::TaskDeployment do
context 'noop_run' do
it 'should run noop deployment without error states' do
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
@ -373,7 +373,7 @@ describe Astute::TaskDeployment do
it 'should setup max nodes per call using config' do
Astute.config.max_nodes_per_call = 33
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
@ -396,7 +396,7 @@ describe Astute::TaskDeployment do
context 'subgraphs' do
it 'should call subgraph set up if subgraphs are present' do
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
task_deployment.stubs(:write_graph_to_file)
Astute::TaskCluster.any_instance.expects(:run).returns({:success => true})
@ -424,7 +424,7 @@ describe Astute::TaskDeployment do
tasks_directory: tasks_directory)
end
it 'should not call subgraph setup if subgraphs are not present' do
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
task_deployment.stubs(:write_graph_to_file)
ctx.stubs(:report)
Astute::TaskCluster.any_instance.expects(:run).returns({:success => true})
@ -451,7 +451,7 @@ describe Astute::TaskDeployment do
it 'succeed status and 100 progress for all nodes' do
Astute::TaskCluster.any_instance.stubs(:run).returns({:success => true})
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
task_deployment.stubs(:write_graph_to_file)
ctx.expects(:report).with('nodes' => [
{'uid' => '1', 'progress' => 100},
@ -474,7 +474,7 @@ describe Astute::TaskDeployment do
:failed_nodes => [failed_node],
:failed_tasks => [failed_task],
:status => 'Failed because of'})
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
task_deployment.stubs(:write_graph_to_file)
ctx.expects(:report).with('nodes' => [
{'uid' => '1', 'progress' => 100},
@ -503,7 +503,7 @@ describe Astute::TaskDeployment do
it 'should write if disable' do
Astute.config.enable_graph_file = false
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
ctx.stubs(:report)
Astute::TaskCluster.any_instance.stubs(:run).returns({:success => true})
@ -521,7 +521,7 @@ describe Astute::TaskDeployment do
it 'should write graph if enable' do
Astute.config.enable_graph_file = true
task_deployment.stubs(:fail_offline_nodes).returns([])
task_deployment.stubs(:detect_offline_nodes).returns([])
ctx.stubs(:report)
Astute::TaskCluster.any_instance.stubs(:run).returns({:success => true})