Merge lp:~mpontillo/maas/signal-handlers-add-logging--bug-1685963 into lp:maas/trunk

[Mike Pontillo]

Adds messages to regiond.log about interface changes due to Django signal handlers.

Note: these logs should already be covered by existing tests. This is debug logging, (so that we can get additional information to solve related bugs) so I don't feel it's worth the effort to test the messages themselves.

[Blake Rouse (blake-rouse)]

Looks good. Nice messages.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~mpontillo/maas/signal-handlers-add-logging--bug-1685963 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://security.ubuntu.com/ubuntu xenial-security InRelease [102 kB]
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]
Fetched 306 kB in 0s (634 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....

[MAAS Lander (maas-lander)]

The attempt to merge lp:~mpontillo/maas/signal-handlers-add-logging--bug-1685963 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://security.ubuntu.com/ubuntu xenial-security InRelease [102 kB]
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]
Fetched 306 kB in 0s (582 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....

[Mike Pontillo (mpontillo)]

Random failure. http://paste.ubuntu.com/24455688/

=== modified file 'src/maasserver/models/signals/interfaces.py'
--- src/maasserver/models/signals/interfaces.py	2016-09-08 19:32:10 +0000
+++ src/maasserver/models/signals/interfaces.py	2017-04-25 18:11:41 +0000
@@ -35,6 +35,7 @@
 )
 from maasserver.models.staticipaddress import StaticIPAddress
 from maasserver.utils.signals import SignalsManager
+from provisioningserver.logger import LegacyLogger
 
 
 INTERFACE_CLASSES = [
@@ -48,6 +49,8 @@
 
 signals = SignalsManager()
 
+log = LegacyLogger()
+
 
 def interface_enabled_or_disabled(instance, old_values, **kwargs):
     """When an interface is enabled be sure at minimum a LINK_UP is created.
@@ -56,11 +59,15 @@
     if instance.type != INTERFACE_TYPE.PHYSICAL:
         return
     if instance.is_enabled():
+        log.msg("%s: Physical interface enabled; ensuring link-up." % (
+            instance.get_log_string()))
         # Make sure it has a LINK_UP link, and for its children.
         instance.ensure_link_up()
         for rel in instance.children_relationships.all():
             rel.child.ensure_link_up()
     else:
+        log.msg("%s: Physical interface disabled; removing links." % (
+            instance.get_log_string()))
         # Was disabled. Remove the links.
         for ip_address in instance.ip_addresses.exclude(
                 alloc_type=IPADDRESS_TYPE.DISCOVERED):
@@ -80,6 +87,13 @@
         klass, ['enabled'], delete=False)
 
 
+def _update_mtu(interface, mtu, instance):
+    log.msg("%s: MTU updated to %d (for consistency with %s)." % (
+        interface.get_log_string(), mtu, instance.get_log_string()))
+    interface.params['mtu'] = mtu
+    interface.save()
+
+
 def interface_mtu_params_update(instance, old_values, **kwargs):
     """When an interfaces MTU is changed we need to do two things.
 
@@ -104,7 +118,7 @@
     # Update the children before the parents. Because calling update on the
     # parent will call save on all the children again.
 
-    # If the children have a larger MTU than the parent, we update the childs
+    # If the children have a larger MTU than the parent, we update the child's
     # MTU to be the same size as the parents. If the child doesn't have MTU
     # set then it is ignored.
     for rel in instance.children_relationships.all():
@@ -112,11 +126,10 @@
         if child.params and 'mtu' in child.params:
             child_mtu = child.params['mtu']
             if child_mtu > new_mtu:
-                child.params['mtu'] = new_mtu
-                child.save()
+                _update_mtu(child, new_mtu, instance)
 
-    # Update the parents MTU to either be the same size of the childs MTU or
-    # larger than the childs MTU.
+    # Update the parents MTU to either be the same size of the child's MTU or
+    # larger than the child's MTU.
     for parent in instance.parents.all():
         if parent.params:
             parent_mtu = parent.params.get('mtu', None)
@@ -124,18 +137,16 @@
                 if parent_mtu < new_mtu:
                     # Parent MTU is to small, make it bigger for the
                     # child interface.
-                    parent.params['mtu'] = new_mtu
-                    parent.save()
+                    _update_mtu(parent, new_mtu, instance)
             else:
-                # Parent doesn't have MTU set. Set it to the childs
+                # Parent doesn't have MTU set. Set it to the child's
                 # MTU value.
-                parent.params['mtu'] = new_mtu
-                parent.save()
+                _update_mtu(parent, new_mtu, instance)
         else:
             # Parent has not parameters at all. Create the parameters
             # object and set the MTU to the size of the child.
-            parent.params = {'mtu': new_mtu}
-            parent.save()
+            parent.params = {}
+            _update_mtu(parent, new_mtu, instance)
 
 
 for klass in INTERFACE_CLASSES:
@@ -167,6 +178,9 @@
                 try:
                     parent.vlan = instance.vlan
                     parent.save()
+                    log.msg("%s: VLAN updated to match %s (vlan=%s)." % (
+                        parent.get_log_string(), instance.get_log_string(),
+                        parent.vlan_id))
                 finally:
                     visiting.discard(parent.id)
 
@@ -209,7 +223,11 @@
                 if ip_address.subnet is not None:
                     ip_address.subnet.vlan = new_vlan
                     ip_address.subnet.save()
-
+                    log.msg(
+                        "%s: IP address [%s] subnet %s: "
+                        "VLAN updated (vlan=%s)." % (
+                            instance.get_log_string(), ip_address.ip,
+                            ip_address.subnet.cidr, ip_address.subnet.vlan_id))
             # If any children are VLAN interfaces then we need to move those
             # VLANs into the same fabric as the parent.
             for rel in instance.children_relationships.all():
@@ -220,12 +238,16 @@
                     rel.child.save()
                     # No need to update the IP addresses here this function
                     # will be called again because the child has been saved.
-
+                    log.msg("%s: updated fabric on %s to %s (vlan=%s)" % (
+                        instance.get_log_string(), rel.child.get_log_string(),
+                        new_vlan.fabric.name, rel.child.vlan_id))
     else:
         # Interface VLAN was changed on a machine or device. Remove all its
         # links except the DISCOVERED ones.
         instance.ip_addresses.exclude(
             alloc_type=IPADDRESS_TYPE.DISCOVERED).delete()
+        log.msg("%s: deleted IP addresses due to VLAN update (%s -> %s)." % (
+            instance.get_log_string(), old_vlan_id, new_vlan_id))
 
 
 for klass in INTERFACE_CLASSES:
@@ -242,6 +264,8 @@
         if len(rel.child.parents.all()) == 1:
             # Last parent of the child, so delete the child.
             rel.child.delete()
+            log.msg("%s has been deleted; orphaned by %s." % (
+                rel.child.get_log_string(), instance.get_log_string()))
 
 
 for klass in INTERFACE_CLASSES:
@@ -265,8 +289,7 @@
             # Remove its link to the interface before the interface
             # is deleted.
             if ip_address.alloc_type != IPADDRESS_TYPE.DISCOVERED:
-                instance.unlink_ip_address(
-                    ip_address, clearing_config=True)
+                instance.unlink_ip_address(ip_address, clearing_config=True)
             else:
                 ip_address.delete()