Merge lp://staging/~mpontillo/maas/iprange-notifications--bug-1393936 into lp://staging/~maas-committers/maas/trunk

Proposed by Mike Pontillo
Status: Merged
Approved by: Mike Pontillo
Approved revision: no longer in the source branch.
Merged at revision: 5986
Proposed branch: lp://staging/~mpontillo/maas/iprange-notifications--bug-1393936
Merge into: lp://staging/~maas-committers/maas/trunk
Diff against target: 570 lines (+394/-16)
7 files modified
src/maasserver/forms/settings.py (+22/-13)
src/maasserver/models/config.py (+3/-1)
src/maasserver/models/signals/__init__.py (+2/-0)
src/maasserver/models/signals/iprange.py (+42/-0)
src/maasserver/models/signals/staticipaddress.py (+21/-0)
src/maasserver/models/subnet.py (+48/-0)
src/maasserver/models/tests/test_subnet.py (+256/-2)
To merge this branch: bzr merge lp://staging/~mpontillo/maas/iprange-notifications--bug-1393936
Reviewer Review Type Date Requested Status
Blake Rouse (community) Approve
MAAS Maintainers Pending
Review via email: mp+322569@code.staging.launchpad.net

This proposal supersedes a proposal from 2017-04-07.

Commit message

Add notification for imminent IP address exhaustion on a subnet.

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

Looks good overall. The way you've done the notifications appears sound. Updating the message on a notification instead of creating a new notification will have the effect of updating the message in the UI, but will not cause the notification to pop up again for people who have already dismissed it.

Revision history for this message
Mike Pontillo (mpontillo) wrote : Posted in a previous version of this proposal

Thanks for looking. Some replies below.

Revision history for this message
Mike Pontillo (mpontillo) wrote : Posted in a previous version of this proposal

Didn't mean to mark this "Needs review" yet. Still need to finalize a few things and do the tests.

Revision history for this message
Mike Pontillo (mpontillo) wrote : Posted in a previous version of this proposal

This is ready for a real review now.

Revision history for this message
Blake Rouse (blake-rouse) wrote : Posted in a previous version of this proposal

Looks good for IPv4. The only thing I see missing is for IPv6? I would assume we would not show notifications for IPv6 because the number of addresses is just huge. I did not see in the code where you check if its IPv6 and not do any work. The unit tests you wrote only cover IPv4.

review: Needs Information
Revision history for this message
Mike Pontillo (mpontillo) wrote : Posted in a previous version of this proposal

Well, yes and no: we should still show notifications for small IPv6 subnets sufficient to reach the threshold. I can add some tests. It should be almost the same as IPv4, but to prevent regressions it would be good to cover IPv6 as well.

Revision history for this message
Mike Pontillo (mpontillo) wrote : Posted in a previous version of this proposal

This branch is blocked on:

 - Adding unit tests for IPv6
 - Bug #1680979

Revision history for this message
Mike Pontillo (mpontillo) wrote :

Added IPv6 test cases (this works with huge IPv6 subnets like 2001::16!); this is ready for another review.

See also:

https://code.launchpad.net/~mpontillo/maas/fix-notification-editing--bug-1680979/+merge/322567

(needed for these notifications to be dismissed properly)

Revision history for this message
Blake Rouse (blake-rouse) wrote :

Looks good. Thanks for adding more tests for IPv6.

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

The attempt to merge lp:~mpontillo/maas/iprange-notifications--bug-1393936 into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [102 kB]
Get:3 http://security.ubuntu.com/ubuntu xenial-security InRelease [102 kB]
Get:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease [102 kB]
Fetched 306 kB in 0s (591 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind avahi-utils bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common isc-dhcp-server libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libnss-wrapper libpq-dev make nodejs-legacy npm postgresql psmisc pxelinux python3-all python3-apt python3-attr python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~rc+dfsg-1ubuntu2).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
psmisc is already the newest version (22.21-2.1build1).
pxelinux is already the newest version (3:6.03+dfsg-11ubuntu1).
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr is already the newest version (0.7.18-1...

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

The attempt to merge lp:~mpontillo/maas/iprange-notifications--bug-1393936 into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com/ubuntu xenial-security InRelease [102 kB]
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [102 kB]
Get:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease [102 kB]
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/main Sources [241 kB]
Get:6 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/main amd64 Packages [512 kB]
Get:7 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/universe amd64 Packages [457 kB]
Fetched 1,517 kB in 0s (2,579 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind avahi-utils bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common isc-dhcp-server libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libnss-wrapper libpq-dev make nodejs-legacy npm postgresql psmisc pxelinux python3-all python3-apt python3-attr python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~rc+dfsg-1ubuntu2).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the new...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.