From ec3cb9e3175850143900b799883df917ccfddb09 Mon Sep 17 00:00:00 2001 From: Martin Slemr Date: Mon, 21 May 2018 15:51:29 +0200 Subject: [PATCH 1/9] Pause/Resume EMS Enables/Disables ems with children and puts to maintenance zone https://bugzilla.redhat.com/show_bug.cgi?id=1455145 --- app/models/ext_management_system.rb | 61 +++++++++++++++++++---- app/models/miq_queue.rb | 1 + app/models/zone.rb | 12 +++++ spec/models/ext_management_system_spec.rb | 47 +++++++++++++++++ spec/models/zone_spec.rb | 2 +- 5 files changed, 111 insertions(+), 12 deletions(-) diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb index 08d4a0f43d4..bef37bcb243 100644 --- a/app/models/ext_management_system.rb +++ b/app/models/ext_management_system.rb @@ -72,6 +72,7 @@ def self.api_allowed_attributes has_one :iso_datastore, :foreign_key => "ems_id", :dependent => :destroy, :inverse_of => :ext_management_system belongs_to :zone + belongs_to :backup_zone, :class_name => "Zone", :inverse_of => :paused_ext_management_systems # used for maintenance mode has_many :metrics, :as => :resource # Destroy will be handled by purger has_many :metric_rollups, :as => :resource # Destroy will be handled by purger @@ -186,6 +187,43 @@ def hostname_format_valid? default_value_for :enabled, true + after_save :change_maintenance_for_child_managers, :if => proc { |ems| ems.changed_attributes.include?('enabled') } + + def disable! + _log.info("Disabling EMS [#{name}] id [#{id}].") + update!(:enabled => false) + _log.info("Disabling EMS [#{name}] id [#{id}] successful.") + end + + def enable! + _log.info("Enabling EMS [#{name}] id [#{id}].") + update!(:enabled => true) + _log.info("Enabling EMS [#{name}] id [#{id}] successful.") + end + + # Move ems to maintenance zone and backup current one + # @param orig_zone [Zone] children original zones can be replaced in provider-specific callbacks + def pause!(orig_zone = nil) + _log.info("Pausing EMS [#{name}] id [#{id}].") + update!( + :backup_zone => orig_zone || zone, + :zone => Zone.maintenance_zone, + :enabled => false + ) + _log.info("Pausing EMS [#{name}] id [#{id}] successful.") + end + + # Move ems to original zone, reschedule task/jobs/.. collected during maintenance + def resume! + _log.info("Resuming EMS [#{name}] id [#{id}].") + update!( + :backup_zone => nil, + :zone => backup_zone || Zone.default_zone, + :enabled => true + ) + _log.info("Resuming EMS [#{name}] id [#{id}] successful.") + end + def self.with_ipaddress(ipaddress) joins(:endpoints).where(:endpoints => {:ipaddress => ipaddress}) end @@ -455,16 +493,6 @@ def self.ems_physical_infra_discovery_types @ems_physical_infra_discovery_types ||= %w(lenovo_ph_infra) end - def disable! - _log.info("Disabling EMS [#{name}] id [#{id}].") - update!(:enabled => false) - end - - def enable! - _log.info("Enabling EMS [#{name}] id [#{id}].") - update!(:enabled => true) - end - # override destroy_queue from AsyncDeleteMixin def self.destroy_queue(ids) find(Array.wrap(ids)).map(&:destroy_queue) @@ -745,6 +773,17 @@ def allow_targeted_refresh? private + # Child managers went to/from maintenance mode with parent + def change_maintenance_for_child_managers + child_managers.each do |child_ems| + if enabled? + child_ems.resume! + else + child_ems.pause!(backup_zone) + end + end + end + def build_connection(options = {}) build_endpoint_by_role(options[:endpoint]) build_authentication_by_role(options[:authentication]) @@ -765,7 +804,7 @@ def build_authentication_by_role(options) role = options.delete(:role) creds = {} creds[role] = options - update_authentication(creds,options) + update_authentication(creds, options) end def clear_association_cache diff --git a/app/models/miq_queue.rb b/app/models/miq_queue.rb index d41e2458bf5..e5eaebbc733 100644 --- a/app/models/miq_queue.rb +++ b/app/models/miq_queue.rb @@ -175,6 +175,7 @@ def self.put(options) def self.submit_job(options) service = options.delete(:service) || "generic" resource = options.delete(:affinity) + case service when "automate" # options[:queue_name] = "generic" diff --git a/app/models/zone.rb b/app/models/zone.rb index 3f85539314c..707898db1b9 100644 --- a/app/models/zone.rb +++ b/app/models/zone.rb @@ -8,6 +8,7 @@ class Zone < ApplicationRecord has_many :miq_servers has_many :ext_management_systems + has_many :paused_ext_management_systems, :class_name => 'ExtManagementSystem', :inverse_of => :backup_zone has_many :container_managers, :class_name => "ManageIQ::Providers::ContainerManager" has_many :miq_schedules, :dependent => :destroy has_many :providers @@ -36,6 +37,8 @@ class Zone < ApplicationRecord include AggregationMixin include ConfigurationManagementMixin + scope :visible, -> { where(:visible => true) } + def active_miq_servers MiqServer.active_miq_servers.where(:zone_id => id) end @@ -49,6 +52,9 @@ def find_master_server end def self.seed + create_with(:description => "Maintenance Zone", :visible => false).find_or_create_by!(:name => 'maintenance') do |_z| + _log.info("Creating maintenance zone...") + end create_with(:description => "Default Zone").find_or_create_by!(:name => 'default') do |_z| _log.info("Creating default zone...") end @@ -78,6 +84,11 @@ def self.default_zone in_my_region.find_by(:name => "default") end + # Zone for suspended providers (no servers in it), not visible by default + def self.maintenance_zone + unscoped.in_my_region.find_by(:name => "maintenance") + end + def remote_cockpit_ws_miq_server role_active?("cockpit_ws") ? miq_servers.find_by(:has_active_cockpit_ws => true) : nil end @@ -209,6 +220,7 @@ def ntp_reload_queue def check_zone_in_use_on_destroy raise _("cannot delete default zone") if name == "default" + raise _("cannot delete maintenance zone") if name == "maintenance" raise _("zone name '%{name}' is used by a server") % {:name => name} unless miq_servers.blank? end end diff --git a/spec/models/ext_management_system_spec.rb b/spec/models/ext_management_system_spec.rb index 7fe0fbe5b25..24e64aff75a 100644 --- a/spec/models/ext_management_system_spec.rb +++ b/spec/models/ext_management_system_spec.rb @@ -396,6 +396,53 @@ end end + context "#pause!" do + it 'disables an ems with child managers and moves them to maintenance zone' do + zone = FactoryGirl.create(:zone) + ems = FactoryGirl.create(:ext_management_system, :zone => zone) + child = FactoryGirl.create(:ext_management_system, :zone => zone) + ems.child_managers << child + + ems.pause! + + expect(ems.enabled).to be_falsy + expect(ems.zone).to eq(Zone.maintenance_zone) + expect(ems.backup_zone).to eq(zone) + + child.reload + expect(child.enabled).to be_falsy + expect(child.zone).to eq(Zone.maintenance_zone) + expect(child.backup_zone).to eq(zone) + end + end + + context "#resume" do + it "enables an ems with child managers and move them from maintenance zone" do + zone = FactoryGirl.create(:zone) + ems = FactoryGirl.create(:ext_management_system, + :backup_zone => zone, + :zone => Zone.maintenance_zone, + :enabled => false) + + child = FactoryGirl.create(:ext_management_system, + :backup_zone => zone, + :zone => Zone.maintenance_zone, + :enabled => false) + ems.child_managers << child + + ems.resume! + + expect(ems.enabled).to be_truthy + expect(ems.zone).to eq(zone) + expect(ems.backup_zone).to be_nil + + child.reload + expect(child.enabled).to be_truthy + expect(child.zone).to eq(zone) + expect(child.backup_zone).to be_nil + end + end + context "destroy" do it "destroys an ems with no active workers" do ems = FactoryGirl.create(:ext_management_system) diff --git a/spec/models/zone_spec.rb b/spec/models/zone_spec.rb index c74fac3a64b..2bc587ca1df 100644 --- a/spec/models/zone_spec.rb +++ b/spec/models/zone_spec.rb @@ -1,5 +1,5 @@ describe Zone do - include_examples ".seed called multiple times" + include_examples ".seed called multiple times", 2 context "with two small envs" do before do From 33526ec03672f5a7977a815edc812b53abfad95c Mon Sep 17 00:00:00 2001 From: Martin Slemr Date: Wed, 4 Jul 2018 13:57:55 +0200 Subject: [PATCH 2/9] Added default_value_for for Zone --- app/models/zone.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/zone.rb b/app/models/zone.rb index 707898db1b9..0cfbc7d9f83 100644 --- a/app/models/zone.rb +++ b/app/models/zone.rb @@ -38,6 +38,7 @@ class Zone < ApplicationRecord include ConfigurationManagementMixin scope :visible, -> { where(:visible => true) } + default_value_for :visible, true def active_miq_servers MiqServer.active_miq_servers.where(:zone_id => id) From 673673abfc3add2f967c8a9169f07cac9697f986 Mon Sep 17 00:00:00 2001 From: Martin Slemr Date: Tue, 10 Jul 2018 14:02:37 +0200 Subject: [PATCH 3/9] Maintenance zone constant Name as constant --- app/models/zone.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/zone.rb b/app/models/zone.rb index 0cfbc7d9f83..c369cf742c6 100644 --- a/app/models/zone.rb +++ b/app/models/zone.rb @@ -40,6 +40,8 @@ class Zone < ApplicationRecord scope :visible, -> { where(:visible => true) } default_value_for :visible, true + MAINTENANCE_ZONE_NAME = '__maintenance__'.freeze + def active_miq_servers MiqServer.active_miq_servers.where(:zone_id => id) end @@ -53,7 +55,7 @@ def find_master_server end def self.seed - create_with(:description => "Maintenance Zone", :visible => false).find_or_create_by!(:name => 'maintenance') do |_z| + create_with(:description => "Maintenance Zone", :visible => false).find_or_create_by!(:name => MAINTENANCE_ZONE_NAME) do |_z| _log.info("Creating maintenance zone...") end create_with(:description => "Default Zone").find_or_create_by!(:name => 'default') do |_z| @@ -87,7 +89,7 @@ def self.default_zone # Zone for suspended providers (no servers in it), not visible by default def self.maintenance_zone - unscoped.in_my_region.find_by(:name => "maintenance") + in_my_region.find_by(:name => MAINTENANCE_ZONE_NAME) end def remote_cockpit_ws_miq_server @@ -221,7 +223,7 @@ def ntp_reload_queue def check_zone_in_use_on_destroy raise _("cannot delete default zone") if name == "default" - raise _("cannot delete maintenance zone") if name == "maintenance" + raise _("cannot delete maintenance zone") if name == MAINTENANCE_ZONE_NAME raise _("zone name '%{name}' is used by a server") % {:name => name} unless miq_servers.blank? end end From 2b24726349168354b10614fb5d9cad581693ee47 Mon Sep 17 00:00:00 2001 From: Martin Slemr Date: Wed, 11 Jul 2018 14:02:46 +0200 Subject: [PATCH 4/9] Disabling MiqQueue for maintenance zone --- app/models/miq_queue.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/miq_queue.rb b/app/models/miq_queue.rb index e5eaebbc733..a6dd0d55f67 100644 --- a/app/models/miq_queue.rb +++ b/app/models/miq_queue.rb @@ -128,6 +128,11 @@ def self.put(options) :handler_id => nil, ) + if options[:zone] == Zone::MAINTENANCE_ZONE_NAME + _log.debug("MiqQueue#put skipped: #{options.inspect}") + return + end + create_with_options = all.values[:create_with] || {} options[:priority] ||= create_with_options[:priority] || NORMAL_PRIORITY options[:queue_name] ||= create_with_options[:queue_name] || "generic" From 402e405fcf7229a041f27e96c5f10f963cb52782 Mon Sep 17 00:00:00 2001 From: Martin Slemr Date: Wed, 11 Jul 2018 18:55:44 +0200 Subject: [PATCH 5/9] ExtManagementSystem and MiqServer validations for invisible zone EMS cannot set invisible zone when enabled and change when disabled, MiqServer cannot set invisible zone --- app/models/ext_management_system.rb | 18 ++++++++++++++ app/models/miq_server.rb | 6 +++++ spec/factories/ext_management_system.rb | 2 +- spec/models/ext_management_system_spec.rb | 30 +++++++++++++++++++++++ spec/models/miq_server_spec.rb | 8 ++++++ 5 files changed, 63 insertions(+), 1 deletion(-) diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb index bef37bcb243..ed3a046d9e7 100644 --- a/app/models/ext_management_system.rb +++ b/app/models/ext_management_system.rb @@ -89,6 +89,8 @@ def self.api_allowed_attributes validates :hostname, :presence => true, :if => :hostname_required? validate :hostname_uniqueness_valid?, :hostname_format_valid?, :if => :hostname_required? + validate :validate_ems_enabled_when_zone_changed?, :validate_zone_visible_when_ems_enabled? + scope :with_eligible_manager_types, ->(eligible_types) { where(:type => eligible_types) } serialize :options @@ -110,6 +112,22 @@ def hostname_format_valid? errors.add(:hostname, _("format is invalid.")) end + # validation - Zone cannot be changed when enabled == false + def validate_ems_enabled_when_zone_changed? + return if changed_attributes.include?('enabled') + + if changed_attributes.include?('zone_id') && !enabled? + errors.add(:zone, N_("cannot be changed when provider paused")) + end + end + + # validation - Zone has to be visible when enabled == true + def validate_zone_visible_when_ems_enabled? + if enabled? && zone.present? && !zone.visible? + errors.add(:zone, N_("cannot be invisible when provider active")) + end + end + include NewWithTypeStiMixin include UuidMixin include EmsRefresh::Manager diff --git a/app/models/miq_server.rb b/app/models/miq_server.rb index af2296a6b49..7bb9a23b142 100644 --- a/app/models/miq_server.rb +++ b/app/models/miq_server.rb @@ -39,6 +39,8 @@ class MiqServer < ApplicationRecord scope :with_zone_id, ->(zone_id) { where(:zone_id => zone_id) } virtual_delegate :description, :to => :zone, :prefix => true + validate :validate_zone_visible?, :if => proc { |server| server.zone.present? } + STATUS_STARTING = 'starting'.freeze STATUS_STARTED = 'started'.freeze STATUS_RESTARTING = 'restarting'.freeze @@ -53,6 +55,10 @@ class MiqServer < ApplicationRecord RESTART_EXIT_STATUS = 123 + def validate_zone_visible? + errors.add(:zone, N_('has to be visible')) unless zone.visible? + end + def hostname h = super h if h.to_s.hostname? diff --git a/spec/factories/ext_management_system.rb b/spec/factories/ext_management_system.rb index d021da928ef..984f2885482 100644 --- a/spec/factories/ext_management_system.rb +++ b/spec/factories/ext_management_system.rb @@ -4,7 +4,7 @@ sequence(:hostname) { |n| "ems-#{seq_padded_for_sorting(n)}" } sequence(:ipaddress) { |n| ip_from_seq(n) } guid { SecureRandom.uuid } - zone { Zone.first || FactoryGirl.create(:zone) } + zone { Zone.visible.first || FactoryGirl.create(:zone) } storage_profiles { [] } # Traits diff --git a/spec/models/ext_management_system_spec.rb b/spec/models/ext_management_system_spec.rb index 24e64aff75a..45af823f35c 100644 --- a/spec/models/ext_management_system_spec.rb +++ b/spec/models/ext_management_system_spec.rb @@ -443,6 +443,36 @@ end end + context "changing zone" do + it 'is allowed when enabled' do + zones = (1..2).collect { FactoryGirl.create(:zone) } + ems = FactoryGirl.create(:ext_management_system, :zone => zones[0]) + + ems.zone = zones[1] + expect(ems.save).to be_truthy + end + + it 'is denied when disabled' do + zones = (1..2).collect { FactoryGirl.create(:zone) } + ems = FactoryGirl.create(:ext_management_system, :zone => zones[0], :enabled => false) + + ems.zone = zones[1] + expect(ems.save).to be_falsy + expect(ems.errors.messages[:zone]).to be_present + end + + it 'to invisible is not possible when provider enabled' do + zone_visible = FactoryGirl.create(:zone) + zone_invisible = FactoryGirl.create(:zone, :visible => false) + + ems = FactoryGirl.create(:ext_management_system, :zone => zone_visible, :enabled => true) + + ems.zone = zone_invisible + expect(ems.save).to be_falsy + expect(ems.errors.messages[:zone]).to be_present + end + end + context "destroy" do it "destroys an ems with no active workers" do ems = FactoryGirl.create(:ext_management_system) diff --git a/spec/models/miq_server_spec.rb b/spec/models/miq_server_spec.rb index b18ffe04972..a2e7f8b55a1 100644 --- a/spec/models/miq_server_spec.rb +++ b/spec/models/miq_server_spec.rb @@ -91,6 +91,14 @@ expect(@miq_server.zone.name).to eq(@zone.name) end + it "cannot have invisible zone" do + zone = FactoryGirl.create(:zone, :visible => false) + + @miq_server.zone = zone + expect(@miq_server.save).to be_falsy + expect(@miq_server.errors.messages[:zone]).to be_present + end + it "shutdown will raise an event and quiesce" do expect(MiqEvent).to receive(:raise_evm_event) expect(@miq_server).to receive(:quiesce) From 93a54c0bb4faab5504a9f5482c065419dfd7f4f7 Mon Sep 17 00:00:00 2001 From: Martin Slemr Date: Thu, 12 Jul 2018 10:39:14 +0200 Subject: [PATCH 6/9] Suspending child managers doesn't change zone. It only changes enabled flag. --- app/models/ext_management_system.rb | 15 ++++++--------- spec/models/ext_management_system_spec.rb | 13 ++++--------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb index ed3a046d9e7..a4be6deb69c 100644 --- a/app/models/ext_management_system.rb +++ b/app/models/ext_management_system.rb @@ -220,11 +220,10 @@ def enable! end # Move ems to maintenance zone and backup current one - # @param orig_zone [Zone] children original zones can be replaced in provider-specific callbacks - def pause!(orig_zone = nil) + def pause! _log.info("Pausing EMS [#{name}] id [#{id}].") update!( - :backup_zone => orig_zone || zone, + :backup_zone => zone, :zone => Zone.maintenance_zone, :enabled => false ) @@ -793,12 +792,10 @@ def allow_targeted_refresh? # Child managers went to/from maintenance mode with parent def change_maintenance_for_child_managers - child_managers.each do |child_ems| - if enabled? - child_ems.resume! - else - child_ems.pause!(backup_zone) - end + if enabled? + child_managers.each(&:enable!) + else + child_managers.each(&:disable!) end end diff --git a/spec/models/ext_management_system_spec.rb b/spec/models/ext_management_system_spec.rb index 45af823f35c..658e33cedfc 100644 --- a/spec/models/ext_management_system_spec.rb +++ b/spec/models/ext_management_system_spec.rb @@ -397,7 +397,7 @@ end context "#pause!" do - it 'disables an ems with child managers and moves them to maintenance zone' do + it 'disables an ems with child managers and moves parent to maintenance zone' do zone = FactoryGirl.create(:zone) ems = FactoryGirl.create(:ext_management_system, :zone => zone) child = FactoryGirl.create(:ext_management_system, :zone => zone) @@ -411,13 +411,11 @@ child.reload expect(child.enabled).to be_falsy - expect(child.zone).to eq(Zone.maintenance_zone) - expect(child.backup_zone).to eq(zone) end end context "#resume" do - it "enables an ems with child managers and move them from maintenance zone" do + it "enables an ems with child managers and move parent from maintenance zone" do zone = FactoryGirl.create(:zone) ems = FactoryGirl.create(:ext_management_system, :backup_zone => zone, @@ -425,9 +423,8 @@ :enabled => false) child = FactoryGirl.create(:ext_management_system, - :backup_zone => zone, - :zone => Zone.maintenance_zone, - :enabled => false) + :zone => zone, + :enabled => false) ems.child_managers << child ems.resume! @@ -438,8 +435,6 @@ child.reload expect(child.enabled).to be_truthy - expect(child.zone).to eq(zone) - expect(child.backup_zone).to be_nil end end From bcce48fbf0bc8f438a202f0839b82497cddec33c Mon Sep 17 00:00:00 2001 From: Martin Slemr Date: Thu, 27 Sep 2018 12:15:28 +0200 Subject: [PATCH 7/9] MiqRegion.maintenance_zone assoc+EMS.zone_when_changed assoc maintenance zone identified by region and EMS.backup_zone renamed BZ https://bugzilla.redhat.com/show_bug.cgi?id=1455145 --- app/models/ext_management_system.rb | 14 +++---- app/models/miq_queue.rb | 3 +- app/models/miq_region.rb | 2 + app/models/zone.rb | 47 ++++++++++++++++++----- spec/models/ext_management_system_spec.rb | 10 ++--- spec/models/zone_spec.rb | 31 +++++++++++++++ 6 files changed, 84 insertions(+), 23 deletions(-) diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb index a4be6deb69c..55afc26381c 100644 --- a/app/models/ext_management_system.rb +++ b/app/models/ext_management_system.rb @@ -72,7 +72,7 @@ def self.api_allowed_attributes has_one :iso_datastore, :foreign_key => "ems_id", :dependent => :destroy, :inverse_of => :ext_management_system belongs_to :zone - belongs_to :backup_zone, :class_name => "Zone", :inverse_of => :paused_ext_management_systems # used for maintenance mode + belongs_to :zone_before_pause, :class_name => "Zone", :inverse_of => :paused_ext_management_systems # used for maintenance mode has_many :metrics, :as => :resource # Destroy will be handled by purger has_many :metric_rollups, :as => :resource # Destroy will be handled by purger @@ -223,9 +223,9 @@ def enable! def pause! _log.info("Pausing EMS [#{name}] id [#{id}].") update!( - :backup_zone => zone, - :zone => Zone.maintenance_zone, - :enabled => false + :zone_before_pause => zone, + :zone => Zone.maintenance_zone, + :enabled => false ) _log.info("Pausing EMS [#{name}] id [#{id}] successful.") end @@ -234,9 +234,9 @@ def pause! def resume! _log.info("Resuming EMS [#{name}] id [#{id}].") update!( - :backup_zone => nil, - :zone => backup_zone || Zone.default_zone, - :enabled => true + :zone_before_pause => nil, + :zone => zone_before_pause || Zone.default_zone, + :enabled => true ) _log.info("Resuming EMS [#{name}] id [#{id}] successful.") end diff --git a/app/models/miq_queue.rb b/app/models/miq_queue.rb index a6dd0d55f67..1d544aca869 100644 --- a/app/models/miq_queue.rb +++ b/app/models/miq_queue.rb @@ -128,7 +128,7 @@ def self.put(options) :handler_id => nil, ) - if options[:zone] == Zone::MAINTENANCE_ZONE_NAME + if options[:zone] == Zone.maintenance_zone&.name _log.debug("MiqQueue#put skipped: #{options.inspect}") return end @@ -180,7 +180,6 @@ def self.put(options) def self.submit_job(options) service = options.delete(:service) || "generic" resource = options.delete(:affinity) - case service when "automate" # options[:queue_name] = "generic" diff --git a/app/models/miq_region.rb b/app/models/miq_region.rb index 29d97802e7a..1894cfcb282 100644 --- a/app/models/miq_region.rb +++ b/app/models/miq_region.rb @@ -1,4 +1,6 @@ class MiqRegion < ApplicationRecord + belongs_to :maintenance_zone, :class_name => 'Zone', :inverse_of => :maintenance_zone_regions + has_many :metrics, :as => :resource # Destroy will be handled by purger has_many :metric_rollups, :as => :resource # Destroy will be handled by purger has_many :vim_performance_states, :as => :resource # Destroy will be handled by purger diff --git a/app/models/zone.rb b/app/models/zone.rb index c369cf742c6..a92c3a56b85 100644 --- a/app/models/zone.rb +++ b/app/models/zone.rb @@ -4,11 +4,12 @@ class Zone < ApplicationRecord serialize :settings, Hash - belongs_to :log_file_depot, :class_name => "FileDepot" + belongs_to :log_file_depot, :class_name => "FileDepot" has_many :miq_servers has_many :ext_management_systems - has_many :paused_ext_management_systems, :class_name => 'ExtManagementSystem', :inverse_of => :backup_zone + has_many :paused_ext_management_systems, :class_name => 'ExtManagementSystem', :inverse_of => :zone_before_pause + has_one :maintenance_zone_region, :class_name => 'MiqRegion', :inverse_of => :maintenance_zone has_many :container_managers, :class_name => "ManageIQ::Providers::ContainerManager" has_many :miq_schedules, :dependent => :destroy has_many :providers @@ -40,7 +41,7 @@ class Zone < ApplicationRecord scope :visible, -> { where(:visible => true) } default_value_for :visible, true - MAINTENANCE_ZONE_NAME = '__maintenance__'.freeze + MAINTENANCE_ZONE_NAME_PREFIX = '__maintenance__'.freeze def active_miq_servers MiqServer.active_miq_servers.where(:zone_id => id) @@ -54,10 +55,38 @@ def find_master_server active_miq_servers.detect(&:is_master?) end - def self.seed - create_with(:description => "Maintenance Zone", :visible => false).find_or_create_by!(:name => MAINTENANCE_ZONE_NAME) do |_z| - _log.info("Creating maintenance zone...") + def self.create_maintenance_zone + if maintenance_zone.nil? + # 1) Create region, if not exists + MiqRegion.seed + + # 2) Create Maintenance zone + threshold = 100 # avoiding infinite loop + zone = nil + (1..threshold).each do |idx| + zone = create(:name => "#{MAINTENANCE_ZONE_NAME_PREFIX}#{idx}", + :description => "Maintenance Zone", + :visible => false) + break if zone.valid? + end + + # 3) Assign zone to region + if zone&.valid? + region = zone.miq_region + region&.maintenance_zone = zone + unless region&.save + _log.error("Saving Maintenance zone to region failed with: #{region&.errors&.messages.inspect}") + end + _log.info("Creating maintenance zone...") + else + _log.error("Maintenance zone not created in #{threshold} attempts") + end end + end + + def self.seed + create_maintenance_zone + create_with(:description => "Default Zone").find_or_create_by!(:name => 'default') do |_z| _log.info("Creating default zone...") end @@ -87,9 +116,9 @@ def self.default_zone in_my_region.find_by(:name => "default") end - # Zone for suspended providers (no servers in it), not visible by default + # Zone for paused providers (no servers in it), not visible by default def self.maintenance_zone - in_my_region.find_by(:name => MAINTENANCE_ZONE_NAME) + MiqRegion.find_by(:region => my_region_number)&.maintenance_zone end def remote_cockpit_ws_miq_server @@ -223,7 +252,7 @@ def ntp_reload_queue def check_zone_in_use_on_destroy raise _("cannot delete default zone") if name == "default" - raise _("cannot delete maintenance zone") if name == MAINTENANCE_ZONE_NAME + raise _("cannot delete maintenance zone") if self == miq_region&.maintenance_zone raise _("zone name '%{name}' is used by a server") % {:name => name} unless miq_servers.blank? end end diff --git a/spec/models/ext_management_system_spec.rb b/spec/models/ext_management_system_spec.rb index 658e33cedfc..82e73106569 100644 --- a/spec/models/ext_management_system_spec.rb +++ b/spec/models/ext_management_system_spec.rb @@ -407,7 +407,7 @@ expect(ems.enabled).to be_falsy expect(ems.zone).to eq(Zone.maintenance_zone) - expect(ems.backup_zone).to eq(zone) + expect(ems.zone_before_pause).to eq(zone) child.reload expect(child.enabled).to be_falsy @@ -418,9 +418,9 @@ it "enables an ems with child managers and move parent from maintenance zone" do zone = FactoryGirl.create(:zone) ems = FactoryGirl.create(:ext_management_system, - :backup_zone => zone, - :zone => Zone.maintenance_zone, - :enabled => false) + :zone_before_pause => zone, + :zone => Zone.maintenance_zone, + :enabled => false) child = FactoryGirl.create(:ext_management_system, :zone => zone, @@ -431,7 +431,7 @@ expect(ems.enabled).to be_truthy expect(ems.zone).to eq(zone) - expect(ems.backup_zone).to be_nil + expect(ems.zone_before_pause).to be_nil child.reload expect(child.enabled).to be_truthy diff --git a/spec/models/zone_spec.rb b/spec/models/zone_spec.rb index 2bc587ca1df..79ff7571016 100644 --- a/spec/models/zone_spec.rb +++ b/spec/models/zone_spec.rb @@ -192,6 +192,37 @@ end end + context "maintenance zone" do + it "creates missing region during seed" do + expect(MiqRegion.find_by(:region => MiqRegion.my_region_number)).to be_nil + described_class.seed + expect(MiqRegion.find_by(:region => MiqRegion.my_region_number)).to be_present + end + + it "is seeded with relation to region" do + described_class.seed + zone = Zone.where("name LIKE ?", "#{Zone::MAINTENANCE_ZONE_NAME_PREFIX}%").first + + expect(described_class.maintenance_zone).to be_present + expect(described_class.maintenance_zone.id).to eq(zone.id) + expect(MiqRegion.find_by(:region => MiqRegion.my_region_number)&.maintenance_zone).to eq(zone) + end + + it "is not visible" do + described_class.seed + expect(described_class.maintenance_zone.visible).to be_falsey + end + + it "is created when default name exists" do + (1..5).each do |idx| + FactoryGirl.create(:zone, :name => "#{Zone::MAINTENANCE_ZONE_NAME_PREFIX}#{idx}") + end + described_class.seed + + expect(Zone.maintenance_zone.name).to eq("#{Zone::MAINTENANCE_ZONE_NAME_PREFIX}6") + end + end + context "validate multi region" do let!(:other_region_id) { ApplicationRecord.id_in_region(1, ApplicationRecord.my_region_number + 1) } let!(:default_in_other_region) { described_class.create(:name => "default", :description => "Default Zone", :id => other_region_id) } From bcbd8286864c1348b18f23592ccb8bfde5586f1c Mon Sep 17 00:00:00 2001 From: Martin Slemr Date: Thu, 27 Sep 2018 12:26:49 +0200 Subject: [PATCH 8/9] MiqQueue.put skip for maintenance_zone Spec and fix for nil zone BZ https://bugzilla.redhat.com/show_bug.cgi?id=1455145 --- app/models/miq_queue.rb | 2 +- app/models/miq_region.rb | 2 +- spec/models/miq_queue_spec.rb | 13 +++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/models/miq_queue.rb b/app/models/miq_queue.rb index 1d544aca869..833fa04ed5a 100644 --- a/app/models/miq_queue.rb +++ b/app/models/miq_queue.rb @@ -128,7 +128,7 @@ def self.put(options) :handler_id => nil, ) - if options[:zone] == Zone.maintenance_zone&.name + if options[:zone].present? && options[:zone] == Zone.maintenance_zone&.name _log.debug("MiqQueue#put skipped: #{options.inspect}") return end diff --git a/app/models/miq_region.rb b/app/models/miq_region.rb index 1894cfcb282..8dc899d7008 100644 --- a/app/models/miq_region.rb +++ b/app/models/miq_region.rb @@ -1,5 +1,5 @@ class MiqRegion < ApplicationRecord - belongs_to :maintenance_zone, :class_name => 'Zone', :inverse_of => :maintenance_zone_regions + belongs_to :maintenance_zone, :class_name => 'Zone', :inverse_of => :maintenance_zone_region has_many :metrics, :as => :resource # Destroy will be handled by purger has_many :metric_rollups, :as => :resource # Destroy will be handled by purger diff --git a/spec/models/miq_queue_spec.rb b/spec/models/miq_queue_spec.rb index 8920e8c7dc6..fc5867e1b8c 100644 --- a/spec/models/miq_queue_spec.rb +++ b/spec/models/miq_queue_spec.rb @@ -368,6 +368,19 @@ expect(MiqQueue.get).to eq(nil) end + it "should skip putting message on queue in maintenance zone" do + Zone.seed + + msg = MiqQueue.put( + :class_name => 'MyClass', + :method_name => 'method1', + :args => [1, 2], + :zone => Zone.maintenance_zone&.name + ) + + expect(MiqQueue.get).to eq(nil) + end + it "should accept non-Array args (for now)" do begin class MiqQueueSpecNonArrayArgs From 53dcf9828360b92ac100f293bcd50f60abcfe50d Mon Sep 17 00:00:00 2001 From: Martin Slemr Date: Mon, 1 Oct 2018 13:41:29 +0200 Subject: [PATCH 9/9] Make EMS's enable/disable methods protected --- app/models/ext_management_system.rb | 30 +++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb index 55afc26381c..81a04727533 100644 --- a/app/models/ext_management_system.rb +++ b/app/models/ext_management_system.rb @@ -207,18 +207,6 @@ def validate_zone_visible_when_ems_enabled? after_save :change_maintenance_for_child_managers, :if => proc { |ems| ems.changed_attributes.include?('enabled') } - def disable! - _log.info("Disabling EMS [#{name}] id [#{id}].") - update!(:enabled => false) - _log.info("Disabling EMS [#{name}] id [#{id}] successful.") - end - - def enable! - _log.info("Enabling EMS [#{name}] id [#{id}].") - update!(:enabled => true) - _log.info("Enabling EMS [#{name}] id [#{id}] successful.") - end - # Move ems to maintenance zone and backup current one def pause! _log.info("Pausing EMS [#{name}] id [#{id}].") @@ -788,14 +776,28 @@ def allow_targeted_refresh? Settings.ems_refresh.fetch_path(emstype, :allow_targeted_refresh) end + protected + + def disable! + _log.info("Disabling EMS [#{name}] id [#{id}].") + update!(:enabled => false) + _log.info("Disabling EMS [#{name}] id [#{id}] successful.") + end + + def enable! + _log.info("Enabling EMS [#{name}] id [#{id}].") + update!(:enabled => true) + _log.info("Enabling EMS [#{name}] id [#{id}] successful.") + end + private # Child managers went to/from maintenance mode with parent def change_maintenance_for_child_managers if enabled? - child_managers.each(&:enable!) + child_managers.each { |ems| ems.enable! } else - child_managers.each(&:disable!) + child_managers.each { |ems| ems.disable! } end end