Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subclass IsoDatastore and transfer from core #621

Merged
merged 2 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/models/manageiq/providers/ovirt/infra_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class ManageIQ::Providers::Ovirt::InfraManager < ManageIQ::Providers::InfraManag
require_nested :ProvisionWorkflow
require_nested :Snapshot
require_nested :Storage
require_nested :IsoDatastore
require_nested :Template
require_nested :Vm
require_nested :DistributedVirtualSwitch
Expand All @@ -27,6 +28,8 @@ class ManageIQ::Providers::Ovirt::InfraManager < ManageIQ::Providers::InfraManag
has_many :vm_and_template_ems_custom_fields, :through => :vms_and_templates, :source => :ems_custom_attributes
has_many :external_distributed_virtual_switches, :dependent => :destroy, :foreign_key => :ems_id, :inverse_of => :ext_management_system
has_many :external_distributed_virtual_lans, -> { distinct }, :through => :external_distributed_virtual_switches, :source => :lans
has_many :iso_datastores, :dependent => :destroy, :foreign_key => :ems_id, :inverse_of => :ext_management_system
has_many :iso_images, :through => :storages

include HasNetworkManagerMixin

Expand All @@ -42,7 +45,7 @@ class ManageIQ::Providers::Ovirt::InfraManager < ManageIQ::Providers::InfraManag
end

supports :create_iso_datastore do
unsupported_reason_add(:create_iso_datastore, _("Already has an ISO datastore")) if iso_datastore
unsupported_reason_add(:create_iso_datastore, _("Already has an ISO datastore")) unless iso_datastores.empty?
end

def ensure_managers
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class ManageIQ::Providers::Ovirt::InfraManager::IsoDatastore < ManageIQ::Providers::Ovirt::InfraManager::Storage
supports :iso_datastore

belongs_to :ext_management_system, :foreign_key => :ems_id

def self.display_name(number = 1)
n_('ISO Datastore', 'ISO Datastores', number)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,8 @@ def advertised_images

sd_service = ems_service.system_service.storage_domains_service.storage_domain_service(iso_sd.id)
iso_images = sd_service.files_service.list
iso_images.collect(&:name)

iso_images
end
rescue OvirtSDK4::Error => err
name = ext_management_system.try(:name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ def templates
collected_inventory[:template]
end

def advertised_images
manager.ovirt_services.advertised_images
end

def collect_networks
collected_inventory[:network]
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ def templates
t
end

def advertised_images
[]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we do handle targeted refresh of storage domains, but the manager.ovirt_services.advertised_images call only works for all storagedomains.

A good follow-up would be to extract the advertised_images method into the collector, then adding a targeted advertised_images that looks at references(:storagedomains)

end

def parse_targets!
target.targets.each do |t|
case t
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def parse
vms
networks
vnic_profiles
advertised_images

$rhevm_log.info("#{log_header}...Complete")
end
Expand Down Expand Up @@ -85,7 +86,7 @@ def storagedomains
storage_type = storagedomain.dig(:storage, :type).upcase
ems_ref = ManageIQ::Providers::Ovirt::InfraManager.make_ems_ref(storagedomain.try(:href))
location = case storage_type
when 'LOCALFS'
when 'LOCALFS', 'ISO'
ems_ref
when 'NFS', 'GLUSTERFS'
"#{storagedomain.dig(:storage, :address)}:#{storagedomain.dig(:storage, :path)}"
Expand All @@ -98,17 +99,32 @@ def storagedomains
total = free + used
committed = storagedomain.try(:committed).to_i

storage_domain_type = storagedomain.dig(:type, :downcase)
agrare marked this conversation as resolved.
Show resolved Hide resolved
type = storage_domain_type == 'iso' ? "IsoDatastore" : "Storage"

persister.storages.find_or_build(ems_ref).assign_attributes(
:ems_ref => ems_ref,
:name => storagedomain.try(:name),
:store_type => storage_type,
:storage_domain_type => storagedomain.dig(:type, :downcase),
:storage_domain_type => storage_domain_type,
:total_space => total,
:free_space => free,
:uncommitted => total - committed,
:multiplehostaccess => true,
:location => location,
:master => storagedomain.try(:master)
:master => storagedomain.try(:master),
:type => "ManageIQ::Providers::Ovirt::InfraManager::#{type}"
)
end
end

def advertised_images
collector.advertised_images.each do |image|
nasark marked this conversation as resolved.
Show resolved Hide resolved
parent_ems_ref = ManageIQ::Providers::Ovirt::InfraManager.make_ems_ref(image.dig(:storage_domain, :href))

persister.iso_images.build(
:name => image.name,
:storage => persister.storages.lazy_find(parent_ems_ref)
)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def initialize_inventory_collections
add_collection(infra, :networks)
add_collection(infra, :operating_systems)
add_collection(infra, :vms)
add_collection(infra, :iso_images)

add_miq_templates
add_resource_pools
Expand Down
3 changes: 3 additions & 0 deletions spec/factories/iso_datastore.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
FactoryBot.define do
factory :iso_datastore, :class => 'ManageIQ::Providers::Ovirt::InfraManager::IsoDatastore', :parent => :storage_redhat
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
RSpec.describe ManageIQ::Providers::Ovirt::InfraManager::IsoDatastore do
include Spec::Support::ArelHelper

let(:ems) { FactoryBot.create(:ems_redhat) }
let(:iso_datastore) { FactoryBot.create(:iso_datastore, :ext_management_system => ems, :name => ems.name) }

describe "#advertised_images" do
subject(:advertised_images) { ems.ovirt_services.advertised_images }

context "ems is rhv" do
context "supports api4" do
it "send the method to ovirt services v4" do
expect_any_instance_of(ManageIQ::Providers::Redhat::InfraManager::OvirtServices::V4)
.to receive(:advertised_images)
advertised_images
end
end
end

describe "#name" do
it "has a name" do
expect(iso_datastore.name).to eq(ems.name)
end

it "fetches name via sql" do
iso_datastore
expect(virtual_column_sql_value(Storage, "name")).to eq(ems.name)
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
context "there are iso domains attached to the data-center" do
context "there are active iso domains" do
it 'returns iso images from an active domain' do
expect(advertised_images).to match_array(%w[iso_1 iso_2])
res = advertised_images.map { |img| img.name }
expect(res).to match_array(%w[iso_1 iso_2])
end
end

Expand Down
Loading