Skip to content

Commit

Permalink
Merge pull request #18391 from agrare/bz_1668020_accessible_storages
Browse files Browse the repository at this point in the history
Add method for restricting to accessible storages
  • Loading branch information
gmcculloug authored Jan 25, 2019
2 parents 4794a6d + 62f834e commit e800c48
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 15 deletions.
2 changes: 2 additions & 0 deletions app/models/host.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions app/models/host_storage.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion app/models/miq_request_workflow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/models/vm_or_template/operations/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 3 additions & 13 deletions spec/models/vm_or_template/operations/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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
Expand Down

0 comments on commit e800c48

Please sign in to comment.