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.

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

Jump to bottom

Allow advanced modifications of Amazon cloud volumes #676

Merged
merged 1 commit into from Mar 24, 2017
Merged

Allow advanced modifications of Amazon cloud volumes #676

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 10 additions & 6 deletions app/assets/javascripts/controllers/cloud_volume/cloud_volume_form_controller.js
View file
Open in desktop
@@ -1,4 +1,4 @@
ManageIQ.angular.app.controller('cloudVolumeFormController', ['$http', '$scope', 'cloudVolumeFormId', 'miqService', 'API', function($http, $scope, cloudVolumeFormId, miqService, API) {
ManageIQ.angular.app.controller('cloudVolumeFormController', ['$scope', 'cloudVolumeFormId', 'miqService', 'API', function($scope, cloudVolumeFormId, miqService, API) {
var init = function() {
$scope.cloudVolumeModel = {
aws_encryption: false,
Expand All @@ -14,7 +14,7 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['$http', '$scope',
{ type: "io1", name: "Provisioned IOPS SSD (IO1)" },
{ type: "st1", name: "Throughput Optimized HDD (ST1)" },
{ type: "sc1", name: "Cold HDD (SC1)" },
{ type: "magnetic", name: "Magnetic" },
{ type: "standard", name: "Magnetic" },
Copy link
Contributor

Choose a reason for hiding this comment

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

Would have really liked if we could fetch this list on-the-fly using some API (AWS API perhaps?)
But don't worry about it, if that's not doable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AWS does not provide an API for this; we initially thought about making CloudVolumeType model where storage managers would be able to list their options (for example in OpenStack admins are allowed to define different volume types so it would make sense).

My concern is that there are specific rules in the UI that need to be handled with regards to the chosen volume type, so I am not sure if providing it through API/backend would benefit much. Still, we could use something like this for regions or instance types. Would that make any sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, yes. I know you mentioned this even before.

];

ManageIQ.angular.scope = $scope;
Expand All @@ -23,17 +23,21 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['$http', '$scope',
$scope.cloudVolumeModel.name = "";
} else {
miqService.sparkleOn();
$http.get('/cloud_volume/cloud_volume_form_fields/' + cloudVolumeFormId)
API.get('/api/cloud_volumes/' + cloudVolumeFormId + '?attributes=ext_management_system.type')
.then(getCloudVolumeFormDataComplete)
.catch(miqService.handleFailure);
}
};

function getCloudVolumeFormDataComplete(response) {
var data = response.data;

function getCloudVolumeFormDataComplete(data) {
$scope.afterGet = true;
$scope.cloudVolumeModel.emstype = data.ext_management_system.type;
$scope.cloudVolumeModel.name = data.name;
// We have to display size in GB.
$scope.cloudVolumeModel.size = data.size / 1073741824;
// Currently, this is only relevant for AWS volumes so we are prefixing the
// model attribute with AWS.
$scope.cloudVolumeModel.aws_volume_type = data.volume_type;

$scope.modelCopy = angular.copy( $scope.cloudVolumeModel );
miqService.sparkleOff();
Expand Down
11 changes: 3 additions & 8 deletions app/controllers/cloud_volume_controller.rb
View file
Open in desktop
Expand Up @@ -121,14 +121,6 @@ def show
replace_gtl_main_div if pagination_request?
end

def cloud_volume_form_fields
assert_privileges("cloud_volume_edit")
volume = find_by_id_filtered(CloudVolume, params[:id])
render :json => {
:name => volume.name
}
end

def attach
params[:id] = checked_item_id unless params[:id].present?
assert_privileges("cloud_volume_attach")
Expand Down Expand Up @@ -696,6 +688,9 @@ def form_params
options[:cloud_tenant_id] = params[:cloud_tenant_id] if params[:cloud_tenant_id]
options[:vm_id] = params[:vm_id] if params[:vm_id]
options[:device_path] = params[:device_path] if params[:device_path]
options[:volume_type] = params[:aws_volume_type] if params[:aws_volume_type]
# Only set IOPS if io1 (provisioned IOPS) and IOPS available
options[:iops] = params[:aws_iops] if options[:volume_type] == 'io1' && params[:aws_iops]
options
end

Expand Down
60 changes: 60 additions & 0 deletions app/views/cloud_volume/_common_new_edit.html.haml
View file
Open in desktop
@@ -0,0 +1,60 @@
.form-group{"ng-class" => "{'has-error': angularForm.name.$invalid}"}
%label.col-md-2.control-label
= _('Volume Name')
.col-md-8
%input.form-control{:type => "text",
:name => "name",
'ng-model' => "cloudVolumeModel.name",
'ng-maxlength' => 128,
:required => "",
:checkchange => true}
%span.help-block{"ng-show" => "angularForm.name.$error.required"}
= _("Required")

.form-group{"ng-class" => "{'has-error': angularForm.aws_volume_type.$invalid}",
"ng-if" => "cloudVolumeModel.emstype == 'ManageIQ::Providers::Amazon::StorageManager::Ebs'"}
%label.col-md-2.control-label
= _('Cloud Volume Type')
.col-md-8
%select{"name" => "aws_volume_type",
"ng-model" => "cloudVolumeModel.aws_volume_type",
"ng-options" => "voltype.type as voltype.name for voltype in awsVolumeTypes",
"ng-change" => "awsVolumeTypeChanged(cloudVolumeModel.aws_volume_type)",
"ng-disabled" => "formId != 'new' && cloudVolumeModel.aws_volume_type == 'standard'",
"required" => "",
:checkchange => true,
"selectpicker-for-select-tag" => ""}
%option{"value" => "", "disabled" => ""}
= "<#{_('Choose')}>"
%span.help-block{"ng-show" => "angularForm.aws_volume_type.$error.required"}
= _("Required")

.form-group{"ng-class" => "{'has-error': angularForm.size.$invalid}"}
%label.col-md-2.control-label
= _('Size (in gigabytes)')
.col-md-8
%input.form-control{:type => "text",
:name => "size",
'ng-model' => "cloudVolumeModel.size",
'ng-maxlength' => 10,
'ng-change' => "sizeChanged(cloudVolumeModel.size)",
'ng-disabled' => "formId != 'new' && (cloudVolumeModel.emstype != 'ManageIQ::Providers::Amazon::StorageManager::Ebs' || cloudVolumeModel.aws_volume_type == 'standard')",
:required => "",
:checkchange => true}
%span.help-block{"ng-show" => "angularForm.size.$error.required"}
= _("Required")

.form-group{"ng-class" => "{'has-error': angularForm.aws_iops.$invalid}",
"ng-if" => "cloudVolumeModel.emstype == 'ManageIQ::Providers::Amazon::StorageManager::Ebs'"}
%label.col-md-2.control-label
= _('IOPS')
.col-md-8
%input.form-control{:type => "text",
:name => "aws_iops",
'ng-model' => "cloudVolumeModel.aws_iops",
'ng-maxlength' => 50,
'ng-disabled' => "cloudVolumeModel.aws_volume_type != 'io1'",
"ng-required" => "cloudVolumeModel.aws_volume_type == 'io1'",
:checkchange => true}
%span.help-block{"ng-show" => "cloudVolumeModel.aws_volume_type == 'io1' && angularForm.aws_iops.$error.required"}
= _("Required")
13 changes: 2 additions & 11 deletions app/views/cloud_volume/edit.html.haml
View file
Open in desktop
@@ -1,18 +1,9 @@
%form#form_div{:name => "angularForm", 'ng-controller' => "cloudVolumeFormController"}
%form#form_div{:name => "angularForm", 'ng-controller' => "cloudVolumeFormController", "ng-cloak" => ""}
= render :partial => "layouts/flash_msg"
%h3
= _('Edit Volume')
.form-horizontal
.form-group
%label.col-md-2.control-label
= _('Volume Name')
.col-md-8
%input.form-control{:type => "text",
:name => "name",
'ng-model' => "cloudVolumeModel.name",
'ng-maxlength' => 128,
:miqrequired => false,
:checkchange => true}
= render :partial => "common_new_edit"

%table{:width => '100%'}
%tr
Copy link
Contributor

Choose a reason for hiding this comment

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

In this haml, down below, can you use this partial to render the buttons?
"layouts/angular/x_edit_buttons_angular"
instead of rendering the buttons individually.
(I probably missed telling you this in your earlier PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sure - I didn't know about this before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AparnaKarve I noticed there are some differences between the way how the button table is created for "new volume" than in the about partial layout. For example, in cloud volume case it checks if @volume.id.nil? to decide whether to show Add or Save, while the layout partial uses newRecord.

I guess we should update the whole controller/view to make it compliant with the partial layout. Perhaps in a separate PR. What do you think?

Expand Down
59 changes: 1 addition & 58 deletions app/views/cloud_volume/new.html.haml
View file
Open in desktop
Expand Up @@ -47,64 +47,7 @@
%span.help-block{"ng-show" => "angularForm.aws_availability_zone_id.$error.required"}
= _("Required")

.form-group{"ng-class" => "{'has-error': angularForm.name.$invalid}"}
%label.col-md-2.control-label
= _('Volume Name')
.col-md-8
%input.form-control{:type => "text",
:name => "name",
'ng-model' => "cloudVolumeModel.name",
'ng-maxlength' => 128,
:required => "",
:checkchange => true}
%span.help-block{"ng-show" => "angularForm.name.$error.required"}
= _("Required")

.form-group{"ng-class" => "{'has-error': angularForm.aws_volume_type.$invalid}",
"ng-if" => "cloudVolumeModel.emstype == 'ManageIQ::Providers::Amazon::StorageManager::Ebs'"}
%label.col-md-2.control-label
= _('Cloud Volume Type')
.col-md-8
%select{"name" => "aws_volume_type",
"ng-model" => "cloudVolumeModel.aws_volume_type",
"ng-options" => "voltype.type as voltype.name for voltype in awsVolumeTypes",
"ng-change" => "awsVolumeTypeChanged(cloudVolumeModel.aws_volume_type)",
"required" => "",
:checkchange => true,
"selectpicker-for-select-tag" => ""}
%option{"value" => "", "disabled" => ""}
= "<#{_('Choose')}>"
%span.help-block{"ng-show" => "angularForm.aws_volume_type.$error.required"}
= _("Required")

.form-group{"ng-class" => "{'has-error': angularForm.size.$invalid}"}
%label.col-md-2.control-label
= _('Size (in gigabytes)')
.col-md-8
%input.form-control{:type => "text",
:name => "size",
'ng-model' => "cloudVolumeModel.size",
'ng-maxlength' => 10,
'ng-change' => "sizeChanged(cloudVolumeModel.size)",
:required => "",
:checkchange => true}
%span.help-block{"ng-show" => "angularForm.size.$error.required"}
= _("Required")

.form-group{"ng-class" => "{'has-error': angularForm.aws_iops.$invalid}",
"ng-if" => "cloudVolumeModel.emstype == 'ManageIQ::Providers::Amazon::StorageManager::Ebs'"}
%label.col-md-2.control-label
= _('IOPS')
.col-md-8
%input.form-control{:type => "text",
:name => "aws_iops",
'ng-model' => "cloudVolumeModel.aws_iops",
'ng-maxlength' => 50,
'ng-disabled' => "cloudVolumeModel.aws_volume_type != 'io1'",
"required" => "",
:checkchange => true}
%span.help-block{"ng-show" => "cloudVolumeModel.aws_volume_type == 'io1' && angularForm.aws_iops.$error.required"}
= _("Required")
= render :partial => "common_new_edit"

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above for the button partial

.form-group{"ng-if" => "cloudVolumeModel.emstype == 'ManageIQ::Providers::Amazon::StorageManager::Ebs'"}
%label.col-md-2.control-label
Expand Down
1 change: 0 additions & 1 deletion config/routes.rb
View file
Open in desktop
Expand Up @@ -481,7 +481,6 @@
backup_select
snapshot_new
edit
cloud_volume_form_fields
cloud_volume_tenants
index
new
Expand Down