Merge ~ltrager/maas:lp1807991_2.5 into maas:2.5

[Lee Trager]

Backport 51b971 LP: #1807991 - Directly go into commissioning during enlistment.

The anonymous API now accepts the 'commission' flag. When set to true
newly created machines will be created in COMMISSIONING and a ScriptSet
with all builtin Scripts will be set as the current_commissioning_script_set.

Commissioning during enlistment can now be disabled using the
'enlist_commissioning' configuration option.

[Lee Trager (ltrager)]

Approved in https://code.launchpad.net/~ltrager/maas/+git/maas/+merge/360849

review: Approve

diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
index 0ac51b1..0af350b 100644
--- a/src/maasserver/api/machines.py
+++ b/src/maasserver/api/machines.py
@@ -1611,6 +1611,12 @@ class AnonMachinesHandler(AnonNodesHandler):
         power_type. `Power types`_ section for a list of the available power
         parameters for each power type.
 
+        @param (boolean) "commission" [required=false,formatting=true] Request
+        the newly created machine to be created with status set to
+        COMMISSIONING. Machines will wait for COMMISSIONING results and not
+        time out. After commissioning is complete machines will still have to
+        be accepted by an administrator.
+
         @success (http-status-code) "200" 200
         @success (json) "success-json" A JSON object containing the machine
         information.
@@ -1621,6 +1627,8 @@ class AnonMachinesHandler(AnonNodesHandler):
         power_type = request.data.get('power_type')
         power_parameters = request.data.get('power_parameters')
         mac_addresses = request.data.getlist('mac_addresses')
+        commission = get_optional_param(
+            request.data, 'commission', default=False, validator=StringBool)
         machine = None
 
         # BMC enlistment - Check if there is a pre-existing machine within MAAS
@@ -1667,12 +1675,12 @@ class AnonMachinesHandler(AnonNodesHandler):
         if machine is None:
             machine = create_machine(request, requires_arch=True)
 
-        if machine.status == NODE_STATUS.NEW:
-            # Make sure an enlisting NodeMetadata object exists if the machine
-            # is NEW. When commissioning finishes this is how MAAS knows to
-            # set the status to NEW instead of READY.
-            NodeMetadata.objects.update_or_create(
-                node=machine, key='enlisting', defaults={'value': 'True'})
+            if commission:
+                # Make sure an enlisting NodeMetadata object exists if the
+                # machine is NEW. When commissioning finishes this is how
+                # MAAS knows to set the status to NEW instead of READY.
+                NodeMetadata.objects.update_or_create(
+                    node=machine, key='enlisting', defaults={'value': 'True'})
 
         return machine
 
@@ -1743,6 +1751,11 @@ class MachinesHandler(NodesHandler, PowersMixin):
         power_type. `Power types`_ section for a list of the available power
         parameters for each power type.
 
+        @param (boolean) "commission" [required=false,formatting=true] Request
+        the newly created machine to be created with status set to
+        COMMISSIONING. Machines will wait for COMMISSIONING results and not
+        time out.
+
         @success (http-status-code) "200" 200
         @success (json) "success-json" A JSON object containing the machine
         information.
diff --git a/src/maasserver/api/tests/test_enlistment.py b/src/maasserver/api/tests/test_enlistment.py
index 30a3413..8111f6b 100644
--- a/src/maasserver/api/tests/test_enlistment.py
+++ b/src/maasserver/api/tests/test_enlistment.py
@@ -452,8 +452,6 @@ class AnonymousEnlistmentAPITest(APITestCase.ForAnonymous):
         self.assertEqual(architecture, machine.architecture)
         self.assertDictContainsSubset(
             machine.bmc.power_parameters, power_parameters)
-        node_metadata = NodeMetadata.objects.get(node=machine, key='enlisting')
-        self.assertEqual(node_metadata.value, 'True')
         self.assertThat(mock_create_machine, MockNotCalled())
         self.assertEqual(
             machine.system_id, json_load_bytes(response.content)['system_id'])
@@ -501,12 +499,33 @@ class AnonymousEnlistmentAPITest(APITestCase.ForAnonymous):
             })
         self.assertEqual(http.client.OK, response.status_code)
         node_metadata = NodeMetadata.objects.get(key='enlisting')
-        self.assertEqual(node_metadata.value, 'True')
+        self.assertIsNone(node_metadata)
         [machine] = Machine.objects.filter(hostname=hostname)
         self.assertEqual(architecture, machine.architecture)
         self.assertEqual(
             machine.system_id, json_load_bytes(response.content)['system_id'])
 
+    def test_POST_create_creates_machine_commission(self):
+        hostname = factory.make_name("hostname")
+        architecture = make_usable_architecture(self)
+        response = self.client.post(
+            reverse('machines_handler'),
+            {
+                'hostname': hostname,
+                'architecture': architecture,
+                'power_type': 'manual',
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'],
+                'commission': True,
+            })
+        self.assertEqual(http.client.OK, response.status_code)
+        node_metadata = NodeMetadata.objects.get(key='enlisting')
+        self.assertEqual('True', node_metadata.value)
+        [machine] = Machine.objects.filter(hostname=hostname)
+        self.assertEqual(architecture, machine.architecture)
+        self.assertEqual(
+            machine.system_id, json_load_bytes(response.content)['system_id'])
+        self.assertEqual(NODE_STATUS.COMMISSIONING, machine.status)
+
     def test_POST_create_requires_architecture(self):
         hostname = factory.make_name("hostname")
         response = self.client.post(
diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
index 582b2af..6c3ab95 100644
--- a/src/maasserver/forms/__init__.py
+++ b/src/maasserver/forms/__init__.py
@@ -113,6 +113,7 @@ from maasserver.enum import (
     FILESYSTEM_GROUP_RAID_TYPE_CHOICES,
     FILESYSTEM_TYPE,
     INTERFACE_TYPE,
+    NODE_STATUS,
     NODE_TYPE,
 )
 from maasserver.exceptions import NodeActionError
@@ -893,6 +894,27 @@ class MachineForm(NodeForm):
         self.is_bound = True
         self.data['install_kvm'] = install_kvm
 
+    def save(self, *args, **kwargs):
+        # Prevent circular imports
+        from metadataserver.models import ScriptSet
+        # LP:1807991 - If requested when creating a new Machine, set the status
+        # to COMMISSIONING when the object is created.
+        commission = not self.instance.id and self.cleaned_data['commission']
+        if commission:
+            self.instance.status = NODE_STATUS.COMMISSIONING
+        machine = super(MachineForm, self).save(*args, **kwargs)
+        # For a ScriptSet to be created it must be associated with a Node
+        # object in the database.
+        if commission:
+            script_set = ScriptSet.objects.create_commissioning_script_set(
+                machine, ['none'])
+            machine.current_commissioning_script_set = script_set
+            machine.save(update_fields=['current_commissioning_script_set'])
+
+        return machine
+
+    commission = forms.BooleanField(required=False, widget=forms.HiddenInput())
+
     class Meta:
         model = Machine
 
@@ -905,6 +927,7 @@ class MachineForm(NodeForm):
             'hwe_kernel',
             'install_rackd',
             'install_kvm',
+            'commission'
         )
 
 
diff --git a/src/maasserver/forms/settings.py b/src/maasserver/forms/settings.py
index d318759..e6bae82 100644
--- a/src/maasserver/forms/settings.py
+++ b/src/maasserver/forms/settings.py
@@ -766,6 +766,17 @@ CONFIG_ITEMS = {
                 "in minutes.")
         }
     },
+    'enlist_commissioning': {
+        'default': True,
+        'form': forms.BooleanField,
+        'form_kwargs': {
+            'label': 'Whether to run commissioning during enlistment.',
+            'required': False,
+            'help_text': (
+                'Enables running all built-in commissioning scripts during '
+                'enlistment.'),
+        }
+    },
 }
 
 
diff --git a/src/maasserver/forms/tests/test_machine.py b/src/maasserver/forms/tests/test_machine.py
index c8ed726..b4564d6 100644
--- a/src/maasserver/forms/tests/test_machine.py
+++ b/src/maasserver/forms/tests/test_machine.py
@@ -51,6 +51,7 @@ class TestMachineForm(MAASServerTestCase):
                 'hwe_kernel',
                 'install_rackd',
                 'install_kvm',
+                'commission',
             ], list(form.fields))
 
     def test_accepts_usable_architecture(self):
@@ -374,6 +375,7 @@ class TestAdminMachineForm(MAASServerTestCase):
                 'power_parameters',
                 'power_type',
                 'pool',
+                'commission',
             ],
             list(form.fields))
 
diff --git a/src/maasserver/forms/tests/test_machinewithmacaddresses.py b/src/maasserver/forms/tests/test_machinewithmacaddresses.py
index fefaaf9..8bc5f98 100644
--- a/src/maasserver/forms/tests/test_machinewithmacaddresses.py
+++ b/src/maasserver/forms/tests/test_machinewithmacaddresses.py
@@ -6,7 +6,10 @@
 __all__ = []
 
 from django.http import QueryDict
-from maasserver.enum import INTERFACE_TYPE
+from maasserver.enum import (
+    INTERFACE_TYPE,
+    NODE_STATUS,
+)
 from maasserver.forms import MachineWithMACAddressesForm
 from maasserver.testing.architecture import (
     make_usable_architecture,
@@ -160,15 +163,18 @@ class MachineWithMACAddressesFormTest(MAASServerTestCase):
         macs = ['aa:bb:cc:dd:ee:ff', '9a:bb:c3:33:e5:7f']
         form = MachineWithMACAddressesForm(
             data=self.make_params(mac_addresses=macs))
+        self.assertTrue(form.is_valid())
         node = form.save()
 
         self.assertIsNotNone(node.id)  # The node is persisted.
+        self.assertEquals(NODE_STATUS.NEW, node.status)
         self.assertItemsEqual(
             macs,
             [nic.mac_address for nic in node.interface_set.all()])
 
     def test_form_without_hostname_generates_hostname(self):
         form = MachineWithMACAddressesForm(data=self.make_params(hostname=''))
+        self.assertTrue(form.is_valid())
         node = form.save()
         self.assertTrue(len(node.hostname) > 0)
 
@@ -176,6 +182,7 @@ class MachineWithMACAddressesFormTest(MAASServerTestCase):
         ip_based_hostname = '192-168-12-10.maas'
         form = MachineWithMACAddressesForm(
             data=self.make_params(hostname=ip_based_hostname))
+        self.assertTrue(form.is_valid())
         node = form.save()
         self.assertNotEqual('192-168-12-10', node.hostname)
 
@@ -183,5 +190,14 @@ class MachineWithMACAddressesFormTest(MAASServerTestCase):
         ip_prefixed_hostname = '192-168-12-10-extra.maas'
         form = MachineWithMACAddressesForm(
             data=self.make_params(hostname=ip_prefixed_hostname))
+        self.assertTrue(form.is_valid())
         node = form.save()
         self.assertEqual('192-168-12-10-extra', node.hostname)
+
+    def test_form_with_commissioning(self):
+        form = MachineWithMACAddressesForm(data={
+            'commission': True, **self.make_params()})
+        self.assertTrue(form.is_valid())
+        machine = form.save()
+        self.assertEquals(NODE_STATUS.COMMISSIONING, machine.status)
+        self.assertIsNotNone(machine.current_commissioning_script_set)
diff --git a/src/maasserver/models/config.py b/src/maasserver/models/config.py
index a95246b..0c76e3c 100644
--- a/src/maasserver/models/config.py
+++ b/src/maasserver/models/config.py
@@ -130,6 +130,8 @@ def get_default_config():
         'prometheus_enabled': False,
         'prometheus_push_gateway': None,
         'prometheus_push_interval': 60,
+        # Enlistment options
+        'enlist_commissioning': True,
     }
 
 
diff --git a/src/metadataserver/api.py b/src/metadataserver/api.py
index e76a651..3470a29 100644
--- a/src/metadataserver/api.py
+++ b/src/metadataserver/api.py
@@ -509,9 +509,9 @@ class VersionIndexHandler(MetadataViewHandler):
                     script_result.save(update_fields=['status'])
 
     def _process_new(self, node, request, status):
-        # MAAS only cares if a NEW node is trying to commission itself. Ignore
-        # other signals.
         if status != SIGNAL_STATUS.COMMISSIONING:
+            # MAAS only cares if a NEW node is trying to commission itself.
+            # Ignore other signals.
             return None
 
         # Check if the node currently has a pending or running commissioning
@@ -525,9 +525,6 @@ class VersionIndexHandler(MetadataViewHandler):
                 node, ['none'])
             node.current_commissioning_script_set = script_set
 
-        node.min_hwe_kernel = Config.objects.get_config(
-            'default_min_hwe_kernel')
-
         return NODE_STATUS.COMMISSIONING
 
     def _process_testing(self, node, request, status):
@@ -1155,7 +1152,10 @@ class EnlistUserDataHandler(OperationsHandler):
         return HttpResponse(
             generate_user_data_for_status(
                 None, NODE_STATUS.NEW, rack_controller=rack_controller,
-                request=request),
+                request=request, extra_content={
+                    'enlist_commissioning': Config.objects.get_config(
+                        'enlist_commissioning'),
+                }),
             content_type="text/plain")
 
 
diff --git a/src/metadataserver/tests/test_api.py b/src/metadataserver/tests/test_api.py
index 2e7b6cb..68d8b2f 100644
--- a/src/metadataserver/tests/test_api.py
+++ b/src/metadataserver/tests/test_api.py
@@ -47,7 +47,6 @@ from maasserver.exceptions import (
     Unauthorized,
 )
 from maasserver.models import (
-    Config,
     NodeMetadata,
     Event,
     SSHKey,
@@ -2546,8 +2545,6 @@ class TestNewAPI(MAASServerTestCase):
         factory.make_Script(script_type=SCRIPT_TYPE.COMMISSIONING)
         factory.make_Script(
             script_type=SCRIPT_TYPE.TESTING, tags=['commissioning'])
-        min_hwe_kernel = factory.make_name('hwe_kernel')
-        Config.objects.set_config('default_min_hwe_kernel', min_hwe_kernel)
 
         client = make_node_client(node)
         response = call_signal(client, status=SIGNAL_STATUS.COMMISSIONING)
@@ -2560,7 +2557,6 @@ class TestNewAPI(MAASServerTestCase):
             [script.name for script in node.current_commissioning_script_set])
         self.assertIsNone(node.current_testing_script_set)
         self.assertEqual(NODE_STATUS.COMMISSIONING, node.status)
-        self.assertEqual(min_hwe_kernel, node.min_hwe_kernel)
 
     def test_signal_commissioning_only_creates_scriptsets_when_needed(self):
         # This happens when commissioning is started by the user with correct
diff --git a/src/metadataserver/user_data/templates/enlistment.template b/src/metadataserver/user_data/templates/enlistment.template
index f389108..726be19 100644
--- a/src/metadataserver/user_data/templates/enlistment.template
+++ b/src/metadataserver/user_data/templates/enlistment.template
@@ -48,7 +48,7 @@ main() {
     # already exists and is not in a NEW state this will fail.
     output=$(maas-enlist --quite --serverurl "{{server_url}}" \
         ${power_params:+--power-params "${power_params}" \
-        --power-type "${power_type}"})
+        --power-type "${power_type}"}{{if enlist_commissioning}} --commission{{endif}})
     ret=$?
     echo $output | jq .
     if [ $ret -ne 0 ]; then
@@ -57,6 +57,7 @@ main() {
 	exit 1
     fi
 
+    {{if enlist_commissioning}}
     # Reload cloud-init preseed using the system_id to get OAUTH credentials.
     system_id=$(echo $output | jq -r .system_id)
     mv ${CRED_CFG} ${CRED_CFG}.orig
@@ -65,6 +66,7 @@ main() {
     signal COMMISSIONING "Enlistment complete, starting commissioning"
 
     maas-run-remote-scripts "--config=${CRED_CFG}" "${TEMP_D}"
+    {{endif}}
 }
 
 ### begin writing files ###
@@ -85,6 +87,7 @@ add_bin "maas-wedge-autodetect" <<"END_MAAS_WEDGE_AUTODETECT"
 {{maas_wedge_autodetect_sh}}
 END_MAAS_WEDGE_AUTODETECT
 
+{{if enlist_commissioning}}
 add_bin "maas_api_helper.py" <<"END_MAAS_API_HELPER"
 {{maas_api_helper_py}}
 END_MAAS_API_HELPER
@@ -96,6 +99,7 @@ END_MAAS_SIGNAL
 add_bin "maas-run-remote-scripts" <<"END_MAAS_RUN_REMOTE_SCRIPTS"
 {{maas_run_remote_scripts_py}}
 END_MAAS_RUN_REMOTE_SCRIPTS
+{{endif}}
 
 add_bin "maas-enlist" <<"END_MAAS_ENLIST"
 {{maas_enlist_sh}}
diff --git a/src/metadataserver/user_data/templates/snippets/maas_enlist.sh b/src/metadataserver/user_data/templates/snippets/maas_enlist.sh
index 394b0bd..895c0d8 100644
--- a/src/metadataserver/user_data/templates/snippets/maas_enlist.sh
+++ b/src/metadataserver/user_data/templates/snippets/maas_enlist.sh
@@ -2,7 +2,7 @@
 #
 #    maas-enlist: MAAS Enlistment Tool
 #
-#    Copyright (C) 2014-2016 Canonical Ltd.
+#    Copyright (C) 2014-2018 Canonical Ltd.
 #
 #    Authors: Andres Rodriguez <[e-mail]>
 #
@@ -107,6 +107,7 @@ enlist_node() {
 	hostname="${5}"
 	power_type="${6}"
 	power_params="${7}"
+	commission="${8}"
 
 	local macparms=""
 	macparms=$(get_mac_address_curl_parms "$mac")
@@ -119,6 +120,7 @@ enlist_node() {
 	    --data-urlencode "subarchitecture=${subarch}" \
 	    --data-urlencode "power_type=${power_type}" \
 	    --data-urlencode "power_parameters=${power_params}" \
+	    --data-urlencode "commission=${commission}" \
 	    ${macparms} \
 	    "${serverurl}"
 
@@ -171,6 +173,8 @@ Usage: ${0##*/} [ options ]
       -e | --exists           checks if the machine already exists in MAAS
       -w | --in-action        checks if the machine already exists in MAAS in a 'in-progress'
                               action like 'deploying' or 'commissioning'.
+      -c | --commission       tell MAAS when creating a new machine to set the status to
+                              'commissioning'.
       --subarch               subarchitecture of the node to register
 
    Example:
@@ -181,8 +185,8 @@ EOF
 
 bad_Usage() { Usage 1>&2; [ $# -eq 0 ] || Error "$@"; exit 1; }
 
-short_opts="hs:n:i:a:t:p:ewq"
-long_opts="help,serverurl:,hostname:,interface:,arch:,subarch:,power-type:,power-params:,exists,in-action,quite"
+short_opts="hs:n:i:a:t:p:ewqc"
+long_opts="help,serverurl:,hostname:,interface:,arch:,subarch:,power-type:,power-params:,exists,in-action,quite,commission"
 getopt_out=$(getopt --name "${0##*/}" \
 	--options "${short_opts}" --long "${long_opts}" -- "$@") &&
 	eval set -- "${getopt_out}" ||
@@ -202,6 +206,7 @@ while [ $# -ne 0 ]; do
 		-e|--exists) check_exists=true;;
 		-w|--in-action) check_action_in_progress=true;;
 		-q|--quite) quite=true;;
+		-c|--commission) commission=true;;
 		--) shift; break;;
 	esac
 	shift;
@@ -268,5 +273,5 @@ elif [ "$check_action_in_progress" = true ]; then
   check_node "$protocol://$servername/$api_url" "${mac_addrs}" "is_action_in_progress"
   exit $?
 else
-  enlist_node "$protocol://$servername/$api_url" "${mac_addrs}" "$arch" "$subarch" "$hostname" "$power_type" "$power_parameters"
+  enlist_node "$protocol://$servername/$api_url" "${mac_addrs}" "$arch" "$subarch" "$hostname" "$power_type" "$power_parameters" "$commission"
 fi
diff --git a/src/metadataserver/user_data/tests/test_generate_user_data.py b/src/metadataserver/user_data/tests/test_generate_user_data.py
index 3632799..4b51a79 100644
--- a/src/metadataserver/user_data/tests/test_generate_user_data.py
+++ b/src/metadataserver/user_data/tests/test_generate_user_data.py
@@ -26,7 +26,8 @@ class TestGenerateUserData(MAASServerTestCase):
         # both definitions and use of various commands in python.
         rack = factory.make_RackController()
         user_data = generate_user_data_for_status(
-            None, NODE_STATUS.NEW, rack_controller=rack)
+            None, NODE_STATUS.NEW, rack_controller=rack,
+            extra_content={'enlist_commissioning': True})
         parsed_data = email.message_from_string(user_data.decode("utf-8"))
         self.assertTrue(parsed_data.is_multipart())