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

[WIP] Make provider's enable/disable methods protected #18037

Closed
78 changes: 67 additions & 11 deletions app/models/ext_management_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 :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
Expand All @@ -88,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
Expand All @@ -109,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
Expand Down Expand Up @@ -186,6 +205,30 @@ def hostname_format_valid?

default_value_for :enabled, true

after_save :change_maintenance_for_child_managers, :if => proc { |ems| ems.changed_attributes.include?('enabled') }

# Move ems to maintenance zone and backup current one
def pause!
_log.info("Pausing EMS [#{name}] id [#{id}].")
update!(
:zone_before_pause => 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!(
:zone_before_pause => nil,
:zone => zone_before_pause || 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
Expand Down Expand Up @@ -455,16 +498,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)
Expand Down Expand Up @@ -743,8 +776,31 @@ 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 { |ems| ems.enable! }
else
child_managers.each { |ems| ems.disable! }
end
end

def build_connection(options = {})
build_endpoint_by_role(options[:endpoint])
build_authentication_by_role(options[:authentication])
Expand All @@ -765,7 +821,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
Expand Down
5 changes: 5 additions & 0 deletions app/models/miq_queue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ def self.put(options)
:handler_id => nil,
)

if options[:zone].present? && 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"
Expand Down
2 changes: 2 additions & 0 deletions app/models/miq_region.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class MiqRegion < ApplicationRecord
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
has_many :vim_performance_states, :as => :resource # Destroy will be handled by purger
Expand Down
6 changes: 6 additions & 0 deletions app/models/miq_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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?
Expand Down
46 changes: 45 additions & 1 deletion app/models/zone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +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 => :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
Expand Down Expand Up @@ -36,6 +38,11 @@ class Zone < ApplicationRecord
include AggregationMixin
include ConfigurationManagementMixin

scope :visible, -> { where(:visible => true) }
default_value_for :visible, true

MAINTENANCE_ZONE_NAME_PREFIX = '__maintenance__'.freeze

def active_miq_servers
MiqServer.active_miq_servers.where(:zone_id => id)
end
Expand All @@ -48,7 +55,38 @@ def find_master_server
active_miq_servers.detect(&:is_master?)
end

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
Expand Down Expand Up @@ -78,6 +116,11 @@ def self.default_zone
in_my_region.find_by(:name => "default")
end

# Zone for paused providers (no servers in it), not visible by default
def self.maintenance_zone
MiqRegion.find_by(:region => my_region_number)&.maintenance_zone
end

def remote_cockpit_ws_miq_server
role_active?("cockpit_ws") ? miq_servers.find_by(:has_active_cockpit_ws => true) : nil
end
Expand Down Expand Up @@ -209,6 +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 self == miq_region&.maintenance_zone
raise _("zone name '%{name}' is used by a server") % {:name => name} unless miq_servers.blank?
end
end
2 changes: 1 addition & 1 deletion spec/factories/ext_management_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 72 additions & 0 deletions spec/models/ext_management_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,78 @@
end
end

context "#pause!" 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)
ems.child_managers << child

ems.pause!

expect(ems.enabled).to be_falsy
expect(ems.zone).to eq(Zone.maintenance_zone)
expect(ems.zone_before_pause).to eq(zone)

child.reload
expect(child.enabled).to be_falsy
end
end

context "#resume" 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,
:zone_before_pause => zone,
:zone => Zone.maintenance_zone,
:enabled => false)

child = FactoryGirl.create(:ext_management_system,
:zone => zone,
:enabled => false)
ems.child_managers << child

ems.resume!

expect(ems.enabled).to be_truthy
expect(ems.zone).to eq(zone)
expect(ems.zone_before_pause).to be_nil

child.reload
expect(child.enabled).to be_truthy
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)
Expand Down
13 changes: 13 additions & 0 deletions spec/models/miq_queue_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions spec/models/miq_server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading