Merge pull request #18434 from djberg96/conversion_host_resource_type…

…_validation

Change ConversionHost resource_type validation

(cherry picked from commit 6ecc883)

https://bugzilla.redhat.com/show_bug.cgi?id=1696437

app/models/conversion_host.rb

@@ -1,8 +1,6 @@
 class ConversionHost < ApplicationRecord
   include NewWithTypeStiMixin
 
-  VALID_RESOURCE_TYPES = %w(VmOrTemplate Host).freeze
-
   acts_as_miq_taggable
 
   belongs_to :resource, :polymorphic => true
@@ -11,15 +9,16 @@ class ConversionHost < ApplicationRecord
   delegate :ext_management_system, :hostname, :ems_ref, :to => :resource, :allow_nil => true
 
   validates :name, :presence => true
-  validates :resource_id, :presence => true
-  validates :resource_type, :presence => true, :inclusion => { :in => VALID_RESOURCE_TYPES }
+  validates :resource, :presence => true
 
   validates :address,
     :uniqueness => true,
     :format     => { :with => Resolv::AddressRegex },
     :inclusion  => { :in => -> { resource.ipaddresses } },
     :unless     => -> { resource.blank? || resource.ipaddresses.blank? }
 
+  validate :resource_supports_conversion_host
+
   include_concern 'Configurations'
 
   # To be eligible, a conversion host must have the following properties
@@ -112,6 +111,15 @@ def disable_conversion_host_role
 
   private
 
+  # The Vm or Host provider subclass must support conversion hosts
+  # using the SupportsFeature mixin.
+  #
+  def resource_supports_conversion_host
+    if resource && !resource.supports_conversion_host?
+      errors.add(:resource, resource.unsupported_reason(:conversion_host))
+    end
+  end
+
   def check_resource_credentials(fatal = false, extra_msg = nil)
     success = send("check_resource_credentials_#{resource.ext_management_system.emstype}")
     if !success and fatal

spec/models/conversion_host/configurations_spec.rb

@@ -10,7 +10,7 @@
   end
 
   context "processing configuration requests" do
-    let(:vm) { FactoryGirl.create(:vm) }
+    let(:vm) { FactoryBot.create(:vm_openstack) }
     before(:each) do
       allow(ConversionHost).to receive(:new).and_return(conversion_host)
     end
@@ -66,7 +66,7 @@
 
   context "queuing configuration requests" do
     let(:ext_management_system) { FactoryGirl.create(:ext_management_system) }
-    let(:vm) { FactoryGirl.create(:vm, :ext_management_system => ext_management_system) }
+    let(:vm) { FactoryBot.create(:vm_openstack, :ext_management_system => ext_management_system) }
     let(:expected_task_action) { "Configuring a conversion_host: operation=#{op} resource=(type: #{vm.class.name} id:#{vm.id})" }
 
     context ".enable_queue" do

spec/models/conversion_host_spec.rb

@@ -4,8 +4,9 @@
   let(:apst) { FactoryGirl.create(:service_template_ansible_playbook) }
 
   context "provider independent methods" do
-    let(:host) { FactoryGirl.create(:host) }
-    let(:vm) { FactoryGirl.create(:vm) }
+    let(:ems) { FactoryBot.create(:ems_redhat, :zone => FactoryBot.create(:zone), :api_version => '4.2.4') }
+    let(:host) { FactoryBot.create(:host_redhat, :ext_management_system => ems) }
+    let(:vm) { FactoryBot.create(:vm_openstack) }
     let(:conversion_host_1) { FactoryGirl.create(:conversion_host, :resource => host, :address => '10.0.0.1') }
     let(:conversion_host_2) { FactoryGirl.create(:conversion_host, :resource => vm, :address => '10.0.1.1') }
     let(:task_1) { FactoryGirl.create(:service_template_transformation_plan_task, :state => 'active', :conversion_host => conversion_host_1) }
@@ -140,7 +141,7 @@
   end
 
   context "resource provider is rhevm" do
-    let(:ems) { FactoryGirl.create(:ems_redhat, :zone => FactoryGirl.create(:zone)) }
+    let(:ems) { FactoryBot.create(:ems_redhat, :zone => FactoryBot.create(:zone), :api_version => '4.2.4') }
     let(:host) { FactoryGirl.create(:host_redhat, :ext_management_system => ems) }
     let(:conversion_host) { FactoryGirl.create(:conversion_host, :resource => host, :vddk_transport_supported => true) }
 
@@ -186,4 +187,27 @@
       it_behaves_like "#check_ssh_connection"
     end
   end
+
+  context "resource validation" do
+    let(:ems) { FactoryBot.create(:ems_redhat, :zone => FactoryBot.create(:zone), :api_version => '4.2.4') }
+    let(:redhat_host) { FactoryBot.create(:host_redhat, :ext_management_system => ems) }
+    let(:azure_vm) { FactoryBot.create(:vm_azure) }
+
+    it "is valid if the associated resource supports conversion hosts" do
+      conversion_host = ConversionHost.new(:name => "test", :resource => redhat_host)
+      expect(conversion_host.valid?).to be(true)
+    end
+
+    it "is invalid if the associated resource does not support conversion hosts" do
+      conversion_host = ConversionHost.new(:name => "test", :resource => azure_vm)
+      expect(conversion_host.valid?).to be(false)
+      expect(conversion_host.errors.messages[:resource].first).to eql("Feature not available/supported")
+    end
+
+    it "is invalid if there is no associated resource" do
+      conversion_host = ConversionHost.new(:name => "test2")
+      expect(conversion_host.valid?).to be(false)
+      expect(conversion_host.errors[:resource].first).to eql("can't be blank")
+    end
+  end
 end

spec/models/service_template_transformation_plan_request_spec.rb

@@ -60,14 +60,14 @@
       let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }
 
       it 'returns false' do
-        host = FactoryGirl.create(:host, :ext_management_system => FactoryGirl.create(:ext_management_system, :zone => FactoryGirl.create(:zone)))
+        host = FactoryBot.create(:host_redhat, :ext_management_system => FactoryBot.create(:ext_management_system, :zone => FactoryBot.create(:zone), :api_version => '4.2.4'))
         conversion_host = FactoryGirl.create(:conversion_host, :resource => host)
         expect(request.validate_conversion_hosts).to be false
       end
     end
 
     context 'conversion host exists in EMS and resource is a Host' do
-      let(:dst_ems) { FactoryGirl.create(:ems_redhat) }
+      let(:dst_ems) { FactoryBot.create(:ems_redhat, :api_version => '4.2.4') }
       let(:src_cluster) { FactoryGirl.create(:ems_cluster) }
       let(:dst_cluster) { FactoryGirl.create(:ems_cluster, :ext_management_system => dst_ems) }
 
@@ -96,7 +96,7 @@
       let(:request) { FactoryGirl.create(:service_template_transformation_plan_request, :source => plan) }
 
       it 'returns true' do
-        host = FactoryGirl.create(:host, :ext_management_system => dst_ems, :ems_cluster => dst_cluster)
+        host = FactoryBot.create(:host_redhat, :ext_management_system => dst_ems, :ems_cluster => dst_cluster)
         conversion_host = FactoryGirl.create(:conversion_host, :resource => host)
         expect(request.validate_conversion_hosts).to be true
       end

spec/models/service_template_transformation_plan_task_spec.rb

@@ -21,7 +21,7 @@
     let(:vm)  { FactoryGirl.create(:vm_or_template) }
     let(:vm2)  { FactoryGirl.create(:vm_or_template) }
     let(:apst) { FactoryGirl.create(:service_template_ansible_playbook) }
-    let(:conversion_host) { FactoryGirl.create(:conversion_host, :resource => host) }
+    let(:conversion_host) { FactoryBot.create(:conversion_host, :skip_validate, :resource => host) }
 
     let(:mapping) do
       FactoryGirl.create(
@@ -344,8 +344,8 @@
 
       let(:src_hardware) { FactoryGirl.create(:hardware, :nics => [src_nic_1, src_nic_2]) }
 
-      let(:src_vm_1) { FactoryGirl.create(:vm_openstack, :ext_management_system => src_ems, :ems_cluster => src_cluster, :host => src_host, :hardware => src_hardware) }
-      let(:src_vm_2) { FactoryGirl.create(:vm_openstack, :ext_management_system => src_ems, :ems_cluster => src_cluster, :host => src_host) }
+      let(:src_vm_1) { FactoryBot.create(:vm_openstack, :ext_management_system => src_ems, :ems_cluster => src_cluster, :host => src_host, :hardware => src_hardware) }
+      let(:src_vm_2) { FactoryBot.create(:vm_openstack, :ext_management_system => src_ems, :ems_cluster => src_cluster, :host => src_host) }
 
       let(:src_network_1) { FactoryBot.create(:network, :ipaddress => '10.0.1.1') }
       let(:src_network_2) { FactoryBot.create(:network, :ipaddress => nil) }
@@ -470,11 +470,11 @@
       end
 
       context 'destination is rhevm' do
-        let(:dst_ems) { FactoryGirl.create(:ems_redhat, :zone => FactoryGirl.create(:zone)) }
+        let(:dst_ems) { FactoryBot.create(:ems_redhat, :zone => FactoryBot.create(:zone), :api_version => '4.2.4') }
         let(:dst_storage) { FactoryGirl.create(:storage) }
         let(:dst_lan_1) { FactoryGirl.create(:lan) }
         let(:dst_lan_2) { FactoryGirl.create(:lan) }
-        let(:conversion_host) { FactoryGirl.create(:conversion_host, :resource => FactoryGirl.create(:host, :ext_management_system => dst_ems)) }
+        let(:conversion_host) { FactoryBot.create(:conversion_host, :resource => FactoryBot.create(:host_redhat, :ext_management_system => dst_ems)) }
 
         let(:mapping) do
           FactoryGirl.create(