moving conditional supports logic from validate and launch to console…

… supports

vnc, webmks, and console are not consistent
webmks also adds login validation checking.

Added the vm must be running check from :launch_{}_console
feature. That feature has been rolled into :{}_console

app/models/manageiq/providers/vmware/infra_manager.rb

@@ -40,7 +40,11 @@ class Vmware::InfraManager < InfraManager
     supports :metrics
     supports :native_console
     supports :vmrc_console do
-      "vCenter needs to be refreshed to determine remote console support." if api_version.blank? || hostname.blank? || uid_ems.blank?
+      if api_version.blank? || hostname.blank? || uid_ems.blank?
+        "vCenter needs to be refreshed to determine remote console support."
+      elsif ext_management_system.authentication_type(:console).nil?
+        "remote console requires console credentials"
+      end
     end
     supports :webmks_console
 

app/models/manageiq/providers/vmware/infra_manager/vm/remote_console.rb

@@ -2,20 +2,23 @@ module ManageIQ::Providers::Vmware::InfraManager::Vm::RemoteConsole
   extend ActiveSupport::Concern
 
   included do
-    supports :console
-    supports :html5_console
-    supports :vmrc_console
-    supports :vnc_console
-    supports :webmks_console
-  end
-
-  def validate_remote_console_acquire_ticket(protocol, options = {})
-    raise(MiqException::RemoteConsoleNotSupportedError, "#{protocol} remote console requires the vm to be registered with a management system.") if ext_management_system.nil?
-
-    raise(MiqException::RemoteConsoleNotSupportedError, "remote console requires console credentials") if ext_management_system.authentication_type(:console).nil? && protocol == "vmrc"
-
-    options[:check_if_running] = true unless options.key?(:check_if_running)
-    raise(MiqException::RemoteConsoleNotSupportedError, "#{protocol} remote console requires the vm to be running.") if options[:check_if_running] && state != "on"
+    supports :console do
+      if ext_management_system.nil?
+        "VM must be registered with a management system."
+      elsif state != "on"
+        "VM must be running."
+      end
+    end
+    supports(:html5_console) { unsupported_reason(:console) }
+    supports :vmrc_console do
+      unsupported_reason(:console) ||
+        ext_management_system.unsupported_reason(:vmrc_console)
+    end
+    supports :vnc_console { unsupported_reason(:console) }
+    supports :webmks_console do
+      unsupported_reason(:console) ||
+        ext_management_system.unsupported_reason(:webmks_console)
+    end
   end
 
   def remote_console_acquire_ticket(userid, originating_server, protocol)
@@ -47,7 +50,7 @@ def remote_console_acquire_ticket_queue(protocol, userid)
   #
 
   def remote_console_vmrc_acquire_ticket(_userid = nil, _originating_server = nil)
-    validate_remote_console_acquire_ticket("vmrc")
+    validate_supports(:vmrc_console)
     ticket = ext_management_system.remote_console_vmrc_acquire_ticket
 
     {
@@ -57,18 +60,12 @@ def remote_console_vmrc_acquire_ticket(_userid = nil, _originating_server = nil)
     }
   end
 
-  def validate_remote_console_vmrc_support
-    validate_remote_console_acquire_ticket("vmrc")
-    validate_supports(ext_management_system.unsupported_reason(:vmrc_console))
-    true
-  end
-
   #
   # WebMKS
   #
 
   def remote_console_webmks_acquire_ticket(userid, originating_server = nil)
-    validate_remote_console_acquire_ticket("webmks")
+    validate_supports(:webmks_console)
     ticket = ext_management_system.vm_remote_console_webmks_acquire_ticket(self)
 
     SystemConsole.force_vm_invalid_token(id)
@@ -85,12 +82,6 @@ def remote_console_webmks_acquire_ticket(userid, originating_server = nil)
     SystemConsole.launch_proxy_if_not_local(console_args, originating_server, ticket['host'].to_s, ticket['port'].to_i)
   end
 
-  def validate_remote_console_webmks_support
-    validate_remote_console_acquire_ticket("webmks")
-    validate_supports(ext_management_system.unsupported_reason(:webmks_console))
-    true
-  end
-
   #
   # HTML5 selects the best available console type (VNC or WebMKS)
   #
@@ -105,7 +96,7 @@ def remote_console_html5_acquire_ticket(userid, originating_server = nil)
   def remote_console_vnc_acquire_ticket(userid, originating_server)
     require 'securerandom'
 
-    validate_remote_console_acquire_ticket("vnc")
+    validate_supports(:vnc_console)
 
     password     = SecureRandom.base64[0, 8] # Random password from the Base64 character set
     host_port    = host.reserve_next_available_vnc_port
@@ -144,12 +135,10 @@ def remote_console_vnc_acquire_ticket(userid, originating_server)
 
   private
 
-  # @param unsupported_reason [Nil,String, Symbol]
-  #        a symbol will lookup the unsupported reason
-  #        otherwise, will raise an error if there is a reason to
-  def validate_supports(unsupported_reason)
-    unsupported_reason = unsupported_reason(unsupported_reason) if unsupported_reason.kind_of?(Symbol)
-    raise(MiqException::RemoteConsoleNotSupportedError, unsupported_reason) if unsupported_reason
+  def validate_supports(feature)
+    if (unsupported_reason = unsupported_reason(feature))
+      raise(MiqException::RemoteConsoleNotSupportedError, unsupported_reason)
+    end
   end
 
   # Method to generate the remote URI for the VMRC console

spec/models/manageiq/providers/vmware/infra_manager/vm/remote_console_spec.rb

@@ -126,25 +126,24 @@
     end
   end
 
-  context '#validate_remote_console_webmks_support' do
+  context '#supports?(:webmks_console)' do
     before do
       ems.authentications << FactoryBot.create(:authentication, :authtype => :console, :userid => "root", :password => "vmware")
     end
 
     it 'normal case' do
       ems.update_attribute(:api_version, '6.0')
-      expect(vm.validate_remote_console_webmks_support).to be_truthy
+      expect(vm.supports?(:webmks_console)).to be_truthy
     end
 
     it 'with vm with no ems' do
-      vm.ext_management_system = nil
-      vm.save!
-      expect { vm.validate_remote_console_webmks_support }.to raise_error MiqException::RemoteConsoleNotSupportedError
+      vm.update!(:ext_management_system => nil)
+      expect(vm.supports?(:webmks_console)).to be_falsey
     end
 
     it 'with vm off' do
-      vm.update_attribute(:raw_power_state, 'poweredOff')
-      expect { vm.validate_remote_console_webmks_support }.to raise_error MiqException::RemoteConsoleNotSupportedError
+      vm.update(:raw_power_state => 'poweredOff')
+      expect(vm.supports?(:webmks_console)).to be_falsey
     end
   end
 
@@ -180,24 +179,24 @@
     end
   end
 
-  context '#validate_remote_console_vmrc_support' do
+  context '#supports?(:vmrc_console)' do
     before do
       ems.authentications << FactoryBot.create(:authentication, :authtype => :console, :userid => "root", :password => "vmware")
     end
 
     it 'normal case' do
-      expect(vm.validate_remote_console_vmrc_support).to be_truthy
+      expect(vm.supports?(:vmrc_console)).to be_truthy
     end
 
     it 'with vm with no ems' do
       vm.ext_management_system = nil
       vm.save!
-      expect { vm.validate_remote_console_vmrc_support }.to raise_error MiqException::RemoteConsoleNotSupportedError
+      expect(vm.supports?(:vmrc_console)).to be_falsey
     end
 
     it 'with vm off' do
       vm.update_attribute(:raw_power_state, 'poweredOff')
-      expect { vm.validate_remote_console_vmrc_support }.to raise_error MiqException::RemoteConsoleNotSupportedError
+      expect(vm.supports?(:vmrc_console)).to be_falsey
     end
   end
 

spec/models/manageiq/providers/vmware/infra_manager_spec.rb

@@ -168,7 +168,7 @@
 
   context "#supports?(:vmrc_console)" do
     before(:each) do
-      @ems = FactoryBot.create(:ems_vmware)
+      @ems = FactoryBot.create(:ems_vmware_with_authentication, :authtype => 'console')
     end
 
     it "true with nothing missing/blank" do