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 VM Reconfigure for VMware vCloud provider #3854

Merged
merged 1 commit into from
May 24, 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
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ ManageIQ.angular.app.controller('reconfigureFormController', ['$http', '$scope',
vm.validateClicked = miqService.validateWithAjax;
vm.modelCopy = angular.copy( vm.reconfigureModel );
vm.model = 'reconfigureModel';
vm.vm_vendor = '';
vm.vm_type = '';
vm.disk_default_type = '';

ManageIQ.angular.scope = vm;

Expand Down Expand Up @@ -179,6 +182,7 @@ ManageIQ.angular.app.controller('reconfigureFormController', ['$http', '$scope',
vm.reconfigureModel.vmAddDisks.push({disk_size_in_mb: diskSizeAdd,
persistent: dmode,
thin_provisioned: dtype,
type: disk.hdType,
new_controller_type: disk.new_controller_type,
dependent: disk.cb_dependent,
bootable: disk.cb_bootable});
Expand Down Expand Up @@ -213,7 +217,7 @@ ManageIQ.angular.app.controller('reconfigureFormController', ['$http', '$scope',
};

vm.resetAddValues = function() {
vm.reconfigureModel.hdType = 'thin';
vm.reconfigureModel.hdType = vm.disk_default_type;
vm.reconfigureModel.hdMode = 'persistent';
vm.reconfigureModel.hdSize = '';
vm.reconfigureModel.hdUnit = 'MB';
Expand Down Expand Up @@ -285,6 +289,7 @@ ManageIQ.angular.app.controller('reconfigureFormController', ['$http', '$scope',
};

vm.enableDiskAdd = function() {
vm.resetAddValues();
vm.reconfigureModel.addEnabled = true;
};

Expand Down Expand Up @@ -408,6 +413,9 @@ ManageIQ.angular.app.controller('reconfigureFormController', ['$http', '$scope',
vm.cb_cpu = data.cb_cpu;
vm.reconfigureModel.vmdisks = angular.copy(data.disks);
vm.reconfigureModel.vmNetworkAdapters = angular.copy(data.network_adapters);
vm.vm_vendor = data.vm_vendor;
vm.vm_type = data.vm_type;
vm.disk_default_type = data.disk_default_type;
vm.updateDisksAddRemove();
vm.updateNetworkAdaptersAddRemove();

Expand Down
12 changes: 5 additions & 7 deletions app/controllers/mixins/actions/vm_actions/reconfigure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def get_reconfig_info(reconfigure_ids)

# if only one vm that supports disk reconfiguration is selected, get the disks information
vmdisks = []
@reconfigureitems.first.hardware.disks.each do |disk|
@reconfigureitems.first.hardware.disks.sort_by(&:filename).each do |disk|
next if disk.device_type != 'disk'
dsize, dunit = reconfigure_calculations(disk.size / (1024 * 1024))
vmdisks << {:hdFilename => disk.filename,
Expand Down Expand Up @@ -246,7 +246,10 @@ def get_reconfig_info(reconfigure_ids)
:socket_count => socket_count.to_s,
:cores_per_socket_count => cores_per_socket.to_s,
:disks => vmdisks,
:network_adapters => network_adapters}
:network_adapters => network_adapters,
:vm_vendor => @reconfigureitems.first.vendor,
:vm_type => @reconfigureitems.first.class.name,
:disk_default_type => @reconfigureitems.first.try(:disk_default_type) || 'thin'}
end

def supports_reconfigure_disks?
Expand Down Expand Up @@ -353,11 +356,6 @@ def reconfigure_handle_submit_button
url = previous_breadcrumb_url.split('/')
javascript_redirect(:controller => url[1], :action => url[2])
end

if @flash_array
Copy link
Contributor

Choose a reason for hiding this comment

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

This bit is surprising, why would you remove a flash message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for asking. This flash rendering breaks redirect as far as I've tested because it happend AFTER redirecting to other page. See the condition in L352:

if ...
   javascript_redirect(...)
else
   javascript_redirect(...)
end

There is no way we can arrive to L357 without callind redirect first. Or am I missing something? Looking forward to your answer, I'm curious how this can work for other providers. Most probably it doesn't :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Aah... I think you're right.. A few lines above, there is a flash_to_session call which saves the flash messages for later.

Since both branches do javascript_redirect, unconditionally, I agree this is broken.
And since flash_to_session I think we can expect this won't hide any flash messages that would otherwise appear.

👍

javascript_flash
return
end
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions app/helpers/application_helper/toolbar/x_vm_cloud_center.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ class ApplicationHelper::Toolbar::XVmCloudCenter < ApplicationHelper::Toolbar::B
'pficon pficon-edit fa-lg',
t = N_('Edit Management Engine Relationship'),
t),
button(
:vm_reconfigure,
'pficon pficon-edit fa-lg',
N_('Reconfigure the Memory/CPU of this VM'),
N_('Reconfigure this VM'),
:klass => ApplicationHelper::Button::VmReconfigure
),
]
),
])
Expand Down
30 changes: 16 additions & 14 deletions app/views/vm_common/_reconfigure.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,13 @@
%thead
%th= _('Name')
%th= _('Type')
%th{"ng-if" => @reconfigitems.first.vendor == 'vmware'}= _('Mode')
%th{"ng-if" => @reconfigitems.first.vendor == 'vmware'}= _('Controller Type')
%th{"ng-if" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('InfraManager')"}= _('Mode')
%th{"ng-if" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('InfraManager')"}= _('Controller Type')
%th= _('Size')
%th= _('Unit')
%th{"ng-if" => @reconfigitems.first.vendor == 'vmware'}= _('Dependent')
%th= _('Delete Backing')
%th{"ng-if" => @reconfigitems.first.vendor == 'redhat'}= _('Bootable')
%th{"ng-if" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('InfraManager')"}= _('Dependent')
%th{"ng-hide" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('CloudManager')"}= _('Delete Backing')
%th{"ng-if" => "vm.vm_vendor == 'redhat'"}= _('Bootable')
- if supports_reconfigure_disksize?
%th{:colspan => 2}= _('Actions')
- else
Expand All @@ -144,14 +144,16 @@
"ng-form" => "rowForm"}
%td
%td
- options = @reconfigitems.first.try(:disk_types) || %w(thick thin)
- default_option = @reconfigitems.first.try(:disk_default_type) || 'thin'
= select_tag('hdType',
options_for_select(%w(thick thin)),
options_for_select(options, :selected => default_option),
"ng-model" => "vm.reconfigureModel.hdType",
"ng-change" => "",
"data-width" => "auto",
"required" => "",
"selectpicker-for-select-tag" => "")
%td{"ng-if" => @reconfigitems.first.vendor == 'vmware'}
%td{"ng-if" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('InfraManager')"}
= select_tag('hdMode',
options_for_select(%w(persistent nonpersistent)),
"ng-model" => "vm.reconfigureModel.hdMode",
Expand All @@ -160,7 +162,7 @@
"required" => "",
"selectpicker-for-select-tag" => "",
"width" => "10")
%td{"ng-if" => @reconfigitems.first.vendor == 'vmware'}
%td{"ng-if" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('InfraManager')"}
- options = @reconfigitems.first.try(:scsi_controller_types) || {}
= select_tag('Controller',
options_for_select(options),
Expand All @@ -187,13 +189,13 @@
"data-width" => "auto",
"required" => "",
"selectpicker-for-select-tag" => "")
%td{"ng-if" => @reconfigitems.first.vendor == 'vmware'}
%td{"ng-if" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('InfraManager')"}
%input{"bs-switch" => "",
:data => {:on_text => _('Yes'), :off_text => _('No'), :size => 'mini'},
"type" => "checkbox",
"name" => "vm.cb_dependent",
"ng-model" => "vm.reconfigureModel.cb_dependent"}
%td
%td{"ng-hide" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('CloudManager')"}
%td{"ng-if" => @reconfigitems.first.vendor == 'redhat'}
%input{"bs-switch" => "",
:data => {:on_text => _('Yes'), :off_text => _('No'), :size => 'mini'},
Expand All @@ -213,9 +215,9 @@
{{disk.hdFilename}}
%td
{{disk.hdType}}
%td{"ng-if" => @reconfigitems.first.vendor == 'vmware'}
%td{"ng-if" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('InfraManager')"}
{{disk.hdMode}}
%td{"ng-if" => @reconfigitems.first.vendor == 'vmware'}
%td{"ng-if" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('InfraManager')"}
{{disk.new_controller_type}}
%td{"ng-if" => "disk.add_remove != 'resizing'"}
{{disk.hdSize}}
Expand Down Expand Up @@ -244,7 +246,7 @@
"data-width" => "auto",
"required" => "",
"selectpicker-for-select-tag" => "")
%td{"ng-if" => @reconfigitems.first.vendor == 'vmware'}
%td{"ng-if" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('InfraManager')"}
%input{"bs-switch" => "",
:data => {:on_text => _('Yes'), :off_text => _('No'), :size => 'mini'},
"type" => "checkbox",
Expand All @@ -253,7 +255,7 @@
"switch-active" => "{{disk.add_remove!='add'}}",
"ng-readonly" => "disk.add_remove=='add'",
"ng-if" => "disk.add_remove=='add'"}
%td
%td{"ng-hide" => "vm.vm_vendor == 'vmware' && vm.vm_type.includes('CloudManager')"}
%input{"bs-switch" => "",
:data => {:on_text => _('Yes'), :off_text => _('No'), :size => 'mini'},
"type" => "checkbox",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ describe('reconfigureFormController', function() {
socket_count: '2',
cores_per_socket_count: '3',
disks: [{hdFilename: "test_disk.vmdk", hdType: "thick", hdMode: "persistent", new_controller_type: "VirtualLsiLogicController", hdSize: "0", hdUnit: "MB", add_remove: ""}],
network_adapters: [{name: "Network adapter 1", vlan: "test_network", mac: "00:00:00:00:00:00", add_remove: ""}]};
network_adapters: [{name: "Network adapter 1", vlan: "test_network", mac: "00:00:00:00:00:00", add_remove: ""}],
vm_vendor: 'vm-vendor',
vm_type: 'vm-type',
disk_default_type: 'disk-default-type'};

$httpBackend = _$httpBackend_;
$httpBackend.whenGET('reconfigure_form_fields/1000000000003,1000000000001,1000000000002').respond(reconfigureFormResponse);
Expand Down Expand Up @@ -60,6 +63,18 @@ describe('reconfigureFormController', function() {
it('sets the network adapter data to the network adapter data returned with the http request', function() {
expect(vm.reconfigureModel.vmNetworkAdapters).toEqual([{name: "Network adapter 1", vlan: "test_network", mac: "00:00:00:00:00:00", add_remove: ""}]);
});

it('sets the vm vendor data to the data returned with the http request', function() {
expect(vm.vm_vendor).toEqual('vm-vendor');
});

it('sets the vm type data to the data returned with the http request', function() {
expect(vm.vm_type).toEqual('vm-type');
});

it('sets the vm disk_default_type to the data returned with the http request', function() {
expect(vm.disk_default_type).toEqual('disk-default-type');
});
});

describe('#cancelClicked', function() {
Expand Down