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

Refactoring service retirement start_retirement method. #538

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,32 +1,46 @@
#
# Description: This method sets the retirement_state to retiring
#
module ManageIQ
module Automate
module Service
module Retirement
module StateMachines
module Methods
class StartRetirement
def initialize(handle = $evm)
@handle = handle
end

service = $evm.root['service']
if service.nil?
$evm.log('error', "Service Object not found")
exit MIQ_ABORT
end
def main
@handle.log('info', "Service Start Retirement for #{service.inspect}.try")
@handle.create_notification(:type => :service_retiring, :subject => service)
service.start_retirement

$evm.log('info', "Service before start_retirement: #{service.inspect} ")
@handle.log('info', "Service after start_retirement: #{service.inspect} ")
end

if service.retired?
$evm.log('error', "Service is already retired. Aborting current State Machine.")
exit MIQ_ABORT
end
private

if service.retiring?
$evm.log('error', "Service is in the process of being retired. Aborting current State Machine.")
exit MIQ_ABORT
def service
@service ||= @handle.root["service"].tap do |service|
if service.nil?
@handle.log(:error, 'Service Object not found')
Copy link
Member

Choose a reason for hiding this comment

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

Seems perfect places to use ManageIQ::Automate::System::CommonMethods::Utils::LogObject.log_and_raise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to do this in a followup PR

raise 'Service Object not found'
end
if service.retired?
@handle.log(:error, 'Service is already retired. Aborting current State Machine.')
raise 'Service is already retired. Aborting current State Machine.'
end
if service.retiring?
@handle.log(:error, 'Service is in the process of being retired. Aborting current State Machine.')
raise 'Service is in the process of being retired. Aborting current State Machine.'
end
end
end
end
end
end
end
end
end
end

unless $evm.root['service_retire_task']
$evm.log(:error, "Service retire task not found")
$evm.log(:error, "The old style retirement is incompatible with the new retirement state machine.")
exit(MIQ_ABORT)
end

$evm.create_notification(:type => :service_retiring, :subject => service)
service.start_retirement

$evm.log('info', "Service after start_retirement: #{service.inspect} ")
ManageIQ::Automate::Service::Retirement::StateMachines::Methods::StartRetirement.new.main
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
require_domain_file

describe ManageIQ::Automate::Service::Retirement::StateMachines::Methods::StartRetirement do
let(:admin) { FactoryBot.create(:user_admin) }
let(:request) { FactoryBot.create(:service_retire_request, :requester => admin) }
let(:service) { FactoryBot.create(:service) }
let(:task) { FactoryBot.create(:service_retire_task, :destination => service, :miq_request => request) }
let(:svc_task) { MiqAeMethodService::MiqAeServiceServiceRetireTask.find(task.id) }
let(:svc_service) { MiqAeMethodService::MiqAeServiceService.find(service.id) }
let(:root_object) do
Spec::Support::MiqAeMockObject.new('service' => svc_service,
'service_retire_task' => svc_task)
end
let(:ae_service) do
Spec::Support::MiqAeMockService.new(root_object).tap do |service|
current_object = Spec::Support::MiqAeMockObject.new
current_object.parent = root_object
service.object = current_object
end
end

let(:svc_model_request) do
MiqAeMethodService::MiqAeServiceServiceRetireRequest.find(request.id)
end

it "#Service retiring - good" do
expect(service.retirement_state).to be_nil
service.start_retirement

expect(service.retirement_state).to eq("retiring")
end

it "starts retirement" do
Copy link
Member

Choose a reason for hiding this comment

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

What does this test?

Copy link
Member

Choose a reason for hiding this comment

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

This does not test anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to test the good condition. Retirement_state is nil and then I start retirement and the retirement_state is equal to retiring.

allow(ae_service).to receive(:create_notification)
Copy link
Member

Choose a reason for hiding this comment

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

If this is a test, then it should be expect instead of allow. Otherwise this is not testing anything.

end

context "with no service" do
let(:root_object) do
Spec::Support::MiqAeMockObject.new('service' => nil,
'service_retire_task' => svc_task)
end

it "raises the ERROR - Service Object not found" do
expect { described_class.new(ae_service).main }.to raise_error(
'Service Object not found'
)
end
end

it "raises the ERROR - Service is already retired. Aborting current State Machine." do
service.update(
:retired => true,
:retirement_last_warn => Time.zone.now,
:retirement_state => "retired"
)
service.reload

expect { described_class.new(ae_service).main }.to raise_error(
'Service is already retired. Aborting current State Machine.'
)
end

it "raises the ERROR - Service is in the process of being retired. Aborting current State Machine." do
service.update(
:retired => false,
:retirement_last_warn => Time.zone.now,
:retirement_state => "retiring"
)
service.reload

expect { described_class.new(ae_service).main }.to raise_error(
'Service is in the process of being retired. Aborting current State Machine.'
)
end
end