Merge lp:~julian-edwards/maas/mark-failed-twice into lp:~maas-committers/maas/trunk

[Julian Edwards]

If Node.mark_failed() is called when the node is already in a failed state, ignore it.

This could be seen as papering over an underlying problem and I won't disagree with that, but this is not a bad thing to do anyway.

[Jeroen T. Vermeulen (jtv)]

I think you're doing the right thing. A second mark-as-failed might come from a concurrent job, such as the power poller.

review: Approve

[Julian Edwards (julian-edwards)]

Cheers.

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2014-09-10 16:20:57 +0000
+++ src/maasserver/models/node.py	2014-09-12 02:42:45 +0000
@@ -1272,6 +1272,9 @@
             self.status = failed_statuses_mapping[self.status]
             self.error_description = error_description
             self.save()
+        elif self.status in failed_statuses_mapping.viewvalues():
+            # Silently ignore a request to fail an already failed node.
+            pass
         else:
             raise NodeStateViolation(
                 "The status of the node is %s; this status cannot "

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2014-09-10 16:20:57 +0000
+++ src/maasserver/models/tests/test_node.py	2014-09-12 02:42:45 +0000
@@ -1152,6 +1152,14 @@
         description = factory.make_name('error-description')
         self.assertRaises(NodeStateViolation, node.mark_failed, description)
 
+    def test_mark_failed_ignores_if_already_failed(self):
+        status = random.choice([
+            NODE_STATUS.FAILED_DEPLOYMENT, NODE_STATUS.FAILED_COMMISSIONING])
+        node = factory.make_Node(status=status)
+        description = factory.make_name('error-description')
+        node.mark_failed(description)
+        self.assertEqual(status, node.status)
+
     def test_mark_broken_changes_status_to_broken(self):
         node = factory.make_Node(
             status=NODE_STATUS.NEW, owner=factory.make_User())