Merge lp:~rvb/maas/check_valid_inter_network into lp:~maas-committers/maas/trunk

Proposed by Raphaël Badin
Status: Merged
Approved by: Raphaël Badin
Approved revision: no longer in the source branch.
Merged at revision: 2454
Proposed branch: lp:~rvb/maas/check_valid_inter_network
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 51 lines (+25/-1)
2 files modified
src/maasserver/api.py (+7/-1)
src/maasserver/tests/test_api_nodegroup.py (+18/-0)
To merge this branch: bzr merge lp:~rvb/maas/check_valid_inter_network
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve
Review via email: mp+224082@code.launchpad.net

Commit message

Skip unconfigured interfaces in update_mac_cluster_interfaces.

To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 review: approve

On 23/06/14 19:47, Raphaël Badin wrote:
> for interface in cluster.nodegroupinterface_set.all(): + #
> Skip unconfigured interfaces. + if interface.network is
> None: + break

Looks good. Is it worth putting this in a query to do it all in one
go? The conditional break is kinda ugly.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlOn/8cACgkQWhGlTF8G/Hf0wgCfTNTPAmcesJiCAQDoS3JX6tZ8
UYUAnA3y7jw4WhjWBxbXjHM/70e7FBgc
=RqOV
-----END PGP SIGNATURE-----

review: Approve
Revision history for this message
Raphaël Badin (rvb) wrote :

> Looks good. Is it worth putting this in a query to do it all in one
> go? The conditional break is kinda ugly.

Good point, done.

Thanks for the review.

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

The attempt to merge lp:~rvb/maas/check_valid_inter_network 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]
Get:2 http://security.ubuntu.com trusty-security Release [58.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
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 [58.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [25.3 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [4,727 B]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [87.2 kB]
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:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [28.0 kB]
Get:9 http://security.ubuntu.com trusty-security/main Translation-en [39.0 kB]
Get:10 http://security.ubuntu.com trusty-security/universe Translation-en [15.1 kB]
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [67.1 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [51.1 kB]
Get:13 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [165 kB]
Get:14 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [125 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-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Fetched 727 kB in 0s (1,661 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 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 postgresql 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-formencode python-hi...

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-06-20 13:38:39 +0000
3+++ src/maasserver/api.py 2014-06-23 14:36:15 +0000
4@@ -1522,7 +1522,13 @@
5 def update_mac_cluster_interfaces(leases, cluster):
6 """Calculate and store which interface a MAC is attached to."""
7 interface_ranges = {}
8- for interface in cluster.nodegroupinterface_set.all():
9+ # Only consider configured interfaces.
10+ interfaces = (
11+ cluster.nodegroupinterface_set
12+ .exclude(ip_range_low__isnull=True)
13+ .exclude(ip_range_high__isnull=True)
14+ )
15+ for interface in interfaces:
16 ip_range = netaddr.IPRange(
17 interface.ip_range_low, interface.ip_range_high)
18 interface_ranges[interface] = ip_range
19
20=== modified file 'src/maasserver/tests/test_api_nodegroup.py'
21--- src/maasserver/tests/test_api_nodegroup.py 2014-06-18 20:43:23 +0000
22+++ src/maasserver/tests/test_api_nodegroup.py 2014-06-23 14:36:15 +0000
23@@ -30,6 +30,7 @@
24 from maasserver.enum import (
25 NODEGROUP_STATUS,
26 NODEGROUP_STATUS_CHOICES,
27+ NODEGROUPINTERFACE_MANAGEMENT,
28 )
29 from maasserver.models import (
30 Config,
31@@ -850,3 +851,20 @@
32 update_mac_cluster_interfaces(leases, cluster)
33 self.assertFalse(
34 MACAddress.objects.filter(mac_address=mac_address).exists())
35+
36+ def test_ignores_unconfigured_interfaces(self):
37+ cluster = factory.make_node_group()
38+ factory.make_node_group_interface(
39+ nodegroup=cluster, subnet_mask='', broadcast_ip='',
40+ static_ip_range_low='', static_ip_range_high='',
41+ ip_range_low='', ip_range_high='', router_ip='',
42+ ip=factory.getRandomIPAddress(),
43+ management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
44+ mac_address = factory.getRandomMACAddress()
45+ leases = {
46+ factory.getRandomIPAddress(): mac_address
47+ }
48+ self.assertIsNone(update_mac_cluster_interfaces(leases, cluster))
49+ # The real test is that update_mac_cluster_interfaces() doesn't
50+ # stacktrace because of the unconfigured interface (see bug
51+ # 1332596).