Merge lp:~julian-edwards/maas/network-gateway-ui into lp:~maas-committers/maas/trunk

Proposed by Julian Edwards
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 2904
Proposed branch: lp:~julian-edwards/maas/network-gateway-ui
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 170 lines (+25/-4)
8 files modified
src/maasserver/api/networks.py (+2/-1)
src/maasserver/api/tests/test_network.py (+3/-0)
src/maasserver/api/tests/test_networks.py (+3/-1)
src/maasserver/forms.py (+1/-0)
src/maasserver/templates/maasserver/network_detail.html (+4/-0)
src/maasserver/templates/maasserver/network_list.html (+2/-0)
src/maasserver/testing/factory.py (+4/-2)
src/maasserver/views/tests/test_networks.py (+6/-0)
To merge this branch: bzr merge lp:~julian-edwards/maas/network-gateway-ui
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Review via email: mp+233445@code.launchpad.net

Commit message

Trivial changes to add default_gateway to the web UI pages for viewing, editing and adding Networks. Also ensure default_gateway can be edited in the API.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Two nitpicks about this bit you're adding in the factory:

        if default_gateway is None and random.choice((True, False)):
            default_gateway = unicode(IPAddress(network.first + 1))

The factory has a helper for random.choice((True, False)): pick_bool().

Why are you forcing the gateway to be network.first + 1? I'd use self.pick_ip_in_network(network).

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

On Friday 05 September 2014 03:20:51 you wrote:
> Review: Approve
>
> Two nitpicks about this bit you're adding in the factory:
>
> if default_gateway is None and random.choice((True, False)):
> default_gateway = unicode(IPAddress(network.first + 1))
>
> The factory has a helper for random.choice((True, False)): pick_bool().
>
> Why are you forcing the gateway to be network.first + 1? I'd use
> self.pick_ip_in_network(network).

Both good points, I'll change things, thank you!

Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (47.1 KiB)

The attempt to merge lp:~julian-edwards/maas/network-gateway-ui into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:2 http://security.ubuntu.com trusty-security Release [59.7 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [59.7 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:5 http://security.ubuntu.com trusty-security/main Sources [43.5 kB]
Get:6 http://security.ubuntu.com trusty-security/universe Sources [10.8 kB]
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [136 kB]
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [47.1 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [116 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [82.6 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [311 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [199 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 1,068 kB in 0s (1,619 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsgi python...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/maasserver/api/networks.py'
--- src/maasserver/api/networks.py 2014-08-16 12:04:25 +0000
+++ src/maasserver/api/networks.py 2014-09-05 05:05:52 +0000
@@ -45,7 +45,8 @@
45 api_doc_section_name = "Network"45 api_doc_section_name = "Network"
4646
47 model = Network47 model = Network
48 fields = ('name', 'ip', 'netmask', 'vlan_tag', 'description')48 fields = (
49 'name', 'ip', 'netmask', 'vlan_tag', 'description', 'default_gateway')
4950
50 # Creation happens on the NetworksHandler.51 # Creation happens on the NetworksHandler.
51 create = None52 create = None
5253
=== modified file 'src/maasserver/api/tests/test_network.py'
--- src/maasserver/api/tests/test_network.py 2014-08-16 05:43:33 +0000
+++ src/maasserver/api/tests/test_network.py 2014-09-05 05:05:52 +0000
@@ -55,6 +55,7 @@
55 network.netmask,55 network.netmask,
56 network.vlan_tag,56 network.vlan_tag,
57 network.description,57 network.description,
58 network.default_gateway,
58 ),59 ),
59 (60 (
60 parsed_result['name'],61 parsed_result['name'],
@@ -62,6 +63,7 @@
62 parsed_result['netmask'],63 parsed_result['netmask'],
63 parsed_result['vlan_tag'],64 parsed_result['vlan_tag'],
64 parsed_result['description'],65 parsed_result['description'],
66 parsed_result['default_gateway'],
65 ))67 ))
6668
67 def test_GET_returns_404_for_unknown_network(self):69 def test_GET_returns_404_for_unknown_network(self):
@@ -79,6 +81,7 @@
79 'netmask': '%s' % new_net.netmask,81 'netmask': '%s' % new_net.netmask,
80 'vlan_tag': factory.make_vlan_tag(),82 'vlan_tag': factory.make_vlan_tag(),
81 'description': "Changed description",83 'description': "Changed description",
84 'default_gateway': factory.getRandomIPAddress(),
82 }85 }
8386
84 response = self.client_put(self.get_url(network.name), new_values)87 response = self.client_put(self.get_url(network.name), new_values)
8588
=== modified file 'src/maasserver/api/tests/test_networks.py'
--- src/maasserver/api/tests/test_networks.py 2014-08-16 05:43:33 +0000
+++ src/maasserver/api/tests/test_networks.py 2014-09-05 05:05:52 +0000
@@ -61,7 +61,9 @@
61 parsed_result = json.loads(response.content)61 parsed_result = json.loads(response.content)
62 self.assertEqual(1, len(parsed_result))62 self.assertEqual(1, len(parsed_result))
63 [returned_network] = parsed_result63 [returned_network] = parsed_result
64 fields = {'name', 'ip', 'netmask', 'vlan_tag', 'description'}64 fields = {
65 'name', 'ip', 'netmask', 'vlan_tag', 'description',
66 'default_gateway'}
65 self.assertEqual(67 self.assertEqual(
66 fields.union({'resource_uri'}),68 fields.union({'resource_uri'}),
67 set(returned_network.keys()))69 set(returned_network.keys()))
6870
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2014-09-04 08:14:17 +0000
+++ src/maasserver/forms.py 2014-09-05 05:05:52 +0000
@@ -1914,6 +1914,7 @@
1914 'ip',1914 'ip',
1915 'netmask',1915 'netmask',
1916 'vlan_tag',1916 'vlan_tag',
1917 'default_gateway',
1917 )1918 )
19181919
1919 mac_addresses = NodeMACAddressChoiceField(1920 mac_addresses = NodeMACAddressChoiceField(
19201921
=== modified file 'src/maasserver/templates/maasserver/network_detail.html'
--- src/maasserver/templates/maasserver/network_detail.html 2014-02-24 07:48:20 +0000
+++ src/maasserver/templates/maasserver/network_detail.html 2014-09-05 05:05:52 +0000
@@ -40,6 +40,10 @@
40 <span>{{ network.vlan_tag|default_if_none:"" }}</span>40 <span>{{ network.vlan_tag|default_if_none:"" }}</span>
41 </li>41 </li>
42 <li class="block size2">42 <li class="block size2">
43 <h4>Default Gateway</h4>
44 <span>{{ network.default_gateway|default_if_none:"" }}</span>
45 </li>
46 <li class="block size2">
43 <h4>Attached nodes</h4>47 <h4>Attached nodes</h4>
44 <span id="nodecount">48 <span id="nodecount">
45 <a title="View nodes attached to network {{ network.name }}"49 <a title="View nodes attached to network {{ network.name }}"
4650
=== modified file 'src/maasserver/templates/maasserver/network_list.html'
--- src/maasserver/templates/maasserver/network_list.html 2014-03-25 11:03:39 +0000
+++ src/maasserver/templates/maasserver/network_list.html 2014-09-05 05:05:52 +0000
@@ -23,6 +23,7 @@
23 <th>Description</th>23 <th>Description</th>
24 <th>Network</th>24 <th>Network</th>
25 <th>VLAN tag</th>25 <th>VLAN tag</th>
26 <th>Default gateway</th>
26 <th>Attached nodes</th>27 <th>Attached nodes</th>
27 {% if user.is_superuser %}28 {% if user.is_superuser %}
28 {% comment %} Action buttons {% endcomment %}29 {% comment %} Action buttons {% endcomment %}
@@ -41,6 +42,7 @@
41 <td>{{ network_item.description|truncatechars:40 }}</td>42 <td>{{ network_item.description|truncatechars:40 }}</td>
42 <td>{{ network_item.get_network }}</td>43 <td>{{ network_item.get_network }}</td>
43 <td>{{ network_item.vlan_tag|default_if_none:"" }}</td>44 <td>{{ network_item.vlan_tag|default_if_none:"" }}</td>
45 <td>{{ network_item.default_gateway|default_if_none:"" }}</td>
44 <td>46 <td>
45 <a title="View nodes attached to network {{ network_item.name }}"47 <a title="View nodes attached to network {{ network_item.name }}"
46 href="{% url 'node-list' %}?query=networks%3D{{ network_item.name }}">48 href="{% url 'node-list' %}?query=networks%3D{{ network_item.name }}">
4749
=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py 2014-08-29 12:49:20 +0000
+++ src/maasserver/testing/factory.py 2014-09-05 05:05:52 +0000
@@ -834,7 +834,7 @@
834834
835 def make_network(self, name=None, network=None, vlan_tag=NO_VALUE,835 def make_network(self, name=None, network=None, vlan_tag=NO_VALUE,
836 description=None, sortable_name=False,836 description=None, sortable_name=False,
837 disjoint_from=None):837 disjoint_from=None, default_gateway=None):
838 """Create a `Network`.838 """Create a `Network`.
839839
840 :param network: An `IPNetwork`. If given, the `ip` and `netmask`840 :param network: An `IPNetwork`. If given, the `ip` and `netmask`
@@ -867,6 +867,8 @@
867 ]867 ]
868 if network is None:868 if network is None:
869 network = self.getRandomNetwork(disjoint_from=disjoint_from)869 network = self.getRandomNetwork(disjoint_from=disjoint_from)
870 if default_gateway is None and self.pick_bool():
871 default_gateway = self.pick_ip_in_network(network)
870 ip = unicode(network.ip)872 ip = unicode(network.ip)
871 netmask = unicode(network.netmask)873 netmask = unicode(network.netmask)
872 if description is None:874 if description is None:
@@ -875,7 +877,7 @@
875 vlan_tag = self.make_vlan_tag()877 vlan_tag = self.make_vlan_tag()
876 network = Network(878 network = Network(
877 name=name, ip=ip, netmask=netmask, vlan_tag=vlan_tag,879 name=name, ip=ip, netmask=netmask, vlan_tag=vlan_tag,
878 description=description)880 description=description, default_gateway=default_gateway)
879 network.save()881 network.save()
880 return network882 return network
881883
882884
=== modified file 'src/maasserver/views/tests/test_networks.py'
--- src/maasserver/views/tests/test_networks.py 2014-08-13 21:49:35 +0000
+++ src/maasserver/views/tests/test_networks.py 2014-09-05 05:05:52 +0000
@@ -68,6 +68,9 @@
68 network.description[:20],68 network.description[:20],
69 '%s' % network.get_network(),69 '%s' % network.get_network(),
70 '%s' % network.vlan_tag if network.vlan_tag else '',70 '%s' % network.vlan_tag if network.vlan_tag else '',
71 '%s' % (
72 network.default_gateway if network.default_gateway
73 else ''),
71 '%d' % network.get_connected_nodes().count(),74 '%d' % network.get_connected_nodes().count(),
72 ]75 ]
73 for network in networks]76 for network in networks]
@@ -196,6 +199,7 @@
196 'ip': "%s" % network.cidr.ip,199 'ip': "%s" % network.cidr.ip,
197 'netmask': "%s" % network.netmask,200 'netmask': "%s" % network.netmask,
198 'vlan_tag': factory.make_vlan_tag(),201 'vlan_tag': factory.make_vlan_tag(),
202 'default_gateway': factory.getRandomIPAddress(),
199 }203 }
200 response = self.client.post(reverse('network-add'), definition)204 response = self.client.post(reverse('network-add'), definition)
201 self.assertEqual(httplib.FOUND, response.status_code)205 self.assertEqual(httplib.FOUND, response.status_code)
@@ -310,11 +314,13 @@
310 new_macs = [314 new_macs = [
311 factory.make_mac_address()315 factory.make_mac_address()
312 for _ in range(3)]316 for _ in range(3)]
317 new_gateway = factory.getRandomIPAddress()
313 response = self.client.post(318 response = self.client.post(
314 reverse('network-edit', args=[network.name]),319 reverse('network-edit', args=[network.name]),
315 data={320 data={
316 'name': new_name,321 'name': new_name,
317 'description': new_description,322 'description': new_description,
323 'default_gateway': new_gateway,
318 'mac_addresses': new_macs,324 'mac_addresses': new_macs,
319 })325 })
320 self.assertEqual(326 self.assertEqual(