From b1ed3c82bfc7760eeba9464775c636edda23661f Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Thu, 27 Jul 2017 15:09:54 -0400 Subject: [PATCH] Return VMs and Templates for EMS prev_relats When retrieving vmdb_relats_ems we weren't including templates in the list of children for folders. This isn't an issue for clusters, hosts, and resource_pools because templates don't belong to any of these but they do belong to folders. If a VM was marked as a template and moved into a new folder it was not being included in the prev_relats which meant that its previous resource_pool and folder relat were not being deleted, but we were adding a new folder relationship. This caused the "multiple parents found" exception seen occasionally on the UI while provisioning. https://bugzilla.redhat.com/show_bug.cgi?id=1475405 --- app/models/ems_refresh/metadata_relats.rb | 13 ++++- .../ems_refresh/metadata_relats_spec.rb | 51 +++++++++++-------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/app/models/ems_refresh/metadata_relats.rb b/app/models/ems_refresh/metadata_relats.rb index 56e0852a673..1002b345b9f 100644 --- a/app/models/ems_refresh/metadata_relats.rb +++ b/app/models/ems_refresh/metadata_relats.rb @@ -38,8 +38,17 @@ def vmdb_relats_ems(ems, relats = nil) next if p_type == :clusters && c_type == :vms next if p_type == :hosts && c_type == :vms - # Handle default resource pools being called differently - c_meth = ([:hosts, :clusters].include?(p_type) && c_type == :resource_pools) ? :resource_pools_with_default : c_type + c_meth = + # Handle default resource pools being called differently + if [:hosts, :clusters].include?(p_type) && c_type == :resource_pools + :resource_pools_with_default + # Handle both Vm and Template child types + elsif c_type == :vms + :vms_and_templates + else + c_type + end + next unless x.respond_to?(c_meth) ids = x.send(c_meth).collect(&:id).uniq diff --git a/spec/models/ems_refresh/metadata_relats_spec.rb b/spec/models/ems_refresh/metadata_relats_spec.rb index 476472d83d1..d77f92bad87 100644 --- a/spec/models/ems_refresh/metadata_relats_spec.rb +++ b/spec/models/ems_refresh/metadata_relats_spec.rb @@ -1,51 +1,60 @@ describe EmsRefresh::MetadataRelats do context ".vmdb_relats" do before(:each) do - @zone = FactoryGirl.create(:zone) - @ems = FactoryGirl.create(:ems_vmware, :zone => @zone) - @cluster = FactoryGirl.create(:ems_cluster, :ext_management_system => @ems) - @host = FactoryGirl.create(:host, :ext_management_system => @ems, :ems_cluster => @cluster) - @vm = FactoryGirl.create(:vm_vmware, :ext_management_system => @ems, :ems_cluster => @cluster, :host => @host) - - @rp = FactoryGirl.create(:resource_pool, :ext_management_system => @ems) - @folder = FactoryGirl.create(:ems_folder, :ext_management_system => @ems) - @folder.add_cluster(@cluster) + @zone = FactoryGirl.create(:zone) + @ems = FactoryGirl.create(:ems_vmware, :zone => @zone) + + @cluster = FactoryGirl.create(:ems_cluster, :ext_management_system => @ems) + @host = FactoryGirl.create(:host, :ext_management_system => @ems, :ems_cluster => @cluster) + @vm = FactoryGirl.create(:vm_vmware, :ext_management_system => @ems, :ems_cluster => @cluster, :host => @host) + @template = FactoryGirl.create(:template_vmware, :ext_management_system => @ems) + @rp = FactoryGirl.create(:resource_pool, :ext_management_system => @ems) + @host_folder = FactoryGirl.create(:vmware_folder_host, :ext_management_system => @ems) + @vm_folder = FactoryGirl.create(:vmware_folder_vm, :ext_management_system => @ems) + + @host_folder.add_cluster(@cluster) @cluster.add_resource_pool(@rp) @rp.add_vm(@vm) - [@ems, @folder, @cluster, @rp, @host, @vm].each(&:reload) + @vm_folder.add_vm(@vm) + @vm_folder.add_vm(@template) + + [@ems, @host_folder, @vm_folder, @cluster, @rp, @host, @vm, @template].each(&:reload) MiqQueue.delete_all end it "with a Vm" do - expect(EmsRefresh.vmdb_relats(@vm)).to eq(:folders_to_clusters => {@folder.id => [@cluster.id]}, - :clusters_to_resource_pools => {@cluster.id => [@rp.id]}, - :resource_pools_to_vms => {@rp.id => [@vm.id]}) + expect(EmsRefresh.vmdb_relats(@vm)).to eq(:folders_to_clusters => {@host_folder.id => [@cluster.id]}, + :folders_to_vms => {@vm_folder.id => [@vm.id]}, + :clusters_to_resource_pools => {@cluster.id => [@rp.id]}, + :resource_pools_to_vms => {@rp.id => [@vm.id]}) end it "with a Host" do - expect(EmsRefresh.vmdb_relats(@host)).to eq(:folders_to_clusters => {@folder.id => [@cluster.id]}) + expect(EmsRefresh.vmdb_relats(@host)).to eq(:folders_to_clusters => {@host_folder.id => [@cluster.id]}) end it "with an EMS" do - expect(EmsRefresh.vmdb_relats(@ems)).to eq(:folders_to_clusters => {@folder.id => [@cluster.id]}, - :clusters_to_resource_pools => {@cluster.id => [@rp.id]}, - :resource_pools_to_vms => {@rp.id => [@vm.id]}) + expect(EmsRefresh.vmdb_relats(@ems)).to eq(:folders_to_clusters => {@host_folder.id => [@cluster.id]}, + :folders_to_vms => {@vm_folder.id => [@template.id, @vm.id]}, + :clusters_to_resource_pools => {@cluster.id => [@rp.id]}, + :resource_pools_to_vms => {@rp.id => [@vm.id]}) end context "with an invalid relats tree" do before(:each) do @rp2 = FactoryGirl.create(:resource_pool, :ext_management_system => @ems) @host.set_child(@rp2) - @folder.add_host(@host) + @host_folder.add_host(@host) - [@folder, @host, @rp2].each(&:reload) + [@host_folder, @host, @rp2].each(&:reload) MiqQueue.delete_all end it "with an EMS" do - expect(EmsRefresh.vmdb_relats(@ems)).to eq(:folders_to_hosts => {@folder.id => [@host.id]}, - :folders_to_clusters => {@folder.id => [@cluster.id]}, + expect(EmsRefresh.vmdb_relats(@ems)).to eq(:folders_to_hosts => {@host_folder.id => [@host.id]}, + :folders_to_vms => {@vm_folder.id => [@template.id, @vm.id]}, + :folders_to_clusters => {@host_folder.id => [@cluster.id]}, :clusters_to_resource_pools => {@cluster.id => [@rp.id]}, :hosts_to_resource_pools => {@host.id => [@rp2.id]}, :resource_pools_to_vms => {@rp.id => [@vm.id]})