Merge pull request #18068 from bdunne/gap_17987

[GAPRINDASHVILI] Prevent queueing things for a zone that doesn't exist in the region

app/models/miq_queue.rb

@@ -93,6 +93,8 @@ def self.lower_priority?(p1, p2)
  serialize :args, Array
  serialize :miq_callback, Hash
 
+ validates :zone, :inclusion => {:in => proc { Zone.in_my_region.pluck(:name) }}, :allow_nil => true
+
  STATE_READY = 'ready'.freeze
  STATE_DEQUEUE = 'dequeue'.freeze
  STATE_WARN = 'warn'.freeze

spec/lib/extensions/ar_nested_count_by_spec.rb

@@ -1,44 +1,45 @@
 describe "AR Nested Count By extension" do
  context "miq_queue with messages" do
- before(:each) do
- @zone = EvmSpecHelper.local_miq_server.zone
-
- FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_DEQUEUE, :role => "role1", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_DEQUEUE, :role => "role1", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_DEQUEUE, :role => "role2", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_READY, :role => "role1", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_READY, :role => "role1", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_READY, :role => "role1", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => "east", :state => MiqQueue::STATE_DEQUEUE, :role => "role1", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => "west", :state => MiqQueue::STATE_READY, :role => "role3", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_ERROR, :role => "role1", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_WARN, :role => "role3", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => "east", :state => MiqQueue::STATE_DEQUEUE, :role => "role1", :priority => 20)
- FactoryGirl.create(:miq_queue, :zone => "west", :state => MiqQueue::STATE_ERROR, :role => "role2", :priority => 20)
+ let(:zone1) { EvmSpecHelper.local_miq_server.zone }
+ let(:zone2) { FactoryGirl.create(:zone) }
+ let(:zone3) { FactoryGirl.create(:zone) }
+ before do
+ FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_DEQUEUE, :role => "role1", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_DEQUEUE, :role => "role1", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_DEQUEUE, :role => "role2", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_READY, :role => "role1", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_READY, :role => "role1", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_READY, :role => "role1", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone2.name, :state => MiqQueue::STATE_DEQUEUE, :role => "role1", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone3.name, :state => MiqQueue::STATE_READY, :role => "role3", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_ERROR, :role => "role1", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_WARN, :role => "role3", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone2.name, :state => MiqQueue::STATE_DEQUEUE, :role => "role1", :priority => 20)
+ FactoryGirl.create(:miq_queue, :zone => zone3.name, :state => MiqQueue::STATE_ERROR, :role => "role2", :priority => 20)
  end
 
  it "should count by state, zone and role" do
  expect(MiqQueue.nested_count_by(%w(state zone role))).to eq(
  MiqQueue::STATE_READY => {
- @zone.name => {"role1" => 3},
- "west"  => {"role3" => 1},
+ zone1.name => {"role1" => 3},
+ zone3.name => {"role3" => 1},
  },
  MiqQueue::STATE_DEQUEUE => {
- @zone.name => {"role1" => 2, "role2" => 1},
- "east"  => {"role1" => 2},
+ zone1.name => {"role1" => 2, "role2" => 1},
+ zone2.name => {"role1" => 2},
  },
  MiqQueue::STATE_WARN => {
- @zone.name => {"role3" => 1},
+ zone1.name => {"role3" => 1},
  },
  MiqQueue::STATE_ERROR => {
- @zone.name => {"role1" => 1},
- "west"  => {"role2" => 1},
+ zone1.name => {"role1" => 1},
+ zone3.name => {"role2" => 1},
  }
  )
  end
 
  it "should respect nested where, and support individual args (vs an array)" do
- expect(MiqQueue.where(:zone => "west").nested_count_by("role", "state")).to eq(
+ expect(MiqQueue.where(:zone => zone3.name).nested_count_by("role", "state")).to eq(
  "role3" => {MiqQueue::STATE_READY => 1},
  "role2" => {MiqQueue::STATE_ERROR => 1},
  )

spec/models/miq_action_spec.rb

@@ -178,8 +178,8 @@
 
  it "asynchronous" do
  input = {:synchronous => false}
- zone = 'Test Zone'
- allow(@vm).to receive_messages(:my_zone => zone)
+ zone = FactoryGirl.create(:zone)
+ allow(@vm).to receive_messages(:my_zone => zone.name)
 
  Timecop.freeze do
  date = Time.now.utc - 1.day
@@ -190,7 +190,7 @@
  expect(msg.class_name).to eq(@vm.class.name)
  expect(msg.method_name).to eq('retire')
  expect(msg.args).to eq([[@vm], :date => date])
- expect(msg.zone).to eq(zone)
+ expect(msg.zone).to eq(zone.name)
  end
  end
  end

spec/models/miq_queue_spec.rb

@@ -130,7 +130,7 @@
  ]
 
  message_parms.each do |mparms|
- msg = FactoryGirl.create(:miq_queue, mparms)
+ msg = FactoryGirl.build(:miq_queue, mparms)
  expect(MiqQueue.format_short_log_msg(msg)).to eq("Message id: [#{msg.id}]")
  expect(MiqQueue.format_full_log_msg(msg)).to eq("Message id: [#{msg.id}], #{msg.handler_type} id: [#{msg.handler_id}], Zone: [#{msg.zone}], Role: [#{msg.role}], Server: [#{msg.server_guid}], Ident: [#{msg.queue_name}], Target id: [#{msg.target_id}], Instance id: [#{msg.instance_id}], Task id: [#{msg.task_id}], Command: [#{msg.class_name}.#{msg.method_name}], Timeout: [#{msg.msg_timeout}], Priority: [#{msg.priority}], State: [#{msg.state}], Deliver On: [#{msg.deliver_on}], Data: [#{msg.data.nil? ? "" : "#{msg.data.length} bytes"}], Args: #{args_cleaned_password.inspect}")
  end
@@ -661,10 +661,11 @@ def self.some_method(single_arg)
  end
 
  it "should not unqueue a message from a different zone" do
+ zone = FactoryGirl.create(:zone)
  MiqQueue.put(
  :class_name => 'MyClass',
  :method_name => 'method1',
- :zone => 'other_zone'
+ :zone => zone.name
  )
 
  expect(MiqQueue.unqueue(
@@ -730,4 +731,19 @@ def self.some_method(single_arg)
  expect(MiqQueue.where(:id => q.id).count).to eq(0)
  end
  end
+
+ context "validates that the zone exists in the current region" do
+ it "with a matching region" do
+ zone = FactoryGirl.create(:zone)
+ expect(MiqQueue.create!(:state => "ready", :zone => zone.name)).to be_kind_of(MiqQueue)
+ end
+
+ it "without a matching region" do
+ expect { MiqQueue.create!(:state => "ready", :zone => "Missing Zone") }.to raise_error(ActiveRecord::RecordInvalid)
+ end
+
+ it "without a zone" do
+ expect(MiqQueue.create!(:state => "ready")).to be_kind_of(MiqQueue)
+ end
+ end
 end

spec/models/miq_request_spec.rb

@@ -68,7 +68,8 @@
  end
 
  it "#call_automate_event_queue" do
- allow(MiqServer).to receive(:my_zone).and_return("New York")
+ zone = FactoryGirl.create(:zone)
+ allow(MiqServer).to receive(:my_zone).and_return(zone.name)
 
  expect(MiqQueue.count).to eq(0)
 
@@ -79,7 +80,7 @@
  expect(msg.class_name).to eq(request.class.name)
  expect(msg.instance_id).to eq(request.id)
  expect(msg.method_name).to eq("call_automate_event")
- expect(msg.zone).to eq("New York")
+ expect(msg.zone).to eq(zone.name)
  expect(msg.args).to eq([event_name])
  expect(msg.msg_timeout).to eq(1.hour)
  end

spec/models/miq_task_spec.rb

@@ -179,8 +179,8 @@
  end
 
  it "should properly process MiqTask#generic_action_with_callback" do
- zone = 'New York'
- allow(MiqServer).to receive(:my_zone).and_return(zone)
+ zone = FactoryGirl.create(:zone)
+ allow(MiqServer).to receive(:my_zone).and_return(zone.name)
  opts = {
  :action => 'Feed',
  :userid => 'Flintstone'
@@ -203,35 +203,35 @@
  expect(message.class_name).to eq("MyClass")
  expect(message.method_name).to eq("my_method")
  expect(message.args).to eq([1, 2, 3])
- expect(message.zone).to eq(zone)
+ expect(message.zone).to eq(zone.name)
  end
  end
 
  context "when there are multiple MiqTasks" do
- before(:each) do
-  @miq_task1 = FactoryGirl.create(:miq_task_plain)
-  @miq_task2 = FactoryGirl.create(:miq_task_plain)
-  @miq_task3 = FactoryGirl.create(:miq_task_plain)
-  @zone = 'New York'
- allow(MiqServer).to receive(:my_zone).and_return(@zone)
+ let(:miq_task1) { FactoryGirl.create(:miq_task_plain) }
+ let(:miq_task2) { FactoryGirl.create(:miq_task_plain) }
+ let(:miq_task3) { FactoryGirl.create(:miq_task_plain) }
+ let(:zone) { FactoryGirl.create(:zone) }
+ before do
+ allow(MiqServer).to receive(:my_zone).and_return(zone.name)
  end
 
  it "should queue up deletes when calling MiqTask.delete_by_id" do
- MiqTask.delete_by_id([@miq_task1.id, @miq_task3.id])
+ MiqTask.delete_by_id([miq_task1.id, miq_task3.id])
  expect(MiqQueue.count).to eq(1)
  message = MiqQueue.first
 
  expect(message.class_name).to eq("MiqTask")
  expect(message.method_name).to eq("destroy")
  expect(message.args).to be_kind_of(Array)
  expect(message.args.length).to eq(1)
- expect(message.args.first).to match_array([@miq_task1.id, @miq_task3.id])
- expect(message.zone).to eq(@zone)
+ expect(message.args.first).to match_array([miq_task1.id, miq_task3.id])
+ expect(message.zone).to eq(zone.name)
  end
 
  it "should queue up proper deletes when calling MiqTask.delete_older" do
- Timecop.travel(10.minutes.ago) { @miq_task2.state_queued }
- Timecop.travel(12.minutes.ago) { @miq_task3.state_queued }
+ Timecop.travel(10.minutes.ago) { miq_task2.state_queued }
+ Timecop.travel(12.minutes.ago) { miq_task3.state_queued }
  date_5_minutes_ago = 5.minutes.ago.utc
  condition = "name LIKE 'name LIKE 'Performance rollup for %''"
  MiqTask.delete_older(date_5_minutes_ago, condition)
@@ -245,7 +245,7 @@
  expect(message.args.length).to eq(2)
  expect(message.args.first).to eq(date_5_minutes_ago)
  expect(message.args.second).to eq(condition)
- expect(message.zone).to eq(@zone)
+ expect(message.zone).to eq(zone.name)
 
  message.destroy
 
@@ -261,7 +261,7 @@
  expect(message.args.length).to eq(2)
  expect(message.args.first).to eq(date_11_minutes_ago)
  expect(message.args.second).to be nil
- expect(message.zone).to eq(@zone)
+ expect(message.zone).to eq(zone.name)
  end
  end
 

spec/models/mixins/authentication_mixin_spec.rb

@@ -89,8 +89,9 @@ def self.name; "TestClass"; end
  end
 
  context "authorization event and check for container providers" do
- before(:each) do
- allow(MiqServer).to receive(:my_zone).and_return("default")
+ before do
+ zone = FactoryGirl.create(:zone)
+ allow(MiqServer).to receive(:my_zone).and_return(zone.name)
  end
 
  it "should be triggered for kubernetes" do