Merge lp:~blake-rouse/maas/fix-1445941 into lp:~maas-committers/maas/trunk

[Blake Rouse]

Add the ability to filter the machines listing based on the deploying or deployed os/release.

[Mike Pontillo (mpontillo)]

Looks good to me!

One comment below about the usage of hard-coded integers rather than an easier-to-read constant or enum. I won't block you on it, but is there a reason you didn't use what is defined in src/maasserver/static/js/enums.js?

review: Approve

[Blake Rouse (blake-rouse)]

enums.js is for YUI only. That should be converted to an Angular service at some point, once YUI is removed.

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-09-28 13:20:00 +0000
+++ src/maasserver/models/node.py	2016-10-04 21:01:43 +0000
@@ -1943,15 +1943,17 @@
         global_value = Config.objects.get_config('kernel_opts')
         return None, global_value
 
-    def get_osystem(self):
+    def get_osystem(self, default=None):
         """Return the operating system to install that node."""
         use_default_osystem = (self.osystem is None or self.osystem == '')
         if use_default_osystem:
-            return Config.objects.get_config('default_osystem')
+            if default is None:
+                default = Config.objects.get_config('default_osystem')
+            return default
         else:
             return self.osystem
 
-    def get_distro_series(self):
+    def get_distro_series(self, default=None):
         """Return the distro series to install that node."""
         use_default_osystem = (
             self.osystem is None or
@@ -1960,7 +1962,9 @@
             self.distro_series is None or
             self.distro_series == '')
         if use_default_osystem and use_default_distro_series:
-            return Config.objects.get_config('default_distro_series')
+            if default is None:
+                default = Config.objects.get_config('default_distro_series')
+            return default
         else:
             return self.distro_series
 

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2016-09-28 13:20:00 +0000
+++ src/maasserver/models/tests/test_node.py	2016-10-04 21:01:43 +0000
@@ -975,11 +975,21 @@
         osystem = Config.objects.get_config('default_osystem')
         self.assertEqual(osystem, node.get_osystem())
 
+    def test_get_osystem_returns_passed_default(self):
+        node = factory.make_Node(osystem='')
+        default = factory.make_name("default")
+        self.assertEqual(default, node.get_osystem(default=default))
+
     def test_get_distro_series_returns_default_series(self):
         node = factory.make_Node(distro_series='')
         series = Config.objects.get_config('default_distro_series')
         self.assertEqual(series, node.get_distro_series())
 
+    def test_get_distro_series_returns_passed_default(self):
+        node = factory.make_Node(osystem='')
+        default = factory.make_name("default")
+        self.assertEqual(default, node.get_distro_series(default=default))
+
     def test_get_effective_license_key_returns_node_value(self):
         license_key = factory.make_name('license_key')
         node = factory.make_Node(license_key=license_key)

=== modified file 'src/maasserver/static/js/angular/controllers/nodes_list.js'
--- src/maasserver/static/js/angular/controllers/nodes_list.js	2016-09-30 22:45:36 +0000
+++ src/maasserver/static/js/angular/controllers/nodes_list.js	2016-10-04 21:01:43 +0000
@@ -433,14 +433,6 @@
             updateActionErrorCount(tab);
             enterViewSelected(tab);
 
-            var actionOption = $scope.tabs[tab].actionOption;
-            if(angular.isObject(actionOption) &&
-                actionOption.name === "deploy") {
-                GeneralManager.startPolling("osinfo");
-            } else {
-                GeneralManager.stopPolling("osinfo");
-            }
-
             // Hide the add hardware/device section.
             if (tab === 'nodes') {
                 if(angular.isObject($scope.addHardwareScope)) {
@@ -496,7 +488,6 @@
             resetActionProgress(tab);
             leaveViewSelected(tab);
             $scope.tabs[tab].actionOption = null;
-            GeneralManager.stopPolling("osinfo");
         };
 
         // Perform the action on all nodes.
@@ -606,6 +597,17 @@
             return UsersManager.isSuperUser();
         };
 
+        // Returns the release title from osinfo.
+        $scope.getReleaseTitle = function(os_release) {
+            for(i = 0; i < $scope.osinfo.releases.length; i++) {
+                var release = $scope.osinfo.releases[i];
+                if(release[0] === os_release) {
+                    return release[1];
+                }
+            }
+            return os_release;
+        };
+
         // Load the required managers for this controller. The ServicesManager
         // is required by the maasControllerStatus directive that is used
         // in the partial for this controller.
@@ -616,6 +618,9 @@
                 $scope.loading = false;
             });
 
+        // Start polling for the os information.
+        GeneralManager.startPolling("osinfo");
+
         // Stop polling and save the current filter when the scope is destroyed.
         $scope.$on("$destroy", function() {
             $interval.cancel($scope.statusPoll);

=== modified file 'src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js'
--- src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js	2016-09-29 18:31:21 +0000
+++ src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js	2016-10-04 21:01:43 +0000
@@ -152,6 +152,28 @@
         });
     });
 
+    describe("getReleaseTitle", function() {
+        it("returns release title from osinfo", function() {
+            var controller = makeController();
+            $scope.osinfo = {
+                releases: [
+                    ['ubuntu/xenial', 'Ubuntu Xenial']
+                ]
+            };
+            expect($scope.getReleaseTitle('ubuntu/xenial')).toBe(
+                'Ubuntu Xenial');
+        });
+
+        it("returns release name when not in osinfo", function() {
+            var controller = makeController();
+            $scope.osinfo = {
+                releases: []
+            };
+            expect($scope.getReleaseTitle('ubuntu/xenial')).toBe(
+                'ubuntu/xenial');
+        });
+    });
+
     it("sets title and page on $rootScope", function() {
         var controller = makeController();
         expect($rootScope.title).toBe("Machines");
@@ -180,6 +202,13 @@
         expect($scope.loading).toBe(true);
     });
 
+    it("calls startPolling when scope created", function() {
+        spyOn(GeneralManager, "startPolling");
+        var controller = makeController();
+        expect(GeneralManager.startPolling).toHaveBeenCalledWith(
+            "osinfo");
+    });
+
     it("calls stopPolling when scope destroyed", function() {
         var controller = makeController();
         spyOn(GeneralManager, "stopPolling");
@@ -827,35 +856,6 @@
                     expect($scope.tabs[tab].previous_search).toBe(search);
                 });
 
-                it("action deploy calls startPolling for osinfo", function() {
-                    var controller = makeController();
-                    $scope.tabs[tab].actionOption = {
-                        "name": "deploy"
-                    };
-                    spyOn(GeneralManager, "startPolling");
-                    $scope.actionOptionSelected(tab);
-                    expect(GeneralManager.startPolling).toHaveBeenCalledWith(
-                        "osinfo");
-                });
-
-                it("changing away from deploy calls startPolling for osinfo",
-                    function() {
-                        var controller = makeController();
-                        $scope.tabs[tab].actionOption = {
-                            "name": "deploy"
-                        };
-                        spyOn(GeneralManager, "startPolling");
-                        spyOn(GeneralManager, "stopPolling");
-                        $scope.actionOptionSelected(tab);
-
-                        $scope.tabs[tab].actionOption = {
-                            "name": "acquire"
-                        };
-                        $scope.actionOptionSelected(tab);
-                        var expected = expect(GeneralManager.stopPolling);
-                        expected.toHaveBeenCalledWith("osinfo");
-                    });
-
                 it("calls hide on addHardwareScope", function() {
                     var controller;
                     if (tab === 'nodes') {

=== modified file 'src/maasserver/static/js/angular/factories/machines.js'
--- src/maasserver/static/js/angular/factories/machines.js	2016-03-28 13:54:47 +0000
+++ src/maasserver/static/js/angular/factories/machines.js	2016-10-04 21:01:43 +0000
@@ -28,7 +28,14 @@
                 "subnets": null,
                 "fabrics": null,
                 "spaces": null,
-                "storage_tags": null
+                "storage_tags": null,
+                "release": function(machine) {
+                    if(machine.status_code === 6 || machine.status_code === 9) {
+                        return machine.osystem + "/" + machine.distro_series;
+                    } else {
+                        return '';
+                    }
+                }
             };
 
             // Listen for notify events for the machine object.

=== modified file 'src/maasserver/static/js/angular/factories/tests/test_machines.js'
--- src/maasserver/static/js/angular/factories/tests/test_machines.js	2016-03-28 13:54:47 +0000
+++ src/maasserver/static/js/angular/factories/tests/test_machines.js	2016-10-04 21:01:43 +0000
@@ -35,7 +35,7 @@
     it("set requires attributes", function() {
         expect(Object.keys(MachinesManager._metadataAttributes)).toEqual(
             ["status", "owner", "tags", "zone", "subnets", "fabrics",
-            "spaces", "storage_tags"]);
+            "spaces", "storage_tags", "release"]);
     });
 
     describe("mountSpecialFilesystem", function() {

=== modified file 'src/maasserver/static/js/angular/filters/nodes.js'
--- src/maasserver/static/js/angular/filters/nodes.js	2015-06-04 14:58:39 +0000
+++ src/maasserver/static/js/angular/filters/nodes.js	2016-10-04 21:01:43 +0000
@@ -34,6 +34,13 @@
             },
             power: function(node) {
                 return node.power_state;
+            },
+            release: function(node) {
+                if(node.status_code === 6 || node.status_code === 9) {
+                    return node.osystem + "/" + node.distro_series;
+                } else {
+                    return '';
+                }
             }
         };
 

=== modified file 'src/maasserver/static/js/angular/filters/tests/test_nodes.js'
--- src/maasserver/static/js/angular/filters/tests/test_nodes.js	2015-06-03 21:09:38 +0000
+++ src/maasserver/static/js/angular/filters/tests/test_nodes.js	2016-10-04 21:01:43 +0000
@@ -340,4 +340,31 @@
             [matchingNode]);
     });
 
+    it("matches using release mapping function", function() {
+        var deployingNode = {
+            status_code: 9,
+            osystem: 'ubuntu',
+            distro_series: 'xenial'
+        };
+        var deployedNode = {
+            status_code: 6,
+            osystem: 'ubuntu',
+            distro_series: 'xenial'
+        };
+        var allocatedNode = {
+            status_code: 5,
+            osystem: 'ubuntu',
+            distro_series: 'xenial'
+        };
+        var deployedOtherNode = {
+            status_code: 6,
+            osystem: 'ubuntu',
+            distro_series: 'trusty'
+        };
+        var nodes = [
+            deployingNode, deployedNode, allocatedNode, deployedOtherNode];
+        expect(nodesFilter(nodes, "release:ubuntu/xenial")).toEqual(
+            [deployingNode, deployedNode]);
+    });
+
 });

=== modified file 'src/maasserver/static/partials/nodes-list.html'
--- src/maasserver/static/partials/nodes-list.html	2016-09-30 14:59:49 +0000
+++ src/maasserver/static/partials/nodes-list.html	2016-10-04 21:01:43 +0000
@@ -569,6 +569,16 @@
                     </ul>
                 </div>
             </div>
+            <div class="ng-hide accordion__tab" data-ng-show="tabs.nodes.metadata.release.length">
+                <h4 class="accordion__tab-title maas-accordion-tab">OS/Release</h4>
+                <div class="accordion__tab-content">
+                    <ul class="accordion__tab-list">
+                        <li class="accordion__tab-item" data-ng-repeat="release in tabs.nodes.metadata.release | orderBy:['name', '-count']" data-ng-class="{ 'is-active': isFilterActive('release', release.name, 'nodes') }">
+                            <a class="accordion__tab-link" href="" data-ng-click="toggleFilter('release', release.name, 'nodes')">{$ getReleaseTitle(release.name) $} ({$ release.count $})</a>
+                        </li>
+                    </ul>
+                </div>
+            </div>
             <div class="ng-hide accordion__tab" data-ng-show="tabs.nodes.metadata.tags.length">
                 <h4 class="accordion__tab-title maas-accordion-tab">Tags</h4>
                 <div class="accordion__tab-content">

=== modified file 'src/maasserver/websockets/handlers/machine.py'
--- src/maasserver/websockets/handlers/machine.py	2016-09-09 21:49:49 +0000
+++ src/maasserver/websockets/handlers/machine.py	2016-10-04 21:01:43 +0000
@@ -52,6 +52,7 @@
 from maasserver.forms_interface_link import InterfaceLinkForm
 from maasserver.models.blockdevice import BlockDevice
 from maasserver.models.cacheset import CacheSet
+from maasserver.models.config import Config
 from maasserver.models.filesystemgroup import VolumeGroup
 from maasserver.models.interface import Interface
 from maasserver.models.node import (
@@ -164,6 +165,17 @@
         return Machine.objects.get_nodes(
             self.user, NODE_PERMISSION.VIEW, from_nodes=self._meta.queryset)
 
+    def list(self, params):
+        """List objects.
+
+        Caches default_osystem and default_distro_series so only 2 queries are
+        made for the whole list of nodes.
+        """
+        self.default_osystem = Config.objects.get_config('default_osystem')
+        self.default_distro_series = Config.objects.get_config(
+            'default_distro_series')
+        return super(MachineHandler, self).list(params)
+
     def dehydrate(self, obj, data, for_list=False):
         """Add extra fields to `data`."""
         data = super(MachineHandler, self).dehydrate(

=== modified file 'src/maasserver/websockets/handlers/node.py'
--- src/maasserver/websockets/handlers/node.py	2016-09-23 20:13:07 +0000
+++ src/maasserver/websockets/handlers/node.py	2016-10-04 21:01:43 +0000
@@ -61,6 +61,9 @@
 
 class NodeHandler(TimestampedModelHandler):
 
+    default_osystem = None
+    default_distro_series = None
+
     class Meta:
         abstract = True
         pk = 'system_id'
@@ -138,9 +141,11 @@
             tag.name
             for tag in obj.tags.all()
         ]
+        data["osystem"] = obj.get_osystem(
+            default=self.default_osystem)
+        data["distro_series"] = obj.get_distro_series(
+            default=self.default_distro_series)
         if not for_list:
-            data["osystem"] = obj.get_osystem()
-            data["distro_series"] = obj.get_distro_series()
             data["hwe_kernel"] = make_hwe_kernel_ui_text(obj.hwe_kernel)
 
             data["power_type"] = obj.power_type

=== modified file 'src/maasserver/websockets/handlers/tests/test_machine.py'
--- src/maasserver/websockets/handlers/tests/test_machine.py	2016-09-27 23:03:56 +0000
+++ src/maasserver/websockets/handlers/tests/test_machine.py	2016-10-04 21:01:43 +0000
@@ -250,6 +250,8 @@
                 "storage",
                 "storage_tags",
                 "node_type_display",
+                "osystem",
+                "distro_series",
             ]
             for key in list(data):
                 if key not in allowed_fields:
@@ -973,7 +975,7 @@
         # number means regiond has to do more work slowing down its process
         # and slowing down the client waiting for the response.
         self.assertEqual(
-            query_10_count, 10,
+            query_10_count, 12,
             "Number of queries has changed; make sure this is expected.")
         self.assertEqual(
             query_10_count, query_20_count,