Merge lp:~ltrager/maas/round_ram into lp:~maas-committers/maas/trunk

[Lee Trager]

Allow refreshing and import images on rack controllers via the UI and show memory to the first decimal place

As discussed during the sprint round show RAM size to the first decimal place, and allow rack controllers to be refreshed and import images via the node action list.

The node actions themselves are only being seen in the node-list page, not the node-details page.

[Jeffrey C Jones (trapnine)]

Cool - please revert the import images bits from your branch, as I've merged and tested them in my import images branch.

I've also fixed the issue you mention where actions weren't appearing properly on the controller details page.

See https://code.launchpad.net/~trapnine/maas/fix-1583685-force-import/+merge/295434

review: Needs Fixing

[Lee Trager (ltrager)]

I've removed the import images node action so you can drive it in your MP. There is still some duplicated code as its needed for the refresh command. Hopefully bzr will merge it for us, otherwise we'll just have to deal with a small merge conflict.

[Jeffrey C Jones (trapnine)]

Looks good. The merge shouldn't be too bad.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~ltrager/maas/round_ram into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [94.5 kB]
Hit:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:4 http://security.ubuntu.com/ubuntu xenial-security InRelease [94.5 kB]
Fetched 189 kB in 0s (412 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr 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 postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-coverage python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-2ubuntu3).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
build-essential is already the newest version (12.1ubuntu2).
bzr is already the newest version (2.7.0-2ubuntu1).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
isc-dhcp-common is already the newest version (4.3.3-5ubuntu12).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
pxelinux is already the newest version (3:6...

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-05-16 08:36:33 +0000
+++ src/maasserver/models/node.py	2016-05-25 21:25:22 +0000
@@ -1302,7 +1302,14 @@
         """Return memory in GiB."""
         if self.memory < 1024:
             return '%.1f' % (self.memory / 1024.0)
-        return '%d' % (self.memory / 1024)
+        # Commissioning gets all available memory to the system. However some
+        # memory can be reserved by the motherboard(e.g for video memory) or
+        # the kernel itself. Commissioning can't detect reserved RAM so show
+        # the RAM in GiB to the first decimal place. Python rounds the float
+        # which results in the correct value. For example a KVM virt is
+        # configured with 2048 MiB of RAM but only 2047MiB is detectable.
+        # 2047 / 1024 = 1.9990 which rounds to 2.0.
+        return '%.1f' % (self.memory / 1024.0)
 
     @property
     def physicalblockdevice_set(self):

=== modified file 'src/maasserver/models/signals/nodes.py'
--- src/maasserver/models/signals/nodes.py	2016-03-28 13:54:47 +0000
+++ src/maasserver/models/signals/nodes.py	2016-05-25 21:25:22 +0000
@@ -9,6 +9,7 @@
 
 from django.db.models.signals import post_save
 from maasserver.models import (
+    Controller,
     Device,
     Machine,
     Node,
@@ -24,6 +25,7 @@
     Node,
     Machine,
     Device,
+    Controller,
     RackController,
     RegionController,
 ]

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2016-05-16 08:36:33 +0000
+++ src/maasserver/models/tests/test_node.py	2016-05-25 21:25:22 +0000
@@ -578,8 +578,8 @@
         self.assertEqual('0.5', node.display_memory())
 
     def test_display_memory_returns_value_divided_by_1024(self):
-        node = factory.make_Node(memory=2048)
-        self.assertEqual('2', node.display_memory())
+        node = factory.make_Node(memory=2560)
+        self.assertEqual('2.5', node.display_memory())
 
     def test_physicalblockdevice_set_returns_physicalblockdevices(self):
         node = factory.make_Node(with_boot_disk=False)

=== modified file 'src/maasserver/node_action.py'
--- src/maasserver/node_action.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/node_action.py	2016-05-25 21:25:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Node actions.
@@ -90,12 +90,10 @@
         A list of NODE_TYPEs which are applicable for this action.
         """)
 
-    actionable_statuses = abstractproperty("""
-        Node states for which this action makes sense.
-
-        A collection of NODE_STATUS values.  The action will be available
-        only if `node.status in action.actionable_statuses`.
-        """)
+    # Optional node states for which this action makes sense.
+    # A collection of NODE_STATUS values.  The action will be available
+    # only if `node.status in action.actionable_statuses`.
+    actionable_statuses = None
 
     permission = abstractproperty("""
         Required permission.
@@ -211,10 +209,6 @@
         zone = Zone.objects.get(id=zone_id)
         self.node.set_zone(zone)
 
-    def is_actionable(self):
-        """Returns true if the selected nodes can be added to a zone"""
-        return super(SetZone, self).is_actionable()
-
 
 class Commission(NodeAction):
     """Accept a node into the MAAS, and start the commissioning process."""
@@ -350,10 +344,6 @@
         except RPC_EXCEPTIONS + (ExternalProcessError,) as exception:
             raise NodeActionError(exception)
 
-    def is_actionable(self):
-        is_actionable = super(PowerOn, self).is_actionable()
-        return is_actionable
-
 
 FAILED_STATUSES = [
     status for status in map_enum(NODE_STATUS).values()
@@ -470,6 +460,25 @@
         return True
 
 
+class Refresh(NodeAction):
+    """Refresh a rack or region and rack controller."""
+    name = "refresh"
+    display = "Refresh"
+    display_sentence = "refreshed"
+    permission = NODE_PERMISSION.ADMIN
+    for_type = {
+        NODE_TYPE.RACK_CONTROLLER,
+        NODE_TYPE.REGION_AND_RACK_CONTROLLER
+    }
+
+    def execute(self):
+        """See `NodeAction.execute`."""
+        try:
+            self.node.refresh()
+        except RPC_EXCEPTIONS + (ExternalProcessError,) as exception:
+            raise NodeActionError(exception)
+
+
 ACTION_CLASSES = (
     Commission,
     Acquire,
@@ -482,6 +491,7 @@
     MarkFixed,
     SetZone,
     Delete,
+    Refresh,
 )
 
 ACTIONS_DICT = OrderedDict((action.name, action) for action in ACTION_CLASSES)

=== modified file 'src/maasserver/tests/test_node_action.py'
--- src/maasserver/tests/test_node_action.py	2016-05-16 09:21:53 +0000
+++ src/maasserver/tests/test_node_action.py	2016-05-25 21:25:22 +0000
@@ -5,6 +5,7 @@
 
 __all__ = []
 
+import random
 from unittest.mock import ANY
 
 from django.db import transaction
@@ -18,6 +19,7 @@
     NODE_STATUS_CHOICES,
     NODE_STATUS_CHOICES_DICT,
     NODE_TYPE,
+    NODE_TYPE_CHOICES,
     POWER_STATE,
 )
 from maasserver.exceptions import NodeActionError
@@ -25,6 +27,10 @@
     signals,
     StaticIPAddress,
 )
+from maasserver.models.node import (
+    RackController,
+    typecast_to_node_type,
+)
 from maasserver.models.signals.testing import SignalsDisabled
 from maasserver.node_action import (
     Abort,
@@ -38,6 +44,7 @@
     NodeAction,
     PowerOff,
     PowerOn,
+    Refresh,
     Release,
     RPC_EXCEPTIONS,
     SetZone,
@@ -60,7 +67,10 @@
     post_commit_hooks,
     reload_object,
 )
-from maastesting.matchers import MockCalledOnceWith
+from maastesting.matchers import (
+    MockCalledOnce,
+    MockCalledOnceWith,
+)
 from metadataserver.enum import RESULT_TYPE
 from metadataserver.models.noderesult import NodeResult
 from netaddr import IPNetwork
@@ -815,6 +825,33 @@
         self.assertItemsEqual([], actions)
 
 
+class TestRefresh(MAASServerTestCase):
+
+    def test_refresh(self):
+        user = factory.make_admin()
+        rack = factory.make_RackController()
+        mock_refresh = self.patch(rack, 'refresh')
+
+        with post_commit_hooks:
+            Refresh(rack, user).execute()
+
+        self.assertThat(mock_refresh, MockCalledOnce())
+
+    def test_requires_admin_permission(self):
+        user = factory.make_User()
+        rack = factory.make_RackController()
+        self.assertFalse(Refresh(rack, user).is_permitted())
+
+    def test_requires_rack(self):
+        user = factory.make_User()
+        node = factory.make_Node(
+            node_type=factory.pick_choice(
+                NODE_TYPE_CHOICES, but_not=[
+                    NODE_TYPE.RACK_CONTROLLER,
+                    NODE_TYPE.REGION_AND_RACK_CONTROLLER]))
+        self.assertFalse(Refresh(node, user).is_actionable())
+
+
 class TestActionsErrorHandling(MAASServerTestCase):
     """Tests for error handling in actions.
 
@@ -838,13 +875,17 @@
         exception = self.make_exception()
         self.patch(node, '_start').side_effect = exception
         self.patch(node, '_stop').side_effect = exception
+        if isinstance(node, RackController):
+            self.patch(node, 'refresh').side_effect = exception
 
-    def make_action(self, action_class, node_status, power_state=None):
+    def make_action(
+            self, action_class, node_status, power_state=None,
+            node_type=NODE_TYPE.MACHINE):
         node = factory.make_Node(
             interface=True, status=node_status, power_type='manual',
-            power_state=power_state)
+            power_state=power_state, node_type=node_type)
         admin = factory.make_admin()
-        return action_class(node, admin)
+        return action_class(typecast_to_node_type(node), admin)
 
     def test_Commission_handles_rpc_errors(self):
         self.addCleanup(signals.power.signals.enable)
@@ -891,3 +932,15 @@
         self.assertEqual(
             get_error_message_for_exception(action.node._stop.side_effect),
             str(exception))
+
+    def test_Refresh_handles_rpc_errors(self):
+        action = self.make_action(
+            Refresh, NODE_STATUS.ALLOCATED, power_state=POWER_STATE.ON,
+            node_type=random.choice([
+                NODE_TYPE.RACK_CONTROLLER,
+                NODE_TYPE.REGION_AND_RACK_CONTROLLER]))
+        self.patch_rpc_methods(action.node)
+        exception = self.assertRaises(NodeActionError, action.execute)
+        self.assertEqual(
+            get_error_message_for_exception(action.node._stop.side_effect),
+            str(exception))

=== modified file 'src/maasserver/websockets/handlers/tests/test_general.py'
--- src/maasserver/websockets/handlers/tests/test_general.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/websockets/handlers/tests/test_general.py	2016-05-25 21:25:22 +0000
@@ -13,6 +13,7 @@
     BOND_MODE_CHOICES,
     BOND_XMIT_HASH_POLICY_CHOICES,
     BOOT_RESOURCE_TYPE,
+    NODE_TYPE,
 )
 from maasserver.models import BootSourceCache
 from maasserver.models.config import Config
@@ -28,7 +29,7 @@
 
 class TestGeneralHandler(MAASServerTestCase):
 
-    def dehydrate_actions(self, actions):
+    def dehydrate_actions(self, actions, node_type=None):
         return [
             {
                 "name": name,
@@ -36,6 +37,7 @@
                 "sentence": action.display_sentence,
             }
             for name, action in actions.items()
+            if node_type is None or node_type in action.for_type
             ]
 
     def test_architectures(self):
@@ -107,7 +109,8 @@
 
     def test_machine_actions_for_admin(self):
         handler = GeneralHandler(factory.make_admin(), {})
-        actions_expected = self.dehydrate_actions(ACTIONS_DICT)
+        actions_expected = self.dehydrate_actions(
+            ACTIONS_DICT, NODE_TYPE.MACHINE)
         self.assertItemsEqual(actions_expected, handler.machine_actions({}))
 
     def test_machine_actions_for_non_admin(self):
@@ -141,7 +144,7 @@
     def test_rack_controller_actions_for_admin(self):
         handler = GeneralHandler(factory.make_admin(), {})
         self.assertItemsEqual(
-            ['delete', 'off', 'on', 'set-zone'],
+            ['delete', 'off', 'on', 'refresh', 'set-zone'],
             [action['name'] for action in handler.rack_controller_actions({})])
 
     def test_rack_controller_actions_for_non_admin(self):
@@ -151,7 +154,7 @@
     def test_region_and_rack_controller_actions_for_admin(self):
         handler = GeneralHandler(factory.make_admin(), {})
         self.assertItemsEqual(
-            ['set-zone', 'delete'],
+            ['set-zone', 'refresh', 'delete'],
             [action['name']
              for action in handler.region_and_rack_controller_actions({})])