Merge ~ltrager/maas:lp1816651 into maas:master

Proposed by Lee Trager
Status: Merged
Approved by: Lee Trager
Approved revision: 326d99566d56686a4e0bc103be14582e9679e620
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ltrager/maas:lp1816651
Merge into: maas:master
Diff against target: 67 lines (+29/-8)
2 files modified
src/maasserver/models/bmc.py (+9/-7)
src/maasserver/models/tests/test_bmc.py (+20/-1)
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve
MAAS Lander Approve
Review via email: mp+363520@code.launchpad.net

Commit message

LP: #1816651 - Handle duplicate IP addresses when creating a BMC object.

To post a comment you must log in.
~ltrager/maas:lp1816651 updated
e172ae2... by Lee Trager

Copyright++

Revision history for this message
Mike Pontillo (mpontillo) wrote :

Can you see if this slightly simpler change does the trick?

https://paste.ubuntu.com/p/cxqZqmzZHC/

I don't think this change will work as intended, since the BMC IP might be an AUTO IP, not strictly a STICKY. For example, if I deploy a machine and use it for a KVM pod, it might get an AUTO address (or even DHCP), rather than STICKY.

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1816651 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: e172ae26e4215c5ab6ea158069e67f69e2a1d236

review: Approve
~ltrager/maas:lp1816651 updated
951c852... by Lee Trager

mpontillo suggestion

326d995... by Lee Trager

Use mpontillos patch

Revision history for this message
Mike Pontillo (mpontillo) wrote :

Looks good; thanks for the fix!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
index 1e3b865..711e756 100644
--- a/src/maasserver/models/bmc.py
+++ b/src/maasserver/models/bmc.py
@@ -1,4 +1,4 @@
1# Copyright 2015-2018 Canonical Ltd. This software is licensed under the1# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""BMC objects."""4"""BMC objects."""
@@ -337,12 +337,14 @@ class BMC(CleanSave, TimestampedModel):
337 with transaction.atomic():337 with transaction.atomic():
338 subnet = Subnet.objects.get_best_subnet_for_ip(new_ip)338 subnet = Subnet.objects.get_best_subnet_for_ip(new_ip)
339 (self.ip_address,339 (self.ip_address,
340 _) = StaticIPAddress.objects.get_or_create(340 _) = StaticIPAddress.objects.exclude(
341 ip=new_ip,341 alloc_type=IPADDRESS_TYPE.DISCOVERED
342 defaults={342 ).get_or_create(
343 'alloc_type': IPADDRESS_TYPE.STICKY,343 ip=new_ip,
344 'subnet': subnet,344 defaults={
345 })345 'alloc_type': IPADDRESS_TYPE.STICKY,
346 'subnet': subnet,
347 })
346 except Exception as error:348 except Exception as error:
347 maaslog.info(349 maaslog.info(
348 "BMC could not save extracted IP "350 "BMC could not save extracted IP "
diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
index e806ff0..41ff534 100644
--- a/src/maasserver/models/tests/test_bmc.py
+++ b/src/maasserver/models/tests/test_bmc.py
@@ -1,4 +1,4 @@
1# Copyright 2016-2018 Canonical Ltd. This software is licensed under the1# Copyright 2016-2019 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Test maasserver models."""4"""Test maasserver models."""
@@ -146,6 +146,25 @@ class TestBMC(MAASServerTestCase):
146 self.assertNotEqual(machine_ip_addr.id, bmc.ip_address.id)146 self.assertNotEqual(machine_ip_addr.id, bmc.ip_address.id)
147 return machine, bmc, machine_ip147 return machine, bmc, machine_ip
148148
149 def test_make_machine_and_bmc_discovered_ip(self):
150 # Regression test for LP:1816651
151 subnet = factory.make_Subnet()
152 discovered_ip = factory.make_StaticIPAddress(
153 subnet=subnet, alloc_type=IPADDRESS_TYPE.DISCOVERED)
154 sticky_ip = factory.make_StaticIPAddress(
155 ip=discovered_ip.ip, subnet=subnet,
156 alloc_type=IPADDRESS_TYPE.STICKY)
157 bmc = factory.make_BMC(
158 power_type='virsh',
159 power_parameters={
160 'power_address':
161 "protocol://%s:8080/path/to/thing#tag" % (
162 factory.ip_to_url_format(discovered_ip.ip))})
163 self.assertItemsEqual(
164 [discovered_ip, sticky_ip],
165 StaticIPAddress.objects.filter(ip=discovered_ip.ip))
166 self.assertEqual(sticky_ip, bmc.ip_address)
167
149 def test_bmc_save_extracts_ip_address(self):168 def test_bmc_save_extracts_ip_address(self):
150 subnet = factory.make_Subnet()169 subnet = factory.make_Subnet()
151 sticky_ip = factory.make_StaticIPAddress(170 sticky_ip = factory.make_StaticIPAddress(

Subscribers

People subscribed via source and target branches