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

[Lee Trager]

Add global_min_hwe_kernel which is used to set the min_hwe_kernel on new nodes. min_hwe_kernel requirements are now followed during commissioning and enlistment.

This branch adds the global_min_hwe_kernel option to MAAS. When set new nodes, or nodes which are being recommissioned which do not have min_hwe_kernel set, will have min_hwe_kernel set to global_min_hwe_kernel automatically.

This branch also instructions MAAS to follow any requirements set by min_hwe_kernel during commissioning and enlistment. Because min_hwe_kernel is now used during enlistment and commissioning global_min_hwe_kernel is limited to the hwe kernels available with the commissioning release. The user can set min_hwe_kernel when adding a new node in the UI or command line or by setting the new global_min_hwe_kernel option.

[Blake Rouse (blake-rouse)]

The core looks good. But needs to fix some things. See inline.

review: Needs Fixing

[Lee Trager (ltrager)]

Thanks for the review, I've fixed the items you marked below.

[Andres Rodriguez (andreserl)]

Hi Lee,

I have a couple things:

1. I think that solely using min_hwe_kernel can be very confusing to the user, which is why global_min_hwe_kernel was better. I disagree with Blake's comment here really. We can discuss this in the call.

2. see inline

review: Needs Fixing

[Andres Rodriguez (andreserl)]

nevermind comment 2. I'd only like to discuss the global_min_hwe_kernel, or it should at least be default_min_hwe_kernel

review: Needs Information

[Blake Rouse (blake-rouse)]

I am good with default_min_hwe_kernel. That will make it consistent.
"global_" was not consistent.

On Fri, Oct 23, 2015 at 11:38 AM, Andres Rodriguez <email address hidden>
wrote:

> Review: Needs Information
>
> nevermind comment 2. I'd only like to discuss the global_min_hwe_kernel,
> or it should at least be default_min_hwe_kernel
>
> Diff comments:
>
> > === modified file 'src/maasserver/api/pxeconfig.py'
> > --- src/maasserver/api/pxeconfig.py 2015-09-08 18:41:57 +0000
> > +++ src/maasserver/api/pxeconfig.py 2015-10-23 07:37:11 +0000
> > @@ -227,6 +227,10 @@
> > # we give precedence to any kernel defined in the
> subarchitecture field
> > if subarch == "generic" and node.hwe_kernel:
> > subarch = node.hwe_kernel
> > + elif(subarch == "generic"
> > + and node.get_boot_purpose() == "commissioning"
> > + and node.min_hwe_kernel):
> > + subarch = node.min_hwe_kernel
>
> nevermind this comment, I see in the code belowe this is being addressed
>
> > else:
> > nodegroup = find_nodegroup_for_pxeconfig_request(request)
> > preseed_url =
> compose_enlistment_preseed_url(nodegroup=nodegroup)
>
>
> --
>
> https://code.launchpad.net/~ltrager/maas/global_min_hwe_kernel/+merge/275477
> You are reviewing the proposed merge of
> lp:~ltrager/maas/global_min_hwe_kernel into lp:maas.
>

[Blake Rouse (blake-rouse)]

Looks good. Thanks for all the fixes.

review: Approve

[MAAS Lander (maas-lander)]

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

Hit http://security.ubuntu.com trusty-security InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://nova.clouds.archive.ubuntu.com trusty-updates InRelease [64.4 kB]
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:2 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [240 kB]
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [140 kB]
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [635 kB]
Get:5 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [323 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 1,403 kB in 3s (372 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-testtools python-twisted python-txtftp python-tz python...

=== modified file 'src/maasserver/api/pxeconfig.py'
--- src/maasserver/api/pxeconfig.py	2015-09-08 18:41:57 +0000
+++ src/maasserver/api/pxeconfig.py	2015-10-23 17:54:22 +0000
@@ -227,6 +227,10 @@
         # we give precedence to any kernel defined in the subarchitecture field
         if subarch == "generic" and node.hwe_kernel:
             subarch = node.hwe_kernel
+        elif(subarch == "generic"
+             and node.get_boot_purpose() == "commissioning"
+             and node.min_hwe_kernel):
+            subarch = node.min_hwe_kernel
     else:
         nodegroup = find_nodegroup_for_pxeconfig_request(request)
         preseed_url = compose_enlistment_preseed_url(nodegroup=nodegroup)
@@ -254,7 +258,14 @@
                 else:
                     arch, _ = resource.split_arch()
 
-        subarch = get_optional_param(request.GET, 'subarch', 'generic')
+        default_min_hwe_kernel = Config.objects.get_config(
+            'default_min_hwe_kernel')
+        if default_min_hwe_kernel:
+            subarch = get_optional_param(
+                request.GET, 'subarch', default_min_hwe_kernel)
+        else:
+            subarch = get_optional_param(
+                request.GET, 'subarch', 'generic')
 
     # If we are booting with "xinstall", then we should always return the
     # commissioning operating system and distro_series.
 
=== modified file 'src/maasserver/api/tests/test_pxeconfig.py'
--- src/maasserver/api/tests/test_pxeconfig.py	2015-08-29 03:27:16 +0000
+++ src/maasserver/api/tests/test_pxeconfig.py	2015-10-23 17:54:22 +0000
@@ -361,6 +361,15 @@
             (ip, hostname),
             (json.loads(response.content)["log_host"], mock.call_args[0][0]))
 
+    def test_pxeconfig_enlistment_checks_default_min_hwe_kernel(self):
+        params = self.get_default_params()
+        params['arch'] = 'armhf'
+        Config.objects.set_config('default_min_hwe_kernel', 'hwe-v')
+        response = self.client.get(reverse('pxeconfig'), params)
+        self.assertEqual(
+            "hwe-v",
+            json.loads(response.content)["subarch"])
+
     def test_pxeconfig_has_preseed_url_for_known_node(self):
         params = self.get_mac_params()
         node = Interface.objects.get(mac_address=params['mac']).node
@@ -631,6 +640,17 @@
         self.assertEqual(osystem, params_out["osystem"])
         self.assertEqual(release, params_out["release"])
 
+    def test_pxeconfig_commissioning_node_uses_min_hwe_kernel(self):
+        node = factory.make_Node(min_hwe_kernel="hwe-v")
+        nic = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
+        self.patch(Node, 'get_boot_purpose').return_value = "commissioning"
+        params = self.get_default_params()
+        params['mac'] = nic.mac_address
+        response = self.client.get(reverse('pxeconfig'), params)
+        self.assertEqual(
+            "hwe-v",
+            json.loads(response.content)["subarch"])
+
     def test_pxeconfig_returns_ubuntu_os_series_for_ubuntu_xinstall(self):
         nodegroup = factory.make_NodeGroup()
         ubuntu_image = make_rpc_boot_image(
 
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2015-10-21 23:36:45 +0000
+++ src/maasserver/forms.py	2015-10-23 17:54:22 +0000
@@ -566,7 +566,11 @@
             return boot_type
 
     def clean_min_hwe_kernel(self):
-        return validate_min_hwe_kernel(self.cleaned_data.get('min_hwe_kernel'))
+        min_hwe_kernel = self.cleaned_data.get('min_hwe_kernel')
+        if self.new_node and not min_hwe_kernel:
+            min_hwe_kernel = Config.objects.get_config(
+                'default_min_hwe_kernel')
+        return validate_min_hwe_kernel(min_hwe_kernel)
 
     def clean(self):
         cleaned_data = super(NodeForm, self).clean()
@@ -1306,6 +1310,8 @@
         Form.__init__(self, *args, **kwargs)
         self.fields['commissioning_distro_series'] = get_config_field(
             'commissioning_distro_series')
+        self.fields['default_min_hwe_kernel'] = get_config_field(
+            'default_min_hwe_kernel')
         self._load_initials()
 
 
 
=== modified file 'src/maasserver/forms_settings.py'
--- src/maasserver/forms_settings.py	2015-08-18 18:02:11 +0000
+++ src/maasserver/forms_settings.py	2015-10-23 17:54:22 +0000
@@ -27,6 +27,7 @@
 from django.core.exceptions import ValidationError
 from maasserver.bootresources import IMPORT_RESOURCES_SERVICE_PERIOD
 from maasserver.fields import IPListFormField
+from maasserver.models import BootResource
 from maasserver.models.config import (
     Config,
     DEFAULT_OS,
@@ -39,6 +40,7 @@
     list_all_usable_releases,
     list_commissioning_choices,
     list_osystem_choices,
+    release_a_newer_than_b,
 )
 
 
@@ -114,6 +116,31 @@
     return field
 
 
+def make_default_min_hwe_kernel_field(*args, **kwargs):
+    """Build and return the default_min_hwe_kernel field."""
+    kernel_choices = [('', '--- No minimum kernel ---')]
+    # Global choices are limited to the commissioning release as min_hwe_kernel
+    # is used during commissioning.
+    commissioning_series = Config.objects.get_config(
+        'commissioning_distro_series')
+    if commissioning_series:
+        commissioning_os_release = "ubuntu/" + commissioning_series
+        kernel_choices += [
+            (kernel, kernel)
+            for kernel in BootResource.objects.get_usable_hwe_kernels(
+                commissioning_os_release)
+            if release_a_newer_than_b(kernel, commissioning_series)]
+    field = forms.ChoiceField(
+        initial=Config.objects.get_config('default_min_hwe_kernel'),
+        choices=kernel_choices,
+        error_messages={
+            'invalid_choice': compose_invalid_choice_text(
+                'default_min_hwe_kernel', kernel_choices)
+        },
+        **kwargs)
+    return field
+
+
 def make_commissioning_distro_series_field(*args, **kwargs):
     """Build and return the commissioning_distro_series field."""
     usable_oses = list_all_usable_osystems()
@@ -257,6 +284,18 @@
             # at run-time to avoid a race condition.
         }
     },
+    'default_min_hwe_kernel': {
+        'default': None,
+        'form': make_default_min_hwe_kernel_field,
+        'form_kwargs': {
+            'label': "Default Minimum Kernel Version",
+            'required': False,
+            'help_text': (
+                "The default minimum kernel version used on all new and"
+                " commissioned nodes."
+            )
+        }
+    },
     'default_storage_layout': {
         'default': 'lvm',
         'form': forms.ChoiceField,
 
=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py	2015-08-18 18:02:11 +0000
+++ src/maasserver/models/config.py	2015-10-23 17:54:22 +0000
@@ -51,6 +51,7 @@
         'commissioning_osystem': DEFAULT_OS.name,
         'commissioning_distro_series':
         DEFAULT_OS.get_default_commissioning_release(),
+        'default_min_hwe_kernel': '',
         'default_storage_layout': 'lvm',
         # Network section configuration.
         'maas_name': gethostname(),
 
=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2015-10-21 00:43:01 +0000
+++ src/maasserver/models/node.py	2015-10-23 17:54:22 +0000
@@ -998,6 +998,12 @@
         old_status = self.status
         self.status = NODE_STATUS.COMMISSIONING
         self.owner = user
+        # Set min_hwe_kernel to min_hwe_kernel if it isn't set incase
+        # commissioning failed and the user set the min globally and is
+        # retrying.
+        if not self.min_hwe_kernel:
+            self.min_hwe_kernel = Config.objects.get_config(
+                'default_min_hwe_kernel')
         self.save()
 
         # Prepare a transition monitor for later.
 
=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2015-10-21 00:43:01 +0000
+++ src/maasserver/models/tests/test_node.py	2015-10-23 17:54:22 +0000
@@ -1492,6 +1492,20 @@
         post_commit_hooks.reset()  # Ignore these for now.
         self.assertThat(node_start, MockCalledOnceWith(admin, user_data))
 
+    def test_start_commissioning_sets_min_hwe_kernel(self):
+        node = factory.make_Node(status=NODE_STATUS.NEW)
+        node_start = self.patch(node, '_start')
+        node_start.side_effect = lambda user, user_data: post_commit()
+        user_data = factory.make_string().encode('ascii')
+        generate_user_data = self.patch(
+            commissioning, 'generate_user_data')
+        generate_user_data.return_value = user_data
+        admin = factory.make_admin()
+        Config.objects.set_config('default_min_hwe_kernel', 'hwe-v')
+        node.start_commissioning(admin)
+        post_commit_hooks.reset()  # Ignore these for now.
+        self.assertEqual('hwe-v', node.min_hwe_kernel)
+
     def test_start_commissioning_clears_node_commissioning_results(self):
         node = factory.make_Node(status=NODE_STATUS.NEW)
         NodeResult.objects.store_data(
 
=== modified file 'src/maasserver/static/js/angular/controllers/add_hardware.js'
--- src/maasserver/static/js/angular/controllers/add_hardware.js	2015-08-11 15:07:10 +0000
+++ src/maasserver/static/js/angular/controllers/add_hardware.js	2015-10-23 17:54:22 +0000
@@ -24,6 +24,8 @@
         $scope.zones = ZonesManager.getItems();
         $scope.architectures = GeneralManager.getData("architectures");
         $scope.hwe_kernels = GeneralManager.getData("hwe_kernels");
+        $scope.default_min_hwe_kernel = GeneralManager.getData(
+            "default_min_hwe_kernel");
         $scope.error = null;
 
         // Input values.
@@ -281,6 +283,7 @@
                 macs: [newMAC()],
                 zone: defaultZone(),
                 architecture: defaultArchitecture(),
+                min_hwe_kernel: $scope.default_min_hwe_kernel.text,
                 power: {
                     type: null,
                     parameters: {}
@@ -439,7 +442,6 @@
                 $scope.machine.cluster === null ||
                 $scope.machine.zone === null ||
                 $scope.machine.architecture === '' ||
-                $scope.machine.min_hwe_kernel === '' ||
                 $scope.machine.power.type === null ||
                 $scope.invalidName($scope.machine));
             if(in_error) {
 
=== modified file 'src/maasserver/static/js/angular/factories/general.js'
--- src/maasserver/static/js/angular/factories/general.js	2015-10-21 19:45:50 +0000
+++ src/maasserver/static/js/angular/factories/general.js	2015-10-23 17:54:22 +0000
@@ -49,6 +49,16 @@
                     polling: false,
                     nextPromise: null
                 },
+                default_min_hwe_kernel: {
+                    method: "general.default_min_hwe_kernel",
+                    data: { text: '' },
+                    loaded: false,
+                    polling: false,
+                    nextPromise: null,
+                    replaceData: function(oldData, newData) {
+                        oldData.text = newData;
+                    }
+                },
                 osinfo: {
                     method: "general.osinfo",
                     data: {},
 
=== modified file 'src/maasserver/static/js/angular/factories/tests/test_general.js'
--- src/maasserver/static/js/angular/factories/tests/test_general.js	2015-10-21 19:45:50 +0000
+++ src/maasserver/static/js/angular/factories/tests/test_general.js	2015-10-23 17:54:22 +0000
@@ -51,7 +51,7 @@
     it("_data has expected keys", function() {
         expect(Object.keys(GeneralManager._data)).toEqual(
             ["node_actions", "device_actions", "architectures", "hwe_kernels",
-             "osinfo", "bond_options", "version"]);
+             "default_min_hwe_kernel", "osinfo", "bond_options", "version"]);
     });
 
     it("_data.node_actions has correct data", function() {
@@ -90,6 +90,17 @@
         expect(hwe_kernels.nextPromise).toBeNull();
     });
 
+    it("_data.default_min_hwe_kernels has correct data", function() {
+        var default_min_hwe_kernel =
+            GeneralManager._data.default_min_hwe_kernel;
+        expect(default_min_hwe_kernel.method).toBe(
+            "general.default_min_hwe_kernel");
+        expect(default_min_hwe_kernel.data).toEqual({text: ''});
+        expect(default_min_hwe_kernel.loaded).toBe(false);
+        expect(default_min_hwe_kernel.polling).toBe(false);
+        expect(default_min_hwe_kernel.nextPromise).toBeNull();
+    });
+
     it("_data.osinfo has correct data", function() {
         var osinfo = GeneralManager._data.osinfo;
         expect(osinfo.method).toBe("general.osinfo");
@@ -166,6 +177,7 @@
             GeneralManager._data.device_actions.loaded = true;
             GeneralManager._data.architectures.loaded = true;
             GeneralManager._data.hwe_kernels.loaded = true;
+            GeneralManager._data.default_min_hwe_kernel.loaded = true;
             GeneralManager._data.osinfo.loaded = true;
             GeneralManager._data.bond_options.loaded = true;
             GeneralManager._data.version.loaded = true;
@@ -416,7 +428,7 @@
             spyOn(GeneralManager, "_loadData").and.returnValue(
                 $q.defer().promise);
             GeneralManager.loadItems();
-            expect(GeneralManager._loadData.calls.count()).toBe(7);
+            expect(GeneralManager._loadData.calls.count()).toBe(8);
         });
 
         it("resolve defer once all resolve", function(done) {
@@ -427,6 +439,7 @@
                 $q.defer(),
                 $q.defer(),
                 $q.defer(),
+                $q.defer(),
                 $q.defer()
             ];
             var i = 0;
 
=== modified file 'src/maasserver/tests/test_forms_commissioning.py'
--- src/maasserver/tests/test_forms_commissioning.py	2015-05-07 18:14:38 +0000
+++ src/maasserver/tests/test_forms_commissioning.py	2015-10-23 17:54:22 +0000
@@ -36,6 +36,14 @@
                 'commissioning_distro_series', field.choices),
             field.error_messages['invalid_choice'])
 
+    def test_commissioningform_error_msg_lists_min_hwe_kernel_choices(self):
+        form = CommissioningForm()
+        field = form.fields['default_min_hwe_kernel']
+        self.assertEqual(
+            compose_invalid_choice_text(
+                'default_min_hwe_kernel', field.choices),
+            field.error_messages['invalid_choice'])
+
 
 class TestCommissioningScriptForm(MAASServerTestCase):
 
 
=== modified file 'src/maasserver/websockets/handlers/general.py'
--- src/maasserver/websockets/handlers/general.py	2015-10-21 19:45:50 +0000
+++ src/maasserver/websockets/handlers/general.py	2015-10-23 17:54:22 +0000
@@ -45,6 +45,7 @@
         allowed_methods = [
             'architectures',
             'hwe_kernels',
+            'default_min_hwe_kernel',
             'osinfo',
             'node_actions',
             'device_actions',
@@ -61,6 +62,10 @@
         """Return all supported hwe_kernels."""
         return BootResource.objects.get_usable_hwe_kernels()
 
+    def default_min_hwe_kernel(self, params):
+        """Return the default_min_hwe_kernel."""
+        return Config.objects.get_config('default_min_hwe_kernel')
+
     def osinfo(self, params):
         """Return all available operating systems and releases information."""
         osystems = list_all_usable_osystems()