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

[Lee Trager]

Only pass custom storage and networking options to curtin if the OS is supported

When generating the preseed data to pass to curtin check if the osystem is Ubuntu. If it isn't warn the user and pass no storage and networking data. In the absence of data this tells curtin to use a flat storage layout and the OS networking default configuration.

In the UI we also display a warning to the user in both the networking and storage section telling them that the shown configuration was not used on the due to an OS other than Ubuntu being used. We currently have no way of informing CLI users this so I added a note in the documentation.

I also came across a bug where when deploying a non Ubuntu OS with min_hwe_kernel set we were deploying with the commissioning OS(trusty) and the default kernel(hwe-t). This patch also ensures that when a non-Ubuntu OS is deployed min_hwe_kernel is respected during deployment.

[Blake Rouse (blake-rouse)]

Looks good. Got some comments that need to be addressed. Not going to block you on landing this branch because of the issues. just fix them before landing. Mainly the extra MockNeverCalled? Don't know if that is any different than MockNotCalled.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~ltrager/maas/otheros_qwerks 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 [98.0 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 [357 kB]
Get:7 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [143 kB]
Get:8 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [637 kB]
Get:9 http://security.ubuntu.com trusty-security/universe amd64 Packages [117 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 [326 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,080 kB in 4s (515 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-tes...

=== modified file 'docs/networking.rst'
--- docs/networking.rst	2015-10-19 18:31:19 +0000
+++ docs/networking.rst	2015-10-28 21:00:41 +0000
@@ -8,7 +8,8 @@
 
 .. note::
 
-  This feature is available in MAAS versions 1.9 and above.
+  This feature is available in MAAS versions 1.9 and above on Ubuntu
+  deployments.
 
 MAAS 1.9 adds support for the modeling of a wide variety of networking concepts
 and configurations.

=== modified file 'docs/storage.rst'
--- docs/storage.rst	2015-10-20 16:02:23 +0000
+++ docs/storage.rst	2015-10-28 21:00:41 +0000
@@ -8,10 +8,11 @@
 
 .. note::
 
-  This feature is available in MAAS versions 1.9 and above.
-  If you're writing a client application, you can check if MAAS
-  supports this feature via the web API; see the documentation for the
-  ``storage-deployment`` capability :ref:`here<cap-storage-deployment>`.
+  This feature is available in MAAS versions 1.9 and above on Ubuntu
+  deployments.  If you're writing a client application, you can check
+  if MAAS supports this feature via the web API; see the documentation
+  for the ``storage-deployment`` capability
+  :ref:`here<cap-storage-deployment>`.
 
 MAAS has the ability to configure any storage layout during node deployment.
 MAAS doesn't just do simple partitioning it supports complex storage layouts,

=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py	2015-09-24 16:22:12 +0000
+++ src/maasserver/preseed.py	2015-10-28 21:00:41 +0000
@@ -230,16 +230,29 @@
     kernel_config = compose_curtin_kernel_preseed(node)
     verbose_config = compose_curtin_verbose_preseed()
 
+    supports_custom_storage = True
     # Get the storage configration if curtin supports custom storage.
-    storage_config = compose_curtin_storage_config(node)
     if not curtin_supports_custom_storage():
         maaslog.error(
             "%s: cannot deploy with custom storage config; missing support "
             "from curtin." % node.hostname)
+        supports_custom_storage = False
+
+    if node.osystem != "ubuntu":
+        maaslog.error(
+            "%s: is not being deployed with custom storage or networking "
+            "config; using operating system other than Ubuntu." %
+            node.hostname)
+        supports_custom_storage = False
+        network_config = []
+    else:
+        network_config = compose_curtin_network_config(node)
+
+    if supports_custom_storage:
+        storage_config = compose_curtin_storage_config(node)
+    else:
         storage_config = []
 
-    network_config = compose_curtin_network_config(node)
-
     # Pack the curtin and the configuration into a script to execute on the
     # deploying node.
     return pack_install(

=== modified file 'src/maasserver/static/js/angular/controllers/node_details.js'
--- src/maasserver/static/js/angular/controllers/node_details.js	2015-10-19 23:54:54 +0000
+++ src/maasserver/static/js/angular/controllers/node_details.js	2015-10-28 21:00:41 +0000
@@ -566,6 +566,20 @@
             return os_release;
         };
 
+        $scope.isUbuntuOS = function() {
+            // This will get called very early and node can be empty.
+            // In that case just return an empty string. It will be
+            // called again to show the correct information.
+            if(!angular.isObject($scope.node)) {
+                return false;
+            }
+
+            if($scope.node.osystem === "ubuntu") {
+                return true;
+            }
+            return false;
+        };
+
         // Return true if there is an action error.
         $scope.isActionError = function() {
             return $scope.actionError !== null;

=== modified file 'src/maasserver/static/js/angular/controllers/tests/test_node_details.js'
--- src/maasserver/static/js/angular/controllers/tests/test_node_details.js	2015-09-27 20:01:05 +0000
+++ src/maasserver/static/js/angular/controllers/tests/test_node_details.js	2015-10-28 21:00:41 +0000
@@ -837,6 +837,24 @@
         });
     });
 
+    describe("isUbuntuOS", function() {
+        it("returns true when ubuntu", function() {
+            var controller = makeController();
+            $scope.node = node;
+            node.osystem = 'ubuntu';
+            node.distro_series = makeName("distro_series");
+            expect($scope.isUbuntuOS()).toBe(true);
+        });
+
+        it("returns false when otheros", function() {
+            var controller = makeController();
+            $scope.node = node;
+            node.osystem = makeName("osystem");
+            node.distro_series = makeName("distro_series");
+            expect($scope.isUbuntuOS()).toBe(false);
+        });
+    });
+
     describe("isActionError", function() {
 
         it("returns true if actionError", function() {

=== modified file 'src/maasserver/static/partials/node-details.html'
--- src/maasserver/static/partials/node-details.html	2015-10-28 18:53:35 +0000
+++ src/maasserver/static/partials/node-details.html	2015-10-28 21:00:41 +0000
@@ -249,8 +249,8 @@
                             </dd>
                         <dt class="two-col ng-hide" data-ng-show="node.show_os_info">Operating System</dt>
                             <dd class="four-col last-col ng-hide" data-ng-show="node.show_os_info">{$ getOSText() $}</dd>
-                        <dt class="two-col ng-hide" data-ng-show="node.show_os_info">Kernel</dt>
-                            <dd class="four-col last-col ng-hide" data-ng-show="node.show_os_info">{$ node.hwe_kernel $}</dd>
+                        <dt class="two-col ng-hide" data-ng-show="node.show_os_info && isUbuntuOS()">Kernel</dt>
+                            <dd class="four-col last-col ng-hide" data-ng-show="node.show_os_info && isUbuntuOS()">{$ node.hwe_kernel $}</dd>
                         <dt class="two-col ng-hide" data-ng-show="node.show_os_info && node.third_party_driver">Third Party Driver</dt>
                             <dd class="four-col last-col ng-hide" data-ng-show="node.show_os_info && node.third_party_driver">
                                 {$ node.third_party_driver.module $} "{$ node.third_party_driver.comment $}"
@@ -323,6 +323,13 @@
                             </li>
                         </ul>
                     </div>
+                    <div class="twelve-col error ng-hide" data-ng-show="!isUbuntuOS()">
+                        <ul class="flash-messages">
+                            <li class="flash-messages__item info">
+                                Custom network configuration only supported on Ubuntu. Using OS default configuration.
+                            </li>
+                        </ul>
+                    </div>
                     <div class="table margin-bottom">
                         <header class="table__head">
                             <div class="table__row">
@@ -633,6 +640,13 @@
                         </li>
                     </ul>
                 </div>
+                <div class="twelve-col error ng-hide" data-ng-show="!isUbuntuOS()">
+                    <ul class="flash-messages">
+                        <li class="flash-messages__item info">
+                            Custom storage configuration only supported on Ubuntu. Using flat layout.
+                        </li>
+                    </ul>
+                </div>
                 <div class="twelve-col padding-bottom">
                     <h3>File System</h3>
                     <section class="table">

=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2015-10-27 20:53:16 +0000
+++ src/maasserver/testing/factory.py	2015-10-28 21:00:41 +0000
@@ -26,6 +26,7 @@
 import random
 import time
 
+from distro_info import UbuntuDistroInfo
 from django.contrib.auth.models import User
 from django.test.client import RequestFactory
 from django.utils import timezone
@@ -236,6 +237,21 @@
         releases = osystem.get_supported_commissioning_releases()
         return random.choice(releases)
 
+    def pick_ubuntu_release(self, but_not=None):
+        """Pick a random supported Ubuntu release.
+
+        :param but_not: Exclude these releases from the result
+        :type but_not: Sequence
+        """
+        ubuntu_releases = UbuntuDistroInfo()
+        supported_releases = ubuntu_releases.all[
+            ubuntu_releases.all.index('precise'):]
+        if but_not is None:
+            but_not = []
+        return random.choice(
+            [choice for choice in supported_releases if choice not in but_not],
+            ).decode("utf-8")
+
     def _save_node_unchecked(self, node):
         """Save a :class:`Node`, but circumvent status transition checks."""
         valid_initial_states = NODE_TRANSITIONS[None]

=== modified file 'src/maasserver/tests/test_forms_commissioning.py'
--- src/maasserver/tests/test_forms_commissioning.py	2015-10-27 17:50:36 +0000
+++ src/maasserver/tests/test_forms_commissioning.py	2015-10-28 21:00:41 +0000
@@ -14,9 +14,6 @@
 __metaclass__ = type
 __all__ = []
 
-import random
-
-from distro_info import UbuntuDistroInfo
 from django.core.files.uploadedfile import SimpleUploadedFile
 from maasserver.enum import BOOT_RESOURCE_TYPE
 from maasserver.forms import (
@@ -50,8 +47,7 @@
             field.error_messages['invalid_choice'])
 
     def test_commissioningform_contains_real_and_ui_choice(self):
-        ubuntu_releases = UbuntuDistroInfo()
-        release = random.choice(ubuntu_releases.all).decode("utf-8")
+        release = factory.pick_ubuntu_release()
         name = 'ubuntu/%s' % release
         kernel = 'hwe-' + release[0]
         factory.make_usable_boot_resource(

=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py	2015-09-15 01:39:03 +0000
+++ src/maasserver/tests/test_preseed.py	2015-10-28 21:00:41 +0000
@@ -78,7 +78,10 @@
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils import absolute_reverse
 from maasserver.utils.curtin import curtin_supports_webhook_events
-from maastesting.matchers import MockCalledOnceWith
+from maastesting.matchers import (
+    MockCalledOnceWith,
+    MockNotCalled,
+)
 from metadataserver.models import NodeKey
 from provisioningserver.drivers.osystem.ubuntu import UbuntuOS
 from provisioningserver.rpc.exceptions import NoConnectionsAvailable
@@ -734,7 +737,7 @@
         PreseedRPCMixin, BootImageHelperMixin, MAASServerTestCase):
     """Tests for `get_curtin_userdata`."""
 
-    def test_get_curtin_userdata_calls_compose_curtin_storage_config(self):
+    def test_get_curtin_userdata_calls_compose_curtin_config_on_ubuntu(self):
         node = factory.make_Node(
             nodegroup=self.rpc_nodegroup, boot_type=NODE_BOOT.FASTPATH,
             interface=True)
@@ -744,10 +747,35 @@
         self.configure_get_boot_images_for_node(node, 'xinstall')
         mock_compose_storage = self.patch(
             preseed_module, "compose_curtin_storage_config")
-
+        mock_compose_network = self.patch(
+            preseed_module, "compose_curtin_network_config")
+        self.patch(
+            preseed_module, "curtin_supports_custom_storage").value = True
+        node.osystem = u'ubuntu'
         user_data = get_curtin_userdata(node)
         self.assertIn("PREFIX='curtin'", user_data)
         self.assertThat(mock_compose_storage, MockCalledOnceWith(node))
+        self.assertThat(mock_compose_network, MockCalledOnceWith(node))
+
+    def test_get_curtin_userdata_doesnt_call_compose_config_on_otheros(self):
+        node = factory.make_Node(
+            nodegroup=self.rpc_nodegroup, boot_type=NODE_BOOT.FASTPATH,
+            interface=True)
+        factory.make_NodeGroupInterface(
+            node.nodegroup, management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
+        arch, subarch = node.architecture.split('/')
+        self.configure_get_boot_images_for_node(node, 'xinstall')
+        mock_compose_storage = self.patch(
+            preseed_module, "compose_curtin_storage_config")
+        mock_compose_network = self.patch(
+            preseed_module, "compose_curtin_network_config")
+        self.patch(
+            preseed_module, "curtin_supports_custom_storage").value = True
+        node.osystem = factory.make_name("osystem")
+        user_data = get_curtin_userdata(node)
+        self.assertIn("PREFIX='curtin'", user_data)
+        self.assertThat(mock_compose_storage, MockNotCalled())
+        self.assertThat(mock_compose_network, MockNotCalled())
 
     def test_get_curtin_userdata_calls_curtin_supports_custom_storage(self):
         node = factory.make_Node(

=== modified file 'src/maasserver/utils/osystems.py'
--- src/maasserver/utils/osystems.py	2015-10-24 00:07:26 +0000
+++ src/maasserver/utils/osystems.py	2015-10-28 21:00:41 +0000
@@ -34,6 +34,7 @@
 from maasserver.models import (
     BootResource,
     BootSourceCache,
+    Config,
 )
 
 
@@ -286,12 +287,18 @@
     os/release/architecture combination, and that the selected hwe_kernel is >=
     min_hwe_kernel. If no hwe_kernel is selected one will be chosen.
     """
-    # The hwe_kernel feature is only supported on Ubuntu
-    if((osystem and "ubuntu" not in osystem.lower()) or
+    if (not osystem or
        (not architecture or architecture == '') or
        (not distro_series or distro_series == '')):
         return hwe_kernel
 
+    # If we're not deploying Ubuntu we are just setting the kernel to be used
+    # during deployment
+    if osystem != "ubuntu":
+        osystem = Config.objects.get_config('commissioning_osystem')
+        distro_series = Config.objects.get_config(
+            'commissioning_distro_series')
+
     arch, subarch = architecture.split('/')
 
     if (subarch != 'generic' and

=== modified file 'src/maasserver/utils/tests/test_osystems.py'
--- src/maasserver/utils/tests/test_osystems.py	2015-10-27 17:50:36 +0000
+++ src/maasserver/utils/tests/test_osystems.py	2015-10-28 21:00:41 +0000
@@ -24,7 +24,10 @@
     make_rpc_release,
 )
 from maasserver.enum import BOOT_RESOURCE_TYPE
-from maasserver.models import BootResource
+from maasserver.models import (
+    BootResource,
+    Config,
+)
 from maasserver.testing.factory import factory
 from maasserver.testing.osystems import make_usable_osystem
 from maasserver.testing.testcase import MAASServerTestCase
@@ -43,6 +46,7 @@
     validate_hwe_kernel,
     validate_osystem_and_distro_series,
 )
+from maastesting.matchers import MockAnyCall
 
 
 class TestOsystems(MAASServerTestCase):
@@ -264,8 +268,7 @@
         self.assertEqual(choices, list_commissioning_choices([osystem]))
 
     def test_make_hwe_kernel_ui_text_finds_release_from_bootsourcecache(self):
-        ubuntu_releases = UbuntuDistroInfo()
-        release = random.choice(ubuntu_releases.all).decode("utf-8")
+        release = factory.pick_ubuntu_release()
         kernel = 'hwe-' + release[0]
         factory.make_usable_boot_resource(
             name="ubuntu/%s" % release,
@@ -445,3 +448,21 @@
                 ' min_hwe_kernel(hwe-v).', e.message)
             exception_raised = True
         self.assertEqual(True, exception_raised)
+
+    def test_validate_hwe_kern_always_sets_kern_with_commissionable_os(self):
+        self.patch(
+            BootResource.objects,
+            'get_usable_hwe_kernels').return_value = ('hwe-t', 'hwe-v')
+        mock_get_config = self.patch(Config.objects, "get_config")
+        mock_get_config.return_value = 'trusty'
+        kernel = validate_hwe_kernel(
+            None,
+            'hwe-v',
+            '%s/generic' % factory.make_name('arch'),
+            factory.make_name("osystem"),
+            factory.make_name("distro"))
+        self.assertThat(
+            mock_get_config, MockAnyCall('commissioning_osystem'))
+        self.assertThat(
+            mock_get_config, MockAnyCall('commissioning_distro_series'))
+        self.assertEquals('hwe-v', kernel)