Merge ~mpontillo/maas:prevent-ip-removal-on-bridge-release--bug-1792409 into maas:master

Proposed by Mike Pontillo on 2018-10-18
Status: Merged
Approved by: Mike Pontillo on 2018-10-18
Approved revision: bfefc4842bfd0d35bd7f9251b1f41565545f6e77
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~mpontillo/maas:prevent-ip-removal-on-bridge-release--bug-1792409
Merge into: maas:master
Diff against target: 17 lines (+5/-1)
1 file modified
src/maasserver/models/node.py (+5/-1)
Reviewer Review Type Date Requested Status
Alberto Donato 2018-10-18 Approve on 2018-10-18
MAAS Lander Approve on 2018-10-18
Review via email: mp+357010@code.launchpad.net

Commit message

LP #1792409 - Prevent removal of IP addresses when bridge interfaces are removed on release.

Description of the change

This was a strange race condition that seems to be related to the Django result cache - logging suggested that it was finding IP addresses attached to interfaces, when those IP addresses had been previously removed moments earlier.

Invalidating the cache didn't work, but we already had a workaround in the signal handler we could use to prevent the unwanted deletion of any IP addresses.

To post a comment you must log in.
bfefc48... by Mike Pontillo on 2018-10-18

LP: #1792409 - Prevent removal of IP addresses when bridge interfaces are removed on release.

Mike Pontillo (mpontillo) wrote :

Note: I couldn't find a good way to unit test this change, since it's a race condition that we don't see in that environment, and I can't access the same instance of the BridgeInterface to make sure it has the _skip_ip_address_removal attribute set.

MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b prevent-ip-removal-on-bridge-release--bug-1792409 lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: bfefc4842bfd0d35bd7f9251b1f41565545f6e77

review: Approve
Alberto Donato (ack) wrote :

lgtm

review: Approve
Mike Pontillo (mpontillo) :

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 c44ec3c..e1fc47b 100644
3--- a/src/maasserver/models/node.py
4+++ b/src/maasserver/models/node.py
5@@ -3282,7 +3282,11 @@ class Node(CleanSave, TimestampedModel):
6 for sip in interface.ip_addresses.all():
7 sip.interface_set.remove(interface)
8 sip.interface_set.add(parent)
9- # Delete the acquired bridge interface.
10+ # Delete the acquired bridge interface, and set a property
11+ # to prevent a race condition that would otherwise cause
12+ # the IP addresses moved to the physical interface to be
13+ # deleted.
14+ setattr(interface, '_skip_ip_address_removal', True)
15 interface.delete()
16
17 def _clear_networking_configuration(self):

Subscribers

People subscribed via source and target branches

to all changes: