From f111842dbe35795d0e28bf11ac5fb880a1cba5fe Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Thu, 22 Sep 2022 17:14:49 +0100 Subject: [PATCH] Handle different attribute and element ordering (#1592) Normalise the XML to ensure the attributes for both documents have the same ordering to prevent excessive noise when differences are detected. Additionally sort various elements based on attributes that make ordering irrelevant to allow for simpler comparison using xmlsimple. Closes: #1583 --- lib/vagrant-libvirt/action/start_domain.rb | 13 +++- spec/spec_helper.rb | 4 ++ spec/unit/action/start_domain_spec.rb | 20 ++++++ .../start_domain_spec/existing_reordered.xml | 62 +++++++++++++++++++ 4 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 spec/unit/action/start_domain_spec/existing_reordered.xml diff --git a/lib/vagrant-libvirt/action/start_domain.rb b/lib/vagrant-libvirt/action/start_domain.rb index 524c229..67825d7 100644 --- a/lib/vagrant-libvirt/action/start_domain.rb +++ b/lib/vagrant-libvirt/action/start_domain.rb @@ -429,9 +429,20 @@ module VagrantPlugins raise Errors::UpdateServerError, error_message: e.message end + # this normalises the attribute order to be the same as what was sent in the above + # request to update the domain XML. Without this, if the XML documents are not + # equivalent, many more differences will be reported than there actually are. + applied_xml = String.new + REXML::Document.new(libvirt_domain.xml_desc(1)).write(applied_xml) + # need to check whether the updated XML contains all the changes requested proposed = VagrantPlugins::ProviderLibvirt::Util::Xml.new(new_xml) - applied = VagrantPlugins::ProviderLibvirt::Util::Xml.new(libvirt_domain.xml_desc(1)) + applied = VagrantPlugins::ProviderLibvirt::Util::Xml.new(applied_xml) + + # perform some sorting to allow comparison otherwise order of devices differing + # even if they are equivalent will be reported as being different. + proposed.xml['devices'][0].each { |_, v| next unless v.is_a?(Array); v.sort_by! { |e| [e['type'], e['index']]} } + applied.xml['devices'][0].each { |_, v| next unless v.is_a?(Array); v.sort_by! { |e| [e['type'], e['index']]} } if proposed != applied require 'diffy' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dab296c..8dd630c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -63,6 +63,10 @@ RSpec.configure do |config| # don't run acceptance tests by default config.filter_run_excluding :acceptance => true + + config.expect_with :rspec do |c| + c.max_formatted_output_length = 2000 if c.respond_to?("max_formatted_output_length=") + end end require 'vagrant-spec/unit' diff --git a/spec/unit/action/start_domain_spec.rb b/spec/unit/action/start_domain_spec.rb index ae18549..75b6268 100644 --- a/spec/unit/action/start_domain_spec.rb +++ b/spec/unit/action/start_domain_spec.rb @@ -74,6 +74,26 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do end end + context 'when xml elements and attributes reordered' do + let(:test_file) { 'existing.xml' } + let(:updated_test_file) { 'existing_reordered.xml' } + let(:vagrantfile_providerconfig) do + <<-EOF + libvirt.cpu_mode = "host-passthrough" + EOF + end + + it 'should correctly detect the domain was updated' do + expect(ui).to_not receive(:warn) + expect(libvirt_domain).to receive(:autostart=) + expect(connection).to receive(:define_domain).and_return(libvirt_domain) + expect(libvirt_domain).to receive(:xml_desc).and_return(domain_xml, updated_domain_xml) + expect(domain).to receive(:start) + + expect(subject.call(env)).to be_nil + end + end + context 'when xml not applied' do let(:test_file) { 'default_with_different_formatting.xml' } let(:updated_domain_xml) { diff --git a/spec/unit/action/start_domain_spec/existing_reordered.xml b/spec/unit/action/start_domain_spec/existing_reordered.xml new file mode 100644 index 0000000..a557f3a --- /dev/null +++ b/spec/unit/action/start_domain_spec/existing_reordered.xml @@ -0,0 +1,62 @@ + + vagrant-libvirt_default + 881a931b-0110-4d10-81aa-47a1a19f5726 + Source: /home/test/vagrant-libvirt/Vagrantfile + 2097152 + 2097152 + 2 + + hvm + + + + + + + + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + + + + +
+ + + +
+ + + + + +
+ + + + + + + + + + + + + + +