这是indexloc提供的服务，不要输入任何密码
Skip to content

[GAPRINDASHVILI] Add requester info to raise_retirement_event #18106

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 5, 2018
Merged

[GAPRINDASHVILI] Add requester info to raise_retirement_event #18106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 31 additions & 12 deletions app/models/mixins/retirement_mixin.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,19 @@ def raise_retire_audit_event(message)

def retirement_check
return if retired? || retiring? || retirement_initialized?
requester = system_context_requester

if !retirement_warned? && retirement_warning_due?
begin
self.retirement_last_warn = Time.now.utc
save
raise_retirement_event(retire_warn_event_name)
raise_retirement_event(retire_warn_event_name, requester)
rescue => err
_log.log_backtrace(err)
end
end

retire_now if retirement_due?
retire_now(requester) if retirement_due?
end

def retire_now(requester = nil)
Expand Down Expand Up @@ -195,8 +196,7 @@ def retired_event_name
end

def raise_retirement_event(event_name, requester = nil)
requester ||= User.current_user.try(:userid)
q_options = retire_queue_options
q_options = q_user_info(retire_queue_options, requester)
$log.info("Raising Retirement Event for [#{name}] with queue options: #{q_options.inspect}")
MiqEvent.raise_evm_event(self, event_name, setup_event_hash(requester), q_options)
end
Expand Down Expand Up @@ -231,15 +231,34 @@ def valid_zone?
respond_to?(:my_zone) && my_zone.present?
end

def setup_event_hash(requester)
event_hash = {:retirement_initiator => "system"}
event_hash[retirement_base_model_name.underscore.to_sym] = self
event_hash[:host] = host if self.respond_to?(:host)
def system_context_requester
if try(:evm_owner_id)
Copy link
Member

@kbrock kbrock Oct 31, 2018

Choose a reason for hiding this comment

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

Suggested change
if try(:evm_owner_id)
if evm_owner_id

User.find(evm_owner_id)
else
$log.info("System context defaulting to admin user because owner of #{name} not set.")
User.super_admin
end
end

def q_user_info(q_options, requester)
if requester
event_hash[:userid] = requester
event_hash[:retirement_initiator] = "user"
if requester.kind_of?(String)
Copy link
Member

@bdunne bdunne Oct 30, 2018

Choose a reason for hiding this comment

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

Does this ever get called with a string?

Copy link
Contributor Author

@d-m-u d-m-u Oct 30, 2018

Choose a reason for hiding this comment

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

Yup! It can!

Copy link
Contributor Author

@d-m-u d-m-u Oct 30, 2018

Choose a reason for hiding this comment

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

https://github.com/ManageIQ/manageiq/pull/18106/files#diff-9c77044014d41193411f6b6e59033a7aR169

requester = User.find_by(:userid => requester)
end
q_options[:user_id] = requester.id
q_options[:group_id] = requester.current_group.id if requester.current_group
Copy link
Member

@kbrock kbrock Oct 31, 2018

Choose a reason for hiding this comment

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

Heh - this makes me reference the code every time I see it - It is not necessary since the very next line is looking up the current_group and all, but I feel like this is better form. (sorry, think you probably introduced this from my suggestion before)

Suggested change
q_options[:group_id] = requester.current_group.id if requester.current_group
q_options[:group_id] = requester.current_group_id if requester.current_group_id

q_options[:tenant_id] = requester.current_tenant.id if requester.current_tenant
end
q_options
end

def setup_event_hash(requester)
{
:userid => requester,
retirement_base_model_name.underscore.to_sym => self,
:type => self.class.name
}.tap do |event|
event[:host] = host if respond_to?(:host)
end
event_hash[:type] ||= self.class.name
event_hash
end
end
17 changes: 8 additions & 9 deletions spec/models/orchestration_stack/retirement_management_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
describe "Service Retirement Management" do
let(:user) { FactoryGirl.create(:user) }
before(:each) do
@miq_server = EvmSpecHelper.local_miq_server
@stack = FactoryGirl.create(:orchestration_stack)
Expand Down Expand Up @@ -32,20 +33,18 @@
it "#retire_now with userid" do
expect(@stack.retirement_state).to be_nil
event_name = 'request_orchestration_stack_retire'
event_hash = {:orchestration_stack => @stack, :type => "OrchestrationStack",
:retirement_initiator => "user", :userid => "freddy"}
event_hash = {:userid => user.userid, :orchestration_stack => @stack, :type => "OrchestrationStack"}

expect(MiqEvent).to receive(:raise_evm_event).with(@stack, event_name, event_hash, {}).once
expect(MiqEvent).to receive(:raise_evm_event).with(@stack, event_name, event_hash, :user_id => user.id).once

@stack.retire_now('freddy')
@stack.retire_now(user.userid)
@stack.reload
end

it "#retire_now without userid" do
expect(@stack.retirement_state).to be_nil
event_name = 'request_orchestration_stack_retire'
event_hash = {:orchestration_stack => @stack, :type => "OrchestrationStack",
:retirement_initiator => "system"}
event_hash = {:userid => nil, :orchestration_stack => @stack, :type => "OrchestrationStack"}

expect(MiqEvent).to receive(:raise_evm_event).with(@stack, event_name, event_hash, {}).once

Expand Down Expand Up @@ -128,9 +127,9 @@
it "#raise_retirement_event" do
event_name = 'foo'
event_hash = {
:orchestration_stack => @stack,
:type => "OrchestrationStack",
:retirement_initiator => "system"
:userid => nil,
:orchestration_stack => @stack,
:type => "OrchestrationStack",
}

expect(MiqEvent).to receive(:raise_evm_event).with(@stack, event_name, event_hash, {})
Expand Down
13 changes: 6 additions & 7 deletions spec/models/service/retirement_management_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
describe "Service Retirement Management" do
let(:user) { FactoryGirl.create(:user) }
before(:each) do
@server = EvmSpecHelper.local_miq_server
@service = FactoryGirl.create(:service)
Expand Down Expand Up @@ -51,19 +52,17 @@
it "#retire_now with userid" do
expect(@service.retirement_state).to be_nil
event_name = 'request_service_retire'
event_hash = {:service => @service, :type => "Service",
:retirement_initiator => "user", :userid => "freddy"}
event_hash = {:userid => user.userid, :service => @service, :type => "Service"}

expect(MiqEvent).to receive(:raise_evm_event).with(@service, event_name, event_hash, {}).once
expect(MiqEvent).to receive(:raise_evm_event).with(@service, event_name, event_hash, :user_id => user.id).once

@service.retire_now('freddy')
@service.retire_now(user.userid)
end

it "#retire_now without userid" do
expect(@service.retirement_state).to be_nil
event_name = 'request_service_retire'
event_hash = {:service => @service, :type => "Service",
:retirement_initiator => "system"}
event_hash = {:userid => nil, :service => @service, :type => "Service"}

expect(MiqEvent).to receive(:raise_evm_event).with(@service, event_name, event_hash, {}).once

Expand Down Expand Up @@ -199,7 +198,7 @@

it "#raise_retirement_event" do
event_name = 'foo'
event_hash = {:service => @service, :type => "Service", :retirement_initiator => "system"}
event_hash = {:userid => nil, :service => @service, :type => "Service"}
expect(MiqEvent).to receive(:raise_evm_event).with(@service, event_name, event_hash, {})
@service.raise_retirement_event(event_name)
end
Expand Down
26 changes: 10 additions & 16 deletions spec/models/vm/retirement_management_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
describe "VM Retirement Management" do
let(:user) { FactoryGirl.create(:user) }
before(:each) do
miq_server = EvmSpecHelper.local_miq_server
@zone = miq_server.zone
Expand Down Expand Up @@ -52,19 +53,17 @@

it "#retire_now with userid" do
event_name = 'request_vm_retire'
event_hash = {:vm => @vm, :host => @vm.host, :type => "ManageIQ::Providers::Vmware::InfraManager::Vm",
:retirement_initiator => "user", :userid => 'freddy'}
options = {:zone => @zone.name}
event_hash = {:userid => user.userid, :vm => @vm, :host => @vm.host, :type => "ManageIQ::Providers::Vmware::InfraManager::Vm"}
options = {:zone => @zone.name, :user_id => user.id}

expect(MiqEvent).to receive(:raise_evm_event).with(@vm, event_name, event_hash, options).once

@vm.retire_now('freddy')
@vm.retire_now(user.userid)
end

it "#retire_now without userid" do
event_name = 'request_vm_retire'
event_hash = {:vm => @vm, :host => @vm.host, :type => "ManageIQ::Providers::Vmware::InfraManager::Vm",
:retirement_initiator => "system"}
event_hash = {:userid => nil, :vm => @vm, :host => @vm.host, :type => "ManageIQ::Providers::Vmware::InfraManager::Vm"}
options = {:zone => @zone.name}

expect(MiqEvent).to receive(:raise_evm_event).with(@vm, event_name, event_hash, options).once
Expand Down Expand Up @@ -153,8 +152,7 @@

it "#raise_retirement_event without current user" do
event_name = 'foo'
event_hash = {:vm => @vm, :host => @vm.host, :type => "ManageIQ::Providers::Vmware::InfraManager::Vm",
:retirement_initiator => "system"}
event_hash = {:userid => nil, :vm => @vm, :host => @vm.host, :type => "ManageIQ::Providers::Vmware::InfraManager::Vm"}
options = {:zone => @vm.my_zone}

expect(MiqEvent).to receive(:raise_evm_event).with(@vm, event_name, event_hash, options).once
Expand All @@ -163,16 +161,12 @@
end

it "#raise_retirement_event with current user" do
user = FactoryGirl.create(:user_with_group, :userid => 'freddy')
event_name = 'foo'
event_hash = {:vm => @vm, :host => @vm.host, :type => "ManageIQ::Providers::Vmware::InfraManager::Vm",
:retirement_initiator => "user", :userid => 'freddy'}
options = {:zone => @vm.my_zone}
event_hash = {:userid => user.userid, :vm => @vm, :host => @vm.host, :type => "ManageIQ::Providers::Vmware::InfraManager::Vm"}
options = {:zone => @vm.my_zone, :user_id => user.id}

User.with_user(user) do
expect(MiqEvent).to receive(:raise_evm_event).with(@vm, event_name, event_hash, options).once
@vm.raise_retirement_event(event_name)
end
expect(MiqEvent).to receive(:raise_evm_event).with(@vm, event_name, event_hash, options).once
@vm.raise_retirement_event(event_name, user.userid)
end

it "#raise_audit_event" do
Expand Down