Merge lp:~trapnine/maas/fix-1501982 into lp:~maas-committers/maas/trunk

[Jeffrey C Jones]

Replaced node list page 'Networks' filter with 'Subnets' and added new 'Fabrics' and 'Spaces' filters.

[Ricardo Bánffy (rbanffy)]

LGTM. Just add docstrings for the new methods. Remember you'll eventually forget what you did here (this you can find out by re-reading the code) and, most important, why it is the way it is (something code doesn't say most of the time).

review: Approve

[Blake Rouse (blake-rouse)]

Looks good. But missing 3 unit tests. You have unit tests on the overall generated result, but you should unit test each of those new methods explicitly.

review: Needs Fixing

[Jeffrey C Jones (trapnine)]

> Looks good. But missing 3 unit tests. You have unit tests on the overall
> generated result, but you should unit test each of those new methods
> explicitly.

added, thanks for review.

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~trapnine/maas/fix-1501982 into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com trusty-security InRelease [64.4 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:2 http://nova.clouds.archive.ubuntu.com trusty-updates InRelease [64.4 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:3 http://security.ubuntu.com trusty-security/main Sources [97.7 kB]
Get:4 http://security.ubuntu.com trusty-security/universe Sources [31.0 kB]
Get:5 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [241 kB]
Get:6 http://security.ubuntu.com trusty-security/main amd64 Packages [356 kB]
Get:7 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [140 kB]
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [117 kB]
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [636 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [324 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 2,071 kB in 3s (527 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm pep8 phantomjs postgresql pyflakes python-apt python-bson python-bzrlib python-convoy python-coverage python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lxml python-mock python-netaddr python-netifaces python-nose python-oauth python-openssl python-paramiko python-pexpect python-pip python-pocket-lint python-psycopg2 python-pyinotify python-pyparsing python-seamicroclient python-simplejson python-simplestreams python-sphinx python-subunit python-tempita python-testresources python-testscenarios python-testt...

=== modified file 'src/maasserver/static/js/angular/factories/nodes.js'
--- src/maasserver/static/js/angular/factories/nodes.js	2015-10-23 04:03:23 +0000
+++ src/maasserver/static/js/angular/factories/nodes.js	2015-10-27 02:33:24 +0000
@@ -26,7 +26,9 @@
                 "zone": function(node) {
                     return node.zone.name;
                 },
-                "networks": null,
+                "subnets": null,
+                "fabrics": null,
+                "spaces": null,
                 "storage_tags": null
             };
 

=== modified file 'src/maasserver/static/js/angular/factories/tests/test_nodes.js'
--- src/maasserver/static/js/angular/factories/tests/test_nodes.js	2015-10-23 16:34:24 +0000
+++ src/maasserver/static/js/angular/factories/tests/test_nodes.js	2015-10-27 02:33:24 +0000
@@ -47,7 +47,8 @@
         expect(NodesManager._pk).toBe("system_id");
         expect(NodesManager._handler).toBe("node");
         expect(Object.keys(NodesManager._metadataAttributes)).toEqual(
-            ["status", "owner", "tags", "zone", "networks", "storage_tags"]);
+            ["status", "owner", "tags", "zone", "subnets", "fabrics",
+            "spaces", "storage_tags"]);
     });
 
     describe("create", function() {

=== modified file 'src/maasserver/static/partials/nodes-list.html'
--- src/maasserver/static/partials/nodes-list.html	2015-10-13 09:38:21 +0000
+++ src/maasserver/static/partials/nodes-list.html	2015-10-27 02:33:24 +0000
@@ -447,12 +447,32 @@
                     </ul>
                 </div>
             </div>
-            <div class="ng-hide accordion__tab" data-ng-show="tabs.nodes.metadata.networks.length">
-                <h4 class="accordion__tab-title maas-accordion-tab">Networks</h4>
-                <div class="accordion__tab-content">
-                    <ul class="accordion__tab-list">
-                        <li class="accordion__tab-item" data-ng-repeat="network in tabs.nodes.metadata.networks | orderBy:['name', '-count']" data-ng-class="{ active: isFilterActive('networks', network.name, 'nodes') }">
-                            <a class="accordion__tab-link" href="" data-ng-click="toggleFilter('networks', network.name, 'nodes')">{$ network.name $} ({$ network.count $})</a>
+            <div class="ng-hide accordion__tab" data-ng-show="tabs.nodes.metadata.subnets.length">
+                <h4 class="accordion__tab-title maas-accordion-tab">Subnets</h4>
+                <div class="accordion__tab-content">
+                    <ul class="accordion__tab-list">
+                        <li class="accordion__tab-item" data-ng-repeat="subnet in tabs.nodes.metadata.subnets | orderBy:['name', '-count']" data-ng-class="{ active: isFilterActive('subnets', subnet.name, 'nodes') }">
+                            <a class="accordion__tab-link" href="" data-ng-click="toggleFilter('subnets', subnet.name, 'nodes')">{$ subnet.name $} ({$ subnet.count $})</a>
+                        </li>
+                    </ul>
+                </div>
+            </div>
+            <div class="ng-hide accordion__tab" data-ng-show="tabs.nodes.metadata.fabrics.length">
+                <h4 class="accordion__tab-title maas-accordion-tab">Fabrics</h4>
+                <div class="accordion__tab-content">
+                    <ul class="accordion__tab-list">
+                        <li class="accordion__tab-item" data-ng-repeat="fabric in tabs.nodes.metadata.fabrics | orderBy:['name', '-count']" data-ng-class="{ active: isFilterActive('fabrics', fabric.name, 'nodes') }">
+                            <a class="accordion__tab-link" href="" data-ng-click="toggleFilter('fabrics', fabric.name, 'nodes')">{$ fabric.name $} ({$ fabric.count $})</a>
+                        </li>
+                    </ul>
+                </div>
+            </div>
+            <div class="ng-hide accordion__tab" data-ng-show="tabs.nodes.metadata.spaces.length">
+                <h4 class="accordion__tab-title maas-accordion-tab">Spaces</h4>
+                <div class="accordion__tab-content">
+                    <ul class="accordion__tab-list">
+                        <li class="accordion__tab-item" data-ng-repeat="space in tabs.nodes.metadata.spaces | orderBy:['name', '-count']" data-ng-class="{ active: isFilterActive('spaces', space.name, 'nodes') }">
+                            <a class="accordion__tab-link" href="" data-ng-click="toggleFilter('spaces', space.name, 'nodes')">{$ space.name $} ({$ space.count $})</a>
                         </li>
                     </ul>
                 </div>

=== modified file 'src/maasserver/websockets/handlers/node.py'
--- src/maasserver/websockets/handlers/node.py	2015-10-23 04:03:23 +0000
+++ src/maasserver/websockets/handlers/node.py	2015-10-27 02:33:24 +0000
@@ -115,8 +115,11 @@
         queryset = (
             Node.nodes.filter(installable=True)
                 .select_related('nodegroup', 'pxe_mac', 'owner')
-                .prefetch_related('interface_set__ip_addresses__subnet__vlan')
+                .prefetch_related(
+                    'interface_set__ip_addresses__subnet__vlan__fabric')
+                .prefetch_related('interface_set__ip_addresses__subnet__space')
                 .prefetch_related('nodegroup__nodegroupinterface_set__subnet')
+                .prefetch_related('interface_set__vlan__fabric')
                 .prefetch_related('zone')
                 .prefetch_related('tags')
                 .prefetch_related('blockdevice_set__physicalblockdevice')
@@ -260,6 +263,11 @@
                 ]) / (1000 ** 3))
         data["storage_tags"] = self.get_all_storage_tags(blockdevices)
 
+        subnets = self.get_all_subnets(obj)
+        data["subnets"] = [subnet.name for subnet in subnets]
+        data["fabrics"] = self.get_all_fabric_names(obj, subnets)
+        data["spaces"] = self.get_all_space_names(subnets)
+
         data["tags"] = [
             tag.name
             for tag in obj.tags.all()
@@ -577,6 +585,28 @@
             tags = tags.union(blockdevice.tags)
         return list(tags)
 
+    def get_all_subnets(self, obj):
+        subnets = set()
+        for interface in obj.interface_set.all():
+            for ip_address in interface.ip_addresses.all():
+                if ip_address.subnet is not None:
+                    subnets.add(ip_address.subnet)
+        return list(subnets)
+
+    def get_all_fabric_names(self, obj, subnets):
+        fabric_names = set()
+        for interface in obj.interface_set.all():
+            fabric_names.add(interface.vlan.fabric.name)
+        for subnet in subnets:
+            fabric_names.add(subnet.vlan.fabric.name)
+        return list(fabric_names)
+
+    def get_all_space_names(self, subnets):
+        space_names = set()
+        for subnet in subnets:
+            space_names.add(subnet.space.name)
+        return list(space_names)
+
     def get_blockdevices_for(self, obj):
         """Return only `BlockDevice`s using the prefetched query."""
         return [

=== modified file 'src/maasserver/websockets/handlers/tests/test_node.py'
--- src/maasserver/websockets/handlers/tests/test_node.py	2015-10-23 18:39:01 +0000
+++ src/maasserver/websockets/handlers/tests/test_node.py	2015-10-27 02:33:24 +0000
@@ -134,6 +134,7 @@
             for cache_set in CacheSet.objects.get_cache_sets_for_node(node)
         ]
         disks = sorted(disks, key=itemgetter("name"))
+        subnets = handler.get_all_subnets(node)
         data = {
             "actions": compile_node_actions(node, handler.user).keys(),
             "architecture": node.architecture,
@@ -201,6 +202,9 @@
                 for blockdevice in node.physicalblockdevice_set.all()
             ]) / (1000 ** 3)),
             "storage_tags": handler.get_all_storage_tags(blockdevices),
+            "subnets": [subnet.name for subnet in subnets],
+            "fabrics": handler.get_all_fabric_names(node, subnets),
+            "spaces": handler.get_all_space_names(subnets),
             "swap_size": node.swap_size,
             "system_id": node.system_id,
             "tags": [
@@ -223,7 +227,9 @@
                 "pxe_mac_vendor",
                 "extra_macs",
                 "tags",
-                "networks",
+                "subnets",
+                "fabrics",
+                "spaces",
                 "physical_disk_count",
                 "storage",
                 "storage_tags",
@@ -660,6 +666,80 @@
             factory.make_Event(node=node, type=event_type)
         self.assertEquals([], handler.dehydrate_events(node))
 
+    def make_node_with_subnets(self):
+        user = factory.make_User()
+        handler = NodeHandler(user, {})
+        space1 = factory.make_Space()
+        fabric1 = factory.make_Fabric(name=factory.make_name("fabric"))
+        vlan1 = factory.make_VLAN(fabric=fabric1)
+        subnet1 = factory.make_Subnet(space=space1, vlan=vlan1)
+        node = factory.make_Node_with_Interface_on_Subnet(
+            subnet=subnet1, vlan=vlan1)
+        node.save()
+
+        # Bond interface with a VLAN on top. With the bond set to STATIC
+        # and the VLAN set to AUTO.
+        fabric2 = factory.make_Fabric(name=factory.make_name("fabric"))
+        vlan2 = factory.make_VLAN(fabric=fabric2)
+        space2 = factory.make_Space()
+        bond_subnet = factory.make_Subnet(space=space1, vlan=vlan1)
+        vlan_subnet = factory.make_Subnet(space=space2, vlan=vlan2)
+        nic1 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, vlan=vlan1)
+        nic2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, vlan=vlan2)
+        bond = factory.make_Interface(
+            INTERFACE_TYPE.BOND, parents=[nic1, nic2], vlan=vlan1)
+        vlan_int = factory.make_Interface(
+            INTERFACE_TYPE.VLAN, vlan=vlan2, parents=[bond])
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY,
+            ip=factory.pick_ip_in_network(bond_subnet.get_ipnetwork()),
+            subnet=bond_subnet, interface=bond)
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, ip="",
+            subnet=vlan_subnet, interface=vlan_int)
+
+        # LINK_UP interface with no subnet.
+        fabric3 = factory.make_Fabric(name=factory.make_name("fabric"))
+        vlan3 = factory.make_VLAN(fabric=fabric3)
+        nic3 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, vlan=vlan3, node=node)
+        nic3_ip = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, ip="",
+            subnet=None, interface=nic3)
+        nic3_ip.subnet = None
+        nic3_ip.save()
+
+        self.patch_autospec(interface_module, "update_host_maps")
+        boot_interface = node.get_boot_interface()
+        boot_interface.claim_static_ips()
+        node.boot_interface = boot_interface
+        node.save()
+
+        subnets = [subnet1, bond_subnet, vlan_subnet]
+        fabrics = [fabric1, fabric2, fabric3]
+        spaces = [space1, space2]
+        return (handler, node, subnets, fabrics, spaces)
+
+    def test_get_all_subnets(self):
+        (handler, node, subnets, _, _) = self.make_node_with_subnets()
+        self.assertItemsEqual(subnets, handler.get_all_subnets(node))
+
+    def test_get_all_fabric_names(self):
+        (handler, node, _, fabrics, _) = self.make_node_with_subnets()
+        fabric_names = [fabric.name for fabric in fabrics]
+        node_subnets = handler.get_all_subnets(node)
+        self.assertItemsEqual(
+            fabric_names, handler.get_all_fabric_names(node, node_subnets))
+
+    def test_get_all_space_names(self):
+        (handler, node, _, _, spaces) = self.make_node_with_subnets()
+        space_names = [space.name for space in spaces]
+        node_subnets = handler.get_all_subnets(node)
+        self.assertItemsEqual(
+            space_names, handler.get_all_space_names(node_subnets))
+
     def test_get(self):
         user = factory.make_User()
         handler = NodeHandler(user, {})
@@ -757,7 +837,7 @@
         # number means regiond has to do more work slowing down its process
         # and slowing down the client waiting for the response.
         self.assertEquals(
-            query_10_count, 9,
+            query_10_count, 11,
             "Number of queries has changed; make sure this is expected.")
         self.assertEquals(
             query_10_count, query_20_count,