Merge ~andreserl/maas:revert_00f4173 into maas:master

[Andres Rodriguez]

Revert "Allow enlisted nodes that have successfully commissioned to be tested."

This reverts commit 00f417318822634da70081dce5146b65e2a78c48 because this causes newly enlisted machines to go to 'Ready' instead of 'New'. This is also breaking the CI.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b revert_00f4173 lp:~andreserl/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 3ea7fbda19b5e1191e8ea7524448736d0a9c57c7

review: Approve

[Andres Rodriguez (andreserl)]

selfie as this is reverting a branch that changing behavior in non-compatible way and causing CI faillures.

review: Approve

diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index d3b74c5..b09d0dc 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -2039,15 +2039,6 @@ class Node(CleanSave, TimestampedModel):
                 "Unconfigured power type. "
                 "Please configure the power type and try again.")
 
-        # If this is an enlisted node make sure commissioning during enlistment
-        # finished successfully.
-        if (self.status == NODE_STATUS.NEW and
-                not self.current_commissioning_script_set):
-            raise ValidationError(
-                "Unable to start machine testing; this node has never been "
-                "commissioned. Please use the 'Commission' action to "
-                "commission & test this machine.")
-
         # Create a new ScriptSet for the tests to be run.
         script_set = ScriptSet.objects.create_testing_script_set(
             self, testing_scripts)
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index 2538393..7b7c5b7 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -3170,12 +3170,6 @@ class TestNode(MAASServerTestCase):
         self.assertRaises(
             UnknownPowerType, node.start_testing, admin)
 
-    def test_start_testing_errors_for_new_node_no_commissioning(self):
-        node = factory.make_Node(interface=True, status=NODE_STATUS.NEW)
-        admin = factory.make_admin()
-        self.assertRaises(
-            ValidationError, node.start_testing, admin)
-
     def test_start_testing_logs_user_request(self):
         script = factory.make_Script(script_type=SCRIPT_TYPE.TESTING)
         node = factory.make_Node(
@@ -3203,23 +3197,6 @@ class TestNode(MAASServerTestCase):
         self.assertEquals(NODE_STATUS.TESTING, node.status)
         self.assertThat(mock_node_power_cycle, MockCalledOnce())
 
-    def test_start_testing_changes_status_and_starts_new_node(self):
-        script = factory.make_Script(script_type=SCRIPT_TYPE.TESTING)
-        node = factory.make_Node(
-            interface=True, status=NODE_STATUS.NEW, power_type='manual')
-        node.current_commissioning_script_set = factory.make_ScriptSet(
-            node=node, result_type=RESULT_TYPE.COMMISSIONING)
-        factory.make_ScriptResult(
-            script_set=node.current_commissioning_script_set)
-        mock_node_power_cycle = self.patch(node, '_power_cycle')
-        mock_node_power_cycle.return_value = None
-        admin = factory.make_admin()
-        node.start_testing(admin, testing_scripts=[script.name])
-        post_commit_hooks.reset()  # Ignore these for now.
-        node = reload_object(node)
-        self.assertEquals(NODE_STATUS.TESTING, node.status)
-        self.assertThat(mock_node_power_cycle, MockCalledOnce())
-
     def test_start_testing_sets_options(self):
         script = factory.make_Script(script_type=SCRIPT_TYPE.TESTING)
         rack = factory.make_RackController()
diff --git a/src/maasserver/node_action.py b/src/maasserver/node_action.py
index 4ad435e..a457e55 100644
--- a/src/maasserver/node_action.py
+++ b/src/maasserver/node_action.py
@@ -1,4 +1,4 @@
-# Copyright 2012-2018 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.
@@ -283,7 +283,6 @@ class Test(NodeAction):
     display_sentence = "tested"
     actionable_statuses = (
         NODE_STATUS.FAILED_COMMISSIONING,
-        NODE_STATUS.NEW,
         NODE_STATUS.READY,
         NODE_STATUS.RESERVED,
         NODE_STATUS.ALLOCATED,
diff --git a/src/maasserver/node_status.py b/src/maasserver/node_status.py
index c43c4a9..e8e5123 100644
--- a/src/maasserver/node_status.py
+++ b/src/maasserver/node_status.py
@@ -1,4 +1,4 @@
-# Copyright 2014-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Node status utilities."""
@@ -18,7 +18,6 @@ from provisioningserver.utils.enum import map_enum
 # This is a subset of the statuses NODE_STATUS.TESTING can transition to. This
 # allows testing to be aborted.
 NODE_TESTING_RESET_READY_TRANSITIONS = {
-    NODE_STATUS.NEW,
     NODE_STATUS.COMMISSIONING,
     NODE_STATUS.FAILED_DEPLOYMENT,
     NODE_STATUS.MISSING,
@@ -53,7 +52,6 @@ NODE_TRANSITIONS = {
         ],
     NODE_STATUS.NEW: [
         NODE_STATUS.COMMISSIONING,
-        NODE_STATUS.TESTING,
         NODE_STATUS.MISSING,
         NODE_STATUS.READY,
         NODE_STATUS.RETIRED,
diff --git a/src/metadataserver/api.py b/src/metadataserver/api.py
index 3f575c3..66bc1d1 100644
--- a/src/metadataserver/api.py
+++ b/src/metadataserver/api.py
@@ -604,10 +604,9 @@ class VersionIndexHandler(MetadataViewHandler):
         }
         target_status = signaling_statuses.get(status)
         enlisting = NodeMetadata.objects.get(node=node, key='enlisting')
-        if enlisting is not None and status != SIGNAL_STATUS.TESTING:
+        if enlisting is not None and status == SIGNAL_STATUS.OK:
             enlisting.delete()
-            if status == SIGNAL_STATUS.OK:
-                target_status = NODE_STATUS.NEW
+            target_status = NODE_STATUS.NEW
 
         if target_status in [
                 NODE_STATUS.NEW, NODE_STATUS.READY, NODE_STATUS.TESTING]:
diff --git a/src/metadataserver/tests/test_api.py b/src/metadataserver/tests/test_api.py
index 3470d10..5ce5990 100644
--- a/src/metadataserver/tests/test_api.py
+++ b/src/metadataserver/tests/test_api.py
@@ -1763,18 +1763,6 @@ class TestCommissioningAPI(MAASServerTestCase):
         self.assertEqual(http.client.OK, response.status_code)
         self.assertEqual(NODE_STATUS.READY, reload_object(node).status)
 
-    def test_signaling_commissioning_failure_deletes_enlisting(self):
-        node = factory.make_Node(
-            status=NODE_STATUS.COMMISSIONING, with_empty_script_sets=True)
-        nmd = NodeMetadata.objects.create(
-            node=node, key='enlisting', value='True')
-        client = make_node_client(node=node)
-        response = call_signal(client, status=SIGNAL_STATUS.FAILED)
-        self.assertEqual(http.client.OK, response.status_code)
-        self.assertEqual(
-            NODE_STATUS.FAILED_COMMISSIONING, reload_object(node).status)
-        self.assertIsNone(reload_object(nmd))
-
     def test_signaling_commissioning_failure_makes_node_failed_tests(self):
         node = factory.make_Node(
             status=NODE_STATUS.COMMISSIONING, with_empty_script_sets=True)