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

Support moving a VM to another folder during VM Migrate. #17519

Merged
merged 1 commit into from
Jun 22, 2018
Merged
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
22 changes: 14 additions & 8 deletions app/models/vm_migrate_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ def self.get_description(req_obj)
new_settings << "Host: #{host_name}" unless host_name.blank?
respool_name = req_obj.get_option_last(:placement_rp_name)
new_settings << "Resource Pool: #{respool_name}" unless respool_name.blank?
folder_name = req_obj.get_option_last(:placement_folder_name)
new_settings << "Folder: #{folder_name}" if folder_name.present?
storage = req_obj.get_option_last(:placement_ds_name)
new_settings << "Storage: #{storage}" unless storage.blank?
"#{request_class::TASK_DESCRIPTION} for: #{name} - #{new_settings.join(", ")}"
Expand All @@ -44,6 +46,9 @@ def do_request
respool_id = get_option(:placement_rp_name)
respool = ResourcePool.find_by(:id => respool_id)

folder_id = get_option(:placement_folder_name)
folder = EmsFolder.find_by(:id => folder_id)

datastore_id = get_option(:placement_ds_name)
datastore = Storage.find_by(:id => datastore_id)

Expand All @@ -57,27 +62,28 @@ def do_request
:relocate
elsif respool && host.nil?
:relocate
else
elsif host
:migrate
end
Copy link
Member

Choose a reason for hiding this comment

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

Since you've removed the catchall else, is there a new catchall?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no catchall case here. Only do certain thing when the condition meets.


_log.warn("Calling VM #{vc_method} for #{vm.id}:#{vm.name}")

begin
if vc_method == :migrate
vm.migrate(host, respool)
else
vm.relocate(host, respool, datastore, nil, disk_transform)
case vc_method
when :migrate then vm.migrate(host, respool)
when :relocate then vm.relocate(host, respool, datastore, nil, disk_transform)
end

vm.move_into_folder(folder) if folder.present?
rescue => err
update_and_notify_parent(:state => 'finished', :status => 'error', :message => "Failed. Reason[#{err.message}]")
update_and_notify_parent(:state => 'finished', :status => 'Error', :message => "Failed. Reason[#{err.message}]")
return
end

if AUTOMATE_DRIVES
update_and_notify_parent(:state => 'migrated', :message => "Finished #{request_class::TASK_DESCRIPTION}")
update_and_notify_parent(:state => 'migrated', :message => "Finished #{request_class::TASK_DESCRIPTION}", :status => 'Ok')
else
update_and_notify_parent(:state => 'finished', :message => "#{request_class::TASK_DESCRIPTION} complete")
update_and_notify_parent(:state => 'finished', :message => "#{request_class::TASK_DESCRIPTION} complete", :status => 'Ok')
end
end
end
15 changes: 15 additions & 0 deletions app/models/vm_or_template/operations/relocation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,21 @@ def relocate(host, pool = nil, datastore = nil, disk_move_type = nil, transform
raw_relocate(host, pool, datastore, disk_move_type, transform, priority, disk)
end

def raw_move_into_folder(folder)
raise _("VM has no EMS, unable to move VM into a new folder") unless ext_management_system
raise _("Folder not specified, unable to move VM into a new folder") unless folder.kind_of?(EmsFolder)

if parent_blue_folder == folder
raise _("The VM '%{name}' is already running on the same folder as the destination.") % {:name => name}
end

run_command_via_parent(:vm_move_into_folder, :folder => folder)
end

def move_into_folder(folder)
raw_move_into_folder(folder)
end

def migrate_via_ids(host_id, pool_id = nil, priority = "defaultPriority", state = nil)
host = Host.find_by(:id => host_id)
raise _("Host with ID=%{host_id} was not found") % {:host_id => host_id} if host.nil?
Expand Down
8 changes: 8 additions & 0 deletions product/dialogs/miq_dialogs/vm_migrate_dialogs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,14 @@
:required: false
:display: :edit
:data_type: :integer
:placement_folder_name:
:values_from:
:method: :allowed_folders
:auto_select_single: false
:description: Name
:required: false
:display: :edit
:data_type: :integer
:display: :show
:schedule:
:description: Schedule
Expand Down
8 changes: 6 additions & 2 deletions spec/models/vm_migrate_task_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
describe VmMigrateTask do
describe '.do_request' do
let(:vm) { Vm.new }
before { subject.vm = vm }
before do
subject.vm = vm
host = FactoryGirl.create(:host, :name => "test")
subject.update_attributes(:options => {:placement_host_name => [host.id, host.name]})
end

it 'migrates the vm and updates the status' do
expect(vm).to receive(:migrate)
Expand All @@ -11,7 +15,7 @@

it 'catches migrate error and update the status' do
expect(vm).to receive(:migrate).and_raise("Bad things happened")
expect(subject).to receive(:update_and_notify_parent).with(hash_including(:state => 'finished', :status => 'error', :message => 'Failed. Reason[Bad things happened]'))
expect(subject).to receive(:update_and_notify_parent).with(hash_including(:state => 'finished', :status => 'Error', :message => 'Failed. Reason[Bad things happened]'))
subject.do_request
end
end
Expand Down