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

Migrations for suspending provider #222

Closed
wants to merge 10 commits into from
Prev Previous commit
Renaming maintenance zone's string associations.
Like Job,MiqTask,MiqQueue + raises error when renaming existing zone to another existing.
slemrmartin committed Sep 10, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 87bbb6a1e2aa5390dee39bfd64999402e83a7e1c
82 changes: 74 additions & 8 deletions db/migrate/20180626125655_rename_user_maintenance_zone_record.rb
Original file line number Diff line number Diff line change
@@ -1,30 +1,96 @@
class RenameUserMaintenanceZoneRecord < ActiveRecord::Migration[5.0]
class UniqueWithinRegionValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
return if value.nil?
match_case = options.key?(:match_case) ? options[:match_case] : true
record_base_class = record.class.base_class
field_matches =
if match_case
record_base_class.where(attribute => value)
else
record_base_class.where(record_base_class.arel_attribute(attribute).lower.eq(value.downcase))
end
unless field_matches.in_region(record.region_id).where.not(:id => record.id).empty?
record.errors.add(attribute, "is not unique within region #{record.region_id}")
end
end
end

class Zone < ActiveRecord::Base
include ActiveRecord::IdRegions

validates_presence_of :name
validates_presence_of :name
validates :name, "rename_user_maintenance_zone_record/unique_within_region" => true

MAINTENANCE_ZONE_NAME = "__maintenance__".freeze
end

class Job < ActiveRecord::Base
self.inheritance_column = :_type_disabled

belongs_to :miq_task, :dependent => :delete
after_update_commit :update_linked_task

def update_linked_task
miq_task&.update_attributes!(:zone => zone)
end
end

class MiqTask < ActiveRecord::Base
has_one :job, :dependent => :destroy
has_one :miq_queue
end

class MiqQueue < ActiveRecord::Base
belongs_to :miq_task
end

def up
say_with_time("Renaming user-defined Maintenance Zone") do
zone = Zone.in_my_region.where(:name => Zone::MAINTENANCE_ZONE_NAME).first
zone = Zone.in_my_region.find_by(:name => Zone::MAINTENANCE_ZONE_NAME)
if zone.present?
zone.name = "#{zone.name}_0"
zone.save
# STEP 1 - rename zone
Copy link
Member

Choose a reason for hiding this comment

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

I still think this is the wrong approach

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bdunne you were worried about string associations

  1. possibility that user has __maintenance__ as name is almost zero
  2. if so, zone and associations are renamed to name with possibility of duplication zero^2.

could you be more specific? When this fails (with significant probability).
Is it not acceptable for @Fryguy, @agrare, @carbonin too?

Copy link
Member

Choose a reason for hiding this comment

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

  1. possibility that user has maintenance as name is almost zero

I agree, but while unlikely, not impossible

  1. if so, zone and associations are renamed to name with possibility of duplication zero^2.

Right now you're only updating Job and MiqQueue. The following models have a zone column:

=> ["MiqQueue", "Job", "MiqTask", "ManageIQ::Providers::Kubernetes::ContainerManager::Scanning::Job", "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::PlaybookRunner", "ManageIQ::Providers::EmsRefreshWorkflow", "ManageIQ::Providers::AnsibleRunnerWorkflow", "VmScan", "ManageIQ::Providers::NativeOperationWorkflow", "Service::LinkingWorkflow"]

I also believe it's in Settings and who knows how many other places it's buried in a serialized column. I just don't think it's practical to try renaming a zone

zone.name = "#{zone.name}_#{Zone.my_region_number}"
if zone.save
# STEP 2 - rename string associations
rename_string_associations(Zone::MAINTENANCE_ZONE_NAME, zone.name)
else
raise "Zone '#{Zone::MAINTENANCE_ZONE_NAME}' cannot be renamed to #{zone.name}. Reason: #{zone.errors.messages.inspect}"
end
end
end
end

def down
say_with_time("Renaming user-defined Maintenance Zone") do
Zone.in_my_region.where(:name => Zone::MAINTENANCE_ZONE_NAME).where(:visible => false).destroy_all
Zone.in_my_region.where(:name => Zone::MAINTENANCE_ZONE_NAME,
:visible => false).destroy_all

orig = Zone.in_my_region.where(:name => "#{Zone::MAINTENANCE_ZONE_NAME}_0").first
user_zone_name = "#{Zone::MAINTENANCE_ZONE_NAME}_#{Zone.my_region_number}"
orig = Zone.in_my_region.find_by(:name => user_zone_name)
if orig.present?
orig.name = orig.name[0..orig.name.size - 3]
orig.save
# STEP 1 - rename user's zone back
orig.name = Zone::MAINTENANCE_ZONE_NAME
if orig.save
# STEP 2 - rename string associations
rename_string_associations(user_zone_name, Zone::MAINTENANCE_ZONE_NAME)
else
raise "Zone '#{user_zone_name}' cannot be renamed to #{Zone::MAINTENANCE_ZONE_NAME}. Reason: #{orig.errors.messages.inspect}"
end
end
end
end

private

def rename_string_associations(old_zone_name, new_zone_name)
[Job, MiqQueue].each do |klass|
klass.where(:zone => old_zone_name).each do |record|
record.zone = new_zone_name
unless record.save
# This is not critical, migrations can continue
say("WARN: #{klass.name}'s (id: #{record.id}) zone not renamed. Reason: #{record.errors.messages.inspect}")
end
end
end
end
Original file line number Diff line number Diff line change
@@ -2,6 +2,9 @@

describe RenameUserMaintenanceZoneRecord do
let(:zone_stub) { migration_stub(:Zone) }
let(:job_stub) { migration_stub(:Job) }
let(:miq_task_stub) { migration_stub(:MiqTask) }
let(:miq_queue_stub) { migration_stub(:MiqQueue) }

let(:remote_region_start) do
anonymous_class_with_id_regions.rails_sequence_start +
@@ -15,7 +18,7 @@
migrate
orig.reload

expect(orig.name).to eq("#{zone_stub::MAINTENANCE_ZONE_NAME}_0")
expect(orig.name).to eq("#{zone_stub::MAINTENANCE_ZONE_NAME}_#{zone_stub.my_region_number}")
end

it "does not rename zone from remote regions" do
@@ -29,6 +32,47 @@

expect(user_zone.name).to eq(zone_stub::MAINTENANCE_ZONE_NAME)
end

it "renames string associations belonging to former maintenance zone" do
orig = zone_stub.create!(:name => zone_stub::MAINTENANCE_ZONE_NAME)
assoc = {}
assoc[:miq_task] = miq_task_stub.create!(:zone => orig.name)
assoc[:job] = job_stub.create!(:zone => orig.name, :miq_task => assoc[:miq_task])
assoc[:miq_queue] = miq_queue_stub.create!(:zone => orig.name, :miq_task => assoc[:miq_task])

migrate
orig.reload
assoc.each_value do |record|
record.reload
expect(record.zone).to eq(orig.name)
end
end

it "does not rename unrelated jobs, tasks and queues" do
orig = zone_stub.create!(:name => zone_stub::MAINTENANCE_ZONE_NAME)
other_zone_name = "Some other zone"
other = zone_stub.create!(:name => other_zone_name)

assoc = {}
assoc[:miq_task] = miq_task_stub.create!(:zone => other.name)
assoc[:job] = job_stub.create!(:zone => other.name, :miq_task => assoc[:miq_task])
assoc[:miq_queue] = miq_queue_stub.create!(:zone => other.name, :miq_task => assoc[:miq_task])

migrate
orig.reload
other.reload
expect(other.name).to eq(other_zone_name)
assoc.each_value do |record|
record.reload
expect(record.zone).to eq(other_zone_name)
end
end

it "raises error when original zone renamed to existing one" do
zone_stub.create!(:name => zone_stub::MAINTENANCE_ZONE_NAME)
zone_stub.create!(:name => "#{zone_stub::MAINTENANCE_ZONE_NAME}_#{zone_stub.my_region_number}")
expect { migrate }.to raise_error(/is not unique within region/)
end
end

migration_context :down do
@@ -43,7 +87,7 @@
end

it "renames original maintenance zone back" do
orig = zone_stub.create!(:name => "#{zone_stub::MAINTENANCE_ZONE_NAME}_0")
orig = zone_stub.create!(:name => "#{zone_stub::MAINTENANCE_ZONE_NAME}_#{zone_stub.my_region_number}")

migrate
orig.reload
@@ -55,12 +99,27 @@
id = remote_region_start

user_zone = zone_stub.create!(:id => id,
:name => "#{zone_stub::MAINTENANCE_ZONE_NAME}_0")
:name => "#{zone_stub::MAINTENANCE_ZONE_NAME}_#{zone_stub.my_region_number}")

migrate
user_zone.reload

expect(user_zone.name).to eq("#{zone_stub::MAINTENANCE_ZONE_NAME}_0")
expect(user_zone.name).to eq("#{zone_stub::MAINTENANCE_ZONE_NAME}_#{zone_stub.my_region_number}")
end

it "renames string associations belonging to original maintenance zone back" do
orig = zone_stub.create!(:name => "#{zone_stub::MAINTENANCE_ZONE_NAME}_#{zone_stub.my_region_number}")
assoc = {}
assoc[:miq_task] = miq_task_stub.create!(:zone => orig.name)
assoc[:job] = job_stub.create!(:zone => orig.name, :miq_task => assoc[:miq_task])
assoc[:miq_queue] = miq_queue_stub.create!(:zone => orig.name, :miq_task => assoc[:miq_task])

migrate
orig.reload
assoc.each_value do |record|
record.reload
expect(record.zone).to eq(orig.name)
end
end
end
end