Merge lp:~jtv/maas/api-ngi-name into lp:~maas-committers/maas/trunk

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 2536
Proposed branch: lp:~jtv/maas/api-ngi-name
Merge into: lp:~maas-committers/maas/trunk
Prerequisite: lp:~jtv/maas/ngi-name
Diff against target: 251 lines (+34/-32)
3 files modified
src/maasserver/api.py (+18/-14)
src/maasserver/tests/test_api.py (+15/-17)
src/maasserver/urls_api.py (+1/-1)
To merge this branch: bzr merge lp:~jtv/maas/api-ngi-name
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+225650@code.launchpad.net

Commit message

Update the API to use a cluster interface's name, rather than its network interface, as its identifying name within a cluster.

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

Is the `interface` field something like "eth0:1", and name is user-defined? I don't understand what the purpose of this change is. Looks good though!

review: Approve
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Thanks. The name is user-defined, but for backward compatibility, defaults to the network interface name if unambiguous. So yes, it might be eth0:1 if eth0 has aliases.

Where names do clash, they currently get random suffixes. In a later branch we may want to make up more helpful suffixes such as “-ipv6-1.”

Revision history for this message
Gavin Panella (allenap) wrote :

Do you know if we do anything with alias names like "eth0:1"? They're not very stable as configuration changes, afaik, so I kind of hope we're not.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

To us those are just interface names like any other. Julian has mentioned plans to control network interface configuration in the future.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

On 08/07/14 02:10, Jeroen T. Vermeulen wrote:
> To us those are just interface names like any other. Julian has mentioned plans to control network interface configuration in the future.
>

Right, the new DB design should allow is to drive udev rules on the node
such that interface names are consistent. Needs a bit of work to marry
up the MAC etc and stuff it in a preseed but will make things much more
reliable.

Revision history for this message
Gavin Panella (allenap) wrote :

On 8 July 2014 01:12, Julian Edwards <email address hidden> wrote:
> On 08/07/14 02:10, Jeroen T. Vermeulen wrote:
>> To us those are just interface names like any other. Julian has mentioned plans to control network interface configuration in the future.
>>
>
> Right, the new DB design should allow is to drive udev rules on the node
> such that interface names are consistent. Needs a bit of work to marry
> up the MAC etc and stuff it in a preseed but will make things much more
> reliable.

Sounds good. A fly in the ointment might be Windows; I have no idea
how it identifies interfaces. CentOS ought to be straightforward, but
we should try it to be sure.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Ah, so apparently the thing about configuring network interfaces was only for the nodes. The network interfaces that this branch discovers are on the cluster controller.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api.py'
2--- src/maasserver/api.py 2014-07-10 02:57:35 +0000
3+++ src/maasserver/api.py 2014-07-10 09:51:48 +0000
4@@ -1975,7 +1975,7 @@
5
6
7 DISPLAYED_NODEGROUPINTERFACE_FIELDS = (
8- 'ip', 'management', 'interface', 'subnet_mask',
9+ 'name', 'ip', 'management', 'interface', 'subnet_mask',
10 'broadcast_ip', 'ip_range_low', 'ip_range_high',
11 'static_ip_range_low', 'static_ip_range_high',
12 )
13@@ -2004,9 +2004,12 @@
14 def new(self, request, uuid):
15 """Create a new NodeGroupInterface for this NodeGroup.
16
17+ :param name: Name for the interface. Must be unique within this
18+ cluster. Only letters, digits, and dashes are allowed.
19 :param ip: Static IP of the interface.
20 :type ip: unicode (IP Address)
21- :param interface: Name of the interface.
22+ :param interface: Name of the network interface that connects the
23+ cluster controller to this network.
24 :type interface: unicode
25 :param management: The service(s) MAAS should manage on this interface.
26 :type management: Vocabulary `NODEGROUPINTERFACE_MANAGEMENT`
27@@ -2057,7 +2060,7 @@
28 create = delete = None
29 fields = DISPLAYED_NODEGROUPINTERFACE_FIELDS
30
31- def get_interface(self, request, uuid, interface):
32+ def get_interface(self, request, uuid, name):
33 """Return the :class:`NodeGroupInterface` indicated by the request.
34
35 Will check for not-found errors, as well as the user's permission to
36@@ -2067,19 +2070,20 @@
37 if not request.user.is_superuser:
38 check_nodegroup_access(request, nodegroup)
39 nodegroupinterface = get_object_or_404(
40- NodeGroupInterface, nodegroup=nodegroup, interface=interface)
41+ NodeGroupInterface, nodegroup=nodegroup, name=name)
42 return nodegroupinterface
43
44- def read(self, request, uuid, interface):
45+ def read(self, request, uuid, name):
46 """List of NodeGroupInterfaces of a NodeGroup."""
47- return self.get_interface(request, uuid, interface)
48+ return self.get_interface(request, uuid, name)
49
50- def update(self, request, uuid, interface):
51+ def update(self, request, uuid, name):
52 """Update a specific NodeGroupInterface.
53
54+ :param name: Identifying name for the cluster interface.
55 :param ip: Static IP of the interface.
56 :type ip: unicode (IP Address)
57- :param interface: Name of the interface.
58+ :param interface: Network interface.
59 :type interface: unicode
60 :param management: The service(s) MAAS should manage on this interface.
61 :type management: Vocabulary `NODEGROUPINTERFACE_MANAGEMENT`
62@@ -2100,7 +2104,7 @@
63 clients.
64 :type static_ip_range_high: unicode (IP Address)
65 """
66- nodegroupinterface = self.get_interface(request, uuid, interface)
67+ nodegroupinterface = self.get_interface(request, uuid, name)
68 form = NodeGroupInterfaceForm(
69 request.data, instance=nodegroupinterface)
70 if form.is_valid():
71@@ -2108,14 +2112,14 @@
72 else:
73 raise ValidationError(form.errors)
74
75- def delete(self, request, uuid, interface):
76+ def delete(self, request, uuid, name):
77 """Delete a specific NodeGroupInterface."""
78- nodegroupinterface = self.get_interface(request, uuid, interface)
79+ nodegroupinterface = self.get_interface(request, uuid, name)
80 nodegroupinterface.delete()
81 return rc.DELETED
82
83 @operation(idempotent=False)
84- def report_foreign_dhcp(self, request, uuid, interface):
85+ def report_foreign_dhcp(self, request, uuid, name):
86 """Report the result of a probe for foreign DHCP servers.
87
88 Cluster controllers probe for DHCP servers not managed by MAAS, and
89@@ -2125,7 +2129,7 @@
90 during the last probe. Leave blank if none were found.
91 """
92 foreign_dhcp_ip = get_mandatory_param(request.data, 'foreign_dhcp_ip')
93- nodegroupinterface = self.get_interface(request, uuid, interface)
94+ nodegroupinterface = self.get_interface(request, uuid, name)
95
96 form = NodeGroupInterfaceForeignDHCPForm(
97 {'foreign_dhcp_ip': foreign_dhcp_ip}, instance=nodegroupinterface)
98@@ -2143,7 +2147,7 @@
99 if interface is None:
100 interface_name = 'interface'
101 else:
102- interface_name = interface.interface
103+ interface_name = interface.name
104 return ('nodegroupinterface_handler', [uuid, interface_name])
105
106
107
108=== modified file 'src/maasserver/tests/test_api.py'
109--- src/maasserver/tests/test_api.py 2014-07-03 09:43:09 +0000
110+++ src/maasserver/tests/test_api.py 2014-07-10 09:51:48 +0000
111@@ -502,8 +502,7 @@
112 self.assertEqual(httplib.OK, response.status_code)
113 self.assertEqual(
114 [
115- dict_subset(
116- interface, DISPLAYED_NODEGROUPINTERFACE_FIELDS)
117+ dict_subset(interface, DISPLAYED_NODEGROUPINTERFACE_FIELDS)
118 for interface in nodegroup.nodegroupinterface_set.all()
119 ],
120 json.loads(response.content))
121@@ -543,7 +542,7 @@
122 for key, value in interface_settings.items()
123 }
124 new_interface = NodeGroupInterface.objects.get(
125- nodegroup=nodegroup, interface=interface_settings['interface'])
126+ nodegroup=nodegroup, name=interface_settings['name'])
127 self.assertThat(
128 new_interface,
129 MatchesStructure.byEquality(**expected_result))
130@@ -598,7 +597,7 @@
131 response = self.client.get(
132 reverse(
133 'nodegroupinterface_handler',
134- args=[nodegroup.uuid, interface.interface]))
135+ args=[nodegroup.uuid, interface.name]))
136 self.assertEqual(
137 httplib.FORBIDDEN, response.status_code, response.content)
138
139@@ -610,7 +609,7 @@
140 response = client.get(
141 reverse(
142 'nodegroupinterface_handler',
143- args=[nodegroup.uuid, interface.interface]))
144+ args=[nodegroup.uuid, interface.name]))
145 self.assertEqual(httplib.OK, response.status_code)
146
147 def test_update_does_not_work_for_normal_user(self):
148@@ -621,7 +620,7 @@
149 response = self.client_put(
150 reverse(
151 'nodegroupinterface_handler',
152- args=[nodegroup.uuid, interface.interface]),
153+ args=[nodegroup.uuid, interface.name]),
154 {'ip_range_high': factory.getRandomIPAddress()})
155 self.assertEqual(
156 httplib.FORBIDDEN, response.status_code, response.content)
157@@ -635,7 +634,7 @@
158 response = self.client_put(
159 reverse(
160 'nodegroupinterface_handler',
161- args=[nodegroup.uuid, interface.interface]),
162+ args=[nodegroup.uuid, interface.name]),
163 {'ip_range_high': new_ip_range_high})
164 self.assertEqual(httplib.OK, response.status_code)
165
166@@ -647,7 +646,7 @@
167 response = self.client.delete(
168 reverse(
169 'nodegroupinterface_handler',
170- args=[nodegroup.uuid, interface.interface]))
171+ args=[nodegroup.uuid, interface.name]))
172 self.assertEqual(
173 httplib.FORBIDDEN, response.status_code, response.content)
174
175@@ -659,7 +658,7 @@
176 response = self.client.delete(
177 reverse(
178 'nodegroupinterface_handler',
179- args=[nodegroup.uuid, interface.interface]))
180+ args=[nodegroup.uuid, interface.name]))
181 self.assertEqual(httplib.NO_CONTENT, response.status_code)
182
183
184@@ -672,11 +671,10 @@
185 response = self.client.get(
186 reverse(
187 'nodegroupinterface_handler',
188- args=[nodegroup.uuid, interface.interface]))
189+ args=[nodegroup.uuid, interface.name]))
190 self.assertEqual(httplib.OK, response.status_code)
191 self.assertEqual(
192- dict_subset(
193- interface, DISPLAYED_NODEGROUPINTERFACE_FIELDS),
194+ dict_subset(interface, DISPLAYED_NODEGROUPINTERFACE_FIELDS),
195 json.loads(response.content))
196
197 def test_update_interface(self):
198@@ -688,7 +686,7 @@
199 response = self.client_put(
200 reverse(
201 'nodegroupinterface_handler',
202- args=[nodegroup.uuid, interface.interface]),
203+ args=[nodegroup.uuid, interface.name]),
204 {'ip_range_high': new_ip_range_high})
205 self.assertEqual(
206 (httplib.OK, new_ip_range_high),
207@@ -701,11 +699,11 @@
208 response = self.client.delete(
209 reverse(
210 'nodegroupinterface_handler',
211- args=[nodegroup.uuid, interface.interface]))
212+ args=[nodegroup.uuid, interface.name]))
213 self.assertEqual(httplib.NO_CONTENT, response.status_code)
214 self.assertFalse(
215 NodeGroupInterface.objects.filter(
216- interface=interface.interface, nodegroup=nodegroup).exists())
217+ name=interface.name, nodegroup=nodegroup).exists())
218
219 def test_report_foreign_dhcp_sets_value(self):
220 self.become_admin()
221@@ -715,7 +713,7 @@
222 response = self.client.post(
223 reverse(
224 'nodegroupinterface_handler',
225- args=[nodegroup.uuid, interface.interface]),
226+ args=[nodegroup.uuid, interface.name]),
227 {
228 'op': 'report_foreign_dhcp',
229 'foreign_dhcp_ip': ip,
230@@ -732,7 +730,7 @@
231 response = self.client.post(
232 reverse(
233 'nodegroupinterface_handler',
234- args=[nodegroup.uuid, interface.interface]),
235+ args=[nodegroup.uuid, interface.name]),
236 {
237 'op': 'report_foreign_dhcp',
238 'foreign_dhcp_ip': '',
239
240=== modified file 'src/maasserver/urls_api.py'
241--- src/maasserver/urls_api.py 2014-07-02 14:58:18 +0000
242+++ src/maasserver/urls_api.py 2014-07-10 09:51:48 +0000
243@@ -159,7 +159,7 @@
244 url(r'^nodegroups/$', nodegroups_handler, name='nodegroups_handler'),
245 url(r'^nodegroups/(?P<uuid>[^/]+)/interfaces/$',
246 nodegroupinterfaces_handler, name='nodegroupinterfaces_handler'),
247- url(r'^nodegroups/(?P<uuid>[^/]+)/interfaces/(?P<interface>[^/]+)/$',
248+ url(r'^nodegroups/(?P<uuid>[^/]+)/interfaces/(?P<name>[^/]+)/$',
249 nodegroupinterface_handler, name='nodegroupinterface_handler'),
250 url(r'^nodegroups/(?P<uuid>[^/]+)/boot-images/$',
251 boot_images_handler, name='boot_images_handler'),