diff --git a/app/models/host.rb b/app/models/host.rb index 0add9466228..bc29ded42b1 100644 --- a/app/models/host.rb +++ b/app/models/host.rb @@ -47,6 +47,8 @@ class Host < ApplicationRecord has_many :miq_templates, :inverse_of => :host has_many :host_storages, :dependent => :destroy has_many :storages, :through => :host_storages + has_many :writable_accessible_host_storages, -> { writable_accessible }, :class_name => "HostStorage" + has_many :writable_accessible_storages, :through => :writable_accessible_host_storages, :source => :storage has_many :host_switches, :dependent => :destroy has_many :switches, :through => :host_switches has_many :lans, :through => :switches diff --git a/app/models/host_storage.rb b/app/models/host_storage.rb index 31ce55151c9..b47f1ff4732 100644 --- a/app/models/host_storage.rb +++ b/app/models/host_storage.rb @@ -1,4 +1,16 @@ class HostStorage < ApplicationRecord belongs_to :host belongs_to :storage + + scope :writable, -> { where(:read_only => [nil, false]) } + scope :read_only, -> { where(:read_only => true) } + + scope :accessible, -> { where(:accessible => [true, nil]) } + scope :inaccessible, -> { where(:accessible => false) } + + class << self + def writable_accessible + writable.accessible + end + end end diff --git a/app/models/miq_request_workflow.rb b/app/models/miq_request_workflow.rb index 6133a63927b..43d2b7cde10 100644 --- a/app/models/miq_request_workflow.rb +++ b/app/models/miq_request_workflow.rb @@ -1069,7 +1069,7 @@ def allowed_storages(_options = {}) MiqPreloader.preload(hosts, :storages => {}, :host_storages => :storage) storages = hosts.each_with_object({}) do |host, hash| - host.writable_storages.each { |s| hash[s.id] = s } + host.writable_accessible_storages.each { |s| hash[s.id] = s } end.values selected_storage_profile_id = get_value(@values[:placement_storage_profile]) if selected_storage_profile_id diff --git a/app/models/vm_or_template/operations/configuration.rb b/app/models/vm_or_template/operations/configuration.rb index 798e6ee57be..66215829e73 100644 --- a/app/models/vm_or_template/operations/configuration.rb +++ b/app/models/vm_or_template/operations/configuration.rb @@ -75,7 +75,7 @@ def raw_add_disk(disk_name, disk_size_mb, options = {}) raise _("VM has no EMS, unable to add disk") unless ext_management_system if options[:datastore] datastore = ext_management_system.hosts.collect do |h| - h.writable_storages.find_by(:name => options[:datastore]) + h.writable_accessible_storages.find_by(:name => options[:datastore]) end.uniq.compact.first raise _("Datastore does not exist or cannot be accessed, unable to add disk") unless datastore end diff --git a/spec/models/vm_or_template/operations/configuration_spec.rb b/spec/models/vm_or_template/operations/configuration_spec.rb index eebd7f41965..38c8020ee75 100644 --- a/spec/models/vm_or_template/operations/configuration_spec.rb +++ b/spec/models/vm_or_template/operations/configuration_spec.rb @@ -14,21 +14,13 @@ context "when ext_management_system exists" do let(:vm) { FactoryBot.create(:vm_or_template, :ext_management_system => ems) } - let(:ems) { FactoryBot.create(:ext_management_system) } + let(:ems) { FactoryBot.create(:ext_management_system, :with_authentication) } let(:storage_name) { "test_storage" } let(:storage) { FactoryBot.create(:storage, :name => storage_name) } - let(:storages) { double("storages") } - let(:host) { double("host", :writable_storages => storages) } - let(:hosts) { [host] } - - before do - allow(ems).to receive(:hosts).and_return(hosts) - end + let!(:host) { FactoryBot.create(:host, :ext_management_system => ems).tap { |h| h.host_storages.create!(:storage => storage) } } context "when storage exists" do it "adds a disk on the storage" do - allow(storages).to receive(:find_by).with(:name => storage_name).and_return(storage) - allow(ems).to receive(:authentication_status_ok?).and_return(true) allow(ems).to receive(:vm_add_disk) expected_options = { @@ -48,10 +40,8 @@ context "when storage does not exist" do it "raises an exception when doesn't find storage by its name" do - allow(storages).to receive(:find_by).with(:name => storage_name).and_return(nil) - message = "Datastore does not exist or cannot be accessed, unable to add disk" - expect { vm.raw_add_disk(disk_name, disk_size, :datastore => storage_name) }.to raise_error(message) + expect { vm.raw_add_disk(disk_name, disk_size, :datastore => "wrong_storage_name") }.to raise_error(message) end end end