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

Need to specify alternate infra dashboard for OpenStack provider #423

Closed
tzumainn opened this issue Feb 17, 2017 · 9 comments
Closed

Need to specify alternate infra dashboard for OpenStack provider #423

tzumainn opened this issue Feb 17, 2017 · 9 comments
Labels

Comments

@tzumainn
Copy link
Contributor

The OpenStack infrastructure provider needs an alternate dashboard. I've played around with the code a bit, and I think I need to do the following:

  • Create app/services/ems_openstack_infra_dashboard_service.rb to get dashboard data
  • Create app/views/ems_infra/_show_openstack_dashboard.html.haml

Then app/controllers/ems_infra_controller.rb needs to know to use the correct service depending on the ems, and app/views/ems_infra/show_dashboard.html.haml needs to know which view to render.

So here are some initial questions:

  • Do I specify the ems's dashboard service/view at the model level? Is it as simple as creating two methods which return the service/view?
  • I want to use some functions in app/assets/javascripts/controllers/ems_infra_dashboard/ems_infra_dashboard_controller.js, and add others; should I create a new controller instead, or is it okay to just use the same JS controller? If I need to create a new one, what's the proper way to handle re-use of functions?
@tzumainn
Copy link
Contributor Author

@martinpovolny @himdel I think you guys expressed an opinion on this subject, so please feel free to comment!

@martinpovolny
Copy link
Member

We can use the class decorators on a Ems subclass to return the correct dashboard service. Then you can reuse the existing controller. See: #237 (will get hopefully merged asap)

@martinpovolny
Copy link
Member

The JS (angular) controlers are cut-n-pasted at this point and we need to stop doing that. @himdel: can you provide some guidance on how to reuse the angular controller code, please?

@himdel
Copy link
Contributor

himdel commented Feb 24, 2017

Well, app/assets/javascripts/controllers/ems_infra_dashboard/ems_infra_dashboard_controller.js and app/assets/javascripts/controllers/container_dashboard/container_dashboard_controller.js are really very similar (while ContainerLiveDashboard looks rather different), so I'd suggest looking into unifing all three (the similar ones and yours, not Live).

Seems like we could use a .DashboardService (as in, an angular service), with all the common functions there... right now, if we undo the infra/container and node/cluster naming differences, the difference between the two is just this...

--- app/assets/javascripts/controllers/ems_infra_dashboard/ems_infra_dashboard_controller.js	2017-02-24 21:57:09.310581328 +0000
+++ app/assets/javascripts/controllers/container_dashboard/container_dashboard_controller.js	2017-02-24 21:57:10.530563058 +0000
@@ -6,12 +6,15 @@
 
       // Obj-status cards init
       $scope.objectStatus = {
-        providers:     dashboardUtilsFactory.createProvidersIcon(),
-        ems_clusters:  dashboardUtilsFactory.createClustersStatus(),
-        hosts:         dashboardUtilsFactory.createHostsStatus(),
-        datastores:    dashboardUtilsFactory.createDatastoresStatus(),
-        vms:           dashboardUtilsFactory.createVmsStatus(),
-        miq_templates: dashboardUtilsFactory.createMiqTemplatesStatus(),
+        providers:  dashboardUtilsFactory.createProvidersStatus(),
+        nodes:      dashboardUtilsFactory.createNodesStatus(),
+        containers: dashboardUtilsFactory.createContainersStatus(),
+        registries: dashboardUtilsFactory.createRegistriesStatus(),
+        projects:   dashboardUtilsFactory.createProjectsStatus(),
+        pods:       dashboardUtilsFactory.createPodsStatus(),
+        services:   dashboardUtilsFactory.createServicesStatus(),
+        images:     dashboardUtilsFactory.createImagesStatus(),
+        routes:     dashboardUtilsFactory.createRoutesStatus()
       };
 
       $scope.loadingDone = false;
@@ -63,7 +66,7 @@
           $scope.id = '/' + (/^\/[^\/]+\/(\d+)$/.exec(pathname)[1]);
         }
 
-        var url = '/ems_infra_dashboard/data' + $scope.id;
+        var url = '/container_dashboard/data' + $scope.id;
         $http.get(url)
           .then(getDashboardData)
           .catch(miqService.handleFailure);
@@ -104,11 +107,14 @@
           }
         }
 
-        dashboardUtilsFactory.updateStatus($scope.objectStatus.ems_clusters, data.status.ems_clusters);
-        dashboardUtilsFactory.updateStatus($scope.objectStatus.hosts, data.status.hosts);
-        dashboardUtilsFactory.updateStatus($scope.objectStatus.datastores, data.status.datastores);
-        dashboardUtilsFactory.updateStatus($scope.objectStatus.vms, data.status.vms);
-        dashboardUtilsFactory.updateStatus($scope.objectStatus.miq_templates, data.status.miq_templates);
+        dashboardUtilsFactory.updateStatus($scope.objectStatus.nodes, data.status.nodes);
+        dashboardUtilsFactory.updateStatus($scope.objectStatus.containers, data.status.containers);
+        dashboardUtilsFactory.updateStatus($scope.objectStatus.registries, data.status.registries);
+        dashboardUtilsFactory.updateStatus($scope.objectStatus.projects, data.status.projects);
+        dashboardUtilsFactory.updateStatus($scope.objectStatus.pods, data.status.pods);
+        dashboardUtilsFactory.updateStatus($scope.objectStatus.services, data.status.services);
+        dashboardUtilsFactory.updateStatus($scope.objectStatus.images, data.status.images);
+        dashboardUtilsFactory.updateStatus($scope.objectStatus.routes, data.status.routes);
 
         // Node utilization donut
         $scope.cpuUsageData = chartsMixin.processUtilizationData(data.ems_utilization.cpu,

Also, it looks like we pretty much just iterate over all (or some of?) the entities and do the same block all over again...

        // Recent Hosts
        $scope.recentHostsConfig = chartsMixin.chartConfig.recentHostsConfig;
        // recent Hosts chart
        $scope.recentHostsData = chartsMixin.processRecentHostsData(data.recentHosts,
          'dates',
          $scope.recentHostsConfig.label);

is repeated for multiple entities .. so this could be something like...

['recentHosts', 'others'].forEach(function(entity) {
  vm.config[entity] = chartsMixin.chartConfig[entity];
  vm.data[entity] = chartsMixin.process[entity](data[entity], 'dates', vm.config[entity].label);
});

so the controllers could almost share all of the methods except for a lists of entities, and the right url.

@himdel
Copy link
Contributor

himdel commented Feb 24, 2017

..and looks like the exact same method could be applied to the files living under util/ in each of the controller's directories :)

@himdel
Copy link
Contributor

himdel commented Feb 24, 2017

About the exact mechanism of code sharing, I think at this point, just moving the methods to a common service and passing in the relevant configs seems like the most straightforward way.

(It's always possible to Object.assign(this, service) and treat the service as a mixin, but.. seems like iterating over the configs and calling common methods from each controller is DRY enough)

@miq-bot miq-bot added the stale label Jan 3, 2018
@miq-bot
Copy link
Member

miq-bot commented Jan 3, 2018

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

@JPrause
Copy link
Member

JPrause commented Jan 28, 2019

@tzumainn is this still a valid issue? If yes, lease remove the stale label. If not can you close.
If there's no update by next week, I'll be closing this issue.

@JPrause
Copy link
Member

JPrause commented Feb 6, 2019

Closing issue. If you feel the issue needs to remain open, please either reopen or let me know and it will be reopened.
@miq-bot close_issue

@miq-bot miq-bot closed this as completed Feb 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants