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

Lazy load StorageManagers upon volume creation #4549

Merged
merged 1 commit into from
Sep 3, 2018
Merged

Lazy load StorageManagers upon volume creation #4549

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
55 changes: 33 additions & 22 deletions app/assets/javascripts/controllers/cloud_volume/cloud_volume_form_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API', 'cloudVolumeFormId', 'storageManagerId', '$timeout', function(miqService, API, cloudVolumeFormId, storageManagerId, $timeout) {
ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API', 'cloudVolumeFormId', 'cloudVolumeFormOperation', 'storageManagerId', '$timeout', '$q', function(miqService, API, cloudVolumeFormId, cloudVolumeFormOperation, storageManagerId, $timeout, $q) {
var vm = this;

var init = function() {
Expand All @@ -21,19 +21,32 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API
vm.newRecord = cloudVolumeFormId === 'new';

miqService.sparkleOn();
API.get('/api/providers?expand=resources&attributes=id,name,supports_block_storage&filter[]=supports_block_storage=true')
.then(getStorageManagers)
.catch(miqService.handleFailure);

if (cloudVolumeFormId !== 'new') {
// Fetch cloud volume data before showing the form.
API.get('/api/cloud_volumes/' + cloudVolumeFormId + '?attributes=ext_management_system.type,availability_zone.ems_ref,base_snapshot.ems_ref')
.then(getCloudVolumeFormData)
.catch(miqService.handleFailure);
} else {
// Initialise the form immediately when creating a new volume.
setForm();
// Load initial API data depending on what form we're displaying. Do as little requests as possible
// to support fine-grained API permissions.
var apiPromises = [];
switch (cloudVolumeFormOperation) {
case 'EDIT':
// Fetch relevant StorageManager, just enough to populate the disabled drop-down.
apiPromises.push(API.get('/api/providers/' + storageManagerId + '?attributes=id,name')
.then(getStorageManagers));
// Limit form options as permitted by this StorageManager.
apiPromises.push(vm.storageManagerChanged(storageManagerId));
// Fetch the volume.
apiPromises.push(loadVolume(cloudVolumeFormId));
break;
case 'NEW':
// Fetch StorageManagers that we can even create the new volume for.
apiPromises.push(API.get('/api/providers?expand=resources&attributes=id,name,supports_block_storage&filter[]=supports_block_storage=true')
.then(getStorageManagers));
break;
default:
// Fetch the volume.
apiPromises.push(loadVolume(cloudVolumeFormId));
}

// After all the API data is at hand we show the form.
$q.all(apiPromises).then(setForm).catch(miqService.handleFailure);
};

vm.addClicked = function() {
Expand Down Expand Up @@ -115,7 +128,7 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API

vm.storageManagerChanged = function(id) {
miqService.sparkleOn();
API.get('/api/providers/' + id + '?attributes=type,parent_manager.availability_zones,parent_manager.cloud_tenants,parent_manager.cloud_volume_snapshots')
return API.get('/api/providers/' + id + '?attributes=type,parent_manager.availability_zones,parent_manager.cloud_tenants,parent_manager.cloud_volume_snapshots')
.then(getStorageManagerFormData)
.catch(miqService.handleFailure);
};
Expand Down Expand Up @@ -193,6 +206,11 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API
miqService.sparkleOff();
}

var loadVolume = function(id) {
return API.get('/api/cloud_volumes/' + id + '?attributes=ext_management_system.type,availability_zone.ems_ref,base_snapshot.ems_ref')
.then(getCloudVolumeFormData)
};

var loadEBSVolumeTypes = function() {
// This ia a fixed list of available cloud volume types for Amazon EBS.
vm.awsVolumeTypes = [
Expand All @@ -212,13 +230,8 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API
};

var getStorageManagers = function(data) {
vm.storageManagers = data.resources;

// If storage manager ID was provided, we need to refresh the form and show
// corresponding form fields.
if (storageManagerId) {
vm.storageManagerChanged(vm.cloudVolumeModel.storage_manager_id);
}
// Can handle list of all managers or a single manager.
vm.storageManagers = data.resources ? data.resources : [data];
};

var getCloudVolumeFormData = function(data) {
Expand All @@ -243,8 +256,6 @@ ManageIQ.angular.app.controller('cloudVolumeFormController', ['miqService', 'API
if (angular.isUndefined(vm.cloudVolumeModel.aws_iops)) {
vm.sizeChanged(vm.cloudVolumeModel.size);
}

setForm();
};

var getStorageManagerFormData = function(data) {
Expand Down
3 changes: 2 additions & 1 deletion app/views/cloud_volume/attach.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,6 @@

:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
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 this is the reason why StorageManager wasn't getting selected upon volume edit (on master branch). It's because we're passing integer id from rails, but drop-down options somehow have strings. Well hidden bug, spent quite some time figuring it out :)

So the easiest solution is to convert it to string immediately here. Problem is if we would want it be integer, then we could hit overflow as MIQ ID's are huge numbers.

Copy link
Contributor

@himdel himdel Aug 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Agreed, strings are necessary for our long ids in js :). (The to_s is not necessary, but.. not a problem either.)

ManageIQ.angular.app.value('cloudVolumeFormOperation', 'ATTACH');
miq_bootstrap('#form_div');
3 changes: 2 additions & 1 deletion app/views/cloud_volume/backup_new.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,6 @@

:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'BACKUP_NEW');
miq_bootstrap(jQuery('#form_div'));
3 changes: 2 additions & 1 deletion app/views/cloud_volume/backup_select.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,6 @@

:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'BACKUP_SELECT');
miq_bootstrap(jQuery('#form_div'));
3 changes: 2 additions & 1 deletion app/views/cloud_volume/detach.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@

:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'DETACH');
miq_bootstrap('#form_div');
3 changes: 2 additions & 1 deletion app/views/cloud_volume/edit.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@

:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'EDIT');
miq_bootstrap(jQuery('#form_div'));
3 changes: 2 additions & 1 deletion app/views/cloud_volume/new.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@

:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id || "new"}');
ManageIQ.angular.app.value('storageManagerId', #{@storage_manager.try(:id) || "undefined"});
ManageIQ.angular.app.value('storageManagerId', '#{@storage_manager.try(:id).to_s || "undefined"}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'NEW');
miq_bootstrap('#form_div');
3 changes: 2 additions & 1 deletion app/views/cloud_volume/snapshot_new.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,6 @@

:javascript
ManageIQ.angular.app.value('cloudVolumeFormId', '#{@volume.id}');
ManageIQ.angular.app.value('storageManagerId', #{@volume.ext_management_system.id});
ManageIQ.angular.app.value('storageManagerId', '#{@volume.ext_management_system.id.to_s}');
ManageIQ.angular.app.value('cloudVolumeFormOperation', 'SNAPSHOT_NEW');
miq_bootstrap(jQuery('#form_div'));