Merge ~ack/maas:1887797-2.9 into maas:2.9

Proposed by Alberto Donato
Status: Merged
Approved by: Alberto Donato
Approved revision: 5b0816ba478c41316862884118987cd2dca0024c
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ack/maas:1887797-2.9
Merge into: maas:2.9
Diff against target: 84 lines (+24/-12)
2 files modified
src/maasserver/models/node.py (+13/-4)
src/maasserver/models/tests/test_node.py (+11/-8)
Reviewer Review Type Date Requested Status
Alberto Donato Approve
Review via email: mp+407846@code.launchpad.net

Commit message

LP: #1887797 - don't fail machine removal if Pod action fails

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
2index 14c3119..d175d47 100644
3--- a/src/maasserver/models/node.py
4+++ b/src/maasserver/models/node.py
5@@ -2950,7 +2950,7 @@ class Node(CleanSave, TimestampedModel):
6 client_idents = pod.get_client_identifiers()
7
8 @transactional
9- def _save(machine_id, pod_id, hints):
10+ def _save(machine_id, pod_id, result):
11 from maasserver.models.bmc import Pod
12
13 machine = Machine.objects.filter(id=machine_id).first()
14@@ -2966,9 +2966,16 @@ class Node(CleanSave, TimestampedModel):
15 machine_id=machine_id
16 ).delete()
17 super(Node, machine).delete()
18+
19+ if isinstance(result, Failure):
20+ maaslog.warning(
21+ f"{self.hostname}: Failure decomposing machine: {result.value}"
22+ )
23+ return
24+
25 pod = Pod.objects.filter(id=pod_id).first()
26 if pod is not None:
27- pod.sync_hints(hints)
28+ pod.sync_hints(result)
29
30 maaslog.info("%s: Decomposing machine", self.hostname)
31
32@@ -2981,8 +2988,10 @@ class Node(CleanSave, TimestampedModel):
33 pod_id=pod.id,
34 name=pod.name,
35 )
36- d.addCallback(
37- lambda hints: (deferToDatabase(_save, self.id, pod.id, hints))
38+ d.addBoth(
39+ lambda result: (
40+ deferToDatabase(_save, self.id, pod.id, result)
41+ )
42 )
43 d.addCallback(lambda _: request_commissioning_results(pod))
44 else:
45diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
46index 375ab21..ab07168 100644
47--- a/src/maasserver/models/tests/test_node.py
48+++ b/src/maasserver/models/tests/test_node.py
49@@ -74,7 +74,6 @@ from maasserver.enum import (
50 from maasserver.exceptions import (
51 IPAddressCheckFailed,
52 NodeStateViolation,
53- PodProblem,
54 PowerProblem,
55 StaticIPAddressExhaustion,
56 )
57@@ -6088,16 +6087,20 @@ class TestDecomposeMachineTransactional(
58 interface = transactional(reload_object)(interface)
59 self.assertIsNone(interface)
60
61- def test_errors_raised_up(self):
62+ def test_delete_doesnt_fail_removal(self):
63+ mock_log_warning = self.patch(node_module.maaslog, "warning")
64 pod, machine, hints, client = self.create_pod_machine_and_hints(
65 creation_type=NODE_CREATION_TYPE.MANUAL
66 )
67- client.return_value = defer.fail(PodActionFail())
68- with ExpectedException(PodProblem):
69- with post_commit_hooks:
70- machine.delete()
71- machine = transactional(reload_object)(machine)
72- self.assertIsNotNone(machine)
73+ client.return_value = defer.fail(PodActionFail("bang!"))
74+ with post_commit_hooks:
75+ machine.delete()
76+ mock_log_warning.assert_called_with(
77+ f"{machine.hostname}: Failure decomposing machine: "
78+ "Unable to decompose machine because: bang!"
79+ )
80+ # the machine is still deleted
81+ self.assertIsNone(transactional(reload_object)(machine))
82
83 def test_release_deletes_dynamic_machine(self):
84 owner = transactional(factory.make_User)()

Subscribers

People subscribed via source and target branches