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
1diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
2index 5802c5c..6eefa76 100644
3--- a/src/maasserver/models/bmc.py
4+++ b/src/maasserver/models/bmc.py
5@@ -1,4 +1,4 @@
6-# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
7+# Copyright 2015-2021 Canonical Ltd. This software is licensed under the
8 # GNU Affero General Public License version 3 (see the file LICENSE).
9
10 """BMC objects."""
11@@ -30,6 +30,7 @@ from django.db.models import (
12 )
13 from django.db.models.query import QuerySet
14 from django.shortcuts import get_object_or_404
15+from netaddr import AddrFormatError, IPAddress
16 import petname
17 from twisted.internet.defer import inlineCallbacks
18
19@@ -327,10 +328,6 @@ class BMC(CleanSave, TimestampedModel):
20 parameters has changed."""
21 new_ip = BMC.extract_ip_address(self.power_type, self.power_parameters)
22 current_ip = None if self.ip_address is None else self.ip_address.ip
23- # Set the ip_address field. If we have a bracketed address, assume
24- # it's IPv6, and strip the brackets.
25- if new_ip and new_ip.startswith("[") and new_ip.endswith("]"):
26- new_ip = new_ip[1:-1]
27 if new_ip != current_ip:
28 if not new_ip:
29 self.ip_address = None
30@@ -425,7 +422,29 @@ class BMC(CleanSave, TimestampedModel):
31 return None
32 match = re.match(extraction_pattern, field_value)
33 if match:
34- return match.group("address")
35+ ip = match.group("address")
36+ # If we have a bracketed address, assume it's IPv6, and strip the
37+ # brackets.
38+ if ip.startswith("[") and ip.endswith("]"):
39+ ip = ip[1:-1]
40+ if ip == "":
41+ return ip
42+ # self.clean() attempts to map the return value of this method to
43+ # a subnet. If the user gives an FQDN or hostname the mapping fails
44+ # when Subnet.objects.get_best_subnet_for_ip() is called and an
45+ # exception is raised as an IP is expected. If the BMC does not
46+ # have an IP address MAAS will fall back on sending power requests
47+ # to all connected rack controllers.
48+ try:
49+ IPAddress(ip)
50+ except AddrFormatError:
51+ maaslog.info(
52+ "BMC uses FQDN, power action will be sent to all "
53+ "rack controllers"
54+ )
55+ return None
56+ else:
57+ return ip
58 # no match found - return None
59 return None
60
61diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
62index adf1fe4..18597b5 100644
63--- a/src/maasserver/models/tests/test_bmc.py
64+++ b/src/maasserver/models/tests/test_bmc.py
65@@ -1,4 +1,4 @@
66-# Copyright 2016-2020 Canonical Ltd. This software is licensed under the
67+# Copyright 2016-2021 Canonical Ltd. This software is licensed under the
68 # GNU Affero General Public License version 3 (see the file LICENSE).
69
70 """Test maasserver models."""
71@@ -433,14 +433,6 @@ class TestBMC(MAASServerTestCase):
72 power_parameters = {"power_address": None}
73 self.assertEqual(None, BMC.extract_ip_address("hmc", power_parameters))
74
75- def test_bmc_extract_ip_address_from_url(self):
76- power_parameters = {
77- "power_address": "protocol://somehost:8080/path/to/thing#tag"
78- }
79- self.assertEqual(
80- "somehost", BMC.extract_ip_address("virsh", power_parameters)
81- )
82-
83 def test_bmc_extract_ip_address_from_url_blank_gives_none(self):
84 self.assertEqual(None, BMC.extract_ip_address("virsh", None))
85 self.assertEqual(None, BMC.extract_ip_address("virsh", {}))
86@@ -459,6 +451,13 @@ class TestBMC(MAASServerTestCase):
87 power_parameters = {"power_address": "http://:8080/foo/#baz"}
88 self.assertEqual("", BMC.extract_ip_address("virsh", power_parameters))
89
90+ def test_bmc_extract_ip_address_with_fqdn_returns_none(self):
91+ self.assertIsNone(
92+ BMC.extract_ip_address(
93+ "webhook", {"power_address": factory.make_url()}
94+ )
95+ )
96+
97 def test_get_usable_rack_controllers_returns_empty_when_none(self):
98 bmc = factory.make_BMC()
99 self.assertThat(bmc.get_usable_rack_controllers(), HasLength(0))

Subscribers

People subscribed via source and target branches