On 09/12/2012 12:28 PM, [email protected] wrote:
From: Jan Provaznik <[email protected]>When Configserver launches services on each VM, it checks if all param references are satisfied so generally something like this should cause a deadlock because of cyclic reference: Assembly A, param P1 references Assembly B, param P2 Assembly B, param P1 references Assembly A, param P2 But the trick is that the above deadlock is allowed if referenced param is 'OS' type (if it's value is not output of some script). So cyclic references can be allowed in some cases depending on type of referenced parameter. But problem is that type of referenced parameter can't be easily resolved based only on deployable XML file so only solution for Conductor for now is to enable inter-assembly cyclic references and check only cyclic references inside one assembly. --- src/app/models/deployable.rb | 2 +- src/app/util/deployable_xml.rb | 45 +++++++++++++++----------------- src/spec/factories/deployable.rb | 53 +++----------------------------------- src/spec/models/deployable_spec.rb | 27 +++++++------------ 4 files changed, 35 insertions(+), 92 deletions(-) diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb index 374b4e3..811a513 100644 --- a/src/app/models/deployable.rb +++ b/src/app/models/deployable.rb @@ -98,7 +98,7 @@ class Deployable < ActiveRecord::Base errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid_not_existing_param', locale_params)) - elsif reference[:to_service] + elsif reference[:reference][:service] errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid_not_existing_service_reference', locale_params)) diff --git a/src/app/util/deployable_xml.rb b/src/app/util/deployable_xml.rb index e057771..837041b 100644 --- a/src/app/util/deployable_xml.rb +++ b/src/app/util/deployable_xml.rb @@ -184,12 +184,6 @@ class AssemblyXML :service => service.name, :references => service.references} end - # whole assembly is added as a node too because a inter-assembly references - # reference whole assembly, not particular service - nodes << {:assembly => name, - :service => nil, - :references => nodes.map {|n| - n[:references]}.flatten} end private @@ -343,28 +337,31 @@ class DeployableDependencyGraph invalid_refs = [] dependency_nodes.each do |node| - # skip "whole assembly" nodes - next unless node[:service] - node[:references].each do |ref| - if ref[:not_existing_ref] - # in this case, the referenced assembly/service doesn't exist - # at all - invalid_refs << {:assembly => node[:assembly], - :service => node[:service], - :reference => ref} - else - # in this case, the referenced assembly/service exists, but the - # referenced parameter is not listed in <returns> tag of the assembly + no_return_param = false + + if ref[:service].nil? assembly = @assemblies.find {|a| a.name == ref[:assembly]} - next unless assembly # this shouldn't be needed - unless assembly.output_parameters.include?(ref[:param]) - invalid_refs << {:assembly => node[:assembly], - :service => node[:service], - :no_return_param => true, - :reference => ref} + if assembly + if assembly.output_parameters.include?(ref[:param]) + # if referenced assembly exists and returns referenced param, then + # it's valid + next + else + no_return_param = true + end end + else + # if a param references another service, then we check if + # this service exists or not. For current version of config server + # only services from the same assembly can be referenced + next unless ref[:not_existing_ref] end + + invalid_refs << {:assembly => node[:assembly], + :service => node[:service], + :no_return_param => no_return_param, + :reference => ref} end end invalid_refs diff --git a/src/spec/factories/deployable.rb b/src/spec/factories/deployable.rb index 8a4f3ba..b279159 100644 --- a/src/spec/factories/deployable.rb +++ b/src/spec/factories/deployable.rb @@ -73,54 +73,6 @@ FactoryGirl.define do </deployable>" end - factory :deployable_with_cyclic_assembly_references, :parent => :deployable do - xml "<?xml version=\"1.0\"?> - <deployable version='1.0' name='deployable'> - <assemblies> - <assembly name='assembly1' hwp='front_hwp1'> - <image id='85653387-88b2-46b0-b7b2-c779d2af06c7'></image> - <services> - <service name='service1'> - <executable url='executable_url'/> - <parameters> - <parameter name='param1'> - <reference assembly='assembly2' parameter='hostname'/> - </parameter> - </parameters> - </service> - </services> - </assembly> - <assembly name='assembly2' hwp='front_hwp1'> - <image id='85653387-88b2-46b0-b7b2-c779d2af06c7'></image> - <services> - <service name='service1'> - <executable url='executable_url'/> - <parameters> - <parameter name='param1'> - <reference assembly='assembly3' parameter='hostname'/> - </parameter> - </parameters> - </service> - </services> - </assembly> - <assembly name='assembly3' hwp='front_hwp1'> - <image id='85653387-88b2-46b0-b7b2-c779d2af06c7'></image> - <services> - <service name='service1'> - <executable url='executable_url'/> - <parameters> - <parameter name='param1'> - <reference assembly='assembly1' parameter='hostname'/> - </parameter> - </parameters> - </service> - </services> - </assembly> - </assemblies> - </deployable>" - end - - factory :deployable_with_cyclic_service_references, :parent => :deployable do xml "<?xml version=\"1.0\"?> <deployable version='1.0' name='deployable'> @@ -161,7 +113,10 @@ FactoryGirl.define do <executable url='executable_url'/> <parameters> <parameter name='param1'> - <reference assembly='assembly2' service='service2'/> + <reference assembly='assembly1' service='service2'/> + </parameter> + <parameter name='param2'> + <reference assembly='assembly2' parameter='param2'/> </parameter> </parameters> </service> diff --git a/src/spec/models/deployable_spec.rb b/src/spec/models/deployable_spec.rb index 4aec099..0deae4c 100644 --- a/src/spec/models/deployable_spec.rb +++ b/src/spec/models/deployable_spec.rb @@ -98,23 +98,6 @@ describe Deployable do end end - describe "validation of deployable_xml with cyclic assembly references" do - before :each do - @deployable = FactoryGirl.build(:deployable_with_cyclic_assembly_references) - @cycles = DeployableXML.new(@deployable.xml).dependency_graph.cycles - end - - it_behaves_like "deployable XML with cyclic reference" - - it "should detect cycle between multiple assmebliess" do - cycle = @cycles.first - cycle.length.should == 3 - cycle.find {|n| n[:assembly] == 'assembly1' && n[:service] == 'service1'} - cycle.find {|n| n[:assembly] == 'assembly2' && n[:service] == 'service1'} - cycle.find {|n| n[:assembly] == 'assembly3' && n[:service] == 'service1'} - end - end - describe "validation of deployable_xml with not existing references" do before :each do @deployable = FactoryGirl.build(:deployable_with_not_existing_references) @@ -136,9 +119,17 @@ describe Deployable do it "should detect reference to not existing service" do @invalid_refs.find {|r| r[:assembly] == 'assembly1' && r[:service] == 'service1' && - r[:reference][:assembly] == 'assembly2' && + r[:reference][:assembly] == 'assembly1' && r[:reference][:service] == 'service2'}.should_not be_nil end + + it "should detect reference to not existing parameter" do + @invalid_refs.find {|r| r[:assembly] == 'assembly1' && + r[:service] == 'service1' && + r[:no_return_param] && + r[:reference][:assembly] == 'assembly2' && + r[:reference][:param] == 'param2'}.should_not be_nil + end end it "should manage permissions on catalog changes" do
ACK
