Merge lp:~jtv/maas/api-ngi-name into lp:~maas-committers/maas/trunk
- api-ngi-name
- Merge into trunk
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 |
Related bugs: |
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.
Description of the change
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.”
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.
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.
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.
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.
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
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'), |
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!