Merge lp:~mpontillo/maas/allow-multiple-macs-on-the-same-node into lp:maas/trunk

Proposed by Mike Pontillo
Status: Rejected
Rejected by: MAAS Lander
Proposed branch: lp:~mpontillo/maas/allow-multiple-macs-on-the-same-node
Merge into: lp:maas/trunk
Diff against target: 222 lines (+101/-51)
4 files modified
src/maasserver/models/interface.py (+0/-13)
src/maasserver/models/node.py (+2/-1)
src/maasserver/rpc/events.py (+4/-5)
src/metadataserver/builtin_scripts/hooks.py (+95/-32)
To merge this branch: bzr merge lp:~mpontillo/maas/allow-multiple-macs-on-the-same-node
Reviewer Review Type Date Requested Status
MAAS Maintainers Pending
Review via email: mp+319073@code.launchpad.net

Commit message

Allow multiple physical interfaces to have the same MAC if they are on the same node.

To post a comment you must log in.
MAAS Lander (maas-lander) wrote :

Transitioned to Git.

lp:maas has now moved from Bzr to Git.
Please propose your branches with Launchpad using Git.

git clone https://git.launchpad.net/maas

Unmerged revisions

5780. By Mike Pontillo on 2017-03-06

Merge trunk.

5779. By Mike Pontillo on 2017-03-06

Allow multiple physical interfaces to have the same MAC if they are on the same node.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/interface.py'
2--- src/maasserver/models/interface.py 2017-02-17 14:23:04 +0000
3+++ src/maasserver/models/interface.py 2017-03-06 14:57:51 +0000
4@@ -1403,19 +1403,6 @@
5 if len(validation_errors) > 0:
6 raise ValidationError(validation_errors)
7
8- # MAC address must be unique amongst every PhysicalInterface.
9- other_interfaces = PhysicalInterface.objects.filter(
10- mac_address=self.mac_address)
11- if self.id is not None:
12- other_interfaces = other_interfaces.exclude(id=self.id)
13- other_interfaces = other_interfaces.all()
14- if len(other_interfaces) > 0:
15- raise ValidationError({
16- "mac_address": [
17- "This MAC address is already in use by %s." % (
18- other_interfaces[0].get_log_string())]
19- })
20-
21 # No parents are allow for a physical interface.
22 if self.id is not None:
23 # Use the precache so less queries are made.
24
25=== modified file 'src/maasserver/models/node.py'
26--- src/maasserver/models/node.py 2017-03-02 00:22:23 +0000
27+++ src/maasserver/models/node.py 2017-03-06 14:57:51 +0000
28@@ -1625,7 +1625,8 @@
29 UnknownInterface.objects.filter(mac_address=mac).delete()
30 try:
31 iface = PhysicalInterface.objects.get(mac_address=mac)
32- except PhysicalInterface.DoesNotExist:
33+ except (PhysicalInterface.MultipleObjectsReturned,
34+ PhysicalInterface.DoesNotExist):
35 return PhysicalInterface.objects.create(
36 node=self, mac_address=mac, name=name)
37 if iface.node != self:
38
39=== modified file 'src/maasserver/rpc/events.py'
40--- src/maasserver/rpc/events.py 2016-10-25 13:57:02 +0000
41+++ src/maasserver/rpc/events.py 2017-03-06 14:57:51 +0000
42@@ -75,12 +75,11 @@
43 except EventType.DoesNotExist:
44 raise NoSuchEventType.from_name(type_name)
45
46- try:
47- interface = Interface.objects.get(
48- type=INTERFACE_TYPE.PHYSICAL, mac_address=mac_address)
49- except Interface.DoesNotExist:
50+ interface = Interface.objects.filter(
51+ type=INTERFACE_TYPE.PHYSICAL, mac_address=mac_address).first()
52+ if interface is None:
53 # The node doesn't exist, but we don't raise an exception - it's
54- # entirely possible the cluster has started sending events for a node
55+ # entirely possible the rack has started sending events for a node
56 # that we don't know about yet. This is most likely to happen when a
57 # new node is trying to enlist.
58 maaslog.debug(
59
60=== modified file 'src/metadataserver/builtin_scripts/hooks.py'
61--- src/metadataserver/builtin_scripts/hooks.py 2017-03-06 13:32:48 +0000
62+++ src/metadataserver/builtin_scripts/hooks.py 2017-03-06 14:57:51 +0000
63@@ -7,6 +7,7 @@
64 'NODE_INFO_SCRIPTS',
65 ]
66
67+from collections import defaultdict
68 import fnmatch
69 import json
70 import logging
71@@ -91,6 +92,31 @@
72 return interface
73
74
75+def get_mac_to_ifnames_dict(ip_addr_info):
76+ mac_to_ifnames = defaultdict(list)
77+ for link in ip_addr_info.values():
78+ mac = link.get('mac')
79+ if mac is None:
80+ # Ignore loopback interfaces.
81+ continue
82+ else:
83+ ifname = link['name']
84+ mac_to_ifnames[mac].append(ifname)
85+ return mac_to_ifnames
86+
87+
88+def clean_up_moved_nics(node, mac):
89+ interfaces = PhysicalInterface.objects.filter(
90+ mac_address=mac).exclude(node=node)
91+ for interface in interfaces:
92+ logger.warning(
93+ "Interface with MAC %s moved from node %s to %s. "
94+ "(The existing interface will be deleted.)" %
95+ (interface.mac_address, interface.node.fqdn,
96+ node.fqdn))
97+ interface.delete()
98+
99+
100 def update_node_network_information(node, output, exit_status):
101 """Updates the network interfaces from the results of `IPADDR_SCRIPT`.
102
103@@ -107,50 +133,87 @@
104 node.hostname, exit_status))
105 return
106 assert isinstance(output, bytes)
107-
108 # Skip network configuration if set by the user.
109 if node.skip_networking:
110 return
111-
112 # Get the MAC addresses of all connected interfaces.
113 ip_addr_info = parse_ip_addr(output)
114+ # Prepare a set of interfaces found during commissioning, so that any
115+ # physical interfaces no longer present on the machine can be removed.
116 current_interfaces = set()
117-
118- for link in ip_addr_info.values():
119- link_mac = link.get('mac')
120- # Ignore loopback interfaces.
121- if link_mac is None:
122- continue
123- else:
124- ifname = link['name']
125- try:
126+ # Pre-process the list to check if multiple physical interfaces share the
127+ # same MAC.
128+ mac_to_ifnames = get_mac_to_ifnames_dict(ip_addr_info)
129+ # Go through the map of { ifname: [mac, ...] } to determine the best course
130+ # of action. If interface names correspond 1:1 with MACs (the normal case)
131+ # we can avoid losing data (such as child interfaces created by a MAAS
132+ # admin by simply renaming interfaces).
133+ for mac, ifnames in mac_to_ifnames.items():
134+ clean_up_moved_nics(node, mac)
135+ existing_iface_count = PhysicalInterface.objects.filter(
136+ mac_address=mac, node=node).count()
137+ if len(ifnames) == 1:
138+ ifname = ifnames[0]
139+ if existing_iface_count == 0:
140+ # Found a single interface with this MAC, and no interface
141+ # was already on the node. Just go ahead and create it.
142+ interface = _create_default_physical_interface(
143+ node, ifname, mac)
144+ ip_addr_info[ifname]['model'] = interface
145+ elif existing_iface_count == 1:
146+ # Found an interface that was created by a previous
147+ # commissioning, so don't throw away the data. Just change the
148+ # name if necessary.
149 interface = PhysicalInterface.objects.get(
150- mac_address=link_mac)
151- if interface.node is not None and interface.node != node:
152- logger.warning(
153- "Interface with MAC %s moved from node %s to %s. "
154- "(The existing interface will be deleted.)" %
155- (interface.mac_address, interface.node.fqdn,
156- node.fqdn))
157+ mac_address=mac, node=node)
158+ if interface.name != ifname:
159+ interface.name = ifname
160+ interface.save()
161+ ip_addr_info[ifname]['model'] = interface
162+ else:
163+ # Previously there were multiple physical interfaces with the
164+ # same MAC, but now there is only one. Might as well start
165+ # fresh.
166+ for iface in PhysicalInterface.objects.filter(
167+ mac_address=mac, node=node):
168+ iface.delete()
169+ interface = _create_default_physical_interface(
170+ node, ifname, mac)
171+ ip_addr_info[ifname]['model'] = interface
172+ else:
173+ # Multiple interfaces found with the same MAC.
174+ # This means we shouldn't try to preserve renamed interfaces.
175+ existing = set()
176+ for interface in PhysicalInterface.objects.filter(
177+ mac_address=mac, node=node):
178+ ifname = interface.name
179+ if interface.name not in ifnames:
180+ # Interface has been removed.
181 interface.delete()
182+ else:
183+ # Existing interface with the same name as an interface
184+ # we just found during commissioning. Make note of it.
185+ existing.add(interface.name)
186+ ip_addr_info[ifname]['model'] = interface
187+ for ifname in ifnames:
188+ # Go through the list of interfaces with a shared MAC whose
189+ # names don't match any existing interface names and create
190+ # them.
191+ if ifname not in existing:
192 interface = _create_default_physical_interface(
193- node, ifname, link_mac)
194- else:
195- # Interface already exists on this Node, so just update
196- # the name.
197- interface.name = ifname
198- interface.save()
199- except PhysicalInterface.DoesNotExist:
200- interface = _create_default_physical_interface(
201- node, ifname, link_mac)
202-
203+ node, ifname, mac)
204+ ip_addr_info[ifname]['model'] = interface
205+ # Record any observed IP addresses on the interfaces we created.
206+ for link in ip_addr_info.values():
207+ interface = link.get('model')
208+ if interface is not None:
209 current_interfaces.add(interface)
210 ips = link.get('inet', []) + link.get('inet6', [])
211 interface.update_ip_addresses(ips)
212-
213- for iface in PhysicalInterface.objects.filter(node=node):
214- if iface not in current_interfaces:
215- iface.delete()
216+ # Delete any obsolete interfaces on this node.
217+ for interface in PhysicalInterface.objects.filter(node=node):
218+ if interface not in current_interfaces:
219+ interface.delete()
220
221
222 def update_node_network_interface_tags(node, output, exit_status):