From 18aa014cd115e7ab89a668d5f995f5ac573ea25d Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Tue, 27 Oct 2020 14:22:03 -0400 Subject: [PATCH] extend relationships to define the appropriate class names use parent definition of ensure since they are now pretty generic. They may be no longer necessary and a build_ may suffice. Use parent definition of propagation so all attributes go across Extend parent managers to group the functionality together --- .../openstack/cinder_manager_mixin.rb | 2 +- .../providers/openstack/cloud_manager.rb | 59 ++++++------------- .../openstack/network_manager_mixin.rb | 11 ++++ .../openstack/swift_manager_mixin.rb | 11 ++++ 4 files changed, 40 insertions(+), 43 deletions(-) create mode 100644 app/models/manageiq/providers/openstack/network_manager_mixin.rb create mode 100644 app/models/manageiq/providers/openstack/swift_manager_mixin.rb diff --git a/app/models/manageiq/providers/openstack/cinder_manager_mixin.rb b/app/models/manageiq/providers/openstack/cinder_manager_mixin.rb index 99db57bac..dc599f0b7 100644 --- a/app/models/manageiq/providers/openstack/cinder_manager_mixin.rb +++ b/app/models/manageiq/providers/openstack/cinder_manager_mixin.rb @@ -7,7 +7,7 @@ module ManageIQ::Providers::Openstack::CinderManagerMixin # Should use has_many :storage_managers, has_one :cinder_manager, :foreign_key => :parent_ems_id, - :class_name => "ManageIQ::Providers::StorageManager::CinderManager", + :class_name => "ManageIQ::Providers::Openstack::StorageManager::CinderManager", :autosave => true delegate :cloud_volumes, diff --git a/app/models/manageiq/providers/openstack/cloud_manager.rb b/app/models/manageiq/providers/openstack/cloud_manager.rb index 5806eb6d3..cb5ebda62 100644 --- a/app/models/manageiq/providers/openstack/cloud_manager.rb +++ b/app/models/manageiq/providers/openstack/cloud_manager.rb @@ -25,13 +25,18 @@ class ManageIQ::Providers::Openstack::CloudManager < ManageIQ::Providers::CloudM require_nested :Template require_nested :Vm + # may want to just us an array of child managers instead of this + # it looks cleaner having this relationship, + # but this will not be in sync with cinder/swift manager references has_many :storage_managers, :foreign_key => :parent_ems_id, :class_name => "ManageIQ::Providers::StorageManager", :autosave => true has_many :snapshots, :through => :vms_and_templates + include ManageIQ::Providers::Openstack::CinderManagerMixin - include SwiftManagerMixin + include ManageIQ::Providers::Openstack::SwiftManagerMixin + include ManageIQ::Providers::Openstack::NetworkManagerMixin include ManageIQ::Providers::Openstack::ManagerMixin include ManageIQ::Providers::Openstack::IdentitySyncMixin @@ -49,9 +54,12 @@ class ManageIQ::Providers::Openstack::CloudManager < ManageIQ::Providers::CloudM supports :swift_service supports :create_host_aggregate + # TODO: move to after_initialization before_create :ensure_managers + # TODO: move to before_validation before_update :ensure_managers_zone_and_provider_region + # TODO: after fixing inverse_of, this may go away after_save :refresh_parent_infra_manager private_class_method def self.provider_id_options @@ -419,52 +427,19 @@ def ensure_managers ensure_network_manager ensure_cinder_manager ensure_swift_manager + # TODO: remove when this moves to before_initialization ensure_managers_zone_and_provider_region end - def ensure_managers_zone_and_provider_region - if network_manager - network_manager.zone_id = zone_id - network_manager.tenant_id = tenant_id - network_manager.provider_region = provider_region - end - - if cinder_manager - cinder_manager.zone_id = zone_id - cinder_manager.tenant_id = tenant_id - cinder_manager.provider_region = provider_region - end - - if swift_manager - swift_manager.zone_id = zone_id - swift_manager.tenant_id = tenant_id - swift_manager.provider_region = provider_region - end - end - - def ensure_network_manager - build_network_manager(:type => 'ManageIQ::Providers::Openstack::NetworkManager') unless network_manager + # sigh, methods like this always start out innocent enough + def child_manager_references + [network_manager, cinder_manager, swift_manager].compact end - def ensure_cinder_manager - return false if cinder_manager - build_cinder_manager(:type => 'ManageIQ::Providers::Openstack::StorageManager::CinderManager') - true - end - - def ensure_swift_manager - return false if swift_manager - build_swift_manager(:type => 'ManageIQ::Providers::StorageManager::SwiftManager') - true - end - - after_save :save_on_other_managers - - def save_on_other_managers - storage_managers.update_all(:tenant_mapping_enabled => tenant_mapping_enabled) - if network_manager - network_manager.tenant_mapping_enabled = tenant_mapping_enabled - network_manager.save! + def ensure_managers_zone_and_provider_region + child_manager_references.each do |child_manager| + propagate_child_manager_attributes(child_manager) + child_manager.tenant_mapping_enabled = tenant_mapping_enabled end end diff --git a/app/models/manageiq/providers/openstack/network_manager_mixin.rb b/app/models/manageiq/providers/openstack/network_manager_mixin.rb new file mode 100644 index 000000000..87aecdaec --- /dev/null +++ b/app/models/manageiq/providers/openstack/network_manager_mixin.rb @@ -0,0 +1,11 @@ +module ManageIQ::Providers::Openstack::NetworkManagerMixin + extend ActiveSupport::Concern + include ::HasNetworkManagerMixin + + included do + has_one :network_manager, + :foreign_key => :parent_ems_id, + :class_name => "ManageIQ::Providers::Openstack::NetworkManager", + :autosave => true + end +end diff --git a/app/models/manageiq/providers/openstack/swift_manager_mixin.rb b/app/models/manageiq/providers/openstack/swift_manager_mixin.rb new file mode 100644 index 000000000..296047fdb --- /dev/null +++ b/app/models/manageiq/providers/openstack/swift_manager_mixin.rb @@ -0,0 +1,11 @@ +module ManageIQ::Providers::Openstack::SwiftManagerMixin + extend ActiveSupport::Concern + include ::SwiftManagerMixin + + included do + has_one :swift_manager, + :foreign_key => :parent_ems_id, + :class_name => "ManageIQ::Providers::StorageManager::SwiftManager", + :autosave => true + end +end