Merge ~ltrager/maas:lp1911825 into maas:master

Proposed by Lee Trager
Status: Merged
Approved by: Lee Trager
Approved revision: 077855cdbea784ef8e831add002c0592b16119e3
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ltrager/maas:lp1911825
Merge into: maas:master
Diff against target: 99 lines (+33/-15)
2 files modified
src/maasserver/models/bmc.py (+25/-6)
src/maasserver/models/tests/test_bmc.py (+8/-9)
Reviewer Review Type Date Requested Status
Alberto Donato Approve
MAAS Lander Approve
Review via email: mp+396398@code.launchpad.net

Commit message

LP: #1911825 - Don't set BMC.ip_address when power_address contains an FQDN

When a BMC is created or edited MAAS maps its IP address to a subnet which
is stored in the model. This allows MAAS to send power commands only to
rack controllers which are on the same subnet as the BMC. If no rack
controller is on the same subnet as the BMC the power command is sent to
all rack controllers. When an FQDN or hostname were given an exception was
raised as MAAS expected an IP address. The exception is now avoided so the
fallback code is used which allow for FQDN or hostnames in the
power_address.

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

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

STATUS: SUCCESS
COMMIT: f50b041262946304722be1eb4342ef27826c52f4

review: Approve
Revision history for this message
Alberto Donato (ack) wrote :

one question inline

review: Needs Information
~ltrager/maas:lp1911825 updated
077855c... by Lee Trager

ack fix

Revision history for this message
Lee Trager (ltrager) :
Revision history for this message
MAAS Lander (maas-lander) wrote :

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

STATUS: SUCCESS
COMMIT: 077855cdbea784ef8e831add002c0592b16119e3

review: Approve
Revision history for this message
Alberto Donato (ack) wrote :

+1 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 5802c5c..6eefa76 100644
--- a/src/maasserver/models/bmc.py
+++ b/src/maasserver/models/bmc.py
@@ -1,4 +1,4 @@
1# Copyright 2015-2020 Canonical Ltd. This software is licensed under the1# Copyright 2015-2021 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."""
@@ -30,6 +30,7 @@ from django.db.models import (
30)30)
31from django.db.models.query import QuerySet31from django.db.models.query import QuerySet
32from django.shortcuts import get_object_or_40432from django.shortcuts import get_object_or_404
33from netaddr import AddrFormatError, IPAddress
33import petname34import petname
34from twisted.internet.defer import inlineCallbacks35from twisted.internet.defer import inlineCallbacks
3536
@@ -327,10 +328,6 @@ class BMC(CleanSave, TimestampedModel):
327 parameters has changed."""328 parameters has changed."""
328 new_ip = BMC.extract_ip_address(self.power_type, self.power_parameters)329 new_ip = BMC.extract_ip_address(self.power_type, self.power_parameters)
329 current_ip = None if self.ip_address is None else self.ip_address.ip330 current_ip = None if self.ip_address is None else self.ip_address.ip
330 # Set the ip_address field. If we have a bracketed address, assume
331 # it's IPv6, and strip the brackets.
332 if new_ip and new_ip.startswith("[") and new_ip.endswith("]"):
333 new_ip = new_ip[1:-1]
334 if new_ip != current_ip:331 if new_ip != current_ip:
335 if not new_ip:332 if not new_ip:
336 self.ip_address = None333 self.ip_address = None
@@ -425,7 +422,29 @@ class BMC(CleanSave, TimestampedModel):
425 return None422 return None
426 match = re.match(extraction_pattern, field_value)423 match = re.match(extraction_pattern, field_value)
427 if match:424 if match:
428 return match.group("address")425 ip = match.group("address")
426 # If we have a bracketed address, assume it's IPv6, and strip the
427 # brackets.
428 if ip.startswith("[") and ip.endswith("]"):
429 ip = ip[1:-1]
430 if ip == "":
431 return ip
432 # self.clean() attempts to map the return value of this method to
433 # a subnet. If the user gives an FQDN or hostname the mapping fails
434 # when Subnet.objects.get_best_subnet_for_ip() is called and an
435 # exception is raised as an IP is expected. If the BMC does not
436 # have an IP address MAAS will fall back on sending power requests
437 # to all connected rack controllers.
438 try:
439 IPAddress(ip)
440 except AddrFormatError:
441 maaslog.info(
442 "BMC uses FQDN, power action will be sent to all "
443 "rack controllers"
444 )
445 return None
446 else:
447 return ip
429 # no match found - return None448 # no match found - return None
430 return None449 return None
431450
diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
index adf1fe4..18597b5 100644
--- a/src/maasserver/models/tests/test_bmc.py
+++ b/src/maasserver/models/tests/test_bmc.py
@@ -1,4 +1,4 @@
1# Copyright 2016-2020 Canonical Ltd. This software is licensed under the1# Copyright 2016-2021 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."""
@@ -433,14 +433,6 @@ class TestBMC(MAASServerTestCase):
433 power_parameters = {"power_address": None}433 power_parameters = {"power_address": None}
434 self.assertEqual(None, BMC.extract_ip_address("hmc", power_parameters))434 self.assertEqual(None, BMC.extract_ip_address("hmc", power_parameters))
435435
436 def test_bmc_extract_ip_address_from_url(self):
437 power_parameters = {
438 "power_address": "protocol://somehost:8080/path/to/thing#tag"
439 }
440 self.assertEqual(
441 "somehost", BMC.extract_ip_address("virsh", power_parameters)
442 )
443
444 def test_bmc_extract_ip_address_from_url_blank_gives_none(self):436 def test_bmc_extract_ip_address_from_url_blank_gives_none(self):
445 self.assertEqual(None, BMC.extract_ip_address("virsh", None))437 self.assertEqual(None, BMC.extract_ip_address("virsh", None))
446 self.assertEqual(None, BMC.extract_ip_address("virsh", {}))438 self.assertEqual(None, BMC.extract_ip_address("virsh", {}))
@@ -459,6 +451,13 @@ class TestBMC(MAASServerTestCase):
459 power_parameters = {"power_address": "http://:8080/foo/#baz"}451 power_parameters = {"power_address": "http://:8080/foo/#baz"}
460 self.assertEqual("", BMC.extract_ip_address("virsh", power_parameters))452 self.assertEqual("", BMC.extract_ip_address("virsh", power_parameters))
461453
454 def test_bmc_extract_ip_address_with_fqdn_returns_none(self):
455 self.assertIsNone(
456 BMC.extract_ip_address(
457 "webhook", {"power_address": factory.make_url()}
458 )
459 )
460
462 def test_get_usable_rack_controllers_returns_empty_when_none(self):461 def test_get_usable_rack_controllers_returns_empty_when_none(self):
463 bmc = factory.make_BMC()462 bmc = factory.make_BMC()
464 self.assertThat(bmc.get_usable_rack_controllers(), HasLength(0))463 self.assertThat(bmc.get_usable_rack_controllers(), HasLength(0))

Subscribers

People subscribed via source and target branches